Export debug builtins on Windows (#9706)

This allows the debugger to find them. After much research, this has been chosen as the overall best solution. The alternative here would be to create machine code thunks that call these symbols when Jitting code, and describing those thunks in DWARF, which would be significantly more complex.
bytecodealliance · Dec 2, 2024 · 8e4f0cd · 8e4f0cd
1 parent 4761930
commit 8e4f0cd
Show file tree

Hide file tree

Showing 6 changed files with 37 additions and 38 deletions.
diff --git a/crates/cranelift/src/debug/transform/synthetic.rs b/crates/cranelift/src/debug/transform/synthetic.rs
@@ -10,8 +10,8 @@ use crate::debug::{Compilation, ModuleMemoryOffset};
 ///
 /// For unwrapping Wasm pointer, the WasmtimeVMContext has the `set()` method
 /// that allows to control current Wasm memory to inspect.
-/// Notice that "set_vmctx_memory" is an external/builtin subprogram that
-/// is not part of Wasm code.
+/// Notice that "wasmtime_set_vmctx_memory" is an external/builtin subprogram
+/// that is not part of Wasm code.
 ///
 /// This CU is currently per-module since VMContext memory structure is per-module;
 /// some of the contained types could be made global (per-Compilation).
@@ -144,13 +144,13 @@ impl ModuleSyntheticUnit {
         });
 
         // Build vmctx_die's DW_TAG_subprogram for `set` method:
-        //  .. DW_AT_linkage_name = "set_vmctx_memory"
+        //  .. DW_AT_linkage_name = "wasmtime_set_vmctx_memory"
         //  .. DW_AT_name = "set"
         //  .. DW_TAG_formal_parameter
         //  ..  .. DW_AT_type = <vmctx_ptr_die>
         //  ..  .. DW_AT_artificial = 1
         add_tag!(unit, vmctx_die_id, gimli::DW_TAG_subprogram => vmctx_set as vmctx_set_id {
-            gimli::DW_AT_linkage_name = AttributeValue::StringRef(out_strings.add(versioned_stringify_ident!(set_vmctx_memory))),
+            gimli::DW_AT_linkage_name = AttributeValue::StringRef(out_strings.add(versioned_stringify_ident!(wasmtime_set_vmctx_memory))),
             gimli::DW_AT_name = AttributeValue::StringRef(out_strings.add("set"))
         });
         add_tag!(unit, vmctx_set_id, gimli::DW_TAG_formal_parameter => vmctx_set_this_param as vmctx_set_this_param_id {

diff --git a/crates/cranelift/src/debug/transform/unit.rs b/crates/cranelift/src/debug/transform/unit.rs
@@ -186,14 +186,14 @@ fn replace_pointer_type(
     });
 
     // Build wrapper_die's DW_TAG_subprogram for `ptr()`:
-    //  .. DW_AT_linkage_name = "resolve_vmctx_memory_ptr"
+    //  .. DW_AT_linkage_name = "wasmtime_resolve_vmctx_memory_ptr"
     //  .. DW_AT_name = "ptr"
     //  .. DW_AT_type = <ptr_type>
     //  .. DW_TAG_formal_parameter
     //  ..  .. DW_AT_type = <wrapper_ptr_type>
     //  ..  .. DW_AT_artificial = 1
     add_tag!(wrapper_die_id, gimli::DW_TAG_subprogram => deref_op_die as deref_op_die_id {
-        gimli::DW_AT_linkage_name = write::AttributeValue::StringRef(out_strings.add(versioned_stringify_ident!(resolve_vmctx_memory_ptr))),
+        gimli::DW_AT_linkage_name = write::AttributeValue::StringRef(out_strings.add(versioned_stringify_ident!(wasmtime_resolve_vmctx_memory_ptr))),
         gimli::DW_AT_name = write::AttributeValue::StringRef(out_strings.add("ptr")),
         gimli::DW_AT_type = write::AttributeValue::UnitRef(ptr_type_id)
     });
@@ -203,14 +203,14 @@ fn replace_pointer_type(
     });
 
     // Build wrapper_die's DW_TAG_subprogram for `operator*`:
