Mark emit_to_memory as unsafe, and introduce a safe emit. (#281)

* Mark emit_to_memory as unsafe, and provide a safe compile_and_emit.

Mark `Context::emit_to_memory` and `MemoryCodeSink::new` as unsafe, as
`MemoryCodeSink` does not perform bounds checking when writing to
memory.

Add a `Context::compile_and_emit` function which provides a convenient
interface for doing `compile` and `emit_to_memory` in one step, and
which can also provide a safe interface, since it allocates memory of
the needed size itself.

* Mention that `MemoryCodeSink` can't guarantee that the pointer is valid.

cranelift/src/compile.rs

@@ -96,19 +96,18 @@ fn handle_module(
     for (func, _) in test_file.functions {
         let mut context = Context::new();
         context.func = func;
-        let size = context.compile(isa).map_err(|err| {
-            pretty_error(&context.func, Some(isa), err)
-        })?;
-        if flag_print {
-            println!("{}", context.func.display(isa));
-        }
 
-        // Encode the result as machine code.
+        // Compile and encode the result to machine code.
         let mut mem = Vec::new();
         let mut relocs = PrintRelocs { flag_print };
         let mut traps = PrintTraps { flag_print };
-        mem.resize(size as usize, 0);
+        context
-        context.emit_to_memory(mem.as_mut_ptr(), &mut relocs, &mut traps, &*isa);
+            .compile_and_emit(isa, &mut mem, &mut relocs, &mut traps)
+            .map_err(|err| pretty_error(&context.func, Some(isa), err))?;
+
+        if flag_print {
+            println!("{}", context.func.display(isa));
+        }
 
         if flag_print {
             print!(".byte ");

lib/codegen/src/binemit/memorysink.rs

@@ -38,7 +38,10 @@ pub struct MemoryCodeSink<'a> {
 
 impl<'a> MemoryCodeSink<'a> {
     /// Create a new memory code sink that writes a function to the memory pointed to by `data`.
-    pub fn new<'sink>(
+    ///
+    /// This function is unsafe since `MemoryCodeSink` does not perform bounds checking on the
+    /// memory buffer, and it can't guarantee that the `data` pointer is valid.
+    pub unsafe fn new<'sink>(
         data: *mut u8,
         relocs: &'sink mut RelocSink,
         traps: &'sink mut TrapSink,

lib/codegen/src/context.rs

@@ -78,6 +78,36 @@ impl Context {
         self.loop_analysis.clear();
     }
 
+    /// Compile the function, and emit machine code into a `Vec<u8>`.
+    ///
+    /// Run the function through all the passes necessary to generate code for the target ISA
+    /// represented by `isa`, as well as the final step of emitting machine code into a
+    /// `Vec<u8>`. The machine code is not relocated. Instead, any relocations are emitted
+    /// into `relocs`.
+    ///
+    /// This function calls `compile` and `emit_to_memory`, taking care to resize `mem` as
+    /// needed, so it provides a safe interface.
+    pub fn compile_and_emit(
+        &mut self,
+        isa: &TargetIsa,
+        mem: &mut Vec<u8>,
+        relocs: &mut RelocSink,
+        traps: &mut TrapSink,
+    ) -> CtonResult {
+        let code_size = self.compile(isa)?;
+        let old_len = mem.len();
+        mem.resize(old_len + code_size as usize, 0);
+        unsafe {
+            self.emit_to_memory(
+                isa,
+                mem.as_mut_ptr().offset(old_len as isize),
+                relocs,
+                traps,
+            )
+        };
+        Ok(())
+    }
+
     /// Compile the function.
     ///
     /// Run the function through all the passes necessary to generate code for the target ISA
@@ -119,12 +149,15 @@ impl Context {
     /// code is returned by `compile` above.
     ///
     /// The machine code is not relocated. Instead, any relocations are emitted into `relocs`.
-    pub fn emit_to_memory(
+    ///
+    /// This function is unsafe since it does not perform bounds checking on the memory buffer,
+    /// and it can't guarantee that the `mem` pointer is valid.
+    pub unsafe fn emit_to_memory(
         &self,
+        isa: &TargetIsa,
         mem: *mut u8,
         relocs: &mut RelocSink,
         traps: &mut TrapSink,
-        isa: &TargetIsa,
     ) {
         let _tt = timing::binemit();
         isa.emit_function(&self.func, &mut MemoryCodeSink::new(mem, relocs, traps));

lib/faerie/src/backend.rs

@@ -107,12 +107,14 @@ impl Backend for FaerieBackend {
             // that traps.
             let mut trap_sink = NullTrapSink {};
 
-            ctx.emit_to_memory(
+            unsafe {
-                code.as_mut_ptr(),
+                ctx.emit_to_memory(
-                &mut reloc_sink,
+                    &*self.isa,
-                &mut trap_sink,
+                    code.as_mut_ptr(),
-                &*self.isa,
+                    &mut reloc_sink,
-            );
+                    &mut trap_sink,
+                )
+            };
         }
 
         self.artifact.define(name, code).expect(

lib/simplejit/src/backend.rs

@@ -103,7 +103,7 @@ impl<'simple_jit_backend> Backend for SimpleJITBackend {
         // Ignore traps for now. For now, frontends should just avoid generating code
         // that traps.
         let mut trap_sink = NullTrapSink {};
-        ctx.emit_to_memory(ptr, &mut reloc_sink, &mut trap_sink, &*self.isa);
+        unsafe { ctx.emit_to_memory(&*self.isa, ptr, &mut reloc_sink, &mut trap_sink) };
 
         Ok(Self::CompiledFunction {
             code: ptr,