TomBebbington / llvm-rs

LLVM wrappers for Rust
BSD 3-Clause "New" or "Revised" License
68 stars 27 forks source link

Examples Segfault on OS X (LLVM 3.8.1) #25

Closed iwillspeak closed 6 years ago

iwillspeak commented 8 years ago

I'm compiling on OS X with rustc 1.13.0-nightly (f883b0bba 2016-08-19) and LLVM 3.8.1 from Homebrew.

When running any of the examples I receive a segfault:

$ PATH=$PATH:/usr/local/opt/llvm/bin cargo run --example add 
    Finished debug [unoptimized + debuginfo] target(s) in 0.0 secs
     Running `target/debug/examples/add`
1 + 2 = 3
error: Process didn't exit successfully: `target/debug/examples/add` (signal: 11, SIGSEGV: invalid memory reference)

This seems to be from the drop implementation for Module

(lldb) run
Process 61966 launched: '/Users/will/Repositories/llvm-rs/target/debug/examples/add' (x86_64)
1 + 2 = 3
Process 61966 stopped
* thread #1: tid = 0x640f90, 0x000000010063d8b2 add`llvm::LLVMContext::removeModule(llvm::Module*) + 4, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
    frame #0: 0x000000010063d8b2 add`llvm::LLVMContext::removeModule(llvm::Module*) + 4
add`llvm::LLVMContext::removeModule:
->  0x10063d8b2 <+4>:  movq   (%rdi), %rdi
    0x10063d8b5 <+7>:  popq   %rbp
    0x10063d8b6 <+8>:  jmp    0x10069b226               ; llvm::SmallPtrSetImplBase::erase_imp(void const*)
    0x10063d8bb <+13>: nop    
(lldb) bt
* thread #1: tid = 0x640f90, 0x000000010063d8b2 add`llvm::LLVMContext::removeModule(llvm::Module*) + 4, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
  * frame #0: 0x000000010063d8b2 add`llvm::LLVMContext::removeModule(llvm::Module*) + 4
    frame #1: 0x0000000100661901 add`llvm::Module::~Module() + 31
    frame #2: 0x00000001005d7f7d add`LLVMDisposeModule + 22
    frame #3: 0x0000000100002951 add`drop::h3fd646682453ffaf + 97
    frame #4: 0x0000000100004553 add`add::main::h8b97c6143f430fbb + 851 at add.rs:6
    frame #5: 0x000000010001417b add`__rust_maybe_catch_panic + 27
    frame #6: 0x0000000100012aff add`std::rt::lang_start::h352a66f5026f54bd + 399
    frame #7: 0x000000010000482a add`main + 42
    frame #8: 0x00007fff88ba65ad libdyld.dylib`start + 1
    frame #9: 0x00007fff88ba65ad libdyld.dylib`start + 1
(lldb)

I think this is because the JitEngine has taken ownership of the module and dropped it already.

iwillspeak commented 8 years ago

I have managed to prove that this segfault is due to a double-free on the Module. This can be fixed in the examples by using mem::forget:

diff --git a/examples/add.rs b/examples/add.rs
index 9050176..f1f8cf0 100644
--- a/examples/add.rs
+++ b/examples/add.rs
@@ -1,21 +1,25 @@
 extern crate llvm;
+use std::mem;
 use llvm::*;
 use llvm::Attribute::*;
 fn main() {
     let ctx = Context::new();
     let module = Module::new("add", &ctx);
-    let func = module.add_function("add", Type::get::<fn(f64, f64) -> f64>(&ctx));
-    func.add_attributes(&[NoUnwind, ReadNone]);
-    let entry = func.append("entry");
-    let builder = Builder::new(&ctx);
-    builder.position_at_end(entry);
-    let a = &func[0];
-    let b = &func[1];
-    let value = builder.build_add(a, b);
-    builder.build_ret(value);
-    module.verify().unwrap();
-    let ee = JitEngine::new(&module, JitOptions {opt_level: 3}).unwrap();
-    ee.with_function(func, |add:extern fn((f64, f64)) -> f64| {
-        println!("{} + {} = {}", 1., 2., add((1., 2.)));
-    });
+    {
+        let func = module.add_function("add", Type::get::<fn(f64, f64) -> f64>(&ctx));
+        func.add_attributes(&[NoUnwind, ReadNone]);
+        let entry = func.append("entry");
+        let builder = Builder::new(&ctx);
+        builder.position_at_end(entry);
+        let a = &func[0];
+        let b = &func[1];
+        let value = builder.build_add(a, b);
+        builder.build_ret(value);
+        module.verify().unwrap();
+        let ee = JitEngine::new(&module, JitOptions {opt_level: 3}).unwrap();
+        ee.with_function(func, |add:extern fn((f64, f64)) -> f64| {
+            println!("{} + {} = {}", 1., 2., add((1., 2.)));
+        });
+    }
+    mem::forget(module);
 }

I guess this means that JitEngine::new should take ownership of the module rather than receiving a borrow of it. Not sure if it's quite that simple though due to the lifetime of func

willcrichton commented 7 years ago

For anyone still concerned: you can also solve this issue by removing the module from the ExecutionEngine before the engine is dropped, i.e. ee.remove_module(&module);.

iwillspeak commented 7 years ago

Could the execution engine take ownership of the module when created so that Rust's ownership semantics protect us in this case?