Align the start pointer in ink allocator (#2100)

.config/cargo_spellcheck.dic

@@ -25,6 +25,7 @@ Wasm32
 WebAssembly
 adjunctive
 bitvector
+bitmask
 bitwise
 callee
 codegen

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

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<usize> {
-        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::<u8>();
         assert_eq!(inner.next, expected_alloc_start);
     }
+
+    #[test]
+    fn correct_alloc_types() {
+        let mut inner = InnerAlloc::new();
+        let layout1 = Layout::for_value(&Vec::<u128>::with_capacity(3));
+        assert_eq!(inner.alloc(layout1), Some(0));
+        assert_eq!(inner.next, 24);
+
+        let layout2 = Layout::for_value(&Vec::<u128>::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;

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::<u8>::with_capacity(3);
+            let buf2 = Vec::<u64>::with_capacity(1);
+            let ptr1 = buf1.as_ptr() as u64;
+            let ptr2 = buf2.as_ptr() as u64;
+            let align = core::mem::align_of::<Vec<u64>>() 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<Client: E2EBackend>(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::<StaticBuffer>();
+
+            // 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(())
+        }
     }
 }