Bug 1626967: Use the regular Cranelift sinks to keep track of relocations; r=rhunt
authorBenjamin Bouvier <benj@benj.me>
Tue, 07 Apr 2020 10:37:13 +0000
changeset 522558 f48ad40e60afd31b716ff5bb15e69c99c9180945
parent 522557 29aaf3ff21eaa5792c49ce646ee78983d34b1bbc
child 522559 837dab8ce31cb96546ca90b5703331644961ce94
push id37292
push userccoroiu@mozilla.com
push dateTue, 07 Apr 2020 16:05:40 +0000
treeherdermozilla-central@6c6c67870c37 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrhunt
bugs1626967
milestone77.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1626967: Use the regular Cranelift sinks to keep track of relocations; r=rhunt This goes hand in hand with https://github.com/bytecodealliance/wasmtime/pull/1460, which will need to be merged first. This removes most of the Spidermonkey-specific machinery to collect relocations. The only remaining place is for stack-maps, which will be handled later. Differential Revision: https://phabricator.services.mozilla.com/D69394
js/src/wasm/WasmCraneliftCompile.cpp
js/src/wasm/cranelift/baldrapi.h
js/src/wasm/cranelift/src/bindings/mod.rs
js/src/wasm/cranelift/src/compile.rs
--- a/js/src/wasm/WasmCraneliftCompile.cpp
+++ b/js/src/wasm/WasmCraneliftCompile.cpp
@@ -136,19 +136,21 @@ static bool GenerateCraneliftCode(WasmMa
     ArgTypeVector args(funcType);
     wasm::StackMap* functionEntryStackMap = nullptr;
     if (!CreateStackMapForFunctionEntryTrap(
             args, trapExitLayout, trapExitLayoutNumWords,
             nBytesReservedBeforeTrap, nInboundStackArgBytes,
             &functionEntryStackMap)) {
       return false;
     }
+
     // In debug builds, we'll always have a stack map, even if there are no
     // refs to track.
     MOZ_ALWAYS_TRUE(functionEntryStackMap);
+
     if (functionEntryStackMap &&
         !stackMaps->add((uint8_t*)(uintptr_t)trapInsnOffset.offset(),
                         functionEntryStackMap)) {
       functionEntryStackMap->destroy();
       return false;
     }
   }
   MOZ_ASSERT(masm.framePushed() == func.framePushed);
