From 77eb38c41fe5f3fa14f85ed5d682187c08066be8 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Thu, 16 Aug 2018 18:31:05 -0700 Subject: [PATCH] [Module] Remove DataDescription's writable field. It was redundant, as data object declarations also have a writable field, so just use that, avoiding the need for users to declare the same thing twice. Fixes #456. --- lib/faerie/src/backend.rs | 2 +- lib/module/src/backend.rs | 1 + lib/module/src/data_context.rs | 29 +++++------------------------ lib/module/src/lib.rs | 2 +- lib/module/src/module.rs | 1 + lib/simplejit/src/backend.rs | 16 +++++++--------- 6 files changed, 16 insertions(+), 35 deletions(-) diff --git a/lib/faerie/src/backend.rs b/lib/faerie/src/backend.rs index 4f30406a09..b1d5368bad 100644 --- a/lib/faerie/src/backend.rs +++ b/lib/faerie/src/backend.rs @@ -193,11 +193,11 @@ impl Backend for FaerieBackend { fn define_data( &mut self, name: &str, + _writable: bool, data_ctx: &DataContext, namespace: &ModuleNamespace, ) -> ModuleResult { let &DataDescription { - writable: _writable, ref init, ref function_decls, ref data_decls, diff --git a/lib/module/src/backend.rs b/lib/module/src/backend.rs index bc151efce9..44a2aa95af 100644 --- a/lib/module/src/backend.rs +++ b/lib/module/src/backend.rs @@ -65,6 +65,7 @@ where fn define_data( &mut self, name: &str, + writable: bool, data_ctx: &DataContext, namespace: &ModuleNamespace, ) -> ModuleResult; diff --git a/lib/module/src/data_context.rs b/lib/module/src/data_context.rs index e29389517b..4a7fdd7a6e 100644 --- a/lib/module/src/data_context.rs +++ b/lib/module/src/data_context.rs @@ -34,19 +34,8 @@ impl Init { } } -/// A flag specifying whether data is readonly or may be written to. -#[derive(Copy, Clone, PartialEq, Eq, Debug)] -pub enum Writability { - /// Data is readonly, meaning writes to it will trap. - Readonly, - /// Data is writable. - Writable, -} - /// A description of a data object. pub struct DataDescription { - /// Whether the data readonly or writable. - pub writable: Writability, /// How the data should be initialized. pub init: Init, /// External function declarations. @@ -69,7 +58,6 @@ impl DataContext { pub fn new() -> Self { Self { description: DataDescription { - writable: Writability::Readonly, init: Init::Uninitialized, function_decls: PrimaryMap::new(), data_decls: PrimaryMap::new(), @@ -81,7 +69,6 @@ impl DataContext { /// Clear all data structures in this context. pub fn clear(&mut self) { - self.description.writable = Writability::Readonly; self.description.init = Init::Uninitialized; self.description.function_decls.clear(); self.description.data_decls.clear(); @@ -90,18 +77,16 @@ impl DataContext { } /// Define a zero-initialized object with the given size. - pub fn define_zeroinit(&mut self, size: usize, writable: Writability) { + pub fn define_zeroinit(&mut self, size: usize) { debug_assert_eq!(self.description.init, Init::Uninitialized); - self.description.writable = writable; self.description.init = Init::Zeros { 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) { + pub fn define(&mut self, contents: Box<[u8]>) { debug_assert_eq!(self.description.init, Init::Uninitialized); - self.description.writable = writable; self.description.init = Init::Bytes { contents }; } @@ -148,14 +133,13 @@ impl DataContext { #[cfg(test)] mod tests { use cranelift_codegen::ir; - use {DataContext, Init, Writability}; + use {DataContext, Init}; #[test] fn basic_data_context() { let mut data_ctx = DataContext::new(); { let description = &data_ctx.description; - assert_eq!(description.writable, Writability::Readonly); assert_eq!(description.init, Init::Uninitialized); assert!(description.function_decls.is_empty()); assert!(description.data_decls.is_empty()); @@ -163,7 +147,7 @@ mod tests { assert!(description.data_relocs.is_empty()); } - data_ctx.define_zeroinit(256, Writability::Writable); + data_ctx.define_zeroinit(256); let _func_a = data_ctx.import_function(ir::ExternalName::user(0, 0)); let func_b = data_ctx.import_function(ir::ExternalName::user(0, 1)); @@ -177,7 +161,6 @@ mod tests { { let description = data_ctx.description(); - assert_eq!(description.writable, Writability::Writable); assert_eq!(description.init, Init::Zeros { size: 256 }); assert_eq!(description.function_decls.len(), 3); assert_eq!(description.data_decls.len(), 2); @@ -188,7 +171,6 @@ mod tests { data_ctx.clear(); { let description = &data_ctx.description; - assert_eq!(description.writable, Writability::Readonly); assert_eq!(description.init, Init::Uninitialized); assert!(description.function_decls.is_empty()); assert!(description.data_decls.is_empty()); @@ -198,10 +180,9 @@ mod tests { let contents = vec![33, 34, 35, 36]; let contents_clone = contents.clone(); - data_ctx.define(contents.into_boxed_slice(), Writability::Readonly); + data_ctx.define(contents.into_boxed_slice()); { let description = data_ctx.description(); - assert_eq!(description.writable, Writability::Readonly); assert_eq!( description.init, Init::Bytes { diff --git a/lib/module/src/lib.rs b/lib/module/src/lib.rs index 45e3bd683e..479288df57 100644 --- a/lib/module/src/lib.rs +++ b/lib/module/src/lib.rs @@ -33,7 +33,7 @@ mod data_context; mod module; pub use backend::Backend; -pub use data_context::{DataContext, DataDescription, Init, Writability}; +pub use data_context::{DataContext, DataDescription, Init}; pub use module::{ DataId, FuncId, FuncOrDataId, Linkage, Module, ModuleError, ModuleNamespace, ModuleResult, }; diff --git a/lib/module/src/module.rs b/lib/module/src/module.rs index 223f748107..185057b946 100644 --- a/lib/module/src/module.rs +++ b/lib/module/src/module.rs @@ -507,6 +507,7 @@ where } Some(self.backend.define_data( &info.decl.name, + info.decl.writable, data_ctx, &ModuleNamespace:: { contents: &self.contents, diff --git a/lib/simplejit/src/backend.rs b/lib/simplejit/src/backend.rs index 15c3f2b2c1..2a23b93f0c 100644 --- a/lib/simplejit/src/backend.rs +++ b/lib/simplejit/src/backend.rs @@ -5,7 +5,6 @@ use cranelift_codegen::isa::TargetIsa; use cranelift_codegen::{self, ir, settings}; use cranelift_module::{ Backend, DataContext, DataDescription, Init, Linkage, ModuleNamespace, ModuleResult, - Writability, }; use cranelift_native; use libc; @@ -192,11 +191,11 @@ impl<'simple_jit_backend> Backend for SimpleJITBackend { fn define_data( &mut self, _name: &str, + writable: bool, data: &DataContext, _namespace: &ModuleNamespace, ) -> ModuleResult { let &DataDescription { - writable, ref init, ref function_decls, ref data_decls, @@ -205,15 +204,14 @@ impl<'simple_jit_backend> Backend for SimpleJITBackend { } = data.description(); let size = init.size(); - let storage = match writable { - Writability::Readonly => self - .readonly_memory + let storage = if writable { + self.writable_memory .allocate(size) - .expect("TODO: handle OOM etc."), - Writability::Writable => self - .writable_memory + .expect("TODO: handle OOM etc.") + } else { + self.readonly_memory .allocate(size) - .expect("TODO: handle OOM etc."), + .expect("TODO: handle OOM etc.") }; match *init {