Fix/improve stack pointer detection (#4036)

Renamed all instances of "shadow stack" to just "stack".

CHANGELOG.md

@@ -154,6 +154,9 @@
 * Allow ex/importing structs, functions and parameters named with raw identifiers.
   [#4025](https://github.com/rustwasm/wasm-bindgen/pull/4025)
 
+* Implement a more reliable way to detect the stack pointer.
+  [#4036](https://github.com/rustwasm/wasm-bindgen/pull/4036)
+
 --------------------------------------------------------------------------------
 
 ## [0.2.92](https://github.com/rustwasm/wasm-bindgen/compare/0.2.91...0.2.92)

crates/cli-support/src/js/mod.rs

@@ -3963,13 +3963,13 @@ impl<'a> Context<'a> {
         if self.stack_pointer_shim_injected {
             return Ok(());
         }
-        let stack_pointer = match self.aux.shadow_stack_pointer {
+        let stack_pointer = match self.aux.stack_pointer {
             Some(s) => s,
             // In theory this shouldn't happen since malloc is included in
             // most wasm binaries (and may be gc'd out) and that almost
             // always pulls in a stack pointer. We can try to synthesize
             // something here later if necessary.
-            None => bail!("failed to find shadow stack pointer"),
+            None => bail!("failed to find stack pointer"),
         };
 
         use walrus::ir::*;

crates/cli-support/src/multivalue.rs

@@ -18,21 +18,21 @@ pub fn run(module: &mut Module) -> Result<(), Error> {
         extract_xform(module, adapter, &mut to_xform, &mut slots);
     }
     if to_xform.is_empty() {
-        // Early exit to avoid failing if we don't have a memory or shadow stack
+        // Early exit to avoid failing if we don't have a memory or stack
         // pointer because this is a minimal module that doesn't use linear
         // memory.
         module.customs.add(*adapters);
         return Ok(());
     }
 
-    let shadow_stack_pointer = module
+    let stack_pointer = module
         .customs
         .get_typed::<WasmBindgenAux>()
         .expect("aux section should be present")
-        .shadow_stack_pointer
-        .ok_or_else(|| anyhow!("failed to find shadow stack pointer in wasm module"))?;
+        .stack_pointer
+        .ok_or_else(|| anyhow!("failed to find stack pointer in wasm module"))?;
     let memory = wasm_conventions::get_memory(module)?;
-    let wrappers = multi_value_xform::run(module, memory, shadow_stack_pointer, &to_xform)?;
+    let wrappers = multi_value_xform::run(module, memory, stack_pointer, &to_xform)?;
 
     for (slot, id) in slots.into_iter().zip(wrappers) {
         match slot {

crates/cli-support/src/wit/mod.rs

@@ -90,8 +90,7 @@ pub fn process(
 
 impl<'a> Context<'a> {
     fn init(&mut self) -> Result<(), Error> {
-        self.aux.shadow_stack_pointer =
-            wasm_bindgen_wasm_conventions::get_shadow_stack_pointer(self.module);
+        self.aux.stack_pointer = wasm_bindgen_wasm_conventions::get_stack_pointer(self.module);
 
         // Make a map from string name to ids of all exports
         for export in self.module.exports.iter() {

crates/cli-support/src/wit/nonstandard.rs

@@ -57,7 +57,7 @@ pub struct WasmBindgenAux {
 
     /// Various intrinsics used for JS glue generation
     pub exn_store: Option<walrus::FunctionId>,
-    pub shadow_stack_pointer: Option<walrus::GlobalId>,
+    pub stack_pointer: Option<walrus::GlobalId>,
     pub thread_destroy: Option<walrus::FunctionId>,
 }
 
@@ -430,7 +430,7 @@ impl walrus::CustomSection for WasmBindgenAux {
         if let Some(id) = self.exn_store {
             roots.push_func(id);
         }
-        if let Some(id) = self.shadow_stack_pointer {
+        if let Some(id) = self.stack_pointer {
             roots.push_global(id);
         }
         if let Some(id) = self.thread_destroy {

crates/cli-support/src/wit/section.rs

@@ -50,7 +50,7 @@ pub fn add(module: &mut Module) -> Result<(), Error> {
         externref_drop: _,
         externref_drop_slice: _,
         exn_store: _,
-        shadow_stack_pointer: _,
+        stack_pointer: _,
         function_table: _,
         thread_destroy: _,
     } = *aux;

crates/multi-value-xform/src/lib.rs

@@ -58,33 +58,33 @@
 //!
 //! (func $pairWrapper (param i32 i32) (result i32 i32)
 //!   ;; Our return pointer that points to the scratch space we are allocating
-//!   ;; on the shadow stack for calling `$pair`.
+//!   ;; on the stack for calling `$pair`.
 //!   (local i32)
 //!
-//!   ;; Allocate space on the shadow stack for the result.
-//!   global.get $shadowStackPointer
+//!   ;; Allocate space on the stack for the result.
+//!   global.get $stackPointer
 //!   i32.const 8
 //!   i32.sub
 //!   local.tee 2
-//!   global.set $shadowStackPointer
+//!   global.set $stackPointer
 //!
-//!   ;; Call `$pair` with our allocated shadow stack space for its results.
+//!   ;; Call `$pair` with our allocated stack space for its results.
 //!   local.get 2
 //!   local.get 0
 //!   local.get 1
 //!   call $pair
 //!
-//!   ;; Copy the return values from the shadow stack to the wasm stack.
+//!   ;; Copy the return values from the stack to the wasm stack.
 //!   local.get 2
 //!   i32.load
 //!   local.get 2 offset=4
 //!   i32.load
 //!
-//!   ;; Finally, restore the shadow stack pointer.
+//!   ;; Finally, restore the stack pointer.
 //!   local.get 2
 //!   i32.const 8
 //!   i32.add
-//!   global.set $shadowStackPointer)
+//!   global.set $stackPointer)
 //! ```
 //!
 //! This `$pairWrapper` function is what we actually end up exporting instead of
@@ -98,12 +98,11 @@ use anyhow::Context;
 ///
 /// See the module-level docs for details on the transformation.
 ///
-/// * `memory` is the module's memory that has the shadow stack where return
+/// * `memory` is the module's memory that has the stack where return
 ///   pointers are allocated within.
 ///
-/// * `shadow_stack_pointer` is the global that is being used as the stack
-///   pointer for the shadow stack. With LLVM, this is typically the first
-///   global.
+/// * `__stack_pointer` is the global that is being used as the stack
+///   pointer. With LLVM, this is typically the first global.
 ///
 /// * `to_xform` is the set of exported functions we want to transform and
 ///   information required to transform them. The `usize` is the index of the
@@ -116,7 +115,7 @@ use anyhow::Context;
 pub fn run(
     module: &mut walrus::Module,
     memory: walrus::MemoryId,
-    shadow_stack_pointer: walrus::GlobalId,
+    stack_pointer: walrus::GlobalId,
     to_xform: &[(walrus::FunctionId, usize, Vec<walrus::ValType>)],
 ) -> Result<Vec<walrus::FunctionId>, anyhow::Error> {
     // Insert multi-value to the target features section.
@@ -128,7 +127,7 @@ pub fn run(
         wrappers.push(xform_one(
             module,
             memory,
-            shadow_stack_pointer,
+            stack_pointer,
             *func,
             *return_pointer_index,
             results,
@@ -146,13 +145,13 @@ fn round_up_to_alignment(n: u32, align: u32) -> u32 {
 fn xform_one(
     module: &mut walrus::Module,
     memory: walrus::MemoryId,
-    shadow_stack_pointer: walrus::GlobalId,
+    stack_pointer: walrus::GlobalId,
     func: walrus::FunctionId,
     return_pointer_index: usize,
     results: &[walrus::ValType],
 ) -> Result<walrus::FunctionId, anyhow::Error> {
-    if module.globals.get(shadow_stack_pointer).ty != walrus::ValType::I32 {
-        anyhow::bail!("shadow stack pointer global does not have type `i32`");
+    if module.globals.get(stack_pointer).ty != walrus::ValType::I32 {
+        anyhow::bail!("stack pointer global does not have type `i32`");
     }
 
     // Compute the total size of all results, potentially with padding to ensure
@@ -210,18 +209,18 @@ fn xform_one(
     // The locals for the function parameters.
     let params: Vec<_> = new_params.iter().map(|ty| module.locals.add(*ty)).collect();
 
-    // A local to hold our shadow stack-allocated return pointer.
+    // A local to hold our stack-allocated return pointer.
     let return_pointer = module.locals.add(walrus::ValType::I32);
 
     let mut wrapper = walrus::FunctionBuilder::new(&mut module.types, &new_params, results);
     let mut body = wrapper.func_body();
 
-    // Allocate space in the shadow stack for the call.
-    body.global_get(shadow_stack_pointer)
+    // Allocate space in the stack for the call.
+    body.global_get(stack_pointer)
         .i32_const(results_size as i32)
         .binop(walrus::ir::BinaryOp::I32Sub)
         .local_tee(return_pointer)
-        .global_set(shadow_stack_pointer);
+        .global_set(stack_pointer);
 
     // Push the parameters for calling our wrapped function -- including the
     // return pointer! -- on to the stack.
@@ -238,8 +237,7 @@ fn xform_one(
     // Call our wrapped function.
     body.call(func);
 
-    // Copy the return values from our shadow stack-allocated space and onto the
-    // Wasm stack.
+    // Copy the return values from our stack-allocated space and onto the Wasm stack.
     let mut offset = 0;
     for ty in results {
         debug_assert!(offset < results_size);
@@ -294,11 +292,11 @@ fn xform_one(
         }
     }
 
-    // Finally, restore the shadow stack pointer.
+    // Finally, restore the stack pointer.
     body.local_get(return_pointer)
         .i32_const(results_size as i32)
         .binop(walrus::ir::BinaryOp::I32Add)
-        .global_set(shadow_stack_pointer);
+        .global_set(stack_pointer);
 
     let wrapper = wrapper.finish(params, &mut module.funcs);
     if let Some(name) = &module.funcs.get(func).name {

crates/threads-xform/src/lib.rs

@@ -153,8 +153,8 @@ impl Config {
         assert!(self.thread_stack_size % PAGE_SIZE == 0);
 
         let stack = Stack {
-            pointer: wasm_conventions::get_shadow_stack_pointer(module)
-                .ok_or_else(|| anyhow!("failed to find shadow stack pointer"))?,
+            pointer: wasm_conventions::get_stack_pointer(module)
+                .ok_or_else(|| anyhow!("failed to find stack pointer"))?,
             temp: temp_stack as i32,
             temp_lock: thread_counter_addr + 4,
             alloc: stack_alloc,

crates/wasm-conventions/Cargo.toml

@@ -16,3 +16,4 @@ walrus = "0.20.2"
 # Matching the version `walrus` depends on.
 wasmparser = "0.80"
 anyhow = "1.0"
+log = "0.4"

crates/wasm-conventions/src/lib.rs

@@ -3,8 +3,8 @@
 //!
 //! Examples conventions include:
 //!
-//! * The shadow stack pointer
-//! * The canonical linear memory that contains the shadow stack
+//! * The stack pointer
+//! * The canonical linear memory that contains the stack
 
 use std::io::Cursor;
 
@@ -33,8 +33,16 @@ pub fn get_memory(module: &Module) -> Result<MemoryId> {
     })
 }
 
-/// Get the `__shadow_stack_pointer`.
-pub fn get_shadow_stack_pointer(module: &Module) -> Option<GlobalId> {
+/// Get the `__stack_pointer`.
+pub fn get_stack_pointer(module: &Module) -> Option<GlobalId> {
+    if let Some(g) = module
+        .globals
+        .iter()
+        .find(|g| matches!(g.name.as_deref(), Some("__stack_pointer")))
+    {
+        return Some(g.id());
+    }
+
     let candidates = module
         .globals
         .iter()
@@ -51,8 +59,11 @@ pub fn get_shadow_stack_pointer(module: &Module) -> Option<GlobalId> {
 
     match candidates.len() {
         0 => None,
-        // TODO: have an actual check here.
-        1 | 2 => Some(candidates[0].id()),
+        1 => Some(candidates[0].id()),
+        2 => {
+            log::warn!("Unable to accurately determine the location of `__stack_pointer`");
+            Some(candidates[0].id())
+        }
         _ => None,
     }
 }