Merge #2548

2548: Fix probestack r=ptitSeb a=ptitSeb

# Description
The `skip_stack_gard_page` were failing.

The patch first add a trampoline for the PROBEGUARD function on x86_64 build, as this one comes from some runtime and can be too far away from generated code for a regular 32bits relative address (this fix a segfault when a generated function needs a call to probeguard).
The patch also add a special case for StackOverflow error, and doesn't check if the error comes from Wasm code (as the stack overflow will comes from a runtime library, generated by the probeguard function).

Co-authored-by: Syrus Akbary <me@syrusakbary.com>
Co-authored-by: ptitSeb <sebastien.chev@gmail.com>

lib/compiler-cranelift/src/compiler.rs

@@ -29,8 +29,15 @@ use wasmer_compiler::{
     FunctionBodyData, MiddlewareBinaryReader, ModuleMiddleware, ModuleMiddlewareChain,
     SectionIndex,
 };
+#[cfg(target_arch = "x86_64")]
+use wasmer_compiler::{
+    CustomSection, CustomSectionProtection, Relocation, RelocationKind, RelocationTarget,
+    SectionBody,
+};
 use wasmer_types::entity::{EntityRef, PrimaryMap};
 use wasmer_types::{FunctionIndex, LocalFunctionIndex, SignatureIndex};
+#[cfg(target_arch = "x86_64")]
+use wasmer_vm::libcalls::LibCall;
 
 /// A compiler that compiles a WebAssembly module with Cranelift, translating the Wasm to Cranelift IR,
 /// optimizing it and then translating to assembly.
@@ -102,6 +109,29 @@ impl Compiler for CraneliftCompiler {
             }
         };
 
+        let mut custom_sections = PrimaryMap::new();
+
+        #[cfg(target_arch = "x86_64")]
+        let probestack_trampoline = CustomSection {
+            protection: CustomSectionProtection::ReadExecute,
+            // We create a jump to an absolute 64bits address
+            // with an indrect jump immediatly followed but the absolute address
+            // JMP [IP+0]   FF 25 00 00 00 00
+            // 64bits ADDR
+            bytes: SectionBody::new_with_vec(vec![0xff, 0x25, 0x00, 0x00, 0x00, 0x00]),
+            relocations: vec![Relocation {
+                kind: RelocationKind::Abs8,
+                reloc_target: RelocationTarget::LibCall(LibCall::Probestack),
+                // 6 is the size of the jmp instruction. The relocated address must follow
+                offset: 6,
+                addend: 0,
+            }],
+        };
+        #[cfg(target_arch = "x86_64")]
+        custom_sections.push(probestack_trampoline);
+        #[cfg(target_arch = "x86_64")]
+        let probestack_trampoline_relocation_target = SectionIndex::new(custom_sections.len() - 1);
+
         let functions = function_body_inputs
             .iter()
             .collect::<Vec<(LocalFunctionIndex, &FunctionBodyData<'_>)>>()
@@ -138,7 +168,12 @@ impl Compiler for CraneliftCompiler {
                 )?;
 
                 let mut code_buf: Vec<u8> = Vec::new();
-                let mut reloc_sink = RelocSink::new(&module, func_index);
+                let mut reloc_sink = RelocSink::new(
+                    &module,
+                    func_index,
+                    #[cfg(target_arch = "x86_64")]
+                    probestack_trampoline_relocation_target,
+                );
                 let mut trap_sink = TrapSink::new();
                 let mut stackmap_sink = binemit::NullStackMapSink {};
                 context
@@ -204,8 +239,7 @@ impl Compiler for CraneliftCompiler {
             .collect::<PrimaryMap<LocalFunctionIndex, _>>();
 
         #[cfg(feature = "unwind")]
-        let (custom_sections, dwarf) = {
-            let mut custom_sections = PrimaryMap::new();
+        let dwarf = {
             let dwarf = if let Some((dwarf_frametable, _cie_id)) = dwarf_frametable {
                 let mut eh_frame = EhFrame(WriterRelocate::new(target.triple().endianness().ok()));
                 dwarf_frametable
@@ -216,14 +250,14 @@ impl Compiler for CraneliftCompiler {
 
                 let eh_frame_section = eh_frame.0.into_section();
                 custom_sections.push(eh_frame_section);
-                Some(Dwarf::new(SectionIndex::new(0)))
+                Some(Dwarf::new(SectionIndex::new(custom_sections.len() - 1)))
             } else {
                 None
             };
-            (custom_sections, dwarf)
+            dwarf
         };
         #[cfg(not(feature = "unwind"))]
-        let (custom_sections, dwarf) = (PrimaryMap::new(), None);
+        let dwarf = None;
 
         // function call trampolines (only for local functions, by signature)
         let function_call_trampolines = module

lib/compiler-cranelift/src/sink.rs

@@ -2,9 +2,13 @@
 
 use crate::translator::{irlibcall_to_libcall, irreloc_to_relocationkind};
 use cranelift_codegen::binemit;
+#[cfg(target_arch = "x86_64")]
+use cranelift_codegen::ir::LibCall;
 use cranelift_codegen::ir::{self, ExternalName};
 use cranelift_entity::EntityRef as CraneliftEntityRef;
 use wasmer_compiler::{JumpTable, Relocation, RelocationTarget, TrapInformation};
+#[cfg(target_arch = "x86_64")]
+use wasmer_compiler::{RelocationKind, SectionIndex};
 use wasmer_types::entity::EntityRef;
 use wasmer_types::{FunctionIndex, LocalFunctionIndex, ModuleInfo};
 use wasmer_vm::TrapCode;
@@ -18,6 +22,10 @@ pub(crate) struct RelocSink<'a> {
 
     /// Relocations recorded for the function.
     pub func_relocs: Vec<Relocation>,
+
+    /// The section where the probestack trampoline call is located
+    #[cfg(target_arch = "x86_64")]
+    pub probestack_trampoline_relocation_target: SectionIndex,
 }
 
 impl<'a> binemit::RelocSink for RelocSink<'a> {
@@ -37,7 +45,22 @@ impl<'a> binemit::RelocSink for RelocSink<'a> {
                     .expect("The provided function should be local"),
             )
         } else if let ExternalName::LibCall(libcall) = *name {
-            RelocationTarget::LibCall(irlibcall_to_libcall(libcall))
+            match libcall {
+                #[cfg(target_arch = "x86_64")]
+                LibCall::Probestack => {
+                    self.func_relocs.push(Relocation {
+                        kind: RelocationKind::X86CallPCRel4,
+                        reloc_target: RelocationTarget::CustomSection(
+                            self.probestack_trampoline_relocation_target,
+                        ),
+                        offset: offset,
+                        addend: addend,
+                    });
+                    // Skip the default path
+                    return;
+                }
+                _ => RelocationTarget::LibCall(irlibcall_to_libcall(libcall)),
+            }
         } else {
             panic!("unrecognized external name")
         };
@@ -74,14 +97,20 @@ impl<'a> binemit::RelocSink for RelocSink<'a> {
 
 impl<'a> RelocSink<'a> {
     /// Return a new `RelocSink` instance.
-    pub fn new(module: &'a ModuleInfo, func_index: FunctionIndex) -> Self {
+    pub fn new(
+        module: &'a ModuleInfo,
+        func_index: FunctionIndex,
+        #[cfg(target_arch = "x86_64")] probestack_trampoline_relocation_target: SectionIndex,
+    ) -> Self {
         let local_func_index = module
             .local_func_index(func_index)
             .expect("The provided function should be local");
         Self {
             module,
             local_func_index,
             func_relocs: Vec::new(),
+            #[cfg(target_arch = "x86_64")]
+            probestack_trampoline_relocation_target,
         }
     }
 }

lib/compiler-cranelift/src/translator/func_translator.rs

@@ -178,7 +178,7 @@ fn parse_local_decls<FE: FuncEnvironment + ?Sized>(
 
 /// Declare `count` local variables of the same type, starting from `next_local`.
 ///
-/// Fail of too many locals are declared in the function, or if the type is not valid for a local.
+/// Fail if the type is not valid for a local.
 fn declare_locals<FE: FuncEnvironment + ?Sized>(
     builder: &mut FunctionBuilder,
     count: u32,

lib/vm/src/trap/traphandlers.rs

@@ -721,7 +721,8 @@ impl<'a> CallThreadState<'a> {
         }
 
         // If this fault wasn't in wasm code, then it's not our problem
-        if unsafe { !IS_WASM_PC(pc as _) } {
+        // except if it's a StackOverflow (see below)
+        if unsafe { !IS_WASM_PC(pc as _) } && signal_trap != Some(TrapCode::StackOverflow) {
             return ptr::null();
         }
 

tests/ignores.txt

@@ -41,12 +41,6 @@ windows+cranelift spec::memory_grow
 # LLVM/Universal doesn't work in macOS M1. Skip all tests
 llvm+universal+macos+aarch64 *
 
-# TODO: We need to fix this. The issue is caused by libunwind overflowing
-# the stack while creating the stacktrace.
-# https://github.com/rust-lang/backtrace-rs/issues/356
-cranelift spec::skip_stack_guard_page
-llvm      spec::skip_stack_guard_page
-
 # TODO(https://github.com/wasmerio/wasmer/issues/1727): Traps in dylib engine
 cranelift+dylib spec::linking
 cranelift+dylib spec::bulk