Skip to content
This repository was archived by the owner on Mar 24, 2022. It is now read-only.

Commit a88bb29

Browse files
authored
Merge pull request #401 from bytecodealliance/acf/alloc-layout-fix
Fix incorrect globals/sigstack allocation layout
2 parents 9fc93a4 + 7064e22 commit a88bb29

File tree

24 files changed

+359
-215
lines changed

24 files changed

+359
-215
lines changed

Cargo.lock

Lines changed: 69 additions & 69 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

benchmarks/lucet-benchmarks/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[package]
22
name = "lucet-benchmarks"
3-
version = "0.4.1"
3+
version = "0.5.1"
44
description = "Benchmarks for the Lucet runtime"
55
homepage = "https://github.com/fastly/lucet"
66
repository = "https://github.com/fastly/lucet"

lucet-module/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[package]
22
name = "lucet-module"
3-
version = "0.5.0"
3+
version = "0.5.1"
44
description = "A structured interface for Lucet modules"
55
homepage = "https://github.com/fastly/lucet"
66
repository = "https://github.com/fastly/lucet"

lucet-objdump/Cargo.toml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[package]
22
name = "lucet-objdump"
3-
version = "0.5.0"
3+
version = "0.5.1"
44
description = "Analyze object files emitted by the Lucet compiler"
55
homepage = "https://github.com/fastly/lucet"
66
repository = "https://github.com/fastly/lucet"
@@ -13,7 +13,7 @@ edition = "2018"
1313
goblin="0.0.24"
1414
byteorder="1.2.1"
1515
colored="1.8.0"
16-
lucet-module = { path = "../lucet-module", version = "=0.5.0" }
16+
lucet-module = { path = "../lucet-module", version = "=0.5.1" }
1717

1818
[package.metadata.deb]
1919
name = "fst-lucet-objdump"

lucet-runtime/Cargo.toml

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[package]
22
name = "lucet-runtime"
3-
version = "0.5.0"
3+
version = "0.5.1"
44
description = "Pure Rust runtime for Lucet WebAssembly toolchain"
55
homepage = "https://github.com/fastly/lucet"
66
repository = "https://github.com/fastly/lucet"
@@ -11,17 +11,17 @@ edition = "2018"
1111

1212
[dependencies]
1313
libc = "0.2.65"
14-
lucet-runtime-internals = { path = "lucet-runtime-internals", version = "=0.5.0" }
15-
lucet-module = { path = "../lucet-module", version = "=0.5.0" }
14+
lucet-runtime-internals = { path = "lucet-runtime-internals", version = "=0.5.1" }
15+
lucet-module = { path = "../lucet-module", version = "=0.5.1" }
1616
num-traits = "0.2"
1717
num-derive = "0.3.0"
1818

1919
[dev-dependencies]
2020
byteorder = "1.2"
2121
lazy_static = "1.1"
22-
lucetc = { path = "../lucetc", version = "=0.5.0" }
23-
lucet-runtime-tests = { path = "lucet-runtime-tests", version = "=0.5.0" }
24-
lucet-wasi-sdk = { path = "../lucet-wasi-sdk", version = "=0.5.0" }
22+
lucetc = { path = "../lucetc", version = "=0.5.1" }
23+
lucet-runtime-tests = { path = "lucet-runtime-tests", version = "=0.5.1" }
24+
lucet-wasi-sdk = { path = "../lucet-wasi-sdk", version = "=0.5.1" }
2525
nix = "0.15"
2626
rayon = "1.0"
2727
tempfile = "3.0"

lucet-runtime/include/lucet_types.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,13 @@ struct lucet_alloc_limits {
123123
* Size of the globals region in bytes; each global uses 8 bytes. (default 4K)
124124
*/
125125
uint64_t globals_size;
126+
/**
127+
* Size of the signal stack region in bytes.
128+
*
129+
* SIGSTKSZ from <signals.h> is a good default when linking against a Rust release build of
130+
* lucet-runtime, but 12K or more is recommended when using a Rust debug build.
131+
*/
132+
uint64_t signal_stack_size;
126133
};
127134

