Double-free when dropping module as part of execution engine

[RReverser]

Describe the Bug
Current relationship between execution engine and the module can lead to double-free of a module in a "safe" API usage.

To Reproduce

fn main() {
    Target::initialize_native(&InitializationConfig::default()).unwrap();
    let _execution_engine = {
        let context = Context::create();
        let module = context.create_module("sum");
        module.create_jit_execution_engine(OptimizationLevel::None).unwrap()
    };
}

Expected Behavior
Execution engine should either take ownership of the module or borrow it or module should be reference-counted internally or [use any other clear ownership semantic].

LLVM Version (please complete the following information):

• LLVM Version: 7.0.0
• Inkwell Branch Used: master

Desktop (please complete the following information):

• OS: macOS 10.14 (Mojave)

Additional Context
FWIW another Rust wrapper for LLVM had and fixed the same issue: TomBebbington/llvm-rs#25

[TheDan64]

Are you sure the segfault isn't because (in your example) you don't initialize a target? Modules don't call the llvm free method when they're owned by an EE, as in your example case: https://github.com/TheDan64/inkwell/blob/master/src/module.rs#L1339-L1343

[RReverser]

Ugh I guess I minimised the example too much when pasting. Updated code in the description.

But yes, the bug persist when initialising a target and is different from #59 (although I might be wrong about the root cause and it might be different from llvm-rs as well).

Stacktrace:

* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
  * frame #0: 0x000000010085c19e jit`llvm::LLVMContext::removeModule(llvm::Module*) + 4
    frame #1: 0x00000001008840f3 jit`llvm::Module::~Module() + 31
    frame #2: 0x0000000100407123 jit`llvm::MCJIT::OwningModuleContainer::freeModulePtrSet(llvm::SmallPtrSet<llvm::Module*, 4u>&) + 97
    frame #3: 0x0000000100407044 jit`llvm::MCJIT::OwningModuleContainer::~OwningModuleContainer() + 18
    frame #4: 0x0000000100404452 jit`llvm::MCJIT::~MCJIT() + 214
    frame #5: 0x0000000100404654 jit`llvm::MCJIT::~MCJIT() + 14
    frame #6: 0x0000000100008090 jit`_$LT$inkwell..execution_engine..ExecEngineInner$u20$as$u20$core..ops..drop..Drop$GT$::drop::h86f518daac700ebf(self=0x00007ffeefbff310) at execution_engine.rs:421
    frame #7: 0x0000000100001a05 jit`core::ptr::drop_in_place::h91efe68b20f1058b((null)=0x00007ffeefbff310)at ptr.rs:59
    frame #8: 0x0000000100001783 jit`core::ptr::drop_in_place::h198dbc589dc5920e((null)=0x00007ffeefbff310)at ptr.rs:59
    frame #9: 0x0000000100001d40 jit`jit::main::he623d44605966ec7 at jit.rs:18

[TheDan64]

Something weird is going on here, going to take a closer look this weekend.

[TheDan64]

I think it is because Context is the primary owner of all data, so the last rc of context goes away with the module, and then the EE is no longer valid. You can reproduce it this way:

let context = Context::create();
let module = context.create_module("sum");
let ee = module.create_jit_execution_engine(OptimizationLevel::None).unwrap();

drop(context);
drop(module);

sigh This would be easily avoided with lifetimes if they were just Context -> &Module -> &ExecutionEngine

But I guess for ergonomic's sake we should add the Context to the EE.

Feel free to make this change if you'd like (and let me know) otherwise I'll try and get to it sometime this weekend. We should also add a test case to show it is no longer a problem