enable alloc tests for UffdRegion, add new test, fix some things

- Instantiates the suite in `lucet_runtime_internals::alloc::tests` for `UffdRegion

- Fixes early return issues in `UffdRegion` similar to #455

- Adds a test to show that the per-instance heap limit applies to runtime expansions, not just
initial instantiation

- Refactors `validate_runtime_spec` to take the per-instance heap limit as an additional
argument. This centralizes the logic for rejecting initially-oversized heap limits, and makes it
clearer what's happening in each region's instantiation logic.

- Removes the `UffdRegion`'s assertion that signal stack size is a multiple of page size. Since the
user can now control this as a parameter, we reject it gracefully when validating `Limits` rather
than panicking.

Author: acfoltzer

lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs

@@ -242,9 +242,9 @@ macro_rules! alloc_tests {
             max_size: Some(EXPANDPASTLIMIT_MAX_SIZE),
         };
 
-        /// This test shows that a heap refuses to grow past the alloc limits, even if the runtime
-        /// spec says it can grow bigger. This test uses a region with multiple slots in order to
-        /// exercise more edge cases with adjacent managed memory.
+        /// This test shows that a heap refuses to grow past the region's limits, even if the
+        /// runtime spec says it can grow bigger. This test uses a region with multiple slots in
+        /// order to exercise more edge cases with adjacent managed memory.
         #[test]
         fn expand_past_heap_limit() {
             let region = TestRegion::create(10, &LIMITS).expect("region created");
@@ -282,6 +282,55 @@ macro_rules! alloc_tests {
             assert_eq!(heap[new_heap_len - 1], 0xFF);
         }
 
+        /// This test shows that a heap refuses to grow past the instance-specific heap limit, even
+        /// if both the region's limits and the runtime spec says it can grow bigger. This test uses
+        /// a region with multiple slots in order to exercise more edge cases with adjacent managed
+        /// memory.
+        #[test]
+        fn expand_past_instance_heap_limit() {
+            const INSTANCE_LIMIT: usize = LIMITS.heap_memory_size / 2;
+            const SPEC: HeapSpec = HeapSpec {
+                initial_size: INSTANCE_LIMIT as u64 - 64 * 1024,
+                ..EXPAND_PAST_LIMIT_SPEC
+            };
+            assert_eq!(INSTANCE_LIMIT % host_page_size(), 0);
+
+            let region = TestRegion::create(10, &LIMITS).expect("region created");
+            let module = MockModuleBuilder::new().with_heap_spec(SPEC).build();
+            let mut inst = region
+                .new_instance_builder(module.clone())
+                .with_heap_size_limit(INSTANCE_LIMIT)
+                .build()
+                .expect("instantiation succeeds");
+
+            let heap_len = inst.alloc().heap_len();
+            assert_eq!(heap_len, SPEC.initial_size as usize);
+
+            // fill up the rest of the per-instance limit area
+            let new_heap_area = inst
+                .alloc_mut()
+                .expand_heap(64 * 1024, module.as_ref())
+                .expect("expand_heap succeeds");
+            assert_eq!(heap_len, new_heap_area as usize);
+
+            let new_heap_len = inst.alloc().heap_len();
+            assert_eq!(new_heap_len, INSTANCE_LIMIT);
+
+            let past_limit_heap_area = inst.alloc_mut().expand_heap(64 * 1024, module.as_ref());
+            assert!(
+                past_limit_heap_area.is_err(),
+                "heap expansion past limit fails"
+            );
+
+            let still_heap_len = inst.alloc().heap_len();
+            assert_eq!(still_heap_len, INSTANCE_LIMIT);
+
+            let heap = unsafe { inst.alloc_mut().heap_mut() };
+            assert_eq!(heap[new_heap_len - 1], 0);
+            heap[new_heap_len - 1] = 0xFF;
+            assert_eq!(heap[new_heap_len - 1], 0xFF);
+        }
+
         const INITIAL_OVERSIZE_HEAP: HeapSpec = HeapSpec {
             reserved_size: SPEC_HEAP_RESERVED_SIZE,
             guard_size: SPEC_HEAP_GUARD_SIZE,
@@ -1133,4 +1182,11 @@ macro_rules! alloc_tests {
 }
 
 #[cfg(test)]
-alloc_tests!(crate::region::mmap::MmapRegion);
+mod mmap {
+    alloc_tests!(crate::region::mmap::MmapRegion);
+}
+
+#[cfg(all(test, feature = "uffd"))]
+mod uffd {
+    alloc_tests!(crate::region::uffd::UffdRegion);
+}

lucet-runtime/lucet-runtime-internals/src/module.rs

@@ -118,7 +118,6 @@ pub trait ModuleInternal: Send + Sync {
             ));
         }
         let heap_memory_size = std::cmp::min(limits.heap_memory_size, instance_heap_limit);
-
         // Modules without heap specs will not access the heap
         if let Some(heap) = self.heap_spec() {
             // Assure that the total reserved + guard regions fit in the address space.

lucet-runtime/lucet-runtime-internals/src/region/mmap.rs

@@ -82,6 +82,7 @@ impl RegionInternal for MmapRegion {
         mut alloc_strategy: AllocStrategy,
     ) -> Result<InstanceHandle, Error> {
         let limits = self.get_limits();
+
         module.validate_runtime_spec(&limits, heap_memory_size_limit)?;
 
         // Use the supplied alloc_strategy to get the next available slot

lucet-runtime/lucet-runtime-internals/src/region/uffd.rs

@@ -12,7 +12,6 @@ use crate::{lucet_bail, lucet_ensure, lucet_format_err};
 use libc::c_void;
 use nix::poll;
 use nix::sys::mman::{madvise, mmap, munmap, MapFlags, MmapAdvise, ProtFlags};
-use std::cmp::Ordering;
 use std::os::unix::io::{AsRawFd, RawFd};
 use std::ptr;
 use std::sync::{Arc, Mutex, Weak};
@@ -217,30 +216,8 @@ impl RegionInternal for UffdRegion {
         embed_ctx: CtxMap,
         heap_memory_size_limit: usize,
     ) -> Result<InstanceHandle, Error> {
-        let custom_limits;
-        let mut limits = self.get_limits();
-
-        // Affirm that the module, if instantiated, would not violate
-        // any runtime memory limits.
-        match heap_memory_size_limit.cmp(&limits.heap_memory_size) {
-            Ordering::Less => {
-                // The supplied heap_memory_size is smaller than
-                // default. Augment the limits with this custom value
-                // so that it may be validated.
-                custom_limits = Limits {
-                    heap_memory_size: heap_memory_size_limit,
-                    ..*limits
-                };
-                limits = &custom_limits;
-            }
-            Ordering::Equal => (),
-            Ordering::Greater => {
-                return Err(Error::InvalidArgument(
-                    "heap memory size requested for instance is larger than slot allows",
-                ))
-            }
-        }
-        module.validate_runtime_spec(&limits)?;
+        let limits = self.get_limits();
+        module.validate_runtime_spec(&limits, heap_memory_size_limit)?;
 
         let slot = self
             .freelist
@@ -249,12 +226,11 @@ impl RegionInternal for UffdRegion {
             .pop()
             .ok_or(Error::RegionFull(self.instance_capacity))?;
 
-        if slot.heap as usize % host_page_size() != 0 {
-            lucet_bail!("heap is not page-aligned; this is a bug");
-        }
-
-        let limits = &slot.limits;
-        module.validate_runtime_spec(limits)?;
+        assert_eq!(
+            slot.heap as usize % host_page_size(),
+            0,
+            "heap must be page-aligned"
+        );
 
         for (ptr, len) in [
             // zero the globals
@@ -270,7 +246,7 @@ impl RegionInternal for UffdRegion {
                 unsafe {
                     self.uffd
                         .zeropage(*ptr, *len, true)
-                        .map_err(|e| Error::InternalError(e.into()))?;
+                        .expect("uffd.zeropage succeeds");
                 }
             }
         }
@@ -410,10 +386,6 @@ impl UffdRegion {
     /// This also creates and starts a separate thread that is responsible for handling page faults
     /// that occur within the memory region.
     pub fn create(instance_capacity: usize, limits: &Limits) -> Result<Arc<Self>, Error> {
-        assert!(
-            limits.signal_stack_size % host_page_size() == 0,
-            "signal stack size is a multiple of host page size"
-        );
         if instance_capacity == 0 {
             return Err(Error::InvalidArgument(
                 "region must be able to hold at least one instance",