-    //  .. DW_AT_linkage_name = "resolve_vmctx_memory_ptr"
+    //  .. DW_AT_linkage_name = "wasmtime_resolve_vmctx_memory_ptr"
     //  .. DW_AT_name = "operator*"
     //  .. DW_AT_type = <ref_type>
     //  .. DW_TAG_formal_parameter
     //  ..  .. DW_AT_type = <wrapper_ptr_type>
     //  ..  .. DW_AT_artificial = 1
     add_tag!(wrapper_die_id, gimli::DW_TAG_subprogram => deref_op_die as deref_op_die_id {
-        gimli::DW_AT_linkage_name = write::AttributeValue::StringRef(out_strings.add(versioned_stringify_ident!(resolve_vmctx_memory_ptr))),
+        gimli::DW_AT_linkage_name = write::AttributeValue::StringRef(out_strings.add(versioned_stringify_ident!(wasmtime_resolve_vmctx_memory_ptr))),
         gimli::DW_AT_name = write::AttributeValue::StringRef(out_strings.add("operator*")),
         gimli::DW_AT_type = write::AttributeValue::UnitRef(ref_type_id)
     });
@@ -220,14 +220,14 @@ fn replace_pointer_type(
     });
 
     // Build wrapper_die's DW_TAG_subprogram for `operator->`:
-    //  .. DW_AT_linkage_name = "resolve_vmctx_memory_ptr"
+    //  .. DW_AT_linkage_name = "wasmtime_resolve_vmctx_memory_ptr"
     //  .. DW_AT_name = "operator->"
     //  .. DW_AT_type = <ptr_type>
     //  .. DW_TAG_formal_parameter
     //  ..  .. DW_AT_type = <wrapper_ptr_type>
     //  ..  .. DW_AT_artificial = 1
     add_tag!(wrapper_die_id, gimli::DW_TAG_subprogram => deref_op_die as deref_op_die_id {
-        gimli::DW_AT_linkage_name = write::AttributeValue::StringRef(out_strings.add(versioned_stringify_ident!(resolve_vmctx_memory_ptr))),
+        gimli::DW_AT_linkage_name = write::AttributeValue::StringRef(out_strings.add(versioned_stringify_ident!(wasmtime_resolve_vmctx_memory_ptr))),
         gimli::DW_AT_name = write::AttributeValue::StringRef(out_strings.add("operator->")),
         gimli::DW_AT_type = write::AttributeValue::UnitRef(ptr_type_id)
     });