@@ -223,29 +225,26 @@ static bool GenerateCraneliftCode(WasmMa
     if (!offset.isValid()) {
       return false;
     }
 
 #ifdef DEBUG
     // Check code offsets.
     MOZ_ASSERT(offset.value() >= offsets->normalEntry);
     MOZ_ASSERT(offset.value() < offsets->ret);
+    MOZ_ASSERT(metadata.moduleBytecodeOffset != 0);
 
     // Check bytecode offsets.
-    if (metadata.moduleBytecodeOffset > 0 && lineOrBytecode > 0) {
+    if (lineOrBytecode > 0) {
       MOZ_ASSERT(metadata.moduleBytecodeOffset >= lineOrBytecode);
       MOZ_ASSERT(metadata.moduleBytecodeOffset <
                  lineOrBytecode + funcBytecodeSize);
     }
 #endif
-    // TODO(bug 1532716): Cranelift gives null bytecode offsets for symbolic
-    // accesses.
-    uint32_t bytecodeOffset = metadata.moduleBytecodeOffset
-                                  ? metadata.moduleBytecodeOffset
-                                  : lineOrBytecode;
+    uint32_t bytecodeOffset = metadata.moduleBytecodeOffset;
 
     switch (metadata.which) {
       case CraneliftMetadataEntry::Which::DirectCall: {
         CallSiteDesc desc(bytecodeOffset, CallSiteDesc::Func);
         masm.append(desc, CodeOffset(offset.value()), metadata.extra);
         break;
       }
       case CraneliftMetadataEntry::Which::IndirectCall: {
@@ -254,25 +253,24 @@ static bool GenerateCraneliftCode(WasmMa
         break;
       }
       case CraneliftMetadataEntry::Which::Trap: {
         Trap trap = (Trap)metadata.extra;
         BytecodeOffset trapOffset(bytecodeOffset);
         masm.append(trap, wasm::TrapSite(offset.value(), trapOffset));
         break;
       }
-      case CraneliftMetadataEntry::Which::MemoryAccess: {
-        BytecodeOffset trapOffset(bytecodeOffset);
-        masm.appendOutOfBoundsTrap(trapOffset, offset.value());
-        break;
-      }
       case CraneliftMetadataEntry::Which::SymbolicAccess: {
+        CodeOffset raOffset(offset.value());
+        CallSiteDesc desc(bytecodeOffset, CallSiteDesc::Symbolic);
+        masm.append(desc, raOffset);
+
         SymbolicAddress sym =
             ToSymbolicAddress(BD_SymbolicAddress(metadata.extra));
-        masm.append(SymbolicAccess(CodeOffset(offset.value()), sym));
+        masm.append(SymbolicAccess(raOffset, sym));
         break;
       }
       default: {
         MOZ_CRASH("unknown cranelift metadata kind");
       }
     }
   }
 
--- a/js/src/wasm/cranelift/baldrapi.h
+++ b/js/src/wasm/cranelift/baldrapi.h
@@ -102,38 +102,32 @@ struct BD_Stackmaps;
 // struct, but formatted in a Rust-friendly way.
 
 struct CraneliftFuncCompileInput {
   const uint8_t* bytecode;
   size_t bytecodeSize;
   uint32_t index;
   uint32_t offset_in_module;
 
-  // The stackmaps sink to use when compiling this function
+  // The stackmaps sink to use when compiling this function.
   BD_Stackmaps* stackmaps;
 
   // Not bindgen'd because it's inlined.
   explicit inline CraneliftFuncCompileInput(const js::wasm::FuncCompileInput&);
 };
 
 // A single entry in all the metadata array provided after the compilation of a
 // single wasm function. The meaning of the field extra depends on the enum
 // value.
 //
 // XXX should we use a union for this instead? bindgen seems to be able to
 // handle them, with a lot of unsafe'ing.
 
 struct CraneliftMetadataEntry {
-  enum Which {
-    DirectCall,
-    IndirectCall,
-    Trap,
-    MemoryAccess,
-    SymbolicAccess
-  } which;
+  enum Which { DirectCall, IndirectCall, Trap, SymbolicAccess } which;
   uint32_t codeOffset;
   uint32_t moduleBytecodeOffset;
   size_t extra;
 };
 
 // The result of a single function compilation, containing the machine code
 // generated by Cranelift, as well as some useful metadata to generate the
 // prologue/epilogue etc.
--- a/js/src/wasm/cranelift/src/bindings/mod.rs
+++ b/js/src/wasm/cranelift/src/bindings/mod.rs
@@ -259,57 +259,49 @@ impl CompiledFunc {
         self.totalSize = compiled_func.code_buffer.len();
 
         self.numRodataRelocs = compiled_func.rodata_relocs.len();
         self.rodataRelocs = compiled_func.rodata_relocs.as_ptr();
     }
 }
 
 impl MetadataEntry {
-    pub fn direct_call(code_offset: CodeOffset, func_index: FuncIndex, srcloc: SourceLoc) -> Self {
+    pub fn direct_call(code_offset: CodeOffset, srcloc: SourceLoc, func_index: FuncIndex) -> Self {
         Self {
             which: CraneliftMetadataEntry_Which_DirectCall,
             codeOffset: code_offset,
             moduleBytecodeOffset: srcloc.bits(),
             extra: func_index.index(),
         }
     }
-
-    pub fn indirect_call(code_offset: CodeOffset, srcloc: SourceLoc) -> Self {
+    pub fn indirect_call(ret_addr: CodeOffset, srcloc: SourceLoc) -> Self {
         Self {
             which: CraneliftMetadataEntry_Which_IndirectCall,
-            codeOffset: code_offset,
+            codeOffset: ret_addr,
             moduleBytecodeOffset: srcloc.bits(),
             extra: 0,
         }
     }
-
     pub fn trap(code_offset: CodeOffset, srcloc: SourceLoc, which: Trap) -> Self {
         Self {
             which: CraneliftMetadataEntry_Which_Trap,
             codeOffset: code_offset,
             moduleBytecodeOffset: srcloc.bits(),
             extra: which as usize,
         }
     }
-
-    pub fn memory_access(code_offset: CodeOffset, srcloc: SourceLoc) -> Self {
-        Self {
-            which: CraneliftMetadataEntry_Which_MemoryAccess,
-            codeOffset: code_offset,
-            moduleBytecodeOffset: srcloc.bits(),
-            extra: 0,
-        }
-    }
-
-    pub fn symbolic_access(code_offset: CodeOffset, sym: SymbolicAddress) -> Self {
+    pub fn symbolic_access(
+        code_offset: CodeOffset,
+        srcloc: SourceLoc,
+        sym: SymbolicAddress,
+    ) -> Self {
         Self {
             which: CraneliftMetadataEntry_Which_SymbolicAccess,
             codeOffset: code_offset,
-            moduleBytecodeOffset: 0,
+            moduleBytecodeOffset: srcloc.bits(),
             extra: sym as usize,
         }
     }
 }
 
 impl StaticEnvironment {
     /// Returns the default calling convention on this machine.
     pub fn call_conv(&self) -> isa::CallConv {
--- a/js/src/wasm/cranelift/src/compile.rs
+++ b/js/src/wasm/cranelift/src/compile.rs
@@ -17,20 +17,23 @@
 //!
 //! This module defines the `compile()` function which uses Cranelift to compile a single
 //! WebAssembly function.
 
 use std::fmt;
 use std::mem;
 
 use cranelift_codegen::binemit::{
-    Addend, CodeInfo, CodeOffset, NullStackmapSink, NullTrapSink, Reloc, RelocSink, Stackmap,
+    Addend, CodeInfo, CodeOffset, NullStackmapSink, Reloc, RelocSink, Stackmap, TrapSink,
 };
 use cranelift_codegen::entity::EntityRef;
-use cranelift_codegen::ir::{self, constant::ConstantOffset, stackslot::StackSize};
+use cranelift_codegen::ir::{
+    self, constant::ConstantOffset, stackslot::StackSize, ExternalName, JumpTable, SourceLoc,
+    TrapCode,
+};
 use cranelift_codegen::isa::TargetIsa;
 use cranelift_codegen::CodegenResult;
 use cranelift_codegen::Context;
 use cranelift_wasm::{FuncIndex, FuncTranslator, ModuleTranslationState, WasmResult};
 
 use crate::bindings;
 use crate::isa::make_isa;
 use crate::utils::DashResult;
@@ -153,62 +156,87 @@ impl<'a, 'b> BatchCompiler<'a, 'b> {
 
         info!(
             "Emitting {} bytes, frame_pushed={}\n.",
             total_size, frame_pushed
         );
 
         self.current_func.reset(frame_pushed, contains_calls);
 
-        // Generate metadata about function calls and traps now that the emitter knows where the
-        // Cranelift code is going to end up.
-        let mut metadata = mem::replace(&mut self.current_func.metadata, vec![]);
-        self.emit_metadata(&mut metadata, stackmaps);
-        mem::swap(&mut metadata, &mut self.current_func.metadata);
-
         // TODO: If we can get a pointer into `size` pre-allocated bytes of memory, we wouldn't
         // have to allocate and copy here.
         // TODO(bbouvier) try to get this pointer from the C++ caller, with an unlikely callback to
         // C++ if the remaining size is smaller than  needed.
         if self.current_func.code_buffer.len() < total_size {
             let current_size = self.current_func.code_buffer.len();
             // There's no way to do a proper uninitialized reserve, so first reserve and then
             // unsafely set the final size.
             self.current_func
                 .code_buffer
                 .reserve(total_size - current_size);
             unsafe { self.current_func.code_buffer.set_len(total_size) };
         }
 
         {
-            let emit_env = &mut EmitEnv::new(
+            let mut traps = Traps::new();
+            let mut relocs = Relocations::new(
                 &mut self.current_func.metadata,
                 &mut self.current_func.rodata_relocs,
             );
-            let mut trap_sink = NullTrapSink {};
 
+            let code_buffer = &mut self.current_func.code_buffer;
             unsafe {
-                let code_buffer = &mut self.current_func.code_buffer;
                 self.context.emit_to_memory(
                     &*self.isa,
                     code_buffer.as_mut_ptr(),
-                    emit_env,
-                    &mut trap_sink,
+                    &mut relocs,
+                    &mut traps,
                     &mut NullStackmapSink {},
                 )
             };
+
+            self.current_func.metadata.append(&mut traps.metadata);
+        }
+
+        if self.static_environ.refTypesEnabled {
+            self.emit_stackmaps(stackmaps);
         }
 
         self.current_func.code_size = info.code_size;
         self.current_func.jumptables_size = info.jumptables_size;
         self.current_func.rodata_size = info.rodata_size;
 
         Ok(())
     }
 
+    /// Iterate over each instruction to generate a stack map for each instruction that needs it.
+    ///
+    /// Note a stackmap is associated to the address of the next instruction following the actual
+    /// instruction needing the stack map. This is because this is the only information
+    /// Spidermonkey has access to when it looks up a stack map (during stack frame iteration).
+    fn emit_stackmaps(&self, mut stackmaps: bindings::Stackmaps) {
+        let encinfo = self.isa.encoding_info();
+        let func = &self.context.func;
+        let stack_slots = &func.stack_slots;
+        for block in func.layout.blocks() {
+            let mut pending_safepoint = None;
+            for (offset, inst, inst_size) in func.inst_offsets(block, &encinfo) {
+                if let Some(stackmap) = pending_safepoint.take() {
+                    stackmaps.add_stackmap(stack_slots, offset + inst_size, stackmap);
+                }
+                if func.dfg[inst].opcode() == ir::Opcode::Safepoint {
+                    let args = func.dfg.inst_args(inst);
+                    let stackmap = Stackmap::from_values(&args, func, &*self.isa);
+                    pending_safepoint = Some(stackmap);
+                }
+            }
+            debug_assert!(pending_safepoint.is_none());
+        }
+    }
+
     /// Compute the `framePushed` argument to pass to `GenerateFunctionPrologue`. This is the
     /// number of frame bytes used by Cranelift, not counting the values pushed by the standard
     /// prologue generated by `GenerateFunctionPrologue`.
     fn frame_pushed(&self) -> StackSize {
         // Cranelift computes the total stack frame size including the pushed return address,
         // standard SM prologue pushes, and its own stack slots.
         let total = self
             .context
@@ -225,371 +253,213 @@ impl<'a, 'b> BatchCompiler<'a, 'b> {
     }
 
     /// Determine whether the current function may contain calls.
     fn contains_calls(&self) -> bool {
         // Conservatively, just check to see if it contains any function
         // signatures which could be called.
         !self.context.func.dfg.signatures.is_empty()
     }
-
-    #[cfg(feature = "cranelift_x86")]
-    fn platform_specific_ignores_metadata(opcode: ir::Opcode) -> bool {
-        match opcode {
-            ir::Opcode::X86Sdivmodx | ir::Opcode::X86Udivmodx => true,
-            _ => false,
-        }
-    }
-
-    #[cfg(not(feature = "cranelift_x86"))]
-    fn platform_specific_ignores_metadata(_opcode: ir::Opcode) -> bool {
-        false
-    }
-
-    /// Emit metadata by scanning the compiled function before `emit_to_memory`.
-    ///
-    /// - All call sites need metadata: direct, indirect, symbolic.
-    /// - All explicit traps must be registered.
-    ///
-    /// We don't get enough callbacks through the `RelocSink` trait to generate all the metadata we
-    /// need.
-    fn emit_metadata(
-        &self,
-        metadata: &mut Vec<bindings::MetadataEntry>,
-        mut stackmaps: bindings::Stackmaps,
-    ) {
-        let encinfo = self.isa.encoding_info();
-        let func = &self.context.func;
-        let stack_slots = &func.stack_slots;
-        for block in func.layout.blocks() {
-            let mut pending_safepoint = None;
-            for (offset, inst, inst_size) in func.inst_offsets(block, &encinfo) {
-                if let Some(stackmap) = pending_safepoint.take() {
-                    stackmaps.add_stackmap(stack_slots, offset + inst_size, stackmap);
-                }
-                let opcode = func.dfg[inst].opcode();
-                match opcode {
-                    ir::Opcode::Call => self.call_metadata(metadata, inst, offset + inst_size),
-                    ir::Opcode::CallIndirect => {
-                        self.indirect_call_metadata(metadata, inst, offset + inst_size)
-                    }
-                    ir::Opcode::Trap | ir::Opcode::Trapif | ir::Opcode::Trapff => {
-                        self.trap_metadata(metadata, inst, offset)
-                    }
-                    ir::Opcode::Safepoint => {
-                        let args = func.dfg.inst_args(inst);
-                        let stackmap = Stackmap::from_values(&args, func, &*self.isa);
-                        pending_safepoint = Some(stackmap);
-                    }
-                    ir::Opcode::Load
-                    | ir::Opcode::LoadComplex
-                    | ir::Opcode::Uload8
-                    | ir::Opcode::Uload8Complex
-                    | ir::Opcode::Sload8
-                    | ir::Opcode::Sload8Complex
-                    | ir::Opcode::Uload16
-                    | ir::Opcode::Uload16Complex
-                    | ir::Opcode::Sload16
-                    | ir::Opcode::Sload16Complex
-                    | ir::Opcode::Uload32
-                    | ir::Opcode::Uload32Complex
-                    | ir::Opcode::Sload32
-                    | ir::Opcode::Sload32Complex
-                    | ir::Opcode::Store
-                    | ir::Opcode::StoreComplex
-                    | ir::Opcode::Istore8
-                    | ir::Opcode::Istore8Complex
-                    | ir::Opcode::Istore16
-                    | ir::Opcode::Istore16Complex
-                    | ir::Opcode::Istore32
-                    | ir::Opcode::Istore32Complex => self.memory_metadata(metadata, inst, offset),
-
-                    // Instructions that are not going to trap in our use, even though their opcode
-                    // says they can.
-                    ir::Opcode::Spill
-                    | ir::Opcode::Fill
-                    | ir::Opcode::FillNop
-                    | ir::Opcode::JumpTableEntry => {}
-
-                    _ if BatchCompiler::platform_specific_ignores_metadata(opcode) => {}
-
-                    _ => {
-                        debug_assert!(!opcode.is_call(), "Missed call opcode");
-                        debug_assert!(
-                            !opcode.can_trap(),
-                            "Missed trap: {}",
-                            func.dfg.display_inst(inst, Some(self.isa.as_ref()))
-                        );
-                        debug_assert!(
-                            !opcode.can_load(),
-                            "Missed load: {}",
-                            func.dfg.display_inst(inst, Some(self.isa.as_ref()))
-                        );
-                        debug_assert!(
-                            !opcode.can_store(),
-                            "Missed store: {}",
-                            func.dfg.display_inst(inst, Some(self.isa.as_ref()))
-                        );
-                    }
-                }
-            }
-
-            assert!(pending_safepoint.is_none());
-        }
-    }
-
-    fn srcloc(&self, inst: ir::Inst) -> ir::SourceLoc {
-        let srcloc = self.context.func.srclocs[inst];
-        debug_assert!(
-            !srcloc.is_default(),
-            "No source location on {}",
-            self.context
-                .func
-                .dfg
-                .display_inst(inst, Some(self.isa.as_ref()))
-        );
-        srcloc
-    }
-
-    /// Emit metadata for direct call `inst`.
-    fn call_metadata(
-        &self,
-        metadata: &mut Vec<bindings::MetadataEntry>,
-        inst: ir::Inst,
-        ret_addr: CodeOffset,
-    ) {
-        let func = &self.context.func;
-
-        // This is a direct call, so the callee should be a non-imported wasm
-        // function. We register both the call site *and* the target for relocation.
-        let callee = match func.dfg[inst] {
-            ir::InstructionData::Call { func_ref, .. } => &func.dfg.ext_funcs[func_ref].name,
-            _ => panic!("Bad format for call"),
-        };
-
-        let func_index = match *callee {
-            ir::ExternalName::User {
-                namespace: USER_FUNCTION_NAMESPACE,
-                index,
-            } => FuncIndex::new(index as usize),
-            _ => panic!("Direct call to {} unsupported", callee),
-        };
-
-        metadata.push(bindings::MetadataEntry::direct_call(
-            ret_addr,
-            func_index,
-            self.srcloc(inst),
-        ));
-    }
-
-    /// Emit metadata for indirect call `inst`.
-    fn indirect_call_metadata(
-        &self,
-        metadata: &mut Vec<bindings::MetadataEntry>,
-        inst: ir::Inst,
-        ret_addr: CodeOffset,
-    ) {
-        // A call_indirect instruction can represent either a table call or a far call to a runtime
-        // function. The CallSiteDesc::Kind enum does distinguish between the two, but it is not
-        // clear that the information is used anywhere. For now, we won't bother distinguishing
-        // them, and mark all calls as `Kind::Dynamic`.
-        //
-        // If we do need to make a distinction in the future, it is probably easiest to add a
-        // `call_far` instruction to Cranelift that encodes like an indirect call, but includes the
-        // callee like a direct call.
-        metadata.push(bindings::MetadataEntry::indirect_call(
-            ret_addr,
-            self.srcloc(inst),
-        ));
-    }
-
-    fn trap_metadata(
-        &self,
-        metadata: &mut Vec<bindings::MetadataEntry>,
-        inst: ir::Inst,
-        offset: CodeOffset,
-    ) {
-        let func = &self.context.func;
-        let (code, trap_offset) = match func.dfg[inst] {
-            ir::InstructionData::Trap { code, .. } => (code, 0),
-            ir::InstructionData::IntCondTrap { code, .. }
-            | ir::InstructionData::FloatCondTrap { code, .. } => {
-                // This instruction expands to a conditional branch over ud2 on Intel archs.
-                // The actual trap happens on the ud2 instruction.
-                (code, 2)
-            }
-            _ => panic!("Bad format for trap"),
-        };
-
-        // Translate the trap code into one of BaldrMonkey's trap codes.
-        let bd_trap = match code {
-            ir::TrapCode::StackOverflow => bindings::Trap::StackOverflow,
-            ir::TrapCode::HeapOutOfBounds => bindings::Trap::OutOfBounds,
-            ir::TrapCode::OutOfBounds => bindings::Trap::OutOfBounds,
-            ir::TrapCode::TableOutOfBounds => bindings::Trap::OutOfBounds,
-            ir::TrapCode::IndirectCallToNull => bindings::Trap::IndirectCallToNull,
-            ir::TrapCode::BadSignature => bindings::Trap::IndirectCallBadSig,
-            ir::TrapCode::IntegerOverflow => bindings::Trap::IntegerOverflow,
-            ir::TrapCode::IntegerDivisionByZero => bindings::Trap::IntegerDivideByZero,
-            ir::TrapCode::BadConversionToInteger => bindings::Trap::InvalidConversionToInteger,
-            ir::TrapCode::Interrupt => bindings::Trap::CheckInterrupt,
-            ir::TrapCode::UnreachableCodeReached => bindings::Trap::Unreachable,
-            ir::TrapCode::User(x) if x == TRAP_THROW_REPORTED => bindings::Trap::ThrowReported,
-            ir::TrapCode::User(_) => panic!("Uncovered trap code {}", code),
-        };
-
-        metadata.push(bindings::MetadataEntry::trap(
-            offset + trap_offset,
-            self.srcloc(inst),
-            bd_trap,
-        ));
-    }
-
-    fn memory_metadata(
-        &self,
-        metadata: &mut Vec<bindings::MetadataEntry>,
-        inst: ir::Inst,
-        offset: CodeOffset,
-    ) {
-        let func = &self.context.func;
-        let memflags = match func.dfg[inst] {
-            ir::InstructionData::Load { flags, .. }
-            | ir::InstructionData::LoadComplex { flags, .. }
-            | ir::InstructionData::Store { flags, .. }
-            | ir::InstructionData::StoreComplex { flags, .. } => flags,
-            _ => panic!("Bad format for memory access"),
-        };
-
-        // Some load/store instructions may be accessing VM data structures instead of the
-        // WebAssembly heap. These are tagged with `notrap` since their trapping is not part of
-        // the semantics, i.e. that would be a bug.
-        if memflags.notrap() {
-            return;
-        }
-
-        metadata.push(bindings::MetadataEntry::memory_access(
-            offset,
-            self.srcloc(inst),
-        ));
-    }
 }
 
 impl<'a, 'b> fmt::Display for BatchCompiler<'a, 'b> {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         write!(f, "{}", self.context.func.display(self.isa.as_ref()))
     }
 }
 
 /// Create a Cranelift function name representing a WebAssembly function with `index`.
-pub fn wasm_function_name(func: FuncIndex) -> ir::ExternalName {
-    ir::ExternalName::User {
+pub fn wasm_function_name(func: FuncIndex) -> ExternalName {
+    ExternalName::User {
         namespace: USER_FUNCTION_NAMESPACE,
         index: func.index() as u32,
     }
 }
 
 /// Create a Cranelift function name representing a builtin function.
-pub fn symbolic_function_name(sym: bindings::SymbolicAddress) -> ir::ExternalName {
-    ir::ExternalName::User {
+pub fn symbolic_function_name(sym: bindings::SymbolicAddress) -> ExternalName {
+    ExternalName::User {
         namespace: SYMBOLIC_FUNCTION_NAMESPACE,
         index: sym as u32,
     }
 }
 
-/// References joined so we can implement `RelocSink`.
-struct EmitEnv<'a> {
+struct Relocations<'a> {
     metadata: &'a mut Vec<bindings::MetadataEntry>,
     rodata_relocs: &'a mut Vec<CodeOffset>,
 }
 
-impl<'a> EmitEnv<'a> {
-    pub fn new(
+impl<'a> Relocations<'a> {
+    fn new(
         metadata: &'a mut Vec<bindings::MetadataEntry>,
         rodata_relocs: &'a mut Vec<CodeOffset>,
-    ) -> EmitEnv<'a> {
-        EmitEnv {
+    ) -> Self {
+        Self {
             metadata,
             rodata_relocs,
         }
     }
 }
 
-impl<'a> RelocSink for EmitEnv<'a> {
-    fn reloc_block(&mut self, _offset: CodeOffset, _reloc: Reloc, _block_offset: CodeOffset) {
-        unimplemented!();
+impl<'a> RelocSink for Relocations<'a> {
+    /// Add a relocation referencing a block at the current offset.
+    fn reloc_block(&mut self, _at: CodeOffset, _reloc: Reloc, _block_offset: CodeOffset) {
+        unimplemented!("block relocations NYI");
     }
 
+    /// Add a relocation referencing an external symbol at the current offset.
     fn reloc_external(
         &mut self,
-        offset: CodeOffset,
-        _reloc: Reloc,
-        name: &ir::ExternalName,
+        at: CodeOffset,
+        srcloc: SourceLoc,
+        reloc: Reloc,
+        name: &ExternalName,
         _addend: Addend,
     ) {
-        // Decode the function name.
+        debug_assert!(!srcloc.is_default());
+
         match *name {
-            ir::ExternalName::User {
+            ExternalName::User {
                 namespace: USER_FUNCTION_NAMESPACE,
-                ..
+                index,
             } => {
-                // This is a direct function call handled by `call_metadata` above.
+                // A simple function call to another wasm function.
+                let payload_size = match reloc {
+                    Reloc::X86CallPCRel4 => 4,
+                    _ => panic!("unhandled call relocation"),
+                };
+
+                let func_index = FuncIndex::new(index as usize);
+
+                // The Spidermonkey relocation must point to the next instruction. Cranelift gives
+                // us the exact offset to the immediate, so fix it up by the relocation's size.
+                let offset = at + payload_size;
+                self.metadata.push(bindings::MetadataEntry::direct_call(
+                    offset, srcloc, func_index,
+                ));
             }
 
-            ir::ExternalName::User {
+            ExternalName::User {
                 namespace: SYMBOLIC_FUNCTION_NAMESPACE,
                 index,
             } => {
+                let payload_size = match reloc {
+                    Reloc::Abs8 => {
+                        debug_assert_eq!(POINTER_SIZE, 8);
+                        8
+                    }
+                    _ => panic!("unhandled user-space symbolic call relocation"),
+                };
+
                 // This is a symbolic function reference encoded by `symbolic_function_name()`.
                 let sym = index.into();
 
-                // The symbolic access patch address points *after* the stored pointer.
-                let offset = offset + POINTER_SIZE as u32;
-                self.metadata
-                    .push(bindings::MetadataEntry::symbolic_access(offset, sym));
+                // The Spidermonkey relocation must point to the next instruction.
+                let offset = at + payload_size;
+                self.metadata.push(bindings::MetadataEntry::symbolic_access(
+                    offset, srcloc, sym,
+                ));
             }
 
-            ir::ExternalName::LibCall(call) => {
+            ExternalName::LibCall(call) => {
+                let payload_size = match reloc {
+                    Reloc::Abs8 => {
+                        debug_assert_eq!(POINTER_SIZE, 8);
+                        8
+                    }
+                    _ => panic!("unhandled libcall symbolic call relocation"),
+                };
+
                 let sym = match call {
                     ir::LibCall::CeilF32 => bindings::SymbolicAddress::CeilF32,
                     ir::LibCall::CeilF64 => bindings::SymbolicAddress::CeilF64,
                     ir::LibCall::FloorF32 => bindings::SymbolicAddress::FloorF32,
                     ir::LibCall::FloorF64 => bindings::SymbolicAddress::FloorF64,
                     ir::LibCall::NearestF32 => bindings::SymbolicAddress::NearestF32,
                     ir::LibCall::NearestF64 => bindings::SymbolicAddress::NearestF64,
                     ir::LibCall::TruncF32 => bindings::SymbolicAddress::TruncF32,
                     ir::LibCall::TruncF64 => bindings::SymbolicAddress::TruncF64,
                     _ => {
                         panic!("Don't understand external {}", name);
                     }
                 };
 
-                // The symbolic access patch address points *after* the stored pointer.
-                let offset = offset + POINTER_SIZE as u32;
-                self.metadata
-                    .push(bindings::MetadataEntry::symbolic_access(offset, sym));
+                // The Spidermonkey relocation must point to the next instruction.
+                let offset = at + payload_size;
+                self.metadata.push(bindings::MetadataEntry::symbolic_access(
+                    offset, srcloc, sym,
+                ));
             }
 
             _ => {
                 panic!("Don't understand external {}", name);
             }
         }
     }
 
-    fn reloc_jt(&mut self, offset: CodeOffset, reloc: Reloc, _jt: ir::JumpTable) {
+    /// Add a relocation referencing a constant.
+    fn reloc_constant(&mut self, _at: CodeOffset, _reloc: Reloc, _const_offset: ConstantOffset) {
+        unimplemented!("constant pool relocations NYI");
+    }
+
+    /// Add a relocation referencing a jump table.
+    fn reloc_jt(&mut self, at: CodeOffset, reloc: Reloc, _jt: JumpTable) {
         match reloc {
             Reloc::X86PCRelRodata4 => {
-                self.rodata_relocs.push(offset);
+                self.rodata_relocs.push(at);
             }
             _ => {
                 panic!("Unhandled/unexpected reloc type");
             }
         }
     }
 
-    fn reloc_constant(
-        &mut self,
-        _offset: CodeOffset,
-        _reloc: Reloc,
-        _constant_pool_offset: ConstantOffset,
-    ) {
-        unimplemented!("constant pools NYI");
+    /// Track call sites information, giving us the return address offset.
+    fn add_call_site(&mut self, opcode: ir::Opcode, ret_addr: CodeOffset, srcloc: SourceLoc) {
+        // Direct calls need a plain relocation, so we don't need to handle them again.
+        if opcode == ir::Opcode::CallIndirect {
+            self.metadata
+                .push(bindings::MetadataEntry::indirect_call(ret_addr, srcloc));
+        }
+    }
+}
+
+struct Traps {
+    metadata: Vec<bindings::MetadataEntry>,
+}
+
+impl Traps {
+    fn new() -> Self {
+        Self {
+            metadata: Vec::new(),
+        }
     }
 }
+
+impl TrapSink for Traps {
+    /// Add trap information for a specific offset.
+    fn trap(&mut self, trap_offset: CodeOffset, loc: SourceLoc, trap: TrapCode) {
+        // Translate the trap code into one of BaldrMonkey's trap codes.
+        use ir::TrapCode::*;
+        let bd_trap = match trap {
+            StackOverflow => {
+                // Cranelift will give us trap information for every spill/push/call. But
+                // Spidermonkey takes care of tracking stack overflows itself in the function
+                // entries, so we don't have to.
+                return;
+            }
+            HeapOutOfBounds | OutOfBounds | TableOutOfBounds => bindings::Trap::OutOfBounds,
+            IndirectCallToNull => bindings::Trap::IndirectCallToNull,
+            BadSignature => bindings::Trap::IndirectCallBadSig,
+            IntegerOverflow => bindings::Trap::IntegerOverflow,
+            IntegerDivisionByZero => bindings::Trap::IntegerDivideByZero,
+            BadConversionToInteger => bindings::Trap::InvalidConversionToInteger,
+            Interrupt => bindings::Trap::CheckInterrupt,
+            UnreachableCodeReached => bindings::Trap::Unreachable,
+            User(x) if x == TRAP_THROW_REPORTED => bindings::Trap::ThrowReported,
+            User(_) => panic!("Uncovered trap code {}", trap),
+        };
+
+        debug_assert!(!loc.is_default());
+        self.metadata
+            .push(bindings::MetadataEntry::trap(trap_offset, loc, bd_trap));
+    }
+}