128135
typedef enum lucet_signal_behavior (*lucet_signal_handler)(struct lucet_instance * inst,

lucet-runtime/lucet-runtime-internals/Cargo.toml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[package]
22
name = "lucet-runtime-internals"
3-
version = "0.5.0"
3+
version = "0.5.1"
44
description = "Pure Rust runtime for Lucet WebAssembly toolchain (internals)"
55
homepage = "https://github.com/fastly/lucet"
66
repository = "https://github.com/fastly/lucet"
@@ -10,8 +10,8 @@ authors = ["Lucet team <[email protected]>"]
1010
edition = "2018"
1111

1212
[dependencies]
13-
lucet-module = { path = "../../lucet-module", version = "=0.5.0" }
14-
lucet-runtime-macros = { path = "../lucet-runtime-macros", version = "=0.5.0" }
13+
lucet-module = { path = "../../lucet-module", version = "=0.5.1" }
14+
lucet-runtime-macros = { path = "../lucet-runtime-macros", version = "=0.5.1" }
1515

1616
anyhow = "1.0"
1717
bitflags = "1.0"

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

Lines changed: 101 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use crate::error::Error;
22
use crate::module::Module;
33
use crate::region::RegionInternal;
4-
use libc::{c_void, SIGSTKSZ};
4+
use libc::c_void;
55
use lucet_module::GlobalValue;
66
use nix::unistd::{sysconf, SysconfVar};
77
use std::sync::{Arc, Once, Weak};
@@ -112,26 +112,79 @@ impl Drop for Alloc {
112112
}
113113
}
114114