diff --git a/crates/wasmtime/build.rs b/crates/wasmtime/build.rs
@@ -32,6 +32,10 @@ fn build_c_helpers() {
     build.define(&format!("CFG_TARGET_OS_{os}"), None);
     build.define(&format!("CFG_TARGET_ARCH_{arch}"), None);
     build.define("VERSIONED_SUFFIX", Some(versioned_suffix!()));
+    if std::env::var("CARGO_FEATURE_DEBUG_BUILTINS").is_ok() {
+        build.define("FEATURE_DEBUG_BUILTINS", None);
+    }
+
     println!("cargo:rerun-if-changed=src/runtime/vm/helpers.c");
     build.file("src/runtime/vm/helpers.c");
     build.compile("wasmtime-helpers");

diff --git a/crates/wasmtime/src/engine.rs b/crates/wasmtime/src/engine.rs
@@ -99,8 +99,6 @@ impl Engine {
             // handlers, etc.
             #[cfg(all(feature = "signals-based-traps", not(miri)))]
             crate::runtime::vm::init_traps(config.macos_use_mach_ports);
-            #[cfg(feature = "debug-builtins")]
-            crate::runtime::vm::debug_builtins::ensure_exported();
         }
 
         #[cfg(any(feature = "cranelift", feature = "winch"))]

diff --git a/crates/wasmtime/src/runtime/vm/debug_builtins.rs b/crates/wasmtime/src/runtime/vm/debug_builtins.rs
@@ -7,18 +7,8 @@ use wasmtime_versioned_export_macros::versioned_export;
 
 static mut VMCTX_AND_MEMORY: (*mut VMContext, usize) = (std::ptr::null_mut(), 0);
 
-#[versioned_export]
-pub unsafe extern "C" fn resolve_vmctx_memory(ptr: usize) -> *const u8 {
-    Instance::from_vmctx(VMCTX_AND_MEMORY.0, |handle| {
-        assert!(
-            VMCTX_AND_MEMORY.1 < handle.env_module().memories.len(),
-            "memory index for debugger is out of bounds"
-        );
-        let index = MemoryIndex::new(VMCTX_AND_MEMORY.1);
-        let mem = handle.get_memory(index);
-        mem.base.add(ptr)
-    })
-}
+// These implementatations are referenced from C code in "helpers.c". The symbols defined
+// there (prefixed by "wasmtime_") are the real 'public' interface used in the debug info.
 
 #[versioned_export]
 pub unsafe extern "C" fn resolve_vmctx_memory_ptr(p: *const u32) -> *const u8 {
@@ -43,17 +33,3 @@ pub unsafe extern "C" fn set_vmctx_memory(vmctx_ptr: *mut VMContext) {
     // TODO multi-memory
     VMCTX_AND_MEMORY = (vmctx_ptr, 0);
 }
-
-// Ensures that set_vmctx_memory and resolve_vmctx_memory_ptr are linked and
-// exported as symbols. It is a workaround: the executable normally ignores
-// `pub extern "C"`, see rust-lang/rust#25057.
-pub fn ensure_exported() {
-    if cfg!(miri) {
-        return;
-    }
-    unsafe {
-        std::ptr::read_volatile(resolve_vmctx_memory_ptr as *const u8);
-        std::ptr::read_volatile(set_vmctx_memory as *const u8);
-        std::ptr::read_volatile(resolve_vmctx_memory as *const u8);
-    }
-}
diff --git a/crates/wasmtime/src/runtime/vm/helpers.c b/crates/wasmtime/src/runtime/vm/helpers.c
@@ -127,6 +127,27 @@ void VERSIONED_SYMBOL(wasmtime_longjmp)(void *JmpBuf) {
   platform_longjmp(*buf, 1);
 }
 
+#ifdef FEATURE_DEBUG_BUILTINS
+#ifdef CFG_TARGET_OS_windows
+#define DEBUG_BUILTIN_EXPORT __declspec(dllexport)
+#else
+#define DEBUG_BUILTIN_EXPORT __attribute__((used, retain))
+#endif
+
+// This set of symbols is defined here in C because Rust's #[export_name]
+// functions are not dllexported on Windows when building an executable. These
+// symbols are directly referenced by name from the native DWARF info.
+void *VERSIONED_SYMBOL(resolve_vmctx_memory_ptr)(void *);
+DEBUG_BUILTIN_EXPORT void *
+VERSIONED_SYMBOL(wasmtime_resolve_vmctx_memory_ptr)(void *p) {
+  return VERSIONED_SYMBOL(resolve_vmctx_memory_ptr)(p);
+}
+void VERSIONED_SYMBOL(set_vmctx_memory)(void *);
+DEBUG_BUILTIN_EXPORT void VERSIONED_SYMBOL(wasmtime_set_vmctx_memory)(void *p) {
+  VERSIONED_SYMBOL(set_vmctx_memory)(p);
+}
+#endif // FEATURE_DEBUG_BUILTINS
+
 #ifdef CFG_TARGET_OS_windows
 // export required for external access.
 __declspec(dllexport)