T0b1/wasmtime
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Improvements to Modules API (#307)

Browse Source 
* test-no_std: use cargo +nightly

assume folks have rustup set to use stable by default

* cretonne-module, -faerie, -simplejit: use new ModuleError enum

CtonError is not really appropriate for use in the module system.
Instead, create a new enum ModuleError, which implements failure::Fail
(works with no_std). Translate existing panics and unimplemented
error cases to return ModuleErrors.

* cretonne-faerie: export FaerieProduct

* cretonne-module: expose FuncOrDataId, and Module::get_name to lookup

This is helpful for looking up a name that has already been declared.
Also, implement FuncOrDataId -> ExternalName conversion.

* cretonne-faerie: depend on faerie 0.3.0

which has bugfix for data relocations

* cretonne-module: change InvalidDefinition to InvalidImportDefinition

per dan's code review. plus another typo fix

* cretonne-faerie: add optional manifest of all traps from codegen

* cretonne-module: provide more context in panics

* cretonne-faerie: updates to docs

* cretonne-faerie: return an Err instead of debug_assert when isa not pic
This commit is contained in:
Pat Hickey
2018-04-26 22:02:35 -07:00
committed by Dan Gohman
parent 948d5fdeec
commit ee9dcb8367
12 changed files with 252 additions and 74 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
cranelift/test-no_std.sh
View File

@@ -21,10 +21,10 @@ do
cd "lib/$LIB"
# Test with just "core" enabled.
cargo test --no-default-features --features core
cargo +nightly test --no-default-features --features core
# Test with "core" and "std" enabled at the same time.
cargo test --features core
cargo +nightly test --features core
cd "$topdir"
done

2
lib/faerie/Cargo.toml
View File

@@ -11,7 +11,7 @@ readme = "README.md"
[dependencies]
cretonne-codegen = { path = "../codegen", version = "0.6.0" }
cretonne-module = { path = "../module", version = "0.6.0" }
faerie = "0.2.0"
faerie = "0.3.0"
goblin = "0.0.14"
failure = "0.1.1"

92
lib/faerie/src/backend.rs
View File

@@ -3,13 +3,25 @@
use container;
use cretonne_codegen::binemit::{Addend, CodeOffset, Reloc, RelocSink, NullTrapSink};
use cretonne_codegen::isa::TargetIsa;
use cretonne_codegen::result::CtonError;
use cretonne_codegen::{self, binemit, ir};
use cretonne_module::{Backend, DataContext, Linkage, ModuleNamespace, Init, DataDescription};
use faerie;
use cretonne_module::{Backend, DataContext, Linkage, ModuleNamespace, Init, DataDescription,
ModuleError};
use failure::Error;
use faerie;
use std::fs::File;
use target;
use traps::{FaerieTrapManifest, FaerieTrapSink};
#[derive(Debug)]
/// Setting to enable collection of traps. Setting this to `Enabled` in
/// `FaerieBuilder` means that a `FaerieTrapManifest` will be present
/// in the `FaerieProduct`.
pub enum FaerieTrapCollection {
/// `FaerieProduct::trap_manifest` will be `None`
Disabled,
/// `FaerieProduct::trap_manifest` will be `Some`
Enabled,
}
/// A builder for `FaerieBackend`.
pub struct FaerieBuilder {
@@ -17,6 +29,7 @@ pub struct FaerieBuilder {
name: String,
format: container::Format,
faerie_target: faerie::Target,
collect_traps: FaerieTrapCollection,
}
impl FaerieBuilder {
@@ -24,21 +37,28 @@ impl FaerieBuilder {
/// can be passed to
/// [`Module::new`](cretonne_module/struct.Module.html#method.new].
///
/// Note: To support calls JIT'd functions from Rust or other compiled
/// code, it's necessary for the `call_conv` setting in `isa`'s flags
/// to match the host platform.
/// Faerie output requires that TargetIsa have PIC (Position Independent Code) enabled.
///
/// `collect_traps` setting determines whether trap information is collected in a
/// `FaerieTrapManifest` available in the `FaerieProduct`.
pub fn new(
isa: Box<TargetIsa>,
name: String,
format: container::Format,
) -> Result<Self, Error> {
debug_assert!(isa.flags().is_pic(), "faerie requires PIC");
collect_traps: FaerieTrapCollection,
) -> Result<Self, ModuleError> {
if !isa.flags().is_pic() {
return Err(ModuleError::Backend(
"faerie requires TargetIsa be PIC".to_owned(),
));
}
let faerie_target = target::translate(&*isa)?;
Ok(Self {
isa,
name,
format,
faerie_target,
collect_traps,
})
}
}
@@ -48,6 +68,7 @@ pub struct FaerieBackend {
isa: Box<TargetIsa>,
artifact: faerie::Artifact,
format: container::Format,
trap_manifest: Option<FaerieTrapManifest>,
}
pub struct FaerieCompiledFunction {}
@@ -75,6 +96,10 @@ impl Backend for FaerieBackend {
isa: builder.isa,
artifact: faerie::Artifact::new(builder.faerie_target, builder.name),
format: builder.format,
trap_manifest: match builder.collect_traps {
FaerieTrapCollection::Enabled => Some(FaerieTrapManifest::new()),
FaerieTrapCollection::Disabled => None,
},
}
}
@@ -100,7 +125,7 @@ impl Backend for FaerieBackend {
ctx: &cretonne_codegen::Context,
namespace: &ModuleNamespace<Self>,
code_size: u32,
) -> Result<FaerieCompiledFunction, CtonError> {
) -> Result<FaerieCompiledFunction, ModuleError> {
let mut code: Vec<u8> = Vec::with_capacity(code_size as usize);
code.resize(code_size as usize, 0);
@@ -112,18 +137,29 @@ impl Backend for FaerieBackend {
name,
namespace,
};
// Ignore traps for now. For now, frontends should just avoid generating code
// that traps.
let mut trap_sink = NullTrapSink {};
unsafe {
ctx.emit_to_memory(
&*self.isa,
code.as_mut_ptr(),
&mut reloc_sink,
&mut trap_sink,
)
};
if let Some(ref mut trap_manifest) = self.trap_manifest {
let mut trap_sink = FaerieTrapSink::new(name, code_size);
unsafe {
ctx.emit_to_memory(
&*self.isa,
code.as_mut_ptr(),
&mut reloc_sink,
&mut trap_sink,
)
};
trap_manifest.add_sink(trap_sink);
} else {
let mut trap_sink = NullTrapSink {};
unsafe {
ctx.emit_to_memory(
&*self.isa,
code.as_mut_ptr(),
&mut reloc_sink,
&mut trap_sink,
)
};
}
}
self.artifact.define(name, code).expect(
@@ -137,7 +173,7 @@ impl Backend for FaerieBackend {
name: &str,
data_ctx: &DataContext,
namespace: &ModuleNamespace<Self>,
) -> Result<FaerieCompiledData, CtonError> {
) -> Result<FaerieCompiledData, ModuleError> {
let &DataDescription {
writable: _writable,
ref init,
@@ -161,8 +197,6 @@ impl Backend for FaerieBackend {
}
}
// TODO: Change the signature of this function to use something other
// than `CtonError`, as `CtonError` can't convey faerie's errors.
for &(offset, id) in function_relocs {
let to = &namespace.get_function_decl(&function_decls[id]).name;
self.artifact
@@ -171,7 +205,7 @@ impl Backend for FaerieBackend {
to,
at: offset as usize,
})
.map_err(|_e| CtonError::InvalidInput)?;
.map_err(|e| ModuleError::Backend(format!("{}", e)))?;
}
for &(offset, id, addend) in data_relocs {
debug_assert_eq!(
@@ -186,7 +220,7 @@ impl Backend for FaerieBackend {
to,
at: offset as usize,
})
.map_err(|_e| CtonError::InvalidInput)?;
.map_err(|e| ModuleError::Backend(format!("{}", e)))?;
}
self.artifact.define(name, bytes).expect(
@@ -230,6 +264,7 @@ impl Backend for FaerieBackend {
FaerieProduct {
artifact: self.artifact,
format: self.format,
trap_manifest: self.trap_manifest,
}
}
}
@@ -238,7 +273,12 @@ impl Backend for FaerieBackend {
/// [`finish`](../cretonne_module/struct.Module.html#method.finish) function.
/// It provides functions for writing out the object file to memory or a file.
pub struct FaerieProduct {
artifact: faerie::Artifact,
/// Faerie artifact with all functions, data, and links from the module defined
pub artifact: faerie::Artifact,
/// Optional trap manifest. Contains `FaerieTrapManifest` when `FaerieBuilder.collect_traps` is
/// set to `FaerieTrapCollection::Enabled`.
pub trap_manifest: Option<FaerieTrapManifest>,
/// The format that the builder specified for output.
format: container::Format,
}

4
lib/faerie/src/lib.rs
View File

@@ -21,13 +21,13 @@
extern crate cretonne_codegen;
extern crate cretonne_module;
extern crate faerie;
#[macro_use]
extern crate failure;
extern crate goblin;
mod backend;
mod container;
mod target;
pub mod traps;
pub use backend::{FaerieBuilder, FaerieBackend};
pub use backend::{FaerieBuilder, FaerieBackend, FaerieProduct, FaerieTrapCollection};
pub use container::Format;

8
lib/faerie/src/target.rs
View File

@@ -1,9 +1,9 @@
use cretonne_codegen::isa;
use cretonne_module::ModuleError;
use faerie::Target;
use failure::Error;
/// Translate from a Cretonne `TargetIsa` to a Faerie `Target`.
pub fn translate(isa: &isa::TargetIsa) -> Result<Target, Error> {
pub fn translate(isa: &isa::TargetIsa) -> Result<Target, ModuleError> {
let name = isa.name();
match name {
"x86" => Ok(if isa.flags().is_64bit() {
@@ -13,6 +13,8 @@ pub fn translate(isa: &isa::TargetIsa) -> Result<Target, Error> {
}),
"arm32" => Ok(Target::ARMv7),
"arm64" => Ok(Target::ARM64),
_ => Err(format_err!("unsupported isa: {}", name)),
_ => Err(ModuleError::Backend(
format!("unsupported faerie isa: {}", name),
)),
}
}

64
lib/faerie/src/traps.rs Normal file
View File

@@ -0,0 +1,64 @@
//! Faerie trap manifests record every `TrapCode` that cretonne outputs during code generation,
//! for every function in the module. This data may be useful at runtime.
use cretonne_codegen::{ir, binemit};
/// Record of the arguments cretonne passes to `TrapSink::trap`
pub struct FaerieTrapSite {
/// Offset into function
pub offset: binemit::CodeOffset,
/// Source location given to cretonne
pub srcloc: ir::SourceLoc,
/// Trap code, as determined by cretonne
pub code: ir::TrapCode,
}
/// Record of the trap sites for a given function
pub struct FaerieTrapSink {
/// Name of function
pub name: String,
/// Total code size of function
pub code_size: u32,
/// All trap sites collected in function
pub sites: Vec<FaerieTrapSite>,
}
impl FaerieTrapSink {
/// Create an empty `FaerieTrapSink`
pub fn new(name: &str, code_size: u32) -> Self {
Self {
sites: Vec::new(),
name: name.to_owned(),
code_size,
}
}
}
impl binemit::TrapSink for FaerieTrapSink {
fn trap(&mut self, offset: binemit::CodeOffset, srcloc: ir::SourceLoc, code: ir::TrapCode) {
self.sites.push(FaerieTrapSite {
offset,
srcloc,
code,
});
}
}
/// Collection of all `FaerieTrapSink`s for the module
pub struct FaerieTrapManifest {
/// All `FaerieTrapSink` for the module
pub sinks: Vec<FaerieTrapSink>,
}
impl FaerieTrapManifest {
/// Create an empty `FaerieTrapManifest`
pub fn new() -> Self {
Self { sinks: Vec::new() }
}
/// Put a `FaerieTrapSink` into manifest
pub fn add_sink(&mut self, sink: FaerieTrapSink) {
self.sinks.push(sink);
}
}

1
lib/module/Cargo.toml
View File

@@ -12,6 +12,7 @@ readme = "README.md"
cretonne-codegen = { path = "../codegen", version = "0.6.0", default-features = false }
cretonne-entity = { path = "../entity", version = "0.6.0", default-features = false }
hashmap_core = { version = "0.1.4", optional = true }
failure = "0.1.1"
[features]
default = ["std"]

8
lib/module/src/backend.rs
View File

@@ -3,9 +3,9 @@
use DataContext;
use Linkage;
use ModuleNamespace;
use ModuleError;
use cretonne_codegen::Context;
use cretonne_codegen::isa::TargetIsa;
use cretonne_codegen::result::CtonError;
use cretonne_codegen::{binemit, ir};
use std::marker;
@@ -57,19 +57,17 @@ where
ctx: &Context,
namespace: &ModuleNamespace<Self>,
code_size: u32,
) -> Result<Self::CompiledFunction, CtonError>;
) -> Result<Self::CompiledFunction, ModuleError>;
/// Define a zero-initialized data object of the given size.
///
/// Data objects must be declared before being defined.
///
/// TODO: Is CtonError the right error code here?
fn define_data(
&mut self,
name: &str,
data_ctx: &DataContext,
namespace: &ModuleNamespace<Self>,
) -> Result<Self::CompiledData, CtonError>;
) -> Result<Self::CompiledData, ModuleError>;
/// Write the address of `what` into the data for `data` at `offset`. `data` must refer to a
/// defined data object.

2
lib/module/src/data_context.rs
View File

@@ -94,7 +94,7 @@ impl DataContext {
self.description.init = Init::Zeros { size };
}
/// Define a zero-initialized object with the given size.
/// Define an object initialized with the given contents.
///
/// TODO: Can we avoid a Box here?
pub fn define(&mut self, contents: Box<[u8]>, writable: Writability) {

5
lib/module/src/lib.rs
View File

@@ -16,9 +16,12 @@
use_self,
))]
#[macro_use]
extern crate cretonne_codegen;
#[macro_use]
extern crate cretonne_entity;
#[macro_use]
extern crate failure;
mod backend;
mod data_context;
@@ -26,4 +29,4 @@ mod module;
pub use backend::Backend;
pub use data_context::{DataContext, Writability, DataDescription, Init};
pub use module::{DataId, FuncId, Linkage, Module, ModuleNamespace};
pub use module::{DataId, FuncId, FuncOrDataId, Linkage, Module, ModuleNamespace, ModuleError};

129
lib/module/src/module.rs
View File

@@ -7,7 +7,7 @@
use Backend;
use cretonne_codegen::entity::{EntityRef, PrimaryMap};
use cretonne_codegen::result::{CtonError, CtonResult};
use cretonne_codegen::result::CtonError;
use cretonne_codegen::{binemit, ir, Context};
use data_context::DataContext;
use std::collections::HashMap;
@@ -17,11 +17,32 @@ use std::collections::HashMap;
pub struct FuncId(u32);
entity_impl!(FuncId, "funcid");
/// Function identifiers are namespace 0 in `ir::ExternalName`
impl From<FuncId> for ir::ExternalName {
fn from(id: FuncId) -> ir::ExternalName {
ir::ExternalName::User {
namespace: 0,
index: id.0,
}
}
}
/// A data object identifier for use in the `Module` interface.
#[derive(Copy, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)]
pub struct DataId(u32);
entity_impl!(DataId, "dataid");
/// Data identifiers are namespace 1 in `ir::ExternalName`
impl From<DataId> for ir::ExternalName {
fn from(id: DataId) -> ir::ExternalName {
ir::ExternalName::User {
namespace: 1,
index: id.0,
}
}
}
/// Linkage refers to where an entity is defined and who can see it.
#[derive(Copy, Clone, PartialEq, Eq)]
pub enum Linkage {
@@ -73,12 +94,26 @@ impl Linkage {
}
}
#[derive(Copy, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)]
enum FuncOrDataId {
/// A declared name may refer to either a function or data declaration
#[derive(Copy, Clone, PartialEq, Eq, Hash, PartialOrd, Ord, Debug)]
pub enum FuncOrDataId {
/// When it's a FuncId
Func(FuncId),
/// When it's a DataId
Data(DataId),
}
/// Mapping to `ir::ExternalName` is trivial based on the `FuncId` and `DataId` mapping.
impl From<FuncOrDataId> for ir::ExternalName {
fn from(id: FuncOrDataId) -> ir::ExternalName {
match id {
FuncOrDataId::Func(funcid) => ir::ExternalName::from(funcid),
FuncOrDataId::Data(dataid) => ir::ExternalName::from(dataid),
}
}
}
/// Information about a function which can be called.
pub struct FunctionDeclaration {
pub name: String,
@@ -86,6 +121,29 @@ pub struct FunctionDeclaration {
pub signature: ir::Signature,
}
/// Error messages for all `Module` and `Backend` methods
#[derive(Fail, Debug)]
pub enum ModuleError {
/// Indicates an identifier was used before it was declared
#[fail(display = "Undeclared identifier: {}", _0)]
Undeclared(String),
/// Indicates an identifier was used contrary to the way it was declared
#[fail(display = "Incompatible declaration of identifier: {}", _0)]
IncompatibleDeclaration(String),
/// Indicates an identifier was defined more than once
#[fail(display = "Duplicate definition of identifier: {}", _0)]
DuplicateDefinition(String),
/// Indicates an identifier was defined, but was declared as an import
#[fail(display = "Invalid to define identifier declared as an import: {}", _0)]
InvalidImportDefinition(String),
/// Wraps a `cretonne-codegen` error
#[fail(display = "Compilation error: {}", _0)]
Compilation(CtonError),
/// Wraps a generic error from a backend
#[fail(display = "Backend error: {}", _0)]
Backend(String),
}
/// A function belonging to a `Module`.
struct ModuleFunction<B>
where
@@ -157,7 +215,7 @@ where
let func = FuncId::new(index as usize);
&self.functions[func]
} else {
panic!("unexpected ExternalName kind")
panic!("unexpected ExternalName kind {}", name)
}
}
@@ -168,7 +226,7 @@ where
let data = DataId::new(index as usize);
&self.data_objects[data]
} else {
panic!("unexpected ExternalName kind")
panic!("unexpected ExternalName kind {}", name)
}
}
}
@@ -227,7 +285,7 @@ where
if let ir::ExternalName::User { namespace, .. } = *name {
namespace == 0
} else {
panic!("unexpected ExternalName kind")
panic!("unexpected ExternalName kind {}", name)
}
}
}
@@ -258,6 +316,12 @@ where
}
}
/// Get the module identifier for a given name, if that name
/// has been declared.
pub fn get_name(&self, name: &str) -> Option<FuncOrDataId> {
self.names.get(name).map(|e| *e)
}
/// Return then pointer type for the current target.
pub fn pointer_type(&self) -> ir::types::Type {
if self.backend.isa().flags().is_64bit() {
@@ -273,7 +337,7 @@ where
name: &str,
linkage: Linkage,
signature: &ir::Signature,
) -> Result<FuncId, CtonError> {
) -> Result<FuncId, ModuleError> {
// TODO: Can we avoid allocating names so often?
use std::collections::hash_map::Entry::*;
match self.names.entry(name.to_owned()) {
@@ -285,7 +349,9 @@ where
self.backend.declare_function(name, existing.decl.linkage);
Ok(id)
}
FuncOrDataId::Data(..) => unimplemented!(),
FuncOrDataId::Data(..) => Err(
ModuleError::IncompatibleDeclaration(name.to_owned()),
),
}
}
Vacant(entry) => {
@@ -311,7 +377,7 @@ where
name: &str,
linkage: Linkage,
writable: bool,
) -> Result<DataId, CtonError> {
) -> Result<DataId, ModuleError> {
// TODO: Can we avoid allocating names so often?
use std::collections::hash_map::Entry::*;
match self.names.entry(name.to_owned()) {
@@ -328,7 +394,9 @@ where
Ok(id)
}
FuncOrDataId::Func(..) => unimplemented!(),
FuncOrDataId::Func(..) => Err(
ModuleError::IncompatibleDeclaration(name.to_owned()),
),
}
}
Vacant(entry) => {
@@ -386,19 +454,24 @@ where
}
/// Define a function, producing the function body from the given `Context`.
pub fn define_function(&mut self, func: FuncId, ctx: &mut Context) -> CtonResult {
pub fn define_function(&mut self, func: FuncId, ctx: &mut Context) -> Result<(), ModuleError> {
let compiled = {
let code_size = ctx.compile(self.backend.isa())?;
let code_size = ctx.compile(self.backend.isa()).map_err(|e| {
dbg!(
"defining function {}: {}",
func,
ctx.func.display(self.backend.isa())
);
ModuleError::Compilation(e)
})?;
let info = &self.contents.functions[func];
debug_assert!(
info.compiled.is_none(),
"functions can be defined only once"
);
debug_assert!(
info.decl.linkage.is_definable(),
"imported functions cannot be defined"
);
if !info.compiled.is_none() {
return Err(ModuleError::DuplicateDefinition(info.decl.name.clone()));
}
if !info.decl.linkage.is_definable() {
return Err(ModuleError::InvalidImportDefinition(info.decl.name.clone()));
}
Some(self.backend.define_function(
&info.decl.name,
ctx,
@@ -413,17 +486,15 @@ where
}
/// Define a function, producing the data contents from the given `DataContext`.
pub fn define_data(&mut self, data: DataId, data_ctx: &DataContext) -> CtonResult {
pub fn define_data(&mut self, data: DataId, data_ctx: &DataContext) -> Result<(), ModuleError> {
let compiled = {
let info = &self.contents.data_objects[data];
debug_assert!(
info.compiled.is_none(),
"functions can be defined only once"
);
debug_assert!(
info.decl.linkage.is_definable(),
"imported functions cannot be defined"
);
if !info.compiled.is_none() {
return Err(ModuleError::DuplicateDefinition(info.decl.name.clone()));
}
if !info.decl.linkage.is_definable() {
return Err(ModuleError::InvalidImportDefinition(info.decl.name.clone()));
}
Some(self.backend.define_data(
&info.decl.name,
data_ctx,

7
lib/simplejit/src/backend.rs
View File

@@ -2,10 +2,9 @@
use cretonne_codegen::binemit::{Addend, CodeOffset, Reloc, RelocSink, NullTrapSink};
use cretonne_codegen::isa::TargetIsa;
use cretonne_codegen::result::CtonError;
use cretonne_codegen::{self, ir, settings};
use cretonne_module::{Backend, DataContext, Linkage, ModuleNamespace, Writability,
DataDescription, Init};
DataDescription, Init, ModuleError};
use cretonne_native;
use std::ffi::CString;
use std::ptr;
@@ -116,7 +115,7 @@ impl<'simple_jit_backend> Backend for SimpleJITBackend {
ctx: &cretonne_codegen::Context,
_namespace: &ModuleNamespace<Self>,
code_size: u32,
) -> Result<Self::CompiledFunction, CtonError> {
) -> Result<Self::CompiledFunction, ModuleError> {
let size = code_size as usize;
let ptr = self.code_memory.allocate(size).expect(
"TODO: handle OOM etc.",
@@ -139,7 +138,7 @@ impl<'simple_jit_backend> Backend for SimpleJITBackend {
_name: &str,
data: &DataContext,
_namespace: &ModuleNamespace<Self>,
) -> Result<Self::CompiledData, CtonError> {
) -> Result<Self::CompiledData, ModuleError> {
let &DataDescription {
writable,
ref init,