115+
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
116+
pub enum AddrLocation {
117+
Heap,
118+
InaccessibleHeap,
119+
StackGuard,
120+
Stack,
121+
Globals,
122+
SigStackGuard,
123+
SigStack,
124+
Unknown,
125+
}
126+
127+
impl AddrLocation {
128+
/// If a fault occurs in this location, is it fatal to the entire process?
129+
///
130+
/// This is currently a permissive baseline that only returns true for unknown locations and the
131+
/// signal stack guard, in case a `Region` implementation uses faults to populate the accessible
132+
/// locations like the heap and the globals.
133+
pub fn is_fault_fatal(self) -> bool {
134+
use AddrLocation::*;
135+
match self {
136+
SigStackGuard | Unknown => true,
137+
_ => false,
138+
}
139+
}
140+
}
141+
115142
impl Alloc {
116-
pub fn addr_in_guard_page(&self, addr: *const c_void) -> bool {
143+
/// Where in an `Alloc` does a particular address fall?
144+
pub fn addr_location(&self, addr: *const c_void) -> AddrLocation {
117145
let addr = addr as usize;
118-
let heap = self.slot().heap as usize;
119-
let guard_start = heap + self.heap_accessible_size;
120-
let guard_end = heap + self.slot().limits.heap_address_space_size;
121-
// eprintln!(
122-
// "addr = {:p}, guard_start = {:p}, guard_end = {:p}",
123-
// addr, guard_start as *mut c_void, guard_end as *mut c_void
124-
// );
125-
let stack_guard_end = self.slot().stack as usize;
126-
let stack_guard_start = stack_guard_end - host_page_size();
127-
// eprintln!(
128-
// "addr = {:p}, stack_guard_start = {:p}, stack_guard_end = {:p}",
129-
// addr, stack_guard_start as *mut c_void, stack_guard_end as *mut c_void
130-
// );
131-
let in_heap_guard = (addr >= guard_start) && (addr < guard_end);
132-
let in_stack_guard = (addr >= stack_guard_start) && (addr < stack_guard_end);
133-
134-
in_heap_guard || in_stack_guard
146+
147+
let heap_start = self.slot().heap as usize;
148+
let heap_inaccessible_start = heap_start + self.heap_accessible_size;
149+
let heap_inaccessible_end = heap_start + self.slot().limits.heap_address_space_size;
150+
151+
if (addr >= heap_start) && (addr < heap_inaccessible_start) {
152+
return AddrLocation::Heap;
153+
}
154+
if (addr >= heap_inaccessible_start) && (addr < heap_inaccessible_end) {
155+
return AddrLocation::InaccessibleHeap;
156+
}
157+
158+
let stack_start = self.slot().stack as usize;
159+
let stack_end = stack_start + self.slot().limits.stack_size;
160+
let stack_guard_start = stack_start - host_page_size();
161+
162+
if (addr >= stack_guard_start) && (addr < stack_start) {
163+
return AddrLocation::StackGuard;
164+
}
165+
if (addr >= stack_start) && (addr < stack_end) {
166+
return AddrLocation::Stack;
167+
}
168+
169+
let globals_start = self.slot().globals as usize;
170+
let globals_end = globals_start + self.slot().limits.globals_size;
171+
172+
if (addr >= globals_start) && (addr < globals_end) {
173+
return AddrLocation::Globals;
174+
}
175+
176+
let sigstack_start = self.slot().sigstack as usize;
177+
let sigstack_end = sigstack_start + self.slot().limits.signal_stack_size;
178+
let sigstack_guard_start = sigstack_start - host_page_size();
179+
180+
if (addr >= sigstack_guard_start) && (addr < sigstack_start) {
181+
return AddrLocation::SigStackGuard;
182+
}
183+
if (addr >= sigstack_start) && (addr < sigstack_end) {
184+
return AddrLocation::SigStack;
185+
}
186+
187+
AddrLocation::Unknown
135188
}
136189

137190
pub fn expand_heap(&mut self, expand_bytes: u32, module: &dyn Module) -> Result<u32, Error> {
@@ -318,7 +371,10 @@ impl Alloc {
318371

319372
/// Return the sigstack as a mutable byte slice.
320373
pub unsafe fn sigstack_mut(&mut self) -> &mut [u8] {
321-
std::slice::from_raw_parts_mut(self.slot().sigstack as *mut u8, libc::SIGSTKSZ)
374+
std::slice::from_raw_parts_mut(
375+
self.slot().sigstack as *mut u8,
376+
self.slot().limits.signal_stack_size,
377+
)
322378
}
323379

324380
pub fn mem_in_heap<T>(&self, ptr: *const T, len: usize) -> bool {
@@ -351,15 +407,30 @@ pub struct Limits {
351407
pub stack_size: usize,
352408
/// Size of the globals region in bytes; each global uses 8 bytes. (default 4K)
353409
pub globals_size: usize,
410+
/// Size of the signal stack in bytes. (default SIGSTKSZ for release builds, 12K for debug builds)
411+
///
412+
/// This difference is to account for the greatly increased stack size usage in the signal
413+
/// handler when running without optimizations.
414+
///
415+
/// Note that debug vs. release mode is determined by `cfg(debug_assertions)`, so if you are
416+
/// specifically enabling debug assertions in your release builds, the default signal stack will
417+
/// be larger.
418+
pub signal_stack_size: usize,
354419
}
355420

356-
impl Default for Limits {
357-
fn default() -> Limits {
421+
#[cfg(debug_assertions)]
422+
pub const DEFAULT_SIGNAL_STACK_SIZE: usize = 12 * 1024;
423+
#[cfg(not(debug_assertions))]
424+
pub const DEFAULT_SIGNAL_STACK_SIZE: usize = libc::SIGSTKSZ;
425+
426+
impl Limits {
427+
pub const fn default() -> Limits {
358428
Limits {
359429
heap_memory_size: 16 * 64 * 1024,
360430
heap_address_space_size: 0x200000000,
361431
stack_size: 128 * 1024,
362432
globals_size: 4096,
433+
signal_stack_size: DEFAULT_SIGNAL_STACK_SIZE,
363434
}
364435
}
365436
}
@@ -370,19 +441,18 @@ impl Limits {
370441
// * the instance (up to instance_heap_offset)
371442
// * the heap, followed by guard pages
372443
// * the stack (grows towards heap guard pages)
373-
// * one guard page (for good luck?)
374444
// * globals
375445
// * one guard page (to catch signal stack overflow)
376-
// * the signal stack (size given by signal.h SIGSTKSZ macro)
446+
// * the signal stack
377447

378448
[
379449
instance_heap_offset(),
380450
self.heap_address_space_size,
381-
self.stack_size,
382451
host_page_size(),
452+
self.stack_size,
383453
self.globals_size,
384454
host_page_size(),
385-
SIGSTKSZ,
455+
self.signal_stack_size,
386456
]
387457
.iter()
388458
.try_fold(0usize, |acc, &x| acc.checked_add(x))
@@ -419,6 +489,11 @@ impl Limits {
419489
if self.stack_size <= 0 {
420490
return Err(Error::InvalidArgument("stack size must be greater than 0"));
421491
}
492+
if self.signal_stack_size % host_page_size() != 0 {
493+
return Err(Error::InvalidArgument(
494+
"signal stack size must be a multiple of host page size",
495+
));
496+
}
422497
Ok(())
423498
}
424499
}

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ macro_rules! alloc_tests {
2121
heap_address_space_size: LIMITS_HEAP_ADDRSPACE_SIZE,
2222
stack_size: LIMITS_STACK_SIZE,
2323
globals_size: LIMITS_GLOBALS_SIZE,
24+
..Limits::default()
2425
};
2526

2627
const SPEC_HEAP_RESERVED_SIZE: u64 = LIMITS_HEAP_ADDRSPACE_SIZE as u64 / 2;
@@ -264,6 +265,7 @@ macro_rules! alloc_tests {
264265
heap_address_space_size: LIMITS_HEAP_ADDRSPACE_SIZE,
265266
stack_size: LIMITS_STACK_SIZE,
266267
globals_size: LIMITS_GLOBALS_SIZE,
268+
..Limits::default()
267269
};
268270
let res = TestRegion::create(10, &LIMITS);
269271
assert!(res.is_err(), "region creation fails");
@@ -366,7 +368,7 @@ macro_rules! alloc_tests {
366368
}
367369

368370
let sigstack = unsafe { inst.alloc_mut().sigstack_mut() };
369-
assert_eq!(sigstack.len(), libc::SIGSTKSZ);
371+
assert_eq!(sigstack.len(), LIMITS.signal_stack_size);
370372

371373
assert_eq!(sigstack[0], 0);
372374
sigstack[0] = 0xFF;
@@ -569,6 +571,7 @@ macro_rules! alloc_tests {
569571
heap_address_space_size: 2 * 4096,
570572
stack_size: 4096,
571573
globals_size: 4096,
574+
..Limits::default()
572575
};
573576
const CONTEXT_TEST_INITIAL_SIZE: u64 = 4096;
574577
const CONTEXT_TEST_HEAP: HeapSpec = HeapSpec {

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,16 @@ pub struct lucet_alloc_limits {
140140
pub stack_size: u64,
141141
/// Size of the globals region in bytes; each global uses 8 bytes. (default 4K)
142142
pub globals_size: u64,
143+
/// Size of the signal stack in bytes. (default SIGSTKSZ for Rust release builds, 12K for Rust
144+
/// debug builds)
145+
///
146+
/// This difference is to account for the greatly increased stack size usage in the signal
147+
/// handler when running without optimizations.
148+
///
149+
/// Note that debug vs. release mode is determined by `cfg(debug_assertions)`, so if you are
150+
/// specifically enabling Rust debug assertions in your Cargo release builds, the default signal
151+
/// stack will be larger.
152+
pub signal_stack_size: u64,
143153
}
144154

145155
impl From<Limits> for lucet_alloc_limits {
@@ -155,6 +165,7 @@ impl From<&Limits> for lucet_alloc_limits {
155165
heap_address_space_size: limits.heap_address_space_size as u64,
156166
stack_size: limits.stack_size as u64,
157167
globals_size: limits.globals_size as u64,
168+
signal_stack_size: limits.signal_stack_size as u64,
158169
}
159170
}
160171
}
@@ -172,6 +183,7 @@ impl From<&lucet_alloc_limits> for Limits {
172183
heap_address_space_size: limits.heap_address_space_size as usize,
173184
stack_size: limits.stack_size as usize,
174185
globals_size: limits.globals_size as usize,
186+
signal_stack_size: limits.signal_stack_size as usize,
175187
}
176188
}
177189
}

0 commit comments

Comments
 (0)