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

Commit

Permalink
move stack probe to lucetc; generalize tests over lucet_region
Browse files Browse the repository at this point in the history
Moving the stack probe into the compiled Lucet modules lets us dodge Rust's current inability to
reexport dynamic symbols (see <https://github.com/rust-lang/rust/issues/36342>). It loses a small
amount of fidelity that we got with stack overflow traps previously, as we can't distinguish a stack
overflow originating in the stack probe from anywhere else.

The C test suites are now parameterized over region, much like the Rust test suites.
  • Loading branch information
acfoltzer committed Feb 23, 2019
1 parent be0ef55 commit 93c34de
Show file tree
Hide file tree
Showing 22 changed files with 274 additions and 237 deletions.
7 changes: 6 additions & 1 deletion lucet-runtime/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,12 @@ fn main() {

cbindgen::Builder::new()
.with_config(cbindgen::Config::from_root_or_default(&crate_dir))
.with_crate(&crate_dir)
.with_src(
crate_dir
.join("lucet-runtime-internals")
.join("src")
.join("c_api.rs"),
)
.with_language(cbindgen::Language::C)
.with_include("lucet_val.h")
.with_include("lucet_vmctx.h")
Expand Down
3 changes: 0 additions & 3 deletions lucet-runtime/lucet-runtime-internals/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,6 @@ fn main() {
cc::Build::new()
.file("src/context/context_asm.S")
.compile("context_context_asm");
cc::Build::new()
.file("src/probestack/probestack_asm.S")
.compile("probestack_probestack_asm");
cc::Build::new()
.file("src/instance/siginfo_ext.c")
.compile("instance_siginfo_ext");
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,16 @@
#![allow(non_camel_case_types)]

use crate::{
DlModule, Error, Instance, Limits, MmapRegion, Module, Region, SignalBehavior, TrapCode,
use crate::alloc::Limits;
use crate::error::Error;
use crate::instance::signals::SignalBehavior;
use crate::instance::{
instance_handle_from_raw, instance_handle_to_raw, Instance, InstanceInternal,
};
use crate::module::{DlModule, Module};
use crate::region::mmap::MmapRegion;
use crate::region::Region;
use crate::trapcode::TrapCode;
use libc::{c_char, c_int, c_void};
use lucet_runtime_internals::instance::{instance_handle_from_raw, instance_handle_to_raw};
use num_derive::FromPrimitive;
use num_traits::FromPrimitive;
use std::ffi::CStr;
Expand All @@ -14,6 +20,7 @@ use std::sync::Arc;
#[no_mangle]
pub static LUCET_WASM_PAGE_SIZE: u32 = crate::WASM_PAGE_SIZE;

#[macro_export]
macro_rules! assert_nonnull {
( $name:ident ) => {
if $name.is_null() {
Expand All @@ -22,6 +29,7 @@ macro_rules! assert_nonnull {
};
}

#[macro_export]
macro_rules! with_ffi_arcs {
( [ $($name:ident : $ty:ty),+ ], $body:block ) => {{
$(
Expand Down Expand Up @@ -308,7 +316,6 @@ pub unsafe extern "C" fn lucet_instance_state(
) -> lucet_error {
assert_nonnull!(state_out);
with_instance_ptr!(inst, {
use lucet_runtime_internals::instance::InstanceInternal;
state_out.write(inst.state().into());
lucet_error::Ok
})
Expand Down Expand Up @@ -454,12 +461,12 @@ pub unsafe extern "C" fn lucet_instance_set_fatal_handler(
lucet_error::Ok
}

mod lucet_state {
pub mod lucet_state {
use crate::c_api::lucet_val;
use crate::instance::State;
use crate::module::AddrDetails;
use crate::trapcode::{TrapCode, TrapCodeType};
use libc::{c_char, c_void};
use lucet_runtime_internals::instance::State;
use lucet_runtime_internals::module::AddrDetails;
use lucet_runtime_internals::trapcode::{TrapCode, TrapCodeType};
use num_derive::FromPrimitive;
use num_traits::FromPrimitive;
use std::ffi::CString;
Expand Down Expand Up @@ -678,9 +685,9 @@ mod lucet_state {
}
}

mod lucet_val {
pub mod lucet_val {
use crate::val::{UntypedRetVal, UntypedRetValInternal, Val};
use libc::{c_char, c_void};
use lucet_runtime_internals::val::{UntypedRetVal, UntypedRetValInternal, Val};

// Note on the value associated with each type: the most significant bits represent the "class"
// of the type (1: a C pointer, 2: something unsigned that fits in 64 bits, 3: something signed
Expand Down
2 changes: 1 addition & 1 deletion lucet-runtime/lucet-runtime-internals/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@ pub mod error;
pub mod test_helpers;

pub mod alloc;
pub mod c_api;
pub mod context;
pub mod instance;
pub mod module;
pub mod probestack;
pub mod region;
pub mod trapcode;
pub mod val;
Expand Down
18 changes: 2 additions & 16 deletions lucet-runtime/lucet-runtime-internals/src/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,7 @@ pub use lucet_module_data::{Global, GlobalSpec, HeapSpec};

use crate::alloc::Limits;
use crate::error::Error;
use crate::probestack::{lucet_probestack_private, lucet_probestack_size};
use crate::trapcode::{TrapCode, TrapCodeType};
use crate::trapcode::TrapCode;
use libc::c_void;
use std::slice::from_raw_parts;

Expand Down Expand Up @@ -136,20 +135,7 @@ pub trait ModuleInternal: Send + Sync {
}
}
}

// handle the special case when the probe stack is running
let probestack = lucet_probestack_private as *const c_void;
if rip >= probestack
&& rip as usize <= probestack as usize + unsafe { lucet_probestack_size } as usize
{
Some(TrapCode {
ty: TrapCodeType::StackOverflow,
tag: std::u16::MAX,
})
} else {
// we couldn't find a trapcode
None
}
None
}

/// This is a hack to make sure we don't DCE away the `lucet_vmctx_*` C API
Expand Down
13 changes: 0 additions & 13 deletions lucet-runtime/lucet-runtime-internals/src/probestack/mod.rs

This file was deleted.

This file was deleted.

3 changes: 2 additions & 1 deletion lucet-runtime/lucet-runtime-internals/src/vmctx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,8 @@ pub fn vmctx_capi_init() {
read_volatile(lucet_vmctx_terminate as *const extern "C" fn());
read_volatile(lucet_vmctx_get_delegate as *const extern "C" fn());
read_volatile(lucet_vmctx_get_func_from_idx as *const extern "C" fn());
read_volatile(crate::probestack::lucet_probestack as *const c_void);
// need at least one of these to get the symbols exported
read_volatile(crate::c_api::lucet_dl_module_load as *const c_void);
});
}

Expand Down
11 changes: 9 additions & 2 deletions lucet-runtime/lucet-runtime-tests/src/stack.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,15 @@ macro_rules! stack_tests {
assert_eq!(details.fatal, false);
assert_eq!(details.trapcode.ty, TrapCodeType::StackOverflow);
if probestack {
// When the runtime catches probestack, it puts this special tag in the trapcode
assert_eq!(details.trapcode.tag, std::u16::MAX);
// Make sure we overflowed in the stack probe as expected
//
// TODO: this no longer differentiates between different stack overflow
// sites after moving the stack probe into lucetc; figure out a way to
// provide that information or just wait till we can do the stack probe as a
// global symbol again
let addr_details =
details.rip_addr_details.expect("can look up addr details");
assert!(addr_details.in_module_code);
}
}
res => panic!("unexpected result: {:?}", res),
Expand Down
6 changes: 5 additions & 1 deletion lucet-runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,4 +178,8 @@ pub mod vmctx {
pub use lucet_runtime_internals::vmctx::{lucet_vmctx, Vmctx};
}

mod c_api;
#[doc(hidden)]
#[no_mangle]
extern "C" fn lucet_internal_ensure_linked() {
lucet_runtime_internals::vmctx::vmctx_capi_init();
}
4 changes: 3 additions & 1 deletion lucetc/src/compiler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ pub mod table;
pub mod traps;

mod name;
mod stack_probe;

pub use self::name::Name;

Expand Down Expand Up @@ -64,7 +65,7 @@ pub struct Compiler<'p> {
impl<'p> Compiler<'p> {
pub fn new(name: String, prog: &'p Program, opt_level: OptLevel) -> Result<Self, Error> {
let libcalls = Box::new(move |libcall| match libcall {
ir::LibCall::Probestack => "lucet_probestack".to_owned(),
ir::LibCall::Probestack => stack_probe::STACK_PROBE_SYM.to_owned(),
_ => (FaerieBuilder::default_libcall_names())(libcall),
});

Expand Down Expand Up @@ -278,6 +279,7 @@ pub struct ObjectFile {
}
impl ObjectFile {
pub fn new(mut product: FaerieProduct) -> Result<Self, Error> {
stack_probe::declare_and_define(&mut product)?;
let trap_manifest = &product
.trap_manifest
.expect("trap manifest will be present");
Expand Down
67 changes: 67 additions & 0 deletions lucetc/src/compiler/stack_probe.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
//! Manual definition of the stack probe.
//!
//! Rust currently fails to reexport symbols in dynamic libraries. This means that the old way of
//! including an assembly stack probe in the runtime does not work when embedding in C.
//!
//! There is an [issue](https://github.com/rust-lang/rust/issues/36342) tracking this, but until
//! it's closed we are taking the approach of including the stack probe in every Lucet module, and
//! adding custom entries for it into the trap table, so that stack overflows in the probe will be
//! treated like any other guest trap.
use cranelift_codegen::binemit::TrapSink;
use cranelift_codegen::ir;
use cranelift_faerie::traps::{FaerieTrapManifest, FaerieTrapSink};
use cranelift_faerie::FaerieProduct;
use faerie::Decl;
use failure::Error;

/// Stack probe symbol name
pub const STACK_PROBE_SYM: &'static str = "lucet_probestack";

/// The binary of the stack probe.
const STACK_PROBE_BINARY: &'static [u8] = &[
// 49 89 c3 mov %rax,%r11
// 48 81 ec 00 10 00 00 sub $0x1000,%rsp
// 48 85 64 24 08 test %rsp,0x8(%rsp)
// 49 81 eb 00 10 00 00 sub $0x1000,%r11
// 49 81 fb 00 10 00 00 cmp $0x1000,%r11
// 77 e4 ja 4dfd3 <lucet_probestack+0x3>
// 4c 29 dc sub %r11,%rsp
// 48 85 64 24 08 test %rsp,0x8(%rsp)
// 48 01 c4 add %rax,%rsp
// c3 retq
0x49, 0x89, 0xc3, 0x48, 0x81, 0xec, 0x00, 0x10, 0x00, 0x00, 0x48, 0x85, 0x64, 0x24, 0x08, 0x49,
0x81, 0xeb, 0x00, 0x10, 0x00, 0x00, 0x49, 0x81, 0xfb, 0x00, 0x10, 0x00, 0x00, 0x77, 0xe4, 0x4c,
0x29, 0xdc, 0x48, 0x85, 0x64, 0x24, 0x08, 0x48, 0x01, 0xc4, 0xc3,
];

pub fn declare_and_define(product: &mut FaerieProduct) -> Result<(), Error> {
product.artifact.declare_with(
STACK_PROBE_SYM,
Decl::Function { global: false },
STACK_PROBE_BINARY.to_vec(),
)?;
add_sink(
product
.trap_manifest
.as_mut()
.expect("trap manifest is present"),
);
Ok(())
}

fn add_sink(manifest: &mut FaerieTrapManifest) {
let mut stack_probe_trap_sink =
FaerieTrapSink::new(STACK_PROBE_SYM, STACK_PROBE_BINARY.len() as u32);
stack_probe_trap_sink.trap(
10, /* test %rsp,0x8(%rsp) */
ir::SourceLoc::default(),
ir::TrapCode::StackOverflow,
);
stack_probe_trap_sink.trap(
34, /* test %rsp,0x8(%rsp) */
ir::SourceLoc::default(),
ir::TrapCode::StackOverflow,
);
manifest.add_sink(stack_probe_trap_sink);
}
Loading

0 comments on commit 93c34de

Please sign in to comment.