TheDan64 / inkwell

It's a New Kind of Wrapper for Exposing LLVM (Safely)
https://thedan64.github.io/inkwell/
Apache License 2.0
2.33k stars 227 forks source link

unsafe to get or set thread_local_mode on a function #95

Open nlewycky opened 5 years ago

nlewycky commented 5 years ago

Describe the Bug Functions have an as_global_value() method to cast to a GlobalValue. GlobalValues have get_thread_local_mode() which gets the current thread local storage mode. That property only exists on C++ llvm::GlobalVariable which is a subclass of C++ llvm::GlobalValue. (C++ llvm::Function is also a subclass of C++ llvm::GlobalValue, so the as_global_value() method should be there, but maybe inkwell should grow a GlobalVariable class to match the safety of the C++ API.)

To Reproduce test_values.rs' test_function_value_to_global_to_pointer() demonstrates the bug:

    let fn_value = module.add_function("my_func", fn_type, None);

    let fn_global_value = fn_value.as_global_value();
// [...]
    assert!(fn_global_value.get_thread_local_mode().is_none());

Here's what happens if you run this test with an assertions-enabled build of LLVM:

test test_values::test_function_value_to_global_to_pointer ... Assertion failed: (isa<X>(Val) && "cast<Ty>() argument of incompatible type!"), function cast, file ../include/llvm/Support/Casting.h, line 255.

The cast in question is the cast of a C++ llvm::Function to a C++ llvm::GlobalVariable, and that is indeed an invalid cast.

LLVM Version (please complete the following information):

Desktop (please complete the following information):

TheDan64 commented 5 years ago

I'd rather not split up GlobalValue and GlobalVariable because they're so similar and I don't want to further conflate inkwell's API. What if we just have get_thread_local_mode return None if LLVMIsAGlobalVariable returns false?

TheDan64 commented 5 years ago

I suppose we could alternatively make globals generic over Value and Variable, ie Global<Variable> and Global<Value> so that they could share methods as well as have individual ones. This is maybe more ideal but also more complex as far as figuring out what belongs where.