Skip to content

Commit ea74c09

Browse files
committed
Add test for integrity of static and stack regions in memory
1 parent e12db1f commit ea74c09

File tree

5 files changed

+104
-14
lines changed

5 files changed

+104
-14
lines changed

concordium-std/CHANGELOG.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
# Changelog
22

33
## Unreleased changes
4+
- Remove the feature `wee_alloc` and replace it with `bump_alloc`, which enables a small and simple global allocator that can only be used in Wasm.
45

56
## concordium-std 9.0.2 (2024-02-07)
67

78
- Make the `concordium_dbg!` and related macros also usable with the full syntax
89
that `println!` supports.
9-
- Remove the feature `wee_alloc` and replace it with `bump_alloc`, which enables a small and simple global allocator that can only be used in Wasm.
1010

1111
## concordium-std 9.0.1 (2024-01-26)
1212

concordium-std/src/bump_alloc.rs

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,7 @@ impl BumpAllocator {
102102
heap_start: UnsafeCell::new(0),
103103
// Initialized to the dummy value `0`, which is checked during first initialization.
104104
heap_end: UnsafeCell::new(0),
105+
// Keeps track of the number of active allocations.
105106
allocations: UnsafeCell::new(0),
106107
// Initialized to the dummy value `0`.
107108
// Must be set to the same initial address as `next`.
@@ -113,7 +114,7 @@ impl BumpAllocator {
113114
///
114115
/// If successful, it returns the previous number of memory pages.
115116
/// Otherwise, it returns [`ERROR_PAGE_COUNT`] to indicate out of memory.
116-
fn grow_memory(&self, delta: PageCount) -> PageCount {
117+
fn memory_grow(&self, delta: PageCount) -> PageCount {
117118
// The first argument refers to the index of memory to return the size of.
118119
// Currently, Wasm only supports a single slot of memory, so `0` must always be
119120
// used. Source: https://doc.rust-lang.org/beta/core/arch/wasm32/fn.memory_grow.html
@@ -129,7 +130,7 @@ impl BumpAllocator {
129130
/// - The heap.
130131
///
131132
/// To get the start location of the heap, use `__heap_base`.
132-
fn size(&self) -> PageCount {
133+
fn memory_size(&self) -> PageCount {
133134
// The argument refers to the index of memory to return the size of.
134135
// Currently, Wasm only supports a single slot of memory, so `0` must always be
135136
// used. Source: https://doc.rust-lang.org/beta/core/arch/wasm32/fn.memory_size.html
@@ -154,7 +155,7 @@ unsafe impl GlobalAlloc for BumpAllocator {
154155
let heap_base = unsafe { &__heap_base as *const _ as usize };
155156
// Get the actual size of the memory, which is also the end of the heap, as the
156157
// heap is the last section in the memory.
157-
let actual_size = self.size().size_in_bytes();
158+
let actual_size = self.memory_size().size_in_bytes();
158159
// Replace all the dummy values.
159160
*next = heap_base;
160161
*self.heap_start.get() = heap_base;
@@ -175,7 +176,7 @@ unsafe impl GlobalAlloc for BumpAllocator {
175176
if alloc_end > *heap_end {
176177
let space_needed = alloc_end - *heap_end;
177178
let pages_to_request = pages_to_request(space_needed);
178-
let previous_page_count = self.grow_memory(pages_to_request);
179+
let previous_page_count = self.memory_grow(pages_to_request);
179180
// Check if we are out of memory.
180181
if previous_page_count == ERROR_PAGE_COUNT {
181182
return ptr::null_mut();
@@ -225,7 +226,8 @@ fn align_up(addr: usize, align: usize) -> usize { (addr + align - 1) & !(align -
225226
/// Calculates the number of memory pages needed to allocate an additional
226227
/// `space_needed` bytes.
227228
///
228-
/// This function simply performs an integer division and rounds up the result.
229+
/// This function simply performs an integer division with `PAGE_SIZE` and
230+
/// rounds up the result.
229231
///
230232
/// ```
231233
/// assert_eq!(pages_to_request(0), PageCount(0));

concordium-std/src/lib.rs

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -131,11 +131,10 @@
131131
//! i.e, the resulting smart contracts are going to be smaller by about 6-10kB,
132132
//! which means they are cheaper to deploy and run. `bump_alloc` is designed to
133133
//! be simple and fast, but it does not use the memory very efficiently. For
134-
//! short-lived programs, such as smart contracts, this is usually a perfectly
135-
//! acceptable tradeoff.
136-
//! For an allocator with different tradeoffs, see [dlmalloc](https://docs.rs/dlmalloc/).
137-
//!
138-
//! See the Rust [allocator](https://doc.rust-lang.org/std/alloc/index.html#the-global_allocator-attribute)
134+
//! short-lived programs, such as smart contracts, this is usually the right
135+
//! tradeoff. Especially for contracts such as those dealing with tokens.
136+
//! For very complex contracts it may be beneficial to run benchmarks to see
137+
//! whether `bump_alloc` is the best option. See the Rust [allocator](https://doc.rust-lang.org/std/alloc/index.html#the-global_allocator-attribute)
139138
//! documentation for more context and details on using custom allocators.
140139
//!
141140
//! Emit debug information

examples/bump-alloc-tests/src/lib.rs

Lines changed: 68 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ fn allocate_one_mib(_ctx: &ReceiveContext, _host: &Host<State>) -> ReceiveResult
7878
#[receive(contract = "bump_alloc_tests", name = "dealloc_last_optimization")]
7979
fn dealloc_last_optimization(ctx: &ReceiveContext, _host: &Host<State>) -> ReceiveResult<()> {
8080
let n: u64 = ctx.parameter_cursor().get()?;
81-
let mut long_lasting = black_box(Box::new(0u64));
81+
let mut long_lived = black_box(Box::new(0u64));
8282
for i in 0..n {
8383
let addr_a = {
8484
let a = black_box(vec![i]);
@@ -89,11 +89,76 @@ fn dealloc_last_optimization(ctx: &ReceiveContext, _host: &Host<State>) -> Recei
8989
b.as_ptr() as usize
9090
};
9191
if addr_a != addr_b {
92-
*long_lasting += 1;
92+
*long_lived += 1;
9393
}
9494
}
95-
if *long_lasting != 0 {
95+
if *long_lived != 0 {
9696
return Err(Reject::default());
9797
}
9898
Ok(())
9999
}
100+
101+
/// Tests that the allocator does not overwrite the static and stack regions of
102+
/// the memory.
103+
///
104+
/// The general idea is to create static and local variables, perform some
105+
/// recursive calls and subsequently check the integrity of all the data.
106+
/// The details are in written in the internal comments and in the documentation
107+
/// for the recursive helper function `appender`.
108+
#[receive(contract = "bump_alloc_tests", name = "stack_and_static")]
109+
fn stack_and_static(ctx: &ReceiveContext, _host: &Host<State>) -> ReceiveResult<()> {
110+
// Create a static variable and some local variables.
111+
static ON_ODD: &str = "ODD";
112+
let (mut text, n, on_even): (String, u32, String) = ctx.parameter_cursor().get()?;
113+
let original_n = n;
114+
let original_text_len = text.len();
115+
// Allocate a long lived box on the heap.
116+
let long_lived: Box<Vec<u32>> = black_box(Box::new((0..n).collect()));
117+
// Run the appender function. Wrapped in a [`black_box`] to ensure that the
118+
// compiler won't try to optimize the internals away.
119+
black_box(appender(&mut text, n, &on_even, ON_ODD));
120+
// Use some of the local variables defined before the recursive call.
121+
// Abort if `n` should have been altered.
122+
if original_n != n {
123+
return Err(Reject::default());
124+
}
125+
// Abort if the text length hasn't increased when `n` is positive.
126+
if n != 0 && original_text_len == text.len() {
127+
return Err(Reject::default());
128+
}
129+
let n_usize = n as usize;
130+
// Use the box to ensure that it is long lived.
131+
if long_lived[n_usize - 100] != n - 100 {
132+
return Err(Reject::default());
133+
}
134+
// Calculate the expected length of the `text` and check it.
135+
let expected_len = original_text_len
136+
+ (ON_ODD.len() * (n_usize / 2)) // Append `ON_ODD` half the times.
137+
+ (on_even.len() * (n_usize / 2)) // Append `on_even` half the times.
138+
+ (ON_ODD.len() * (n_usize % 2)); // Append an extra `ON_ODD` if `n` is odd.
139+
if expected_len != text.len() {
140+
return Err(Reject::default());
141+
}
142+
Ok(())
143+
}
144+
145+
/// Recursively alternates between appending `on_even` and `on_odd` to the
146+
/// `text` `n` times.
147+
///
148+
/// For example with ("ORIGINAL", 3, "EVEN", "ODD"), the final `text` becomes:
149+
///
150+
/// | | |
151+
/// ORIGINALODDEVENODD
152+
/// | | |
153+
fn appender(text: &mut String, n: u32, on_even: &str, on_odd: &str) {
154+
if n == 0 {
155+
return;
156+
}
157+
let is_even = n % 2 == 0;
158+
if is_even {
159+
text.push_str(on_even);
160+
} else {
161+
text.push_str(on_odd);
162+
}
163+
appender(text, n - 1, on_even, on_odd);
164+
}

examples/bump-alloc-tests/tests/tests.rs

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,30 @@ fn test_dealloc_last_optimization() {
113113
.expect("Update should succeed");
114114
}
115115

116+
/// Tests that stack and static data isn't overwritten in the allocator.
117+
#[test]
118+
fn test_stack_and_static() {
119+
let (chain, contract_address) = initialize_chain_and_contract();
120+
121+
chain
122+
.contract_invoke(
123+
ACC_0,
124+
Address::Account(ACC_0),
125+
Energy::from(100_000_000),
126+
UpdateContractPayload {
127+
amount: Amount::zero(),
128+
address: contract_address,
129+
receive_name: OwnedReceiveName::new_unchecked(
130+
"bump_alloc_tests.stack_and_static".to_string(),
131+
),
132+
message: OwnedParameter::from_serial(&("ORIGINAL", 123456, "EVEN"))
133+
.expect("Parameter size is below limit."),
134+
},
135+
)
136+
.print_emitted_events()
137+
.expect("Update should succeed");
138+
}
139+
116140
/// A helper method for initializing the chain and contract.
117141
fn initialize_chain_and_contract() -> (Chain, ContractAddress) {
118142
// Create the test chain.

0 commit comments

Comments
 (0)