Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add 'free_range' syscall #3467

Merged
merged 51 commits into from
Dec 3, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
51 commits
Select commit Hold shift + click to select a range
1cd654b
feat: Add 'free_range' syscall
holykol Nov 1, 2023
72e0a39
fix tests
holykol Nov 1, 2023
56791f3
revert removing free
holykol Nov 2, 2023
717c97a
fuzzing support for free_range
holykol Nov 2, 2023
e174fa7
validate range
holykol Nov 2, 2023
cd5184e
address feedback
holykol Nov 6, 2023
69395c2
benchmark stuff
holykol Nov 6, 2023
53aec75
fix costs
holykol Nov 7, 2023
792c224
naming
holykol Nov 7, 2023
f770da2
update gsdk
holykol Nov 7, 2023
98a35d4
chore(runtime): update weights (#3477)
github-actions[bot] Nov 7, 2023
aecdcc5
address feedback
holykol Nov 7, 2023
a57bb06
revert changing alloc branch
holykol Nov 7, 2023
0751550
fix benchmarks
holykol Nov 8, 2023
c1562c6
chore(runtime): update weights (#3482)
github-actions[bot] Nov 8, 2023
8fe9e0c
address feedback
holykol Nov 8, 2023
0c6fdaf
move error handling inside syscall implementation
holykol Nov 9, 2023
24bbe91
fix benchmark
holykol Nov 9, 2023
ec644b3
chore(runtime): update weights (#3488)
github-actions[bot] Nov 10, 2023
2e90c3b
reduce free_range banchmark iterations
grishasobol Nov 10, 2023
fb08dd7
chore(runtime): update weights (#3489)
github-actions[bot] Nov 10, 2023
b0ba34c
revert weights
holykol Nov 10, 2023
dfdb792
revert weights 2
holykol Nov 10, 2023
601b58a
trap on any error
holykol Nov 13, 2023
75c38d4
return nothing
holykol Nov 13, 2023
3931497
fix tests
holykol Nov 13, 2023
fca257c
fix fuzzing
holykol Nov 14, 2023
b2e6e74
address feedback
holykol Nov 15, 2023
2123704
Merge branch 'master' into holykol/free-range-syscall
holykol Nov 17, 2023
60b4df9
address feedback
holykol Nov 20, 2023
ecb4317
address feedback 2
holykol Nov 21, 2023
6223615
revert test fix
holykol Nov 21, 2023
3c184b5
address feedback
holykol Nov 27, 2023
0e84818
Merge branch 'master' into holykol/free-range-syscall
holykol Nov 27, 2023
87a4a6c
exchange repetitions and BATCH_SIZE in benchmarks
holykol Nov 27, 2023
6fd6bc5
Merge branch 'master' into holykol/free-range-syscall
holykol Nov 28, 2023
841da55
always return 0 in free_range
holykol Nov 28, 2023
506f13d
fix
holykol Nov 28, 2023
b5355a9
fix 2
holykol Nov 28, 2023
d0f0d22
fix 3
holykol Nov 29, 2023
36664af
chore(runtime): update weights
holykol Nov 29, 2023
9c94842
Revert "chore(runtime): update weights"
holykol Dec 1, 2023
dbb44ab
Merge branch 'master' into holykol/free-range-syscall
holykol Dec 1, 2023
9ca5d86
Merge branch 'master' into holykol/free-range-syscall
holykol Dec 1, 2023
5e81011
return error instead of trapping
holykol Dec 1, 2023
e233c32
revert old benchmarks
holykol Dec 1, 2023
2313e22
fix 999
holykol Dec 2, 2023
449a6ba
fix 999.5
holykol Dec 2, 2023
39f3e4e
test that free_range behaves as free
holykol Dec 2, 2023
659f61f
address feedback
holykol Dec 2, 2023
37d447f
address feedback
holykol Dec 2, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions core-backend/src/env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,7 @@ where

builder.add_func(Alloc, wrap_syscall!(alloc));
builder.add_func(Free, wrap_syscall!(free));
builder.add_func(FreeRange, wrap_syscall!(free_range));
}
}

Expand Down
29 changes: 28 additions & 1 deletion core-backend/src/funcs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -659,7 +659,7 @@ where
))
})?;

let res = ctx.ext_mut().free(page);
let res = ctx.ext_mut().free_range(page..=page);
let res = ctx.process_alloc_func_result(res)?;

match &res {
Expand All @@ -675,6 +675,33 @@ where
})
}

pub fn free_range(start: u32, end: u32) -> impl SysCall<Ext, i32> {
InfallibleSysCall::new(RuntimeCosts::Free, move |ctx: &mut CallerWrap<Ext>| {
holykol marked this conversation as resolved.
Show resolved Hide resolved
let err = |_| {
UndefinedTerminationReason::Actor(ActorTerminationReason::Trap(
TrapExplanation::Unknown,
))
};

let start = WasmPage::new(start).map_err(err)?;
let end = WasmPage::new(end).map_err(err)?;

let res = ctx.ext_mut().free_range(start..=end);
let res = ctx.process_alloc_func_result(res)?;

match &res {
Ok(()) => {
log::trace!("Free range {start:?}:{end:?}");
}
Err(err) => {
log::trace!("Free failed: {err}");
}
};

Ok(res.is_err() as i32)
})
}

pub fn env_vars(vars_ver: u32, vars_ptr: u32) -> impl SysCall<Ext> {
InfallibleSysCall::new(RuntimeCosts::EnvVars, move |ctx: &mut CallerWrap<Ext>| {
let vars = ctx.ext_mut().env_vars(vars_ver)?;
Expand Down
4 changes: 2 additions & 2 deletions core-backend/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ use crate::{
};
use alloc::{collections::BTreeSet, vec, vec::Vec};
use codec::{Decode, Encode};
use core::{cell::Cell, fmt, fmt::Debug};
use core::{cell::Cell, fmt, fmt::Debug, ops::RangeInclusive};
use gear_core::{
costs::RuntimeCosts,
env::{Externalities, PayloadSliceLock, UnlockPayloadBound},
Expand Down Expand Up @@ -112,7 +112,7 @@ impl Externalities for MockExt {
) -> Result<WasmPage, Self::AllocError> {
Err(Error)
}
fn free(&mut self, _page: WasmPage) -> Result<(), Self::AllocError> {
fn free_range(&mut self, _range: RangeInclusive<WasmPage>) -> Result<(), Self::AllocError> {
Err(Error)
}
fn env_vars(&self, version: u32) -> Result<EnvVars, Self::UnrecoverableError> {
Expand Down
6 changes: 4 additions & 2 deletions core-processor/src/ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
// You should have received a copy of the GNU General Public License
// along with this program. If not, see <https://www.gnu.org/licenses/>.

use core::ops::RangeInclusive;

use crate::{
configs::{BlockInfo, PageCosts},
context::SystemReservationContext,
Expand Down Expand Up @@ -721,10 +723,10 @@ impl Externalities for Ext {
.map_err(Into::into)
}

fn free(&mut self, page: WasmPage) -> Result<(), Self::AllocError> {
fn free_range(&mut self, range: RangeInclusive<WasmPage>) -> Result<(), Self::AllocError> {
self.context
.allocations_context
.free(page)
.free_range(range)
.map_err(Into::into)
}

Expand Down
9 changes: 3 additions & 6 deletions core/src/env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ use crate::{
pages::WasmPage,
};
use alloc::collections::BTreeSet;
use core::{fmt::Display, mem};
use core::{fmt::Display, mem, ops::RangeInclusive};
use gear_core_errors::{ReplyCode, SignalCode};
use gear_wasm_instrument::syscalls::SysCallName;

Expand Down Expand Up @@ -192,11 +192,8 @@ pub trait Externalities {
mem: &mut impl Memory,
) -> Result<WasmPage, Self::AllocError>;

/// Free specific memory page.
///
/// Unlike traditional allocator, if multiple pages allocated via `alloc`, all pages
/// should be `free`-d separately.
fn free(&mut self, page: WasmPage) -> Result<(), Self::AllocError>;
/// Free specific memory range
fn free_range(&mut self, range: RangeInclusive<WasmPage>) -> Result<(), Self::AllocError>;

/// Get environment variables currently set in the system and in the form
/// corresponded to the requested version.
Expand Down
46 changes: 34 additions & 12 deletions core/src/memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ use core::{
fmt,
fmt::Debug,
iter,
ops::{Deref, DerefMut},
ops::{Deref, DerefMut, RangeInclusive},
};
use scale_info::{
scale::{self, Decode, Encode, EncodeLike, Input, Output},
Expand Down Expand Up @@ -376,15 +376,21 @@ impl AllocationsContext {
Ok(start)
}

/// Free specific page.
/// Try to free pages in range. Will only return error if range is invalid.
///
/// Currently running program should own this page.
pub fn free(&mut self, page: WasmPage) -> Result<(), AllocError> {
if page < self.static_pages || page >= self.max_pages || !self.allocations.remove(&page) {
Err(AllocError::InvalidFree(page.0))
} else {
Ok(())
/// Currently running program should own this pages.
pub fn free_range(&mut self, range: RangeInclusive<WasmPage>) -> Result<(), AllocError> {
if *range.start() < self.static_pages || *range.end() >= self.max_pages {
breathx marked this conversation as resolved.
Show resolved Hide resolved
let page = if *range.start() < self.static_pages {
holykol marked this conversation as resolved.
Show resolved Hide resolved
range.start().0
} else {
range.end().0
};
return Err(AllocError::InvalidFree(page));
}

self.allocations.retain(|p| !range.contains(p));
Ok(())
}

/// Decomposes this instance and returns allocations.
Expand Down Expand Up @@ -450,14 +456,30 @@ mod tests {
#[test]
fn free_fails() {
let mut ctx = AllocationsContext::new(BTreeSet::default(), WasmPage(0), WasmPage(0));
assert_eq!(ctx.free(WasmPage(1)), Err(AllocError::InvalidFree(1)));
assert_eq!(
ctx.free_range(WasmPage(1)..=WasmPage(1)),
holykol marked this conversation as resolved.
Show resolved Hide resolved
Err(AllocError::InvalidFree(1))
);

let mut ctx = AllocationsContext::new(BTreeSet::default(), WasmPage(1), WasmPage(0));
assert_eq!(ctx.free(WasmPage(0)), Err(AllocError::InvalidFree(0)));
assert_eq!(
ctx.free_range(WasmPage(0)..=WasmPage(0)),
Err(AllocError::InvalidFree(0))
);

let mut ctx =
AllocationsContext::new(BTreeSet::from([WasmPage(0)]), WasmPage(1), WasmPage(1));
assert_eq!(ctx.free(WasmPage(1)), Err(AllocError::InvalidFree(1)));
assert_eq!(
ctx.free_range(WasmPage(1)..=WasmPage(1)),
Err(AllocError::InvalidFree(1))
);

let mut ctx = AllocationsContext::new(
BTreeSet::from([WasmPage(1), WasmPage(3)]),
WasmPage(1),
WasmPage(4),
);
assert_eq!(ctx.free_range(WasmPage(1)..=WasmPage(3)), Ok(()));
}

#[test]
Expand Down Expand Up @@ -606,7 +628,7 @@ mod tests {
}
}
Action::Free { page } => {
if let Err(err) = ctx.free(page) {
if let Err(err) = ctx.free_range(page..=page) {
assert_free_error(err);
}
}
Expand Down
23 changes: 23 additions & 0 deletions pallets/gear/src/benchmarking/syscalls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,29 @@ where
Self::prepare_handle(module, 0)
}

pub fn free_range(r: u32) -> Result<Exec<T>, &'static str> {
holykol marked this conversation as resolved.
Show resolved Hide resolved
assert!(r <= max_pages::<T>() as u32);

use Instruction::*;
let mut instructions = vec![];
for _ in 0..API_BENCHMARK_BATCH_SIZE {
instructions.extend([I32Const(r as i32), Call(0), I32Const(-1)]);
unreachable_condition(&mut instructions, I32Eq); // if alloc returns -1 then it's error

instructions.extend([I32Const(1), I32Const(r - 1 as i32), Call(1), I32Const(0)]);
unreachable_condition(&mut instructions, I32Ne); // if free_range returns 0 then it's error
}

let module = ModuleDefinition {
memory: Some(ImportedMemory::new(0)),
imported_functions: vec![SysCallName::Alloc, SysCallName::FreeRange],
handle_body: Some(body::from_instructions(instructions)),
..Default::default()
};

Self::prepare_handle(module, 0)
}

pub fn gr_reserve_gas(r: u32) -> Result<Exec<T>, &'static str> {
let repetitions = r;
let res_offset = COMMON_OFFSET;
Expand Down
113 changes: 113 additions & 0 deletions pallets/gear/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13673,6 +13673,119 @@ fn free_usage_error() {
});
}

#[test]
fn free_range_oob_error() {
const WAT: &str = r#"
(module
(import "env" "memory" (memory 1))
(import "env" "free_range" (func $free_range (param i32) (param i32) (result i32)))
(export "init" (func $init))
(func $init
;; free impossible and non-existing page
i32.const 0x0
i32.const 0xffffffff
call $free_range
;; free must return 1 so we will get `unreachable` instruction
i32.const 0
i32.ne
if
unreachable
end
)
)
"#;

init_logger();
new_test_ext().execute_with(|| {
let pid = Gear::upload_program(
RuntimeOrigin::signed(USER_1),
ProgramCodeKind::Custom(WAT).to_bytes(),
DEFAULT_SALT.to_vec(),
EMPTY_PAYLOAD.to_vec(),
500_000_000_u64,
0,
false,
)
.map(|_| get_last_program_id())
.unwrap();
let mid = get_last_message_id();

run_to_next_block(None);

assert!(Gear::is_terminated(pid));
assert_failed(
mid,
ActorExecutionErrorReplyReason::Trap(TrapExplanation::Unknown),
);
});
}

#[test]
fn free_range_success() {
const WAT: &str = r#"
(module
(import "env" "memory" (memory 1))
(import "env" "alloc" (func $alloc (param i32) (result i32)))
(import "env" "free" (func $free (param i32) (result i32)))
(import "env" "free_range" (func $free_range (param i32) (param i32) (result i32)))
(export "init" (func $init))
(func $init
;; allocate 4 pages
i32.const 0x4
call $alloc

i32.const 1
i32.ne
if
unreachable
end

;; free one page in range
i32.const 0x2
call $free

i32.const 0
i32.ne
if
unreachable
end

;; free range with one missing page
i32.const 0x1
i32.const 0x4
call $free_range

i32.const 0
i32.ne
if
unreachable
end
)
)
"#;

init_logger();
new_test_ext().execute_with(|| {
let pid = Gear::upload_program(
RuntimeOrigin::signed(USER_1),
ProgramCodeKind::Custom(WAT).to_bytes(),
DEFAULT_SALT.to_vec(),
EMPTY_PAYLOAD.to_vec(),
500_000_000_u64,
0,
false,
)
.map(|_| get_last_program_id())
.unwrap();
let mid = get_last_message_id();

run_to_next_block(None);

assert_succeed(mid);
assert!(Gear::is_active(pid));
holykol marked this conversation as resolved.
Show resolved Hide resolved
});
}

#[test]
fn reject_incorrect_stack_pointer() {
let wat = format!(
Expand Down
1 change: 1 addition & 0 deletions utils/wasm-gen/src/generator/syscalls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,7 @@ impl InvocableSysCall {
| SysCallName::Wait
| SysCallName::Alloc
| SysCallName::Free
| SysCallName::FreeRange
| SysCallName::OutOfGas => false,
SysCallName::CreateProgramWGas
| SysCallName::CreateProgram
Expand Down
3 changes: 3 additions & 0 deletions utils/wasm-instrument/src/syscalls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ pub enum SysCallName {
// Hard under the hood calls, serving proper program execution
Alloc,
Free,
FreeRange,
OutOfGas,

// Miscellaneous
Expand Down Expand Up @@ -120,6 +121,7 @@ impl SysCallName {
SysCallName::OomPanic => "gr_oom_panic",
SysCallName::Exit => "gr_exit",
SysCallName::Free => "free",
SysCallName::FreeRange => "free_range",
SysCallName::GasAvailable => "gr_gas_available",
SysCallName::Leave => "gr_leave",
SysCallName::MessageId => "gr_message_id",
Expand Down Expand Up @@ -242,6 +244,7 @@ impl SysCallName {
match self {
Self::Alloc => SysCallSignature::system([Alloc], [I32]),
Self::Free => SysCallSignature::system([Free], [I32]),
Self::FreeRange => SysCallSignature::system([Free, Free], [I32]),
Self::Debug => SysCallSignature::gr([
Ptr(PtrInfo::new_immutable(PtrType::SizedBufferStart {
length_param_idx: 1,
Expand Down
Loading