diff --git a/.config/cargo_spellcheck.dic b/.config/cargo_spellcheck.dic index 4831da94b3..61a6f3445e 100644 --- a/.config/cargo_spellcheck.dic +++ b/.config/cargo_spellcheck.dic @@ -25,6 +25,7 @@ Wasm32 WebAssembly adjunctive bitvector +bitmask bitwise callee codegen diff --git a/CHANGELOG.md b/CHANGELOG.md index 2ec78ff01b..37aa5ad1e6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed +- Fix alignment in allocator [#2100](https://github.com/paritytech/ink/pull/2100) + ## Version 5.0.0-rc.1 ### Added diff --git a/crates/allocator/src/bump.rs b/crates/allocator/src/bump.rs index a713cc5d04..78db75584f 100644 --- a/crates/allocator/src/bump.rs +++ b/crates/allocator/src/bump.rs @@ -14,9 +14,6 @@ //! A simple bump allocator. //! -//! Its goal to have a much smaller footprint than the admittedly more full-featured -//! `wee_alloc` allocator which is currently being used by ink! smart contracts. -//! //! The heap which is used by this allocator is built from pages of Wasm memory (each page //! is `64KiB`). We will request new pages of memory as needed until we run out of memory, //! at which point we will crash with an `OOM` error instead of freeing any memory. @@ -174,9 +171,10 @@ impl InnerAlloc { /// Note: This implementation results in internal fragmentation when allocating across /// pages. fn alloc(&mut self, layout: Layout) -> Option { - let alloc_start = self.next; + let alloc_start = self.align_ptr(&layout); + + let aligned_size = layout.size(); - let aligned_size = layout.pad_to_align().size(); let alloc_end = alloc_start.checked_add(aligned_size)?; if alloc_end > self.upper_limit { @@ -194,6 +192,20 @@ impl InnerAlloc { Some(alloc_start) } } + + /// Aligns the start pointer of the next allocation. + /// + /// We inductively calculate the start index + /// of a layout in the linear memory. + /// - Initially `self.next` is `0`` and aligned + /// - `layout.align() - 1` accounts for `0` as the first index. + /// - the binary with the inverse of the align creates a + /// bitmask that is used to zero out bits, ensuring alignment according to type + /// requirements and ensures that the next allocated pointer address is of the + /// power of 2. + fn align_ptr(&self, layout: &Layout) -> usize { + (self.next + layout.align() - 1) & !(layout.align() - 1) + } } /// Calculates the number of pages of memory needed for an allocation of `size` bytes. @@ -329,6 +341,18 @@ mod tests { let expected_alloc_start = 2 * PAGE_SIZE + size_of::(); assert_eq!(inner.next, expected_alloc_start); } + + #[test] + fn correct_alloc_types() { + let mut inner = InnerAlloc::new(); + let layout1 = Layout::for_value(&Vec::::with_capacity(3)); + assert_eq!(inner.alloc(layout1), Some(0)); + assert_eq!(inner.next, 24); + + let layout2 = Layout::for_value(&Vec::::with_capacity(1)); + assert_eq!(inner.alloc(layout2), Some(24)); + assert_eq!(inner.next, 48); + } } #[cfg(all(test, feature = "ink-fuzz-tests"))] @@ -351,7 +375,7 @@ mod fuzz_tests { Err(_) => return TestResult::discard(), }; - let size = layout.pad_to_align().size(); + let size = layout.size(); assert_eq!( inner.alloc(layout), Some(0), @@ -390,7 +414,7 @@ mod fuzz_tests { Err(_) => return TestResult::discard(), }; - let size = layout.pad_to_align().size(); + let size = layout.size(); assert_eq!( inner.alloc(layout), Some(0), @@ -459,7 +483,7 @@ mod fuzz_tests { Err(_) => return TestResult::discard(), }; - let size = layout.pad_to_align().size(); + let size = layout.size(); let current_page_limit = PAGE_SIZE * required_pages(inner.next).unwrap(); let is_too_big_for_current_page = inner.next + size > current_page_limit; diff --git a/integration-tests/static-buffer/lib.rs b/integration-tests/static-buffer/lib.rs index c6b733535b..9f813f4655 100644 --- a/integration-tests/static-buffer/lib.rs +++ b/integration-tests/static-buffer/lib.rs @@ -2,6 +2,11 @@ #[ink::contract] pub mod static_buffer { + use ink::prelude::{ + string::String, + vec::Vec, + }; + #[allow(unused_imports)] use ink::env::BUFFER_SIZE; #[ink(storage)] @@ -28,6 +33,19 @@ pub mod static_buffer { pub fn get_caller(&self) -> AccountId { self.env().caller() } + + #[ink(message)] + pub fn buffer(&self) -> Result<(u64, u64), String> { + let buf1 = Vec::::with_capacity(3); + let buf2 = Vec::::with_capacity(1); + let ptr1 = buf1.as_ptr() as u64; + let ptr2 = buf2.as_ptr() as u64; + let align = core::mem::align_of::>() as u64; + let padding = ptr2 + .checked_sub(ptr1) + .ok_or(String::from("Error during padding calculation"))?; + Ok((padding, align)) + } } #[cfg(test)] @@ -75,5 +93,32 @@ pub mod static_buffer { Ok(()) } + + #[ink_e2e::test] + async fn buffer(mut client: Client) -> E2EResult<()> { + // given + let mut constructor = StaticBufferRef::new_default(); + + // when + let contract = client + .instantiate("static_buffer", &ink_e2e::bob(), &mut constructor) + .submit() + .await + .expect("instantiate failed"); + let call_builder = contract.call_builder::(); + + // then + let buffer_call = call_builder.buffer(); + let buffer_call_res = + client.call(&ink_e2e::bob(), &buffer_call).submit().await?; + let value = buffer_call_res.return_value(); + assert!(value.is_ok()); + let value = value.unwrap(); + let padding = value.0; + let align = value.1; + assert_eq!(padding, 8); + assert_eq!(align, 4); + Ok(()) + } } }