inkwell icon indicating copy to clipboard operation
inkwell copied to clipboard

Add wrappers for support functions

Open 71 opened this issue 7 years ago • 5 comments

It would be nice to have "Rust-friendly" (I doubt "safe" is a good word) wrappers around the methods found in llvm_sys::support, namely:

  • LLVMAddSymbol
  • LLVMSearchForAddressOfSymbol
  • LLVMLoadLibraryPermanently
  • LLVMParseCommandLineOptions (a bit less useful)

However, I can't make it work in any way. I created support.rs with the following content, but I can't make a single test that actually succeeds.

use llvm_sys::support::*;
use libc::{c_void};
use std::ffi::CString;

pub fn add_symbol(name: &str, value: *const ()) {
    let name = CString::new(name).unwrap();

    unsafe {
        LLVMAddSymbol(name.as_ptr(), ::std::mem::transmute(value));
    }
}

pub fn search_for_address_of_symbol<T>(name: &str) -> Option<*mut T> {
    let name = CString::new(name).unwrap();

    unsafe {
        let addr = LLVMSearchForAddressOfSymbol(name.as_ptr());

        if addr.is_null() {
            None
        } else {
            Some(addr as *mut T)
        }
    }
}

pub fn load_library_permanently(filename: &str) -> bool {
    let filename = CString::new(filename).unwrap();

    unsafe {
        LLVMLoadLibraryPermanently(filename.as_ptr()) == 1
    }
}

I also tried the following for add_symbol, which didn't work either.

pub fn add_symbol<T>(name: &str, value: &mut T) {
    let name = CString::new(name).unwrap();

    let addr1 = &*value as *const _ as *const c_void;
    let addr2 = value as *const _ as *const ();

    unsafe {
        LLVMAddSymbol(name.as_ptr(),  value as &mut _ as &mut c_void);
    }
}

71 avatar Oct 01 '17 10:10 71

Adding a support module is a good idea. I already started a couple supporty methods in lib.rs (even though they don't come from llvm_sys' support.rs), so those should probably get moved.

I really want Inkwell to be as safe as possible, unless there's absolutely nothing we can do (see ExecutionEngine.get_function_address() for example). I think for a lot of these support methods that take a void pointer, we should be able to research what types of data is generally used in C/++, and provide a safe wrapper for those data types and allow them to be passed into these methods and turn them into void pointers behind the scenes as needed. (Though I'm not familiar with your example functions in particular, so I'm just speculating) What exactly are you trying to do with these methods?

TheDan64 avatar Oct 01 '17 23:10 TheDan64

Other support functions requiring callbacks and/or void pointers:

  • [ ] LLVMAddSymbol:
  • [ ] LLVMSearchForAddressOfSymbol:
  • [x] LLVMLoadLibraryPermanently: 355a238ab28078807eca7d46a630e6b4e666c2f5
  • [ ] LLVMParseCommandLineOptions:
  • [x] LLVMInstallFatalErrorHandler: f9902c600e7e0774351b60e364d97ffa7f24bc51
  • [ ] LLVMContextSetDiagnosticHandler
  • [ ] LLVMContextSetYieldCallback
  • [ ] LLVMGetDiagInfoDescription
  • [ ] LLVMGetDiagInfoSeverity

TheDan64 avatar Oct 06 '17 22:10 TheDan64

I'm working on add_symbol right now, and actually got it working! However, there is a bug when running all tests.

I think I'm having an LLVM bug here, and not an Inkwell / Rust bug.

Basically, I can add one symbol, but once an EE has been created, adding other symbols does not register them for resolution. Really weird issue.

71 avatar Oct 11 '17 19:10 71

Alright, add_symbol has a bug when it is used more than once in a single process, but add_global_mapping DOES work! I'm gonna push a new commit.

In any case, is there a reason why tests.rs exists? With this file, every test is passed twice.

71 avatar Oct 11 '17 19:10 71

Oh, interesting! I thought tests.rs was needed, like a mod.rs. I'll remove it in my next commit to master since it might break code coverage which looks for the tests binary. Thanks!

TheDan64 avatar Oct 13 '17 11:10 TheDan64