`test_values::test_call_site` triggers assertion in llvm 10

[nlewycky]

To Reproduce Build llvm 10 with assertions enabled and run inkwell tests against it.

$ cargo test --features=llvm10-0 test_values::test_call_site
While deleting: void ()* %do_nothing
Use still stuck around after Def is destroyed:  call cc2 align 16 addrspace(0) void @do_nothing()
all-0b042a69785d4445: ../lib/IR/Value.cpp:92: llvm::Value::~Value(): Assertion `use_empty() && "Uses remain when a value is destroyed!"' failed.

Describe the Bug This test builds up a dangling pointer in LLVM IR. LLVM catches this when assertions are enabled. In this case the test happens to work with assertions disabled because the dangling pointer is never dereferenced, but this is exactly the sort of unsafety rust is designed to avoid.

The underlying problem here is creating a builder and then using that to build a call without ever specifying the insertion point for the builder. LLVM's memory model is a tree, each object owns a list of things inside it (module->list of globals->list of basic blocks->list of instructions) which are deleted bottom up. A "de-parented" instruction is one that isn't a member of any list, and therefore is left out of LLVM's memory management.

What happens here is that at the end of test_call_site rust deletes call_site which doesn't do anything in its Drop: the original instruction continues to live in LLVM memory. This is normal, we don't normally want to delete an instruction just because an inkwell handle to it went out of scope. Then the rest of the deletion occurs, including the module which does implement Drop. That triggers LLVM's cleanup which goes to delete the function "do_nothing". However, LLVM detects that there's an Instruction still holding a pointer to "do_nothing" ("%to_infinity_and_beyond" created in call_site) and detects that this would leave us with a dangling pointer, and assertion-fails.

Expected Behavior Deparenting is weird and we should probably simply not support it in inkwell except to use under the hood to build up safe primitives. The reason you deparent an instruction in LLVM IR is as a temporary step to move it to a different basic block, for example.

The builder should not allow us to create deparented instructions. That might mean that we must specify a BasicBlock to create the Builder, or we could have runtime check that it has an insertion position specified when creating instructions.

Or if we do want to support de-parented instructions, we could change all values to have Drop that checks whether they're parented and if so deletes it.

LLVM Version (please complete the following information):

• LLVM Version: 10
• Inkwell Branch Used: master

[TheDan64]

If I understand correctly, I think I was already aware of this issue, and my solution was to have a Builder<Orphaned> and Builder<NotOrphaned> (or whatever better sub-type names) and only the latter type has the standard builder methods. The former is a stand-in until it is positioned. IE Builder<Orphaned>::position_at_instruction(self, i: InstructionValue) -> Builder<NotOrphaned>

Alternatively, like you said, we could just require an instruction or block to be provided on Builder creation and that might be a more user friendly API.