Skip to content

Commit

Permalink
Merge #2693
Browse files Browse the repository at this point in the history
2693: Add extra validation to compilers & engines r=ptitSeb a=Amanieu

- Cranelift and singlepass now properly cross-compile with no dependency on the host target.
- Staticlib engine now panics if you try to run a freshly compiled module.
- CPU features used when a module was compiled are now checked against the host CPU features during instantiation.

Fixes #1567
Fixes #2590 

Co-authored-by: Amanieu d'Antras <[email protected]>
  • Loading branch information
bors[bot] and Amanieu authored Nov 23, 2021
2 parents 277ab29 + af29488 commit 8f2e49d
Show file tree
Hide file tree
Showing 31 changed files with 186 additions and 141 deletions.
5 changes: 5 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions lib/api/src/sys/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,11 @@ pub enum InstantiationError {
#[error(transparent)]
Start(RuntimeError),

/// The module was compiled with a CPU feature that is not available on
/// the current host.
#[error("missing requires CPU features: {0:?}")]
CpuFeature(String),

/// Error occurred when initializing the host environment.
#[error(transparent)]
HostEnvInitialization(HostEnvInitError),
Expand All @@ -68,6 +73,7 @@ impl From<wasmer_engine::InstantiationError> for InstantiationError {
match other {
wasmer_engine::InstantiationError::Link(e) => Self::Link(e),
wasmer_engine::InstantiationError::Start(e) => Self::Start(e),
wasmer_engine::InstantiationError::CpuFeature(e) => Self::CpuFeature(e),
}
}
}
Expand Down
39 changes: 12 additions & 27 deletions lib/c-api/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,49 +49,48 @@
use libc::{c_char, c_int};
use std::cell::RefCell;
use std::error::Error;
use std::fmt::{self, Display, Formatter};
use std::fmt::Display;
use std::ptr::{self, NonNull};
use std::slice;

thread_local! {
static LAST_ERROR: RefCell<Option<Box<dyn Error>>> = RefCell::new(None);
static LAST_ERROR: RefCell<Option<String>> = RefCell::new(None);
}

/// Rust function to register a new error.
///
/// # Example
///
/// ```rust,no_run
/// # use wasmer::error::{update_last_error, CApiError};
/// # use wasmer::error::update_last_error;
///
/// update_last_error(CApiError {
/// msg: "Hello, World!".to_string(),
/// });
/// update_last_error("Hello, World!");
/// ```
pub fn update_last_error<E: Error + 'static>(err: E) {
pub fn update_last_error<E: Display>(err: E) {
LAST_ERROR.with(|prev| {
*prev.borrow_mut() = Some(Box::new(err));
*prev.borrow_mut() = Some(err.to_string());
});
}

/// Retrieve the most recent error, clearing it in the process.
pub(crate) fn take_last_error() -> Option<Box<dyn Error>> {
pub(crate) fn take_last_error() -> Option<String> {
LAST_ERROR.with(|prev| prev.borrow_mut().take())
}

/// Gets the length in bytes of the last error if any, zero otherwise.
/// Gets the length in bytes of the last error if any, zero otherwise. This
/// includes th NUL terminator byte.
///
/// This can be used to dynamically allocate a buffer with the correct number of
/// bytes needed to store a message.
///
/// # Example
///
/// See this module's documentation to get a complete example.
// TODO(Amanieu): This should use size_t
#[no_mangle]
pub extern "C" fn wasmer_last_error_length() -> c_int {
LAST_ERROR.with(|prev| match *prev.borrow() {
Some(ref err) => err.to_string().len() as c_int + 1,
Some(ref err) => err.len() as c_int + 1,
None => 0,
})
}
Expand All @@ -118,6 +117,7 @@ pub extern "C" fn wasmer_last_error_length() -> c_int {
/// # Example
///
/// See this module's documentation to get a complete example.
// TODO(Amanieu): This should use size_t
#[no_mangle]
pub unsafe extern "C" fn wasmer_last_error_message(
buffer: Option<NonNull<c_char>>,
Expand Down Expand Up @@ -156,18 +156,3 @@ pub unsafe extern "C" fn wasmer_last_error_message(

error_message.len() as c_int + 1
}

/// Rust type to represent a C API error.
#[derive(Debug)]
pub struct CApiError {
/// The error message.
pub msg: String,
}

impl Display for CApiError {
fn fmt(&self, f: &mut Formatter) -> fmt::Result {
write!(f, "{}", &self.msg)
}
}

impl Error for CApiError {}
11 changes: 3 additions & 8 deletions lib/c-api/src/wasm_c_api/engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ pub use super::unstable::middlewares::wasm_config_push_middleware;
#[cfg(feature = "middlewares")]
use super::unstable::middlewares::wasmer_middleware_t;
use super::unstable::target_lexicon::wasmer_target_t;
use crate::error::{update_last_error, CApiError};
use crate::error::update_last_error;
use cfg_if::cfg_if;
use std::sync::Arc;
use wasmer_api::Engine;
Expand Down Expand Up @@ -433,13 +433,8 @@ pub extern "C" fn wasm_engine_new_with_config(
config: Option<Box<wasm_config_t>>,
) -> Option<Box<wasm_engine_t>> {
#[allow(dead_code)]
fn return_with_error<M>(msg: M) -> Option<Box<wasm_engine_t>>
where
M: ToString,
{
update_last_error(CApiError {
msg: msg.to_string(),
});
fn return_with_error(msg: &str) -> Option<Box<wasm_engine_t>> {
update_last_error(msg);

return None;
}
Expand Down
6 changes: 6 additions & 0 deletions lib/c-api/src/wasm_c_api/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,12 @@ pub unsafe extern "C" fn wasm_instance_new(
return None;
}

Err(e @ InstantiationError::CpuFeature(_)) => {
crate::error::update_last_error(e);

return None;
}

Err(InstantiationError::HostEnvInitialization(error)) => {
crate::error::update_last_error(error);

Expand Down
3 changes: 1 addition & 2 deletions lib/c-api/src/wasm_c_api/unstable/target_lexicon.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@
//! ```
use super::super::types::wasm_name_t;
use crate::error::CApiError;
use enumset::EnumSet;
use std::slice;
use std::str::{self, FromStr};
Expand Down Expand Up @@ -153,7 +152,7 @@ pub unsafe extern "C" fn wasmer_triple_new(
)));

Some(Box::new(wasmer_triple_t {
inner: c_try!(Triple::from_str(triple).map_err(|e| CApiError { msg: e.to_string() })),
inner: c_try!(Triple::from_str(triple)),
}))
}

Expand Down
8 changes: 2 additions & 6 deletions lib/c-api/src/wasm_c_api/unstable/wasi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ use super::super::{
externals::wasm_extern_t, module::wasm_module_t, store::wasm_store_t, types::wasm_name_t,
wasi::wasi_env_t,
};
use crate::error::CApiError;
use wasmer_api::Extern;
use wasmer_wasi::{generate_import_object_from_env, get_wasi_version};

Expand Down Expand Up @@ -168,11 +167,8 @@ fn wasi_get_unordered_imports_inner(

let store = &store.inner;

let version = c_try!(
get_wasi_version(&module.inner, false).ok_or_else(|| CApiError {
msg: "could not detect a WASI version on the given module".to_string(),
})
);
let version = c_try!(get_wasi_version(&module.inner, false)
.ok_or("could not detect a WASI version on the given module"));

let import_object = generate_import_object_from_env(store, wasi_env.inner.clone(), version);

Expand Down
4 changes: 2 additions & 2 deletions lib/c-api/src/wasm_c_api/value.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use super::types::{wasm_ref_t, wasm_valkind_enum};
use crate::error::{update_last_error, CApiError};
use crate::error::update_last_error;
use std::convert::{TryFrom, TryInto};
use wasmer_api::Val;

Expand Down Expand Up @@ -156,7 +156,7 @@ pub unsafe extern "C" fn wasm_val_copy(
},

Err(e) => {
update_last_error(CApiError { msg: e.to_string() });
update_last_error(e);

return;
}
Expand Down
29 changes: 10 additions & 19 deletions lib/c-api/src/wasm_c_api/wasi/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use super::{
module::wasm_module_t,
store::wasm_store_t,
};
use crate::error::{update_last_error, CApiError};
use crate::error::update_last_error;
use std::cmp::min;
use std::convert::TryFrom;
use std::ffi::CStr;
Expand Down Expand Up @@ -212,9 +212,7 @@ pub unsafe extern "C" fn wasi_env_read_stdout(
if let Some(stdout) = stdout.as_mut() {
stdout
} else {
update_last_error(CApiError {
msg: "could not find a file handle for `stdout`".to_string(),
});
update_last_error("could not find a file handle for `stdout`");
return -1;
}
} else {
Expand All @@ -235,15 +233,11 @@ pub unsafe extern "C" fn wasi_env_read_stderr(
if let Some(stderr) = stderr.as_mut() {
stderr
} else {
update_last_error(CApiError {
msg: "could not find a file handle for `stderr`".to_string(),
});
update_last_error("could not find a file handle for `stderr`");
return -1;
}
} else {
update_last_error(CApiError {
msg: "could not find a file handle for `stderr`".to_string(),
});
update_last_error("could not find a file handle for `stderr`");
return -1;
};
read_inner(stderr, inner_buffer)
Expand Down Expand Up @@ -348,11 +342,8 @@ fn wasi_get_imports_inner(

let store = &store.inner;

let version = c_try!(
get_wasi_version(&module.inner, false).ok_or_else(|| CApiError {
msg: "could not detect a WASI version on the given module".to_string(),
})
);
let version = c_try!(get_wasi_version(&module.inner, false)
.ok_or("could not detect a WASI version on the given module"));

let import_object = generate_import_object_from_env(store, wasi_env.inner.clone(), version);

Expand All @@ -362,18 +353,18 @@ fn wasi_get_imports_inner(
.map(|import_type| {
let export = import_object
.resolve_by_name(import_type.module(), import_type.name())
.ok_or_else(|| CApiError {
msg: format!(
.ok_or_else(|| {
format!(
"Failed to resolve import \"{}\" \"{}\"",
import_type.module(),
import_type.name()
),
)
})?;
let inner = Extern::from_vm_export(store, export);

Ok(Some(Box::new(inner.into())))
})
.collect::<Result<Vec<_>, CApiError>>()));
.collect::<Result<Vec<_>, String>>()));

Some(())
}
Expand Down
2 changes: 1 addition & 1 deletion lib/compiler-cranelift/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,9 @@ more-asserts = "0.2"
gimli = { version = "0.25", optional = true }
smallvec = "1.6"
loupe = "0.1"
target-lexicon = { version = "0.12.2", default-features = false }

[dev-dependencies]
target-lexicon = { version = "0.12.2", default-features = false }
cranelift-codegen = { version = "0.76", features = ["all-arch"] }
lazy_static = "1.4"

Expand Down
Loading

0 comments on commit 8f2e49d

Please sign in to comment.