From b64ee4995b0b472acec6cd8b9e3cf1d6d26194b3 Mon Sep 17 00:00:00 2001 From: "Tanya L. Crenshaw" Date: Mon, 23 Mar 2020 20:43:50 +0000 Subject: [PATCH 01/38] Improved approach. --- lucet-runtime/lucet-runtime-internals/src/region.rs | 8 +++++++- lucet-runtime/lucet-runtime-internals/src/region/mmap.rs | 1 + 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/lucet-runtime/lucet-runtime-internals/src/region.rs b/lucet-runtime/lucet-runtime-internals/src/region.rs index dcebb15d1..7a16757bd 100644 --- a/lucet-runtime/lucet-runtime-internals/src/region.rs +++ b/lucet-runtime/lucet-runtime-internals/src/region.rs @@ -57,6 +57,7 @@ pub trait RegionInternal: Send + Sync { &self, module: Arc, embed_ctx: CtxMap, + heap_memory_size: usize, ) -> Result; /// Unmaps the heap, stack, and globals of an `Alloc`, while retaining the virtual address @@ -90,6 +91,7 @@ pub struct InstanceBuilder<'a> { region: &'a dyn RegionInternal, module: Arc, embed_ctx: CtxMap, + heap_memory_size: usize, } impl<'a> InstanceBuilder<'a> { @@ -98,9 +100,13 @@ impl<'a> InstanceBuilder<'a> { region, module, embed_ctx: CtxMap::default(), + heap_memory_size: 16 * 64 * 1024, } } + /// Add a custom limit for the heap memory size to the built instance. + pub fn with_heap_size_limit() {} + /// Add an embedder context to the built instance. /// /// Up to one context value of any particular type may exist in the instance. If a context value @@ -117,6 +123,6 @@ impl<'a> InstanceBuilder<'a> { /// This function runs the guest code for the WebAssembly `start` section, and running any guest /// code is potentially unsafe; see [`Instance::run()`](struct.Instance.html#method.run). pub fn build(self) -> Result { - self.region.new_instance_with(self.module, self.embed_ctx) + self.region.new_instance_with(self.module, self.embed_ctx, self.heap_memory_size) } } diff --git a/lucet-runtime/lucet-runtime-internals/src/region/mmap.rs b/lucet-runtime/lucet-runtime-internals/src/region/mmap.rs index dd5418480..16cfeb8d8 100644 --- a/lucet-runtime/lucet-runtime-internals/src/region/mmap.rs +++ b/lucet-runtime/lucet-runtime-internals/src/region/mmap.rs @@ -78,6 +78,7 @@ impl RegionInternal for MmapRegion { &self, module: Arc, embed_ctx: CtxMap, + heap_memory_size: usize ) -> Result { let slot = self .freelist From 2160f4e2090d47045ab8a04d5ff05efa4527bc4d Mon Sep 17 00:00:00 2001 From: "Tanya L. Crenshaw" Date: Tue, 24 Mar 2020 01:21:29 +0000 Subject: [PATCH 02/38] Do not make the heap size larger than the slot size limit. --- .../src/alloc/tests.rs | 26 ++++++++++------- .../lucet-runtime-internals/src/region.rs | 7 ++++- .../src/region/mmap.rs | 29 ++++++++++++++++++- 3 files changed, 49 insertions(+), 13 deletions(-) diff --git a/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs b/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs index 16d861959..07beaca8d 100644 --- a/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs +++ b/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs @@ -598,14 +598,16 @@ macro_rules! alloc_tests { } let region = TestRegion::create(1, &CONTEXT_TEST_LIMITS).expect("region created"); - let mut inst = region - .new_instance( - MockModuleBuilder::new() - .with_heap_spec(CONTEXT_TEST_HEAP) - .build(), - ) - .expect("new_instance succeeds"); + let mut inst = region + .new_instance_builder( + MockModuleBuilder::new() + .with_heap_spec(CONTEXT_TEST_HEAP) + .build(),) + .with_heap_size_limit(4096) + .build() + .expect("new_instance succeeds"); + let mut parent = ContextHandle::new(); unsafe { let heap_ptr = inst.alloc_mut().heap_mut().as_ptr() as *mut c_void; @@ -647,12 +649,14 @@ macro_rules! alloc_tests { } let region = TestRegion::create(1, &CONTEXT_TEST_LIMITS).expect("region created"); - let mut inst = region - .new_instance( + + let mut inst = region + .new_instance_builder( MockModuleBuilder::new() .with_heap_spec(CONTEXT_TEST_HEAP) - .build(), - ) + .build(),) + .with_heap_size_limit(4096) + .build() .expect("new_instance succeeds"); let mut parent = ContextHandle::new(); diff --git a/lucet-runtime/lucet-runtime-internals/src/region.rs b/lucet-runtime/lucet-runtime-internals/src/region.rs index 7a16757bd..012473ef1 100644 --- a/lucet-runtime/lucet-runtime-internals/src/region.rs +++ b/lucet-runtime/lucet-runtime-internals/src/region.rs @@ -105,7 +105,12 @@ impl<'a> InstanceBuilder<'a> { } /// Add a custom limit for the heap memory size to the built instance. - pub fn with_heap_size_limit() {} + /// This call is not necessary if the default heap memory size is adequate + /// for the new instance. + pub fn with_heap_size_limit(mut self, heap_memory_size: usize) -> Self { + self.heap_memory_size = heap_memory_size; + self + } /// Add an embedder context to the built instance. /// diff --git a/lucet-runtime/lucet-runtime-internals/src/region/mmap.rs b/lucet-runtime/lucet-runtime-internals/src/region/mmap.rs index 16cfeb8d8..4b6c4ff16 100644 --- a/lucet-runtime/lucet-runtime-internals/src/region/mmap.rs +++ b/lucet-runtime/lucet-runtime-internals/src/region/mmap.rs @@ -8,6 +8,7 @@ use libc::c_void; #[cfg(not(target_os = "linux"))] use libc::memset; use nix::sys::mman::{madvise, mmap, munmap, MapFlags, MmapAdvise, ProtFlags}; +use std::cmp::Ordering; use std::ptr; use std::sync::{Arc, RwLock, Weak}; @@ -91,7 +92,33 @@ impl RegionInternal for MmapRegion { lucet_bail!("heap is not page-aligned; this is a bug"); } - let limits = &slot.limits; + let limits = &slot.limits; + + // Affirm that any custom heap memory size supplied to this + // builder does not exceed the slot limits. + if let Ordering::Greater = + heap_memory_size.cmp(&slot.limits.heap_memory_size) { + return Err(Error::LimitsExceeded("heap memory size limit is larger than slot allows".to_owned())) + } + + + /* + + let limits = match heap_memory_size.cmp(&slot.limits.heap_memory_size) { + Ordering::Less => { + Limits { + heap_memory_size, + heap_address_space_size: slot.limits.heap_address_space_size, + stack_size: slot.limits.stack_size, + globals_size: slot.limits.globals_size, + signal_stack_size: slot.limits.signal_stack_size, + } + }, + Ordering::Equal => &slot.limits, + + }; +*/ + module.validate_runtime_spec(limits)?; for (ptr, len) in [ From e361aecc8ed1edaf1c3067088f262926d3b9c91a Mon Sep 17 00:00:00 2001 From: "Tanya L. Crenshaw" Date: Tue, 24 Mar 2020 03:03:10 +0000 Subject: [PATCH 03/38] Added a test to affirm that an oversized heap memory size limit is rejected. --- .../src/alloc/tests.rs | 45 +++++++++++++----- .../lucet-runtime-internals/src/region.rs | 9 ++-- .../src/region/mmap.rs | 47 +++++++------------ 3 files changed, 57 insertions(+), 44 deletions(-) diff --git a/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs b/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs index 07beaca8d..d39d6e2c3 100644 --- a/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs +++ b/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs @@ -599,15 +599,16 @@ macro_rules! alloc_tests { let region = TestRegion::create(1, &CONTEXT_TEST_LIMITS).expect("region created"); - let mut inst = region - .new_instance_builder( - MockModuleBuilder::new() - .with_heap_spec(CONTEXT_TEST_HEAP) - .build(),) - .with_heap_size_limit(4096) - .build() - .expect("new_instance succeeds"); - + let mut inst = region + .new_instance_builder( + MockModuleBuilder::new() + .with_heap_spec(CONTEXT_TEST_HEAP) + .build(), + ) + .with_heap_size_limit(4096) + .build() + .expect("new_instance succeeds"); + let mut parent = ContextHandle::new(); unsafe { let heap_ptr = inst.alloc_mut().heap_mut().as_ptr() as *mut c_void; @@ -650,11 +651,12 @@ macro_rules! alloc_tests { let region = TestRegion::create(1, &CONTEXT_TEST_LIMITS).expect("region created"); - let mut inst = region + let mut inst = region .new_instance_builder( MockModuleBuilder::new() .with_heap_spec(CONTEXT_TEST_HEAP) - .build(),) + .build(), + ) .with_heap_size_limit(4096) .build() .expect("new_instance succeeds"); @@ -824,6 +826,27 @@ macro_rules! alloc_tests { Ok(_) => panic!("unexpected success"), } } + + #[test] + fn reject_too_large_heap_memory_size() { + let region = TestRegion::create(1, &LIMITS).expect("region created"); + let res = region + .new_instance_builder( + MockModuleBuilder::new() + .with_heap_spec(CONTEXT_TEST_HEAP) + .build(), + ) + .with_heap_size_limit(&LIMITS.heap_memory_size * 2) + .build(); + + match res { + Err(Error::InvalidArgument( + "heap memory size requested for instance is larger than slot allows", + )) => (), + Err(e) => panic!("unexpected error: {}", e), + Ok(_) => panic!("unexpected success"), + } + } }; } diff --git a/lucet-runtime/lucet-runtime-internals/src/region.rs b/lucet-runtime/lucet-runtime-internals/src/region.rs index 012473ef1..5df1e5ce8 100644 --- a/lucet-runtime/lucet-runtime-internals/src/region.rs +++ b/lucet-runtime/lucet-runtime-internals/src/region.rs @@ -100,7 +100,7 @@ impl<'a> InstanceBuilder<'a> { region, module, embed_ctx: CtxMap::default(), - heap_memory_size: 16 * 64 * 1024, + heap_memory_size: 16 * 64 * 1024, } } @@ -108,8 +108,8 @@ impl<'a> InstanceBuilder<'a> { /// This call is not necessary if the default heap memory size is adequate /// for the new instance. pub fn with_heap_size_limit(mut self, heap_memory_size: usize) -> Self { - self.heap_memory_size = heap_memory_size; - self + self.heap_memory_size = heap_memory_size; + self } /// Add an embedder context to the built instance. @@ -128,6 +128,7 @@ impl<'a> InstanceBuilder<'a> { /// This function runs the guest code for the WebAssembly `start` section, and running any guest /// code is potentially unsafe; see [`Instance::run()`](struct.Instance.html#method.run). pub fn build(self) -> Result { - self.region.new_instance_with(self.module, self.embed_ctx, self.heap_memory_size) + self.region + .new_instance_with(self.module, self.embed_ctx, self.heap_memory_size) } } diff --git a/lucet-runtime/lucet-runtime-internals/src/region/mmap.rs b/lucet-runtime/lucet-runtime-internals/src/region/mmap.rs index 4b6c4ff16..631a21e5f 100644 --- a/lucet-runtime/lucet-runtime-internals/src/region/mmap.rs +++ b/lucet-runtime/lucet-runtime-internals/src/region/mmap.rs @@ -79,7 +79,7 @@ impl RegionInternal for MmapRegion { &self, module: Arc, embed_ctx: CtxMap, - heap_memory_size: usize + heap_memory_size: usize, ) -> Result { let slot = self .freelist @@ -92,34 +92,23 @@ impl RegionInternal for MmapRegion { lucet_bail!("heap is not page-aligned; this is a bug"); } - let limits = &slot.limits; - - // Affirm that any custom heap memory size supplied to this - // builder does not exceed the slot limits. - if let Ordering::Greater = - heap_memory_size.cmp(&slot.limits.heap_memory_size) { - return Err(Error::LimitsExceeded("heap memory size limit is larger than slot allows".to_owned())) - } - - - /* - - let limits = match heap_memory_size.cmp(&slot.limits.heap_memory_size) { - Ordering::Less => { - Limits { - heap_memory_size, - heap_address_space_size: slot.limits.heap_address_space_size, - stack_size: slot.limits.stack_size, - globals_size: slot.limits.globals_size, - signal_stack_size: slot.limits.signal_stack_size, - } - }, - Ordering::Equal => &slot.limits, - - }; -*/ - - module.validate_runtime_spec(limits)?; + let mut limits = slot.limits.clone(); + + // Affirm that any custom heap memory size supplied to this + // builder does not exceed the slot limits. + if let Ordering::Greater = heap_memory_size.cmp(&slot.limits.heap_memory_size) { + return Err(Error::InvalidArgument( + "heap memory size requested for instance is larger than slot allows", + )); + } + + // If a custom heap memory size is supplied, augment the limits + // with the value so that it may be validated. + if let Ordering::Less = heap_memory_size.cmp(&slot.limits.heap_memory_size) { + limits.heap_memory_size = heap_memory_size; + } + + module.validate_runtime_spec(&limits)?; for (ptr, len) in [ // make the stack read/writable From 85ef2c88a3efb2e5cf3d887fa5d43e04676de22b Mon Sep 17 00:00:00 2001 From: "Tanya L. Crenshaw" Date: Tue, 24 Mar 2020 23:24:53 +0000 Subject: [PATCH 04/38] Initializing heap_memory_size with a more sensible value. --- lucet-runtime/lucet-runtime-internals/src/region.rs | 5 +++-- lucet-runtime/lucet-runtime-internals/src/region/mmap.rs | 4 ++++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/lucet-runtime/lucet-runtime-internals/src/region.rs b/lucet-runtime/lucet-runtime-internals/src/region.rs index 5df1e5ce8..e5bd16edf 100644 --- a/lucet-runtime/lucet-runtime-internals/src/region.rs +++ b/lucet-runtime/lucet-runtime-internals/src/region.rs @@ -68,7 +68,8 @@ pub trait RegionInternal: Send + Sync { fn expand_heap(&self, slot: &Slot, start: u32, len: u32) -> Result<(), Error>; fn reset_heap(&self, alloc: &mut Alloc, module: &dyn Module) -> Result<(), Error>; - + fn heap_memory_size_limit(&self) -> usize; + fn as_dyn_internal(&self) -> &dyn RegionInternal; } @@ -100,7 +101,7 @@ impl<'a> InstanceBuilder<'a> { region, module, embed_ctx: CtxMap::default(), - heap_memory_size: 16 * 64 * 1024, + heap_memory_size: region.heap_memory_size_limit(), // 16 * 64 * 1024, // TLC The default limit should match the slot limit? } } diff --git a/lucet-runtime/lucet-runtime-internals/src/region/mmap.rs b/lucet-runtime/lucet-runtime-internals/src/region/mmap.rs index 631a21e5f..8fbf06063 100644 --- a/lucet-runtime/lucet-runtime-internals/src/region/mmap.rs +++ b/lucet-runtime/lucet-runtime-internals/src/region/mmap.rs @@ -273,6 +273,10 @@ impl RegionInternal for MmapRegion { Ok(()) } + fn heap_memory_size_limit(&self) -> usize { + self.limits.heap_memory_size + } + fn as_dyn_internal(&self) -> &dyn RegionInternal { self } From cb2270bf1c02d907ef55ac7fdd764fde8db28c70 Mon Sep 17 00:00:00 2001 From: "Tanya L. Crenshaw" Date: Wed, 25 Mar 2020 17:32:26 +0000 Subject: [PATCH 05/38] Fmt --- lucet-runtime/lucet-runtime-internals/src/region.rs | 2 +- lucet-runtime/lucet-runtime-internals/src/region/mmap.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lucet-runtime/lucet-runtime-internals/src/region.rs b/lucet-runtime/lucet-runtime-internals/src/region.rs index e5bd16edf..433b50c37 100644 --- a/lucet-runtime/lucet-runtime-internals/src/region.rs +++ b/lucet-runtime/lucet-runtime-internals/src/region.rs @@ -69,7 +69,7 @@ pub trait RegionInternal: Send + Sync { fn reset_heap(&self, alloc: &mut Alloc, module: &dyn Module) -> Result<(), Error>; fn heap_memory_size_limit(&self) -> usize; - + fn as_dyn_internal(&self) -> &dyn RegionInternal; } diff --git a/lucet-runtime/lucet-runtime-internals/src/region/mmap.rs b/lucet-runtime/lucet-runtime-internals/src/region/mmap.rs index 8fbf06063..89bd31269 100644 --- a/lucet-runtime/lucet-runtime-internals/src/region/mmap.rs +++ b/lucet-runtime/lucet-runtime-internals/src/region/mmap.rs @@ -274,7 +274,7 @@ impl RegionInternal for MmapRegion { } fn heap_memory_size_limit(&self) -> usize { - self.limits.heap_memory_size + self.limits.heap_memory_size } fn as_dyn_internal(&self) -> &dyn RegionInternal { From df004c88bd4573217778a078c1567d230f3db2a5 Mon Sep 17 00:00:00 2001 From: "Tanya L. Crenshaw" Date: Wed, 25 Mar 2020 18:57:11 +0000 Subject: [PATCH 06/38] Remove silly comment. --- lucet-runtime/lucet-runtime-internals/src/region.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lucet-runtime/lucet-runtime-internals/src/region.rs b/lucet-runtime/lucet-runtime-internals/src/region.rs index 433b50c37..776617f06 100644 --- a/lucet-runtime/lucet-runtime-internals/src/region.rs +++ b/lucet-runtime/lucet-runtime-internals/src/region.rs @@ -101,7 +101,7 @@ impl<'a> InstanceBuilder<'a> { region, module, embed_ctx: CtxMap::default(), - heap_memory_size: region.heap_memory_size_limit(), // 16 * 64 * 1024, // TLC The default limit should match the slot limit? + heap_memory_size: region.heap_memory_size_limit(), } } From 252b9b5fbccd8aa58fc2f4fbc9e8b326b49d892e Mon Sep 17 00:00:00 2001 From: "Tanya L. Crenshaw" Date: Wed, 25 Mar 2020 23:19:18 +0000 Subject: [PATCH 07/38] Address PR comments --- .../lucet-runtime-internals/src/alloc/tests.rs | 8 ++------ lucet-runtime/lucet-runtime-internals/src/region.rs | 11 ++++++----- 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs b/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs index d39d6e2c3..4edd24a57 100644 --- a/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs +++ b/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs @@ -600,13 +600,11 @@ macro_rules! alloc_tests { let region = TestRegion::create(1, &CONTEXT_TEST_LIMITS).expect("region created"); let mut inst = region - .new_instance_builder( + .new_instance( MockModuleBuilder::new() .with_heap_spec(CONTEXT_TEST_HEAP) .build(), ) - .with_heap_size_limit(4096) - .build() .expect("new_instance succeeds"); let mut parent = ContextHandle::new(); @@ -652,13 +650,11 @@ macro_rules! alloc_tests { let region = TestRegion::create(1, &CONTEXT_TEST_LIMITS).expect("region created"); let mut inst = region - .new_instance_builder( + .new_instance( MockModuleBuilder::new() .with_heap_spec(CONTEXT_TEST_HEAP) .build(), ) - .with_heap_size_limit(4096) - .build() .expect("new_instance succeeds"); let mut parent = ContextHandle::new(); diff --git a/lucet-runtime/lucet-runtime-internals/src/region.rs b/lucet-runtime/lucet-runtime-internals/src/region.rs index 776617f06..54185fea1 100644 --- a/lucet-runtime/lucet-runtime-internals/src/region.rs +++ b/lucet-runtime/lucet-runtime-internals/src/region.rs @@ -92,7 +92,7 @@ pub struct InstanceBuilder<'a> { region: &'a dyn RegionInternal, module: Arc, embed_ctx: CtxMap, - heap_memory_size: usize, + heap_memory_size_limit: usize, } impl<'a> InstanceBuilder<'a> { @@ -101,15 +101,16 @@ impl<'a> InstanceBuilder<'a> { region, module, embed_ctx: CtxMap::default(), - heap_memory_size: region.heap_memory_size_limit(), + heap_memory_size_limit: region.heap_memory_size_limit(), } } /// Add a custom limit for the heap memory size to the built instance. + /// /// This call is not necessary if the default heap memory size is adequate /// for the new instance. - pub fn with_heap_size_limit(mut self, heap_memory_size: usize) -> Self { - self.heap_memory_size = heap_memory_size; + pub fn with_heap_size_limit(mut self, heap_memory_size_limit: usize) -> Self { + self.heap_memory_size_limit = heap_memory_size_limit; self } @@ -130,6 +131,6 @@ impl<'a> InstanceBuilder<'a> { /// code is potentially unsafe; see [`Instance::run()`](struct.Instance.html#method.run). pub fn build(self) -> Result { self.region - .new_instance_with(self.module, self.embed_ctx, self.heap_memory_size) + .new_instance_with(self.module, self.embed_ctx, self.heap_memory_size_limit) } } From 42e9884927eae2ea2476a974b04fee969a1dd462 Mon Sep 17 00:00:00 2001 From: "Tanya L. Crenshaw" Date: Wed, 25 Mar 2020 23:24:15 +0000 Subject: [PATCH 08/38] Address PR comments --- lucet-runtime/lucet-runtime-internals/src/region.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lucet-runtime/lucet-runtime-internals/src/region.rs b/lucet-runtime/lucet-runtime-internals/src/region.rs index 54185fea1..26bd120b1 100644 --- a/lucet-runtime/lucet-runtime-internals/src/region.rs +++ b/lucet-runtime/lucet-runtime-internals/src/region.rs @@ -68,7 +68,7 @@ pub trait RegionInternal: Send + Sync { fn expand_heap(&self, slot: &Slot, start: u32, len: u32) -> Result<(), Error>; fn reset_heap(&self, alloc: &mut Alloc, module: &dyn Module) -> Result<(), Error>; - fn heap_memory_size_limit(&self) -> usize; + fn get_limits(&self) -> &Limits; fn as_dyn_internal(&self) -> &dyn RegionInternal; } From e31382100ba4ecb857be6fd88fef07e05c797f70 Mon Sep 17 00:00:00 2001 From: "Tanya L. Crenshaw" Date: Wed, 25 Mar 2020 23:36:16 +0000 Subject: [PATCH 09/38] Address PR comments. --- lucet-runtime/lucet-runtime-internals/src/region.rs | 2 +- lucet-runtime/lucet-runtime-internals/src/region/mmap.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lucet-runtime/lucet-runtime-internals/src/region.rs b/lucet-runtime/lucet-runtime-internals/src/region.rs index 26bd120b1..63bf95c34 100644 --- a/lucet-runtime/lucet-runtime-internals/src/region.rs +++ b/lucet-runtime/lucet-runtime-internals/src/region.rs @@ -101,7 +101,7 @@ impl<'a> InstanceBuilder<'a> { region, module, embed_ctx: CtxMap::default(), - heap_memory_size_limit: region.heap_memory_size_limit(), + heap_memory_size_limit: region.get_limits().heap_memory_size, } } diff --git a/lucet-runtime/lucet-runtime-internals/src/region/mmap.rs b/lucet-runtime/lucet-runtime-internals/src/region/mmap.rs index 89bd31269..b622863fc 100644 --- a/lucet-runtime/lucet-runtime-internals/src/region/mmap.rs +++ b/lucet-runtime/lucet-runtime-internals/src/region/mmap.rs @@ -273,8 +273,8 @@ impl RegionInternal for MmapRegion { Ok(()) } - fn heap_memory_size_limit(&self) -> usize { - self.limits.heap_memory_size + fn get_limits(&self) -> &Limits { + &self.limits } fn as_dyn_internal(&self) -> &dyn RegionInternal { From cfb324fb810edc38a90bcabac2605c4b683af16a Mon Sep 17 00:00:00 2001 From: "Tanya L. Crenshaw" Date: Wed, 25 Mar 2020 23:41:21 +0000 Subject: [PATCH 10/38] Address PR comments. --- lucet-runtime/lucet-runtime-internals/src/alloc/mod.rs | 2 +- lucet-runtime/lucet-runtime-internals/src/region/mmap.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lucet-runtime/lucet-runtime-internals/src/alloc/mod.rs b/lucet-runtime/lucet-runtime-internals/src/alloc/mod.rs index cfbca292d..459798355 100644 --- a/lucet-runtime/lucet-runtime-internals/src/alloc/mod.rs +++ b/lucet-runtime/lucet-runtime-internals/src/alloc/mod.rs @@ -396,7 +396,7 @@ impl Alloc { /// Runtime limits for the various memories that back a Lucet instance. /// /// Each value is specified in bytes, and must be evenly divisible by the host page size (4K). -#[derive(Clone, Debug)] +#[derive(Clone, Copy, Debug)] #[repr(C)] pub struct Limits { /// Max size of the heap, which can be backed by real memory. (default 1M) diff --git a/lucet-runtime/lucet-runtime-internals/src/region/mmap.rs b/lucet-runtime/lucet-runtime-internals/src/region/mmap.rs index b622863fc..2fccb49b4 100644 --- a/lucet-runtime/lucet-runtime-internals/src/region/mmap.rs +++ b/lucet-runtime/lucet-runtime-internals/src/region/mmap.rs @@ -92,7 +92,7 @@ impl RegionInternal for MmapRegion { lucet_bail!("heap is not page-aligned; this is a bug"); } - let mut limits = slot.limits.clone(); + let mut limits = slot.limits; // Affirm that any custom heap memory size supplied to this // builder does not exceed the slot limits. From 64ce8b088ae53fd6c535ca0526355fe81fe09bce Mon Sep 17 00:00:00 2001 From: "Tanya L. Crenshaw" Date: Thu, 26 Mar 2020 17:08:42 +0000 Subject: [PATCH 11/38] Use match --- .../src/region/mmap.rs | 30 ++++++++++--------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/lucet-runtime/lucet-runtime-internals/src/region/mmap.rs b/lucet-runtime/lucet-runtime-internals/src/region/mmap.rs index 2fccb49b4..19282f4e2 100644 --- a/lucet-runtime/lucet-runtime-internals/src/region/mmap.rs +++ b/lucet-runtime/lucet-runtime-internals/src/region/mmap.rs @@ -92,20 +92,22 @@ impl RegionInternal for MmapRegion { lucet_bail!("heap is not page-aligned; this is a bug"); } - let mut limits = slot.limits; - - // Affirm that any custom heap memory size supplied to this - // builder does not exceed the slot limits. - if let Ordering::Greater = heap_memory_size.cmp(&slot.limits.heap_memory_size) { - return Err(Error::InvalidArgument( - "heap memory size requested for instance is larger than slot allows", - )); - } - - // If a custom heap memory size is supplied, augment the limits - // with the value so that it may be validated. - if let Ordering::Less = heap_memory_size.cmp(&slot.limits.heap_memory_size) { - limits.heap_memory_size = heap_memory_size; + let mut limits; + + match heap_memory_size.cmp(&slot.limits.heap_memory_size) { + Ordering::Less => { + // The supplied heap_memory_size is smaller than + // default. Augment the new instance limits with this + // // custom value so that it may be validated. + limits = slot.limits; + limits.heap_memory_size = heap_memory_size; + } + Ordering::Equal => limits = slot.limits, + Ordering::Greater => { + return Err(Error::InvalidArgument( + "heap memory size requested for instance is larger than slot allows", + )) + } } module.validate_runtime_spec(&limits)?; From 7c95d267df35eff6a58802a04202f01770c13d2c Mon Sep 17 00:00:00 2001 From: "Tanya L. Crenshaw" Date: Thu, 26 Mar 2020 17:19:02 +0000 Subject: [PATCH 12/38] Address PR comments. --- lucet-runtime/lucet-runtime-internals/src/region/mmap.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lucet-runtime/lucet-runtime-internals/src/region/mmap.rs b/lucet-runtime/lucet-runtime-internals/src/region/mmap.rs index 19282f4e2..bb5842277 100644 --- a/lucet-runtime/lucet-runtime-internals/src/region/mmap.rs +++ b/lucet-runtime/lucet-runtime-internals/src/region/mmap.rs @@ -79,7 +79,7 @@ impl RegionInternal for MmapRegion { &self, module: Arc, embed_ctx: CtxMap, - heap_memory_size: usize, + heap_memory_size_limit: usize, ) -> Result { let slot = self .freelist @@ -94,13 +94,13 @@ impl RegionInternal for MmapRegion { let mut limits; - match heap_memory_size.cmp(&slot.limits.heap_memory_size) { + match heap_memory_size_limit.cmp(&slot.limits.heap_memory_size) { Ordering::Less => { // The supplied heap_memory_size is smaller than // default. Augment the new instance limits with this // // custom value so that it may be validated. limits = slot.limits; - limits.heap_memory_size = heap_memory_size; + limits.heap_memory_size = heap_memory_size_limit; } Ordering::Equal => limits = slot.limits, Ordering::Greater => { From 0216cdfe720efb35d4d32abf89926c4c0d695710 Mon Sep 17 00:00:00 2001 From: "Tanya L. Crenshaw" Date: Thu, 26 Mar 2020 17:20:10 +0000 Subject: [PATCH 13/38] Make diff smaller. --- lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs b/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs index 4edd24a57..15e32eafe 100644 --- a/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs +++ b/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs @@ -598,7 +598,6 @@ macro_rules! alloc_tests { } let region = TestRegion::create(1, &CONTEXT_TEST_LIMITS).expect("region created"); - let mut inst = region .new_instance( MockModuleBuilder::new() From c97049dddd67b4df6c8c780cbefeffeb42bece5d Mon Sep 17 00:00:00 2001 From: "Tanya L. Crenshaw" Date: Thu, 26 Mar 2020 17:20:33 +0000 Subject: [PATCH 14/38] Make diff smaller. --- lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs b/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs index 15e32eafe..1b553fca6 100644 --- a/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs +++ b/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs @@ -647,7 +647,6 @@ macro_rules! alloc_tests { } let region = TestRegion::create(1, &CONTEXT_TEST_LIMITS).expect("region created"); - let mut inst = region .new_instance( MockModuleBuilder::new() From ba3196ac202bf929b1b204f61b51ad09999eec2d Mon Sep 17 00:00:00 2001 From: "Tanya L. Crenshaw" Date: Thu, 26 Mar 2020 20:06:34 +0000 Subject: [PATCH 15/38] Test is failing. Should it? --- .../src/alloc/tests.rs | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs b/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs index 1b553fca6..b723c131f 100644 --- a/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs +++ b/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs @@ -191,6 +191,40 @@ macro_rules! alloc_tests { assert_eq!(heap[new_heap_len - 1], 0xFF); } + /// This test exercises custom limits on the heap_memory_size. + /// In this scenario, the Region has a default limit on the + /// heap memory size, but the instance has a smaller limit. + /// Attemps to expand the heap fail, but the existing heap can + /// still be used. This test uses a region with multiple slots + /// in order to exercise more edge cases with adjacent managed + /// memory. + #[test] + fn expand_past_spec_max_with_custom_limit() { + let region = TestRegion::create(10, &LIMITS).expect("region created"); + let module = MockModuleBuilder::new() + .with_heap_spec(THREE_PAGE_MAX_HEAP) + .build(); + let mut inst = region + .new_instance_builder(module.clone()) + .with_heap_size_limit((THREEPAGE_INITIAL_SIZE) as usize) + .build() + .expect("new_instance succeeds"); + + let heap_len = inst.alloc().heap_len(); + assert_eq!(heap_len, THREEPAGE_INITIAL_SIZE as usize); + + // expand the heap within the heap spec limit but past the + // instance limit. + let new_heap_area = inst.alloc_mut().expand_heap( + (THREEPAGE_MAX_SIZE - THREEPAGE_INITIAL_SIZE) as u32, + module.as_ref(), + ); + assert!( + new_heap_area.is_err(), + "heap expansion past instance limit fails" + ); + } + const EXPANDPASTLIMIT_INITIAL_SIZE: u64 = LIMITS_HEAP_MEM_SIZE as u64 - (64 * 1024); const EXPANDPASTLIMIT_MAX_SIZE: u64 = LIMITS_HEAP_MEM_SIZE as u64 + (64 * 1024); const EXPAND_PAST_LIMIT_SPEC: HeapSpec = HeapSpec { From c153558a7c44a3161daebb14e24dfdaa1a29ecb2 Mon Sep 17 00:00:00 2001 From: "Tanya L. Crenshaw" Date: Fri, 27 Mar 2020 18:43:32 +0000 Subject: [PATCH 16/38] Ok. --- .../lucet-runtime-internals/src/module.rs | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/lucet-runtime/lucet-runtime-internals/src/module.rs b/lucet-runtime/lucet-runtime-internals/src/module.rs index d08ad2a8d..d0004ea75 100644 --- a/lucet-runtime/lucet-runtime-internals/src/module.rs +++ b/lucet-runtime/lucet-runtime-internals/src/module.rs @@ -127,6 +127,15 @@ pub trait ModuleInternal: Send + Sync { if heap.initial_size as usize > limits.heap_memory_size { bail_limits_exceeded!("heap spec initial size: {:?}", heap); } + + /* + if heap.reserved_size > limits.heap_memory_size as u64 { + bail_limits_exceeded!( + "heap spec reserved size exceeds heap memory size limits: {:?}", + heap + ); + } + */ if heap.initial_size > heap.reserved_size { return Err(lucet_incorrect_module!( @@ -134,6 +143,16 @@ pub trait ModuleInternal: Send + Sync { heap )); } + + if let Some(max_size) = heap.max_size { + if max_size > heap.reserved_size { + return Err(lucet_incorrect_module!( + "maximum heap size greater than reserved size: {:?}", + heap + )); + } + } + } if self.globals().len() * std::mem::size_of::() > limits.globals_size { From f7fb5083fea7ffaaec2000aa1daa96c14275672b Mon Sep 17 00:00:00 2001 From: "Tanya L. Crenshaw" Date: Fri, 27 Mar 2020 20:44:12 +0000 Subject: [PATCH 17/38] Remove silly. --- lucet-runtime/lucet-runtime-internals/src/module.rs | 9 --------- 1 file changed, 9 deletions(-) diff --git a/lucet-runtime/lucet-runtime-internals/src/module.rs b/lucet-runtime/lucet-runtime-internals/src/module.rs index d0004ea75..e9b80da37 100644 --- a/lucet-runtime/lucet-runtime-internals/src/module.rs +++ b/lucet-runtime/lucet-runtime-internals/src/module.rs @@ -128,15 +128,6 @@ pub trait ModuleInternal: Send + Sync { bail_limits_exceeded!("heap spec initial size: {:?}", heap); } - /* - if heap.reserved_size > limits.heap_memory_size as u64 { - bail_limits_exceeded!( - "heap spec reserved size exceeds heap memory size limits: {:?}", - heap - ); - } - */ - if heap.initial_size > heap.reserved_size { return Err(lucet_incorrect_module!( "initial heap size greater than reserved size: {:?}", From 7f194468e5ad89fd136f34187a96b99218f52874 Mon Sep 17 00:00:00 2001 From: "Tanya L. Crenshaw" Date: Fri, 27 Mar 2020 21:25:04 +0000 Subject: [PATCH 18/38] Fixed broken test. --- .../lucet-runtime-internals/src/alloc/mod.rs | 2 ++ lucet-runtime/lucet-runtime-internals/src/module.rs | 9 --------- .../lucet-runtime-internals/src/region/mmap.rs | 12 +++++------- 3 files changed, 7 insertions(+), 16 deletions(-) diff --git a/lucet-runtime/lucet-runtime-internals/src/alloc/mod.rs b/lucet-runtime/lucet-runtime-internals/src/alloc/mod.rs index 459798355..cfd86749a 100644 --- a/lucet-runtime/lucet-runtime-internals/src/alloc/mod.rs +++ b/lucet-runtime/lucet-runtime-internals/src/alloc/mod.rs @@ -241,6 +241,8 @@ impl Alloc { } // The runtime sets a limit on how much of the heap can be backed by real memory. Don't let // the heap expand beyond that: + // TLC: I'm not actually updating the slot limits. I am updating my copy of + // the slot limits. Which isn't cool. if self.heap_accessible_size + expand_pagealigned as usize > slot.limits.heap_memory_size { bail_limits_exceeded!( "expansion would exceed runtime-specified heap limit: {:?}", diff --git a/lucet-runtime/lucet-runtime-internals/src/module.rs b/lucet-runtime/lucet-runtime-internals/src/module.rs index e9b80da37..b779a05ca 100644 --- a/lucet-runtime/lucet-runtime-internals/src/module.rs +++ b/lucet-runtime/lucet-runtime-internals/src/module.rs @@ -135,15 +135,6 @@ pub trait ModuleInternal: Send + Sync { )); } - if let Some(max_size) = heap.max_size { - if max_size > heap.reserved_size { - return Err(lucet_incorrect_module!( - "maximum heap size greater than reserved size: {:?}", - heap - )); - } - } - } if self.globals().len() * std::mem::size_of::() > limits.globals_size { diff --git a/lucet-runtime/lucet-runtime-internals/src/region/mmap.rs b/lucet-runtime/lucet-runtime-internals/src/region/mmap.rs index bb5842277..30275b40c 100644 --- a/lucet-runtime/lucet-runtime-internals/src/region/mmap.rs +++ b/lucet-runtime/lucet-runtime-internals/src/region/mmap.rs @@ -81,7 +81,7 @@ impl RegionInternal for MmapRegion { embed_ctx: CtxMap, heap_memory_size_limit: usize, ) -> Result { - let slot = self + let mut slot = self .freelist .write() .unwrap() @@ -92,17 +92,14 @@ impl RegionInternal for MmapRegion { lucet_bail!("heap is not page-aligned; this is a bug"); } - let mut limits; - match heap_memory_size_limit.cmp(&slot.limits.heap_memory_size) { Ordering::Less => { // The supplied heap_memory_size is smaller than // default. Augment the new instance limits with this - // // custom value so that it may be validated. - limits = slot.limits; - limits.heap_memory_size = heap_memory_size_limit; + // custom value so that it may be validated. + slot.limits.heap_memory_size = heap_memory_size_limit; } - Ordering::Equal => limits = slot.limits, + Ordering::Equal => (), Ordering::Greater => { return Err(Error::InvalidArgument( "heap memory size requested for instance is larger than slot allows", @@ -110,6 +107,7 @@ impl RegionInternal for MmapRegion { } } + let limits = &slot.limits; module.validate_runtime_spec(&limits)?; for (ptr, len) in [ From 0902b8263d1765c01cd9951429f2516bc50bd2ad Mon Sep 17 00:00:00 2001 From: "Tanya L. Crenshaw" Date: Fri, 27 Mar 2020 22:02:11 +0000 Subject: [PATCH 19/38] Fmt. --- .../lucet-runtime-internals/src/alloc/mod.rs | 6 +++--- .../lucet-runtime-internals/src/alloc/tests.rs | 11 ++++++++++- lucet-runtime/lucet-runtime-internals/src/module.rs | 3 +-- 3 files changed, 14 insertions(+), 6 deletions(-) diff --git a/lucet-runtime/lucet-runtime-internals/src/alloc/mod.rs b/lucet-runtime/lucet-runtime-internals/src/alloc/mod.rs index cfd86749a..b729a79e9 100644 --- a/lucet-runtime/lucet-runtime-internals/src/alloc/mod.rs +++ b/lucet-runtime/lucet-runtime-internals/src/alloc/mod.rs @@ -241,8 +241,8 @@ impl Alloc { } // The runtime sets a limit on how much of the heap can be backed by real memory. Don't let // the heap expand beyond that: - // TLC: I'm not actually updating the slot limits. I am updating my copy of - // the slot limits. Which isn't cool. + // TLC: I'm not actually updating the slot limits. I am updating my copy of + // the slot limits. Which isn't cool. if self.heap_accessible_size + expand_pagealigned as usize > slot.limits.heap_memory_size { bail_limits_exceeded!( "expansion would exceed runtime-specified heap limit: {:?}", @@ -398,7 +398,7 @@ impl Alloc { /// Runtime limits for the various memories that back a Lucet instance. /// /// Each value is specified in bytes, and must be evenly divisible by the host page size (4K). -#[derive(Clone, Copy, Debug)] +#[derive(Clone, Debug)] #[repr(C)] pub struct Limits { /// Max size of the heap, which can be backed by real memory. (default 1M) diff --git a/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs b/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs index b723c131f..de8c32a29 100644 --- a/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs +++ b/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs @@ -213,7 +213,7 @@ macro_rules! alloc_tests { let heap_len = inst.alloc().heap_len(); assert_eq!(heap_len, THREEPAGE_INITIAL_SIZE as usize); - // expand the heap within the heap spec limit but past the + // Expand the heap within the Region limit but past the // instance limit. let new_heap_area = inst.alloc_mut().expand_heap( (THREEPAGE_MAX_SIZE - THREEPAGE_INITIAL_SIZE) as u32, @@ -223,6 +223,15 @@ macro_rules! alloc_tests { new_heap_area.is_err(), "heap expansion past instance limit fails" ); + + // Affirm that the existing heap can still be used. + let new_heap_len = inst.alloc().heap_len(); + assert_eq!(new_heap_len, heap_len); + + 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 EXPANDPASTLIMIT_INITIAL_SIZE: u64 = LIMITS_HEAP_MEM_SIZE as u64 - (64 * 1024); diff --git a/lucet-runtime/lucet-runtime-internals/src/module.rs b/lucet-runtime/lucet-runtime-internals/src/module.rs index b779a05ca..d08ad2a8d 100644 --- a/lucet-runtime/lucet-runtime-internals/src/module.rs +++ b/lucet-runtime/lucet-runtime-internals/src/module.rs @@ -127,14 +127,13 @@ pub trait ModuleInternal: Send + Sync { if heap.initial_size as usize > limits.heap_memory_size { bail_limits_exceeded!("heap spec initial size: {:?}", heap); } - + if heap.initial_size > heap.reserved_size { return Err(lucet_incorrect_module!( "initial heap size greater than reserved size: {:?}", heap )); } - } if self.globals().len() * std::mem::size_of::() > limits.globals_size { From 0073db0a181dba18187e322a29f3baab04ef1804 Mon Sep 17 00:00:00 2001 From: "Tanya L. Crenshaw" Date: Fri, 27 Mar 2020 22:52:38 +0000 Subject: [PATCH 20/38] Passing all new tests. --- .../lucet-runtime-internals/src/alloc/mod.rs | 2 -- .../src/alloc/tests.rs | 31 ++++++++++++++++--- 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/lucet-runtime/lucet-runtime-internals/src/alloc/mod.rs b/lucet-runtime/lucet-runtime-internals/src/alloc/mod.rs index b729a79e9..cfbca292d 100644 --- a/lucet-runtime/lucet-runtime-internals/src/alloc/mod.rs +++ b/lucet-runtime/lucet-runtime-internals/src/alloc/mod.rs @@ -241,8 +241,6 @@ impl Alloc { } // The runtime sets a limit on how much of the heap can be backed by real memory. Don't let // the heap expand beyond that: - // TLC: I'm not actually updating the slot limits. I am updating my copy of - // the slot limits. Which isn't cool. if self.heap_accessible_size + expand_pagealigned as usize > slot.limits.heap_memory_size { bail_limits_exceeded!( "expansion would exceed runtime-specified heap limit: {:?}", diff --git a/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs b/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs index de8c32a29..c9eb8a4c0 100644 --- a/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs +++ b/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs @@ -192,8 +192,8 @@ macro_rules! alloc_tests { } /// This test exercises custom limits on the heap_memory_size. - /// In this scenario, the Region has a default limit on the - /// heap memory size, but the instance has a smaller limit. + /// In this scenario, the Region has a limit on the heap + /// memory size, but the instance has a smaller limit. /// Attemps to expand the heap fail, but the existing heap can /// still be used. This test uses a region with multiple slots /// in order to exercise more edge cases with adjacent managed @@ -864,13 +864,17 @@ macro_rules! alloc_tests { } } + /// This test exercises custom limits on the heap_memory_size. + /// In this scenario, the Region has a limit on the heap + /// memory size, but the instance has a larger limit. An + /// instance's custom limit must not exceed the Region's. #[test] - fn reject_too_large_heap_memory_size() { + fn reject_heap_memory_size_exeeds_region_limits() { let region = TestRegion::create(1, &LIMITS).expect("region created"); let res = region .new_instance_builder( MockModuleBuilder::new() - .with_heap_spec(CONTEXT_TEST_HEAP) + .with_heap_spec(THREE_PAGE_MAX_HEAP) .build(), ) .with_heap_size_limit(&LIMITS.heap_memory_size * 2) @@ -884,6 +888,25 @@ macro_rules! alloc_tests { Ok(_) => panic!("unexpected success"), } } + + /// This test exercises custom limits on the heap_memory_size. + /// In this scenario, the HeapSpec has a limit on the initial + /// heap memory size, but the instance has a smaller limit. + /// An instance's custom limit must not exceed the HeapSpec. + #[test] + fn reject_heap_memory_size_exeeds_instance_limits() { + let region = TestRegion::create(1, &LIMITS).expect("region created"); + let res = region + .new_instance_builder( + MockModuleBuilder::new() + .with_heap_spec(THREE_PAGE_MAX_HEAP) + .build(), + ) + .with_heap_size_limit((THREE_PAGE_MAX_HEAP.initial_size / 2) as usize) + .build(); + + assert!(res.is_err(), "new_instance fails"); + } }; } From 424ecdb8aa6dfb519f82b767fa013f69ba888663 Mon Sep 17 00:00:00 2001 From: "Tanya L. Crenshaw" Date: Fri, 27 Mar 2020 22:52:48 +0000 Subject: [PATCH 21/38] Fmt. --- .../lucet-runtime-internals/src/alloc/tests.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs b/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs index c9eb8a4c0..a24142bd2 100644 --- a/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs +++ b/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs @@ -864,10 +864,10 @@ macro_rules! alloc_tests { } } - /// This test exercises custom limits on the heap_memory_size. + /// This test exercises custom limits on the heap_memory_size. /// In this scenario, the Region has a limit on the heap /// memory size, but the instance has a larger limit. An - /// instance's custom limit must not exceed the Region's. + /// instance's custom limit must not exceed the Region's. #[test] fn reject_heap_memory_size_exeeds_region_limits() { let region = TestRegion::create(1, &LIMITS).expect("region created"); @@ -889,12 +889,12 @@ macro_rules! alloc_tests { } } - /// This test exercises custom limits on the heap_memory_size. + /// This test exercises custom limits on the heap_memory_size. /// In this scenario, the HeapSpec has a limit on the initial /// heap memory size, but the instance has a smaller limit. /// An instance's custom limit must not exceed the HeapSpec. - #[test] - fn reject_heap_memory_size_exeeds_instance_limits() { + #[test] + fn reject_heap_memory_size_exeeds_instance_limits() { let region = TestRegion::create(1, &LIMITS).expect("region created"); let res = region .new_instance_builder( @@ -905,7 +905,7 @@ macro_rules! alloc_tests { .with_heap_size_limit((THREE_PAGE_MAX_HEAP.initial_size / 2) as usize) .build(); - assert!(res.is_err(), "new_instance fails"); + assert!(res.is_err(), "new_instance fails"); } }; } From 7da7a4e84af80743c8b1c32dc03341ff9941bffd Mon Sep 17 00:00:00 2001 From: "Tanya L. Crenshaw" Date: Fri, 27 Mar 2020 23:24:43 +0000 Subject: [PATCH 22/38] Address PR comments. --- lucet-runtime/lucet-runtime-internals/src/region.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lucet-runtime/lucet-runtime-internals/src/region.rs b/lucet-runtime/lucet-runtime-internals/src/region.rs index 63bf95c34..870eaae67 100644 --- a/lucet-runtime/lucet-runtime-internals/src/region.rs +++ b/lucet-runtime/lucet-runtime-internals/src/region.rs @@ -57,7 +57,7 @@ pub trait RegionInternal: Send + Sync { &self, module: Arc, embed_ctx: CtxMap, - heap_memory_size: usize, + heap_memory_size_limit: usize, ) -> Result; /// Unmaps the heap, stack, and globals of an `Alloc`, while retaining the virtual address @@ -68,6 +68,8 @@ pub trait RegionInternal: Send + Sync { fn expand_heap(&self, slot: &Slot, start: u32, len: u32) -> Result<(), Error>; fn reset_heap(&self, alloc: &mut Alloc, module: &dyn Module) -> Result<(), Error>; + + /// Get the runtime memory size limits fn get_limits(&self) -> &Limits; fn as_dyn_internal(&self) -> &dyn RegionInternal; From 7b40d128de5cdd7e5705b99179db92ce83b78c8b Mon Sep 17 00:00:00 2001 From: "Tanya L. Crenshaw" Date: Fri, 27 Mar 2020 23:31:06 +0000 Subject: [PATCH 23/38] Fmt. --- lucet-runtime/lucet-runtime-internals/src/region.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lucet-runtime/lucet-runtime-internals/src/region.rs b/lucet-runtime/lucet-runtime-internals/src/region.rs index 870eaae67..b744db2d6 100644 --- a/lucet-runtime/lucet-runtime-internals/src/region.rs +++ b/lucet-runtime/lucet-runtime-internals/src/region.rs @@ -69,7 +69,7 @@ pub trait RegionInternal: Send + Sync { fn reset_heap(&self, alloc: &mut Alloc, module: &dyn Module) -> Result<(), Error>; - /// Get the runtime memory size limits + /// Get the runtime memory size limits fn get_limits(&self) -> &Limits; fn as_dyn_internal(&self) -> &dyn RegionInternal; From 3dd8eed89658ae7b5935914059260b48a060a15d Mon Sep 17 00:00:00 2001 From: "Tanya L. Crenshaw" Date: Tue, 31 Mar 2020 02:14:05 +0000 Subject: [PATCH 24/38] Add test for the mundane and the successful. --- .../src/alloc/tests.rs | 32 ++++++++++++++++--- 1 file changed, 27 insertions(+), 5 deletions(-) diff --git a/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs b/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs index a24142bd2..456fcf4ae 100644 --- a/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs +++ b/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs @@ -871,12 +871,11 @@ macro_rules! alloc_tests { #[test] fn reject_heap_memory_size_exeeds_region_limits() { let region = TestRegion::create(1, &LIMITS).expect("region created"); + let module = MockModuleBuilder::new() + .with_heap_spec(THREE_PAGE_MAX_HEAP) + .build(); let res = region - .new_instance_builder( - MockModuleBuilder::new() - .with_heap_spec(THREE_PAGE_MAX_HEAP) - .build(), - ) + .new_instance_builder(module.clone()) .with_heap_size_limit(&LIMITS.heap_memory_size * 2) .build(); @@ -887,6 +886,29 @@ macro_rules! alloc_tests { Err(e) => panic!("unexpected error: {}", e), Ok(_) => panic!("unexpected success"), } + } + + /// This test exercises custom limits on the heap_memory_size. + /// In this scenario, successfully create a custom-sized instance + /// and then a default-sized instance to affirm that a custom size + /// doesn't somehow overwrite the default size. + #[test] + fn custom_size_does_not_break_default() { + let region = TestRegion::create(2, &LIMITS).expect("region created"); + let module = MockModuleBuilder::new() + .with_heap_spec(THREE_PAGE_MAX_HEAP) + .build(); + + // Build an instance that is within custom limits. + let _custom_inst = region + .new_instance_builder(module.clone()) + .with_heap_size_limit(THREEPAGE_INITIAL_SIZE as usize) + .build() + .expect("new instance succeeds"); + + // Build a default-sized instance, to make sure the custom limits + // didn't break the defaults. let default_inst = + let _default_inst = region.new_instance(module.clone()).expect("new_instance succeeds"); } /// This test exercises custom limits on the heap_memory_size. From addc0dba9aa128d8e89439b1d1e0861d880e8527 Mon Sep 17 00:00:00 2001 From: "Tanya L. Crenshaw" Date: Tue, 31 Mar 2020 17:19:52 +0000 Subject: [PATCH 25/38] Passing all tests. The HeapSpec has an expectation of size. The instance has a limit on size. --- .../src/alloc/tests.rs | 59 +++++++++++-------- 1 file changed, 33 insertions(+), 26 deletions(-) diff --git a/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs b/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs index 456fcf4ae..c30e7c48b 100644 --- a/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs +++ b/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs @@ -192,12 +192,9 @@ macro_rules! alloc_tests { } /// This test exercises custom limits on the heap_memory_size. - /// In this scenario, the Region has a limit on the heap - /// memory size, but the instance has a smaller limit. - /// Attemps to expand the heap fail, but the existing heap can - /// still be used. This test uses a region with multiple slots - /// in order to exercise more edge cases with adjacent managed - /// memory. + /// In this scenario, the Region has a limit on the heap memory + /// size, but the instance has a smaller limit. Attemps to expand + /// the heap fail, but the existing heap can still be used. #[test] fn expand_past_spec_max_with_custom_limit() { let region = TestRegion::create(10, &LIMITS).expect("region created"); @@ -886,35 +883,45 @@ macro_rules! alloc_tests { Err(e) => panic!("unexpected error: {}", e), Ok(_) => panic!("unexpected success"), } - } - + } + /// This test exercises custom limits on the heap_memory_size. - /// In this scenario, successfully create a custom-sized instance - /// and then a default-sized instance to affirm that a custom size - /// doesn't somehow overwrite the default size. + /// In this scenario, successfully create a custom-sized + /// instance followed by a default-sized instance to affirm + /// that a custom size doesn't somehow overwrite the default size. #[test] - fn custom_size_does_not_break_default() { + fn custom_size_does_not_break_default() { let region = TestRegion::create(2, &LIMITS).expect("region created"); let module = MockModuleBuilder::new() .with_heap_spec(THREE_PAGE_MAX_HEAP) .build(); - - // Build an instance that is within custom limits. - let _custom_inst = region - .new_instance_builder(module.clone()) - .with_heap_size_limit(THREEPAGE_INITIAL_SIZE as usize) - .build() - .expect("new instance succeeds"); - // Build a default-sized instance, to make sure the custom limits - // didn't break the defaults. let default_inst = - let _default_inst = region.new_instance(module.clone()).expect("new_instance succeeds"); + // Build an instance that is has custom limits that are big + // enough to accommodate the HeapSpec. + let mut custom_inst = region + .new_instance_builder(module.clone()) + .with_heap_size_limit(THREE_PAGE_MAX_HEAP.initial_size as usize) + .build() + .expect("new instance succeeds"); + + // Affirm that its heap is the expected size. + let heap_len = custom_inst.alloc().heap_len(); + assert_eq!(heap_len, THREE_PAGE_MAX_HEAP.initial_size as usize); + + // Build a default-sized instance, to make sure the custom limits + // didn't break the defaults. + let default_inst = region + .new_instance(module.clone()) + .expect("new_instance succeeds"); + + // Affirm that its heap is the expected size. + let heap_len = default_inst.alloc().heap_len(); + assert_eq!(heap_len, THREEPAGE_INITIAL_SIZE as usize); } /// This test exercises custom limits on the heap_memory_size. - /// In this scenario, the HeapSpec has a limit on the initial - /// heap memory size, but the instance has a smaller limit. - /// An instance's custom limit must not exceed the HeapSpec. + /// In this scenario, the HeapSpec has an expectation for the + /// initial heap memory size, but the instance's limit is too small. #[test] fn reject_heap_memory_size_exeeds_instance_limits() { let region = TestRegion::create(1, &LIMITS).expect("region created"); @@ -923,7 +930,7 @@ macro_rules! alloc_tests { MockModuleBuilder::new() .with_heap_spec(THREE_PAGE_MAX_HEAP) .build(), - ) + ) .with_heap_size_limit((THREE_PAGE_MAX_HEAP.initial_size / 2) as usize) .build(); From 7996b40ee8f3f833a0dedae5dbc579e3ed33d97b Mon Sep 17 00:00:00 2001 From: "Tanya L. Crenshaw" Date: Tue, 31 Mar 2020 17:26:38 +0000 Subject: [PATCH 26/38] Passing all tests. The HeapSpec has an expectation of size. The instance has a limit on size. --- .../lucet-runtime-internals/src/alloc/tests.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs b/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs index c30e7c48b..bf9544fa1 100644 --- a/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs +++ b/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs @@ -193,8 +193,8 @@ macro_rules! alloc_tests { /// This test exercises custom limits on the heap_memory_size. /// In this scenario, the Region has a limit on the heap memory - /// size, but the instance has a smaller limit. Attemps to expand - /// the heap fail, but the existing heap can still be used. + /// size, but the instance has a smaller limit. Attemps to expand + /// the heap fail, but the existing heap can still be used. #[test] fn expand_past_spec_max_with_custom_limit() { let region = TestRegion::create(10, &LIMITS).expect("region created"); @@ -895,26 +895,26 @@ macro_rules! alloc_tests { let module = MockModuleBuilder::new() .with_heap_spec(THREE_PAGE_MAX_HEAP) .build(); - + // Build an instance that is has custom limits that are big - // enough to accommodate the HeapSpec. + // enough to accommodate the HeapSpec. let mut custom_inst = region .new_instance_builder(module.clone()) - .with_heap_size_limit(THREE_PAGE_MAX_HEAP.initial_size as usize) + .with_heap_size_limit(THREE_PAGE_MAX_HEAP.initial_size as usize) .build() .expect("new instance succeeds"); - // Affirm that its heap is the expected size. + // Affirm that its heap is the expected size. let heap_len = custom_inst.alloc().heap_len(); assert_eq!(heap_len, THREE_PAGE_MAX_HEAP.initial_size as usize); // Build a default-sized instance, to make sure the custom limits - // didn't break the defaults. + // didn't break the defaults. let default_inst = region .new_instance(module.clone()) .expect("new_instance succeeds"); - // Affirm that its heap is the expected size. + // Affirm that its heap is the expected size. let heap_len = default_inst.alloc().heap_len(); assert_eq!(heap_len, THREEPAGE_INITIAL_SIZE as usize); } @@ -930,7 +930,7 @@ macro_rules! alloc_tests { MockModuleBuilder::new() .with_heap_spec(THREE_PAGE_MAX_HEAP) .build(), - ) + ) .with_heap_size_limit((THREE_PAGE_MAX_HEAP.initial_size / 2) as usize) .build(); From fbb25f62b8821a620e73bda49b5ef782cc155ec7 Mon Sep 17 00:00:00 2001 From: "Tanya L. Crenshaw" Date: Tue, 31 Mar 2020 20:29:04 +0000 Subject: [PATCH 27/38] Make the test less ughy. --- .../src/alloc/tests.rs | 28 +++++++++++-------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs b/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs index bf9544fa1..0e4a8f7b7 100644 --- a/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs +++ b/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs @@ -892,31 +892,37 @@ macro_rules! alloc_tests { #[test] fn custom_size_does_not_break_default() { let region = TestRegion::create(2, &LIMITS).expect("region created"); - let module = MockModuleBuilder::new() - .with_heap_spec(THREE_PAGE_MAX_HEAP) - .build(); // Build an instance that is has custom limits that are big // enough to accommodate the HeapSpec. - let mut custom_inst = region - .new_instance_builder(module.clone()) - .with_heap_size_limit(THREE_PAGE_MAX_HEAP.initial_size as usize) + let custom_inst = region + .new_instance_builder( + MockModuleBuilder::new() + .with_heap_spec(THREE_PAGE_MAX_HEAP) + .build(), + ) + .with_heap_size_limit((THREE_PAGE_MAX_HEAP.initial_size * 2) as usize) .build() .expect("new instance succeeds"); - // Affirm that its heap is the expected size. + // Affirm that its heap is the expected size, the size + // specified in the HeapSpec. let heap_len = custom_inst.alloc().heap_len(); assert_eq!(heap_len, THREE_PAGE_MAX_HEAP.initial_size as usize); - // Build a default-sized instance, to make sure the custom limits - // didn't break the defaults. + // Build a default heap-limited instance, to make sure the + // custom limits didn't break the defaults. let default_inst = region - .new_instance(module.clone()) + .new_instance( + MockModuleBuilder::new() + .with_heap_spec(SMALL_GUARD_HEAP) + .build(), + ) .expect("new_instance succeeds"); // Affirm that its heap is the expected size. let heap_len = default_inst.alloc().heap_len(); - assert_eq!(heap_len, THREEPAGE_INITIAL_SIZE as usize); + assert_eq!(heap_len, SMALL_GUARD_HEAP.initial_size as usize); } /// This test exercises custom limits on the heap_memory_size. From 0e5aed79a1cd989b93cebab45c783890dd0deb96 Mon Sep 17 00:00:00 2001 From: "Tanya L. Crenshaw" <56939192+fst-crenshaw@users.noreply.github.com> Date: Tue, 31 Mar 2020 15:10:22 -0700 Subject: [PATCH 28/38] Update lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs Co-Authored-By: Adam C. Foltzer --- lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs b/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs index 0e4a8f7b7..6ee8b7a24 100644 --- a/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs +++ b/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs @@ -866,7 +866,7 @@ macro_rules! alloc_tests { /// memory size, but the instance has a larger limit. An /// instance's custom limit must not exceed the Region's. #[test] - fn reject_heap_memory_size_exeeds_region_limits() { + fn reject_heap_memory_size_exceeds_region_limits() { let region = TestRegion::create(1, &LIMITS).expect("region created"); let module = MockModuleBuilder::new() .with_heap_spec(THREE_PAGE_MAX_HEAP) From 4a9acdf9a5d4eac1b295808c374d6c421d89ac62 Mon Sep 17 00:00:00 2001 From: "Tanya L. Crenshaw" <56939192+fst-crenshaw@users.noreply.github.com> Date: Tue, 31 Mar 2020 15:10:56 -0700 Subject: [PATCH 29/38] Update lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs Co-Authored-By: Adam C. Foltzer --- lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs b/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs index 6ee8b7a24..d66457675 100644 --- a/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs +++ b/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs @@ -891,7 +891,7 @@ macro_rules! alloc_tests { /// that a custom size doesn't somehow overwrite the default size. #[test] fn custom_size_does_not_break_default() { - let region = TestRegion::create(2, &LIMITS).expect("region created"); + let region = TestRegion::create(1, &LIMITS).expect("region created"); // Build an instance that is has custom limits that are big // enough to accommodate the HeapSpec. From d39619270c7b08a93ffd77227a6516d0f5ea0225 Mon Sep 17 00:00:00 2001 From: "Tanya L. Crenshaw" <56939192+fst-crenshaw@users.noreply.github.com> Date: Tue, 31 Mar 2020 17:53:24 -0700 Subject: [PATCH 30/38] Update lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs Co-Authored-By: Adam C. Foltzer --- lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs b/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs index d66457675..0dd8a2597 100644 --- a/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs +++ b/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs @@ -909,7 +909,7 @@ macro_rules! alloc_tests { // specified in the HeapSpec. let heap_len = custom_inst.alloc().heap_len(); assert_eq!(heap_len, THREE_PAGE_MAX_HEAP.initial_size as usize); - + drop(custom_inst); // Build a default heap-limited instance, to make sure the // custom limits didn't break the defaults. let default_inst = region From a9ac3a214e09603b529fd70ab49e386df4ea455f Mon Sep 17 00:00:00 2001 From: "Tanya L. Crenshaw" Date: Thu, 2 Apr 2020 03:04:30 +0000 Subject: [PATCH 31/38] Be more clear about with_heap_size docs. --- lucet-runtime/lucet-runtime-internals/src/region.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lucet-runtime/lucet-runtime-internals/src/region.rs b/lucet-runtime/lucet-runtime-internals/src/region.rs index b744db2d6..698824014 100644 --- a/lucet-runtime/lucet-runtime-internals/src/region.rs +++ b/lucet-runtime/lucet-runtime-internals/src/region.rs @@ -107,10 +107,10 @@ impl<'a> InstanceBuilder<'a> { } } - /// Add a custom limit for the heap memory size to the built instance. + /// Add a smaller, custom limit for the heap memory size to the built instance. /// - /// This call is not necessary if the default heap memory size is adequate - /// for the new instance. + /// This call is optional. Attempts to create a new instance fail if the + /// limit supplied by with_heap_size_limit() exceeds that of the RegionInternal. pub fn with_heap_size_limit(mut self, heap_memory_size_limit: usize) -> Self { self.heap_memory_size_limit = heap_memory_size_limit; self From 5d7614132e3dc9c21ab5f08a5be6e871c2022365 Mon Sep 17 00:00:00 2001 From: "Tanya L. Crenshaw" Date: Thu, 2 Apr 2020 03:07:23 +0000 Subject: [PATCH 32/38] The usual fussing over words. --- lucet-runtime/lucet-runtime-internals/src/region.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lucet-runtime/lucet-runtime-internals/src/region.rs b/lucet-runtime/lucet-runtime-internals/src/region.rs index 698824014..1f067497b 100644 --- a/lucet-runtime/lucet-runtime-internals/src/region.rs +++ b/lucet-runtime/lucet-runtime-internals/src/region.rs @@ -109,8 +109,8 @@ impl<'a> InstanceBuilder<'a> { /// Add a smaller, custom limit for the heap memory size to the built instance. /// - /// This call is optional. Attempts to create a new instance fail if the - /// limit supplied by with_heap_size_limit() exceeds that of the RegionInternal. + /// This call is optional. Attempts to build a new instance fail if the + /// limit supplied by with_heap_size_limit() exceeds that of the region. pub fn with_heap_size_limit(mut self, heap_memory_size_limit: usize) -> Self { self.heap_memory_size_limit = heap_memory_size_limit; self From c104991fc0f3c0a22623cf8ae6f3c200559276d4 Mon Sep 17 00:00:00 2001 From: "Tanya L. Crenshaw" Date: Thu, 2 Apr 2020 03:58:42 +0000 Subject: [PATCH 33/38] Mmmaybe. --- .../lucet-runtime-internals/src/alloc/mod.rs | 5 +++-- .../lucet-runtime-internals/src/alloc/tests.rs | 4 +++- .../lucet-runtime-internals/src/region/mmap.rs | 12 ++++++------ 3 files changed, 12 insertions(+), 9 deletions(-) diff --git a/lucet-runtime/lucet-runtime-internals/src/alloc/mod.rs b/lucet-runtime/lucet-runtime-internals/src/alloc/mod.rs index cfbca292d..f54a721bc 100644 --- a/lucet-runtime/lucet-runtime-internals/src/alloc/mod.rs +++ b/lucet-runtime/lucet-runtime-internals/src/alloc/mod.rs @@ -101,6 +101,7 @@ impl Slot { pub struct Alloc { pub heap_accessible_size: usize, pub heap_inaccessible_size: usize, + pub heap_memory_size_limit: usize, pub slot: Option, pub region: Arc, } @@ -241,7 +242,7 @@ impl Alloc { } // The runtime sets a limit on how much of the heap can be backed by real memory. Don't let // the heap expand beyond that: - if self.heap_accessible_size + expand_pagealigned as usize > slot.limits.heap_memory_size { + if self.heap_accessible_size + expand_pagealigned as usize > self.heap_memory_size_limit { bail_limits_exceeded!( "expansion would exceed runtime-specified heap limit: {:?}", slot.limits @@ -396,7 +397,7 @@ impl Alloc { /// Runtime limits for the various memories that back a Lucet instance. /// /// Each value is specified in bytes, and must be evenly divisible by the host page size (4K). -#[derive(Clone, Debug)] +#[derive(Clone, Copy, Debug)] #[repr(C)] pub struct Limits { /// Max size of the heap, which can be backed by real memory. (default 1M) diff --git a/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs b/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs index 88d132fd0..90dcee571 100644 --- a/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs +++ b/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs @@ -907,7 +907,7 @@ macro_rules! alloc_tests { /// that a custom size doesn't somehow overwrite the default size. #[test] fn custom_size_does_not_break_default() { - let region = TestRegion::create(1, &LIMITS).expect("region created"); + let region = TestRegion::create(2, &LIMITS).expect("region created"); // Build an instance that is has custom limits that are big // enough to accommodate the HeapSpec. @@ -926,6 +926,7 @@ macro_rules! alloc_tests { let heap_len = custom_inst.alloc().heap_len(); assert_eq!(heap_len, THREE_PAGE_MAX_HEAP.initial_size as usize); drop(custom_inst); + // Build a default heap-limited instance, to make sure the // custom limits didn't break the defaults. let default_inst = region @@ -939,6 +940,7 @@ macro_rules! alloc_tests { // Affirm that its heap is the expected size. let heap_len = default_inst.alloc().heap_len(); assert_eq!(heap_len, SMALL_GUARD_HEAP.initial_size as usize); + } /// This test exercises custom limits on the heap_memory_size. diff --git a/lucet-runtime/lucet-runtime-internals/src/region/mmap.rs b/lucet-runtime/lucet-runtime-internals/src/region/mmap.rs index dcb73bd05..912a360d5 100644 --- a/lucet-runtime/lucet-runtime-internals/src/region/mmap.rs +++ b/lucet-runtime/lucet-runtime-internals/src/region/mmap.rs @@ -81,7 +81,7 @@ impl RegionInternal for MmapRegion { embed_ctx: CtxMap, heap_memory_size_limit: usize, ) -> Result { - let mut slot = self + let slot = self .freelist .write() .unwrap() @@ -92,12 +92,13 @@ impl RegionInternal for MmapRegion { lucet_bail!("heap is not page-aligned; this is a bug"); } + let mut limits = slot.limits; match heap_memory_size_limit.cmp(&slot.limits.heap_memory_size) { Ordering::Less => { // The supplied heap_memory_size is smaller than - // default. Augment the new instance limits with this - // custom value so that it may be validated. - slot.limits.heap_memory_size = heap_memory_size_limit; + // default. Augment the limits with this custom value + // so that it may be validated. + limits.heap_memory_size = heap_memory_size_limit; } Ordering::Equal => (), Ordering::Greater => { @@ -106,8 +107,6 @@ impl RegionInternal for MmapRegion { )) } } - - let limits = &slot.limits; module.validate_runtime_spec(&limits)?; for (ptr, len) in [ @@ -140,6 +139,7 @@ impl RegionInternal for MmapRegion { let alloc = Alloc { heap_accessible_size: 0, // the `reset` call in `new_instance_handle` will set this heap_inaccessible_size: slot.limits.heap_address_space_size, + heap_memory_size_limit, slot: Some(slot), region, }; From 4e98ba4e595ed0f03a92e5537f9307dc7e056223 Mon Sep 17 00:00:00 2001 From: "Tanya L. Crenshaw" Date: Thu, 2 Apr 2020 03:58:47 +0000 Subject: [PATCH 34/38] Mmmaybe. --- lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs | 3 +-- lucet-runtime/lucet-runtime-internals/src/region/mmap.rs | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs b/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs index 90dcee571..4786c921d 100644 --- a/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs +++ b/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs @@ -926,7 +926,7 @@ macro_rules! alloc_tests { let heap_len = custom_inst.alloc().heap_len(); assert_eq!(heap_len, THREE_PAGE_MAX_HEAP.initial_size as usize); drop(custom_inst); - + // Build a default heap-limited instance, to make sure the // custom limits didn't break the defaults. let default_inst = region @@ -940,7 +940,6 @@ macro_rules! alloc_tests { // Affirm that its heap is the expected size. let heap_len = default_inst.alloc().heap_len(); assert_eq!(heap_len, SMALL_GUARD_HEAP.initial_size as usize); - } /// This test exercises custom limits on the heap_memory_size. diff --git a/lucet-runtime/lucet-runtime-internals/src/region/mmap.rs b/lucet-runtime/lucet-runtime-internals/src/region/mmap.rs index 912a360d5..ecddfa88e 100644 --- a/lucet-runtime/lucet-runtime-internals/src/region/mmap.rs +++ b/lucet-runtime/lucet-runtime-internals/src/region/mmap.rs @@ -139,7 +139,7 @@ impl RegionInternal for MmapRegion { let alloc = Alloc { heap_accessible_size: 0, // the `reset` call in `new_instance_handle` will set this heap_inaccessible_size: slot.limits.heap_address_space_size, - heap_memory_size_limit, + heap_memory_size_limit, slot: Some(slot), region, }; From 44bc0b0c0227942a50add046456e06a7857aecb8 Mon Sep 17 00:00:00 2001 From: "Tanya L. Crenshaw" Date: Thu, 2 Apr 2020 04:00:42 +0000 Subject: [PATCH 35/38] Mmmaybe. --- lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs b/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs index 4786c921d..fbf893004 100644 --- a/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs +++ b/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs @@ -907,7 +907,7 @@ macro_rules! alloc_tests { /// that a custom size doesn't somehow overwrite the default size. #[test] fn custom_size_does_not_break_default() { - let region = TestRegion::create(2, &LIMITS).expect("region created"); + let region = TestRegion::create(1, &LIMITS).expect("region created"); // Build an instance that is has custom limits that are big // enough to accommodate the HeapSpec. From 2ebec05b1f13785cabec346cb7b37b1ead77bb7a Mon Sep 17 00:00:00 2001 From: "Tanya L. Crenshaw" Date: Thu, 2 Apr 2020 04:15:10 +0000 Subject: [PATCH 36/38] Fix spelling mistakes --- lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs b/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs index fbf893004..5b1151ee5 100644 --- a/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs +++ b/lucet-runtime/lucet-runtime-internals/src/alloc/tests.rs @@ -193,7 +193,7 @@ macro_rules! alloc_tests { /// This test exercises custom limits on the heap_memory_size. /// In this scenario, the Region has a limit on the heap memory - /// size, but the instance has a smaller limit. Attemps to expand + /// size, but the instance has a smaller limit. Attempts to expand /// the heap fail, but the existing heap can still be used. #[test] fn expand_past_spec_max_with_custom_limit() { @@ -903,8 +903,9 @@ macro_rules! alloc_tests { /// This test exercises custom limits on the heap_memory_size. /// In this scenario, successfully create a custom-sized - /// instance followed by a default-sized instance to affirm - /// that a custom size doesn't somehow overwrite the default size. + /// instance, drop it, then create a default-sized instance to + /// affirm that a custom size doesn't somehow overwrite the + /// default size. #[test] fn custom_size_does_not_break_default() { let region = TestRegion::create(1, &LIMITS).expect("region created"); From 72780f132b84aec94d4cb04b9a9db1fa9d591bb2 Mon Sep 17 00:00:00 2001 From: "Tanya L. Crenshaw" Date: Mon, 6 Apr 2020 20:43:47 +0000 Subject: [PATCH 37/38] But it is just so ugly. --- .../src/region/mmap.rs | 43 ++++++++++--------- 1 file changed, 23 insertions(+), 20 deletions(-) diff --git a/lucet-runtime/lucet-runtime-internals/src/region/mmap.rs b/lucet-runtime/lucet-runtime-internals/src/region/mmap.rs index 3d907bd3c..94141b8b6 100644 --- a/lucet-runtime/lucet-runtime-internals/src/region/mmap.rs +++ b/lucet-runtime/lucet-runtime-internals/src/region/mmap.rs @@ -73,7 +73,7 @@ impl Region for MmapRegion { self.capacity } } - + impl RegionInternal for MmapRegion { fn new_instance_with( &self, @@ -81,29 +81,21 @@ impl RegionInternal for MmapRegion { embed_ctx: CtxMap, heap_memory_size_limit: usize, ) -> Result { - // Affirm that the module, if instantiated, would not violate - // any runtime memory limits. - let limits = self.get_limits(); - module.validate_runtime_spec(limits)?; - - let slot = self - .freelist - .write() - .unwrap() - .pop() - .ok_or(Error::RegionFull(self.capacity))?; - - if slot.heap as usize % host_page_size() != 0 { - lucet_bail!("heap is not page-aligned; this is a bug"); - } + let custom_limits; + let mut limits = self.get_limits(); - let mut limits = slot.limits; - match heap_memory_size_limit.cmp(&slot.limits.heap_memory_size) { + // 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. - limits.heap_memory_size = heap_memory_size_limit; + custom_limits = Limits { + heap_memory_size: heap_memory_size_limit, + .. *limits + } ; + limits = &custom_limits; } Ordering::Equal => (), Ordering::Greater => { @@ -112,7 +104,18 @@ impl RegionInternal for MmapRegion { )) } } - module.validate_runtime_spec(&limits)?; + module.validate_runtime_spec(&limits)?; + + let slot = self + .freelist + .write() + .unwrap() + .pop() + .ok_or(Error::RegionFull(self.capacity))?; + + if slot.heap as usize % host_page_size() != 0 { + lucet_bail!("heap is not page-aligned; this is a bug"); + } for (ptr, len) in [ // make the stack read/writable From c550696165b5174da11670d7bccc6c1690d770ba Mon Sep 17 00:00:00 2001 From: "Tanya L. Crenshaw" Date: Mon, 6 Apr 2020 21:07:13 +0000 Subject: [PATCH 38/38] Fmt. --- .../src/region/mmap.rs | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/lucet-runtime/lucet-runtime-internals/src/region/mmap.rs b/lucet-runtime/lucet-runtime-internals/src/region/mmap.rs index 94141b8b6..4ad313e6e 100644 --- a/lucet-runtime/lucet-runtime-internals/src/region/mmap.rs +++ b/lucet-runtime/lucet-runtime-internals/src/region/mmap.rs @@ -73,7 +73,7 @@ impl Region for MmapRegion { self.capacity } } - + impl RegionInternal for MmapRegion { fn new_instance_with( &self, @@ -81,21 +81,21 @@ impl RegionInternal for MmapRegion { embed_ctx: CtxMap, heap_memory_size_limit: usize, ) -> Result { - let custom_limits; - let mut limits = self.get_limits(); + let custom_limits; + let mut limits = self.get_limits(); - // Affirm that the module, if instantiated, would not violate + // Affirm that the module, if instantiated, would not violate // any runtime memory limits. - match heap_memory_size_limit.cmp(&limits.heap_memory_size) { + 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; + custom_limits = Limits { + heap_memory_size: heap_memory_size_limit, + ..*limits + }; + limits = &custom_limits; } Ordering::Equal => (), Ordering::Greater => { @@ -104,8 +104,8 @@ impl RegionInternal for MmapRegion { )) } } - module.validate_runtime_spec(&limits)?; - + module.validate_runtime_spec(&limits)?; + let slot = self .freelist .write()