Merge #2548

2548: Fix probestack r=syrusakbary 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 <[email protected]> Co-authored-by: ptitSeb <[email protected]>
wasmerio · Sep 2, 2021 · 30b5927 · 30b5927
2 parents 378550a + 5a742ce
commit 30b5927
Show file tree

Hide file tree

Showing 6 changed files with 86 additions and 12 deletions.
diff --git a/.gitignore b/.gitignore
@@ -2,6 +2,7 @@
 **/*.rs.bk
 .DS_Store
 .idea
+.gdb_history
 **/.vscode
 api-docs-repo/
 /.cargo_home/

diff --git a/lib/compiler-cranelift/src/compiler.rs b/lib/compiler-cranelift/src/compiler.rs
@@ -29,8 +29,15 @@ use wasmer_compiler::{
     FunctionBodyData, MiddlewareBinaryReader, ModuleMiddleware, ModuleMiddlewareChain,
     SectionIndex,
 };
+#[cfg(all(target_arch = "x86_64", target_os = "linux"))]
+use wasmer_compiler::{
+    CustomSection, CustomSectionProtection, Relocation, RelocationKind, RelocationTarget,
+    SectionBody,
+};
 use wasmer_types::entity::{EntityRef, PrimaryMap};
 use wasmer_types::{FunctionIndex, LocalFunctionIndex, SignatureIndex};
+#[cfg(all(target_arch = "x86_64", target_os = "linux"))]
+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,31 @@ impl Compiler for CraneliftCompiler {
             }
         };
 
+        let mut custom_sections = PrimaryMap::new();
+
+        #[cfg(all(target_arch = "x86_64", target_os = "linux"))]
+        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  00 00 00 00 00 00 00 00 preset to 0 until the relocation takes place
+            bytes: SectionBody::new_with_vec(vec![
+                0xff, 0x25, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 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(all(target_arch = "x86_64", target_os = "linux"))]
+        custom_sections.push(probestack_trampoline);
+        #[cfg(all(target_arch = "x86_64", target_os = "linux"))]
+        let probestack_trampoline_relocation_target = SectionIndex::new(custom_sections.len() - 1);
+
         let functions = function_body_inputs
             .iter()
             .collect::<Vec<(LocalFunctionIndex, &FunctionBodyData<'_>)>>()
@@ -138,7 +170,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(all(target_arch = "x86_64", target_os = "linux"))]
+                    probestack_trampoline_relocation_target,
+                );
                 let mut trap_sink = TrapSink::new();
                 let mut stackmap_sink = binemit::NullStackMapSink {};
                 context
@@ -204,8 +241,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 +252,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

diff --git a/lib/compiler-cranelift/src/sink.rs b/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(all(target_arch = "x86_64", target_os = "linux"))]
+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(all(target_arch = "x86_64", target_os = "linux"))]
+    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(all(target_arch = "x86_64", target_os = "linux"))]
+                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,21 @@ 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(all(target_arch = "x86_64", target_os = "linux"))]
+        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(all(target_arch = "x86_64", target_os = "linux"))]
+            probestack_trampoline_relocation_target,
         }
     }
 }

diff --git a/lib/compiler-cranelift/src/translator/func_translator.rs b/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,

diff --git a/lib/engine-dylib/src/artifact.rs b/lib/engine-dylib/src/artifact.rs
@@ -17,8 +17,8 @@ use tracing::log::error;
 #[cfg(feature = "compiler")]
 use tracing::trace;
 use wasmer_compiler::{
-    CompileError, CompiledFunctionFrameInfo, Features, FunctionAddressMap, OperatingSystem, Symbol,
-    SymbolRegistry, Triple,
+    Architecture, CompileError, CompiledFunctionFrameInfo, Features, FunctionAddressMap,
+    OperatingSystem, Symbol, SymbolRegistry, Triple,
 };
 #[cfg(feature = "compiler")]
 use wasmer_compiler::{
@@ -351,6 +351,11 @@ impl DylibArtifact {
             Triple::host().to_string(),
         );
 
+        let notext = match (target_triple.operating_system, target_triple.architecture) {
+            (OperatingSystem::Linux, Architecture::X86_64) => "-Wl,-z,notext",
+            _ => "",
+        };
+
         let linker = engine_inner.linker().executable();
         let output = Command::new(linker)
             .arg(&filepath)
@@ -359,6 +364,7 @@ impl DylibArtifact {
             .args(&target_args)
             // .args(&wasmer_symbols)
             .arg("-shared")
+            .arg(&notext)
             .args(&cross_compiling_args)
             .arg("-v")
             .output()

diff --git a/lib/vm/src/trap/traphandlers.rs b/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();
         }