Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add wrappers for support functions #17

Open
71 opened this issue Oct 1, 2017 · 5 comments
Open

Add wrappers for support functions #17

71 opened this issue Oct 1, 2017 · 5 comments
Milestone

Comments

@71
Copy link
Contributor

71 commented Oct 1, 2017

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);
    }
}
@TheDan64 TheDan64 added this to the 0.1.0 milestone Oct 1, 2017
@TheDan64
Copy link
Owner

TheDan64 commented Oct 1, 2017

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
Copy link
Owner

TheDan64 commented Oct 6, 2017

Other support functions requiring callbacks and/or void pointers:

  • LLVMAddSymbol:
  • LLVMSearchForAddressOfSymbol:
  • LLVMLoadLibraryPermanently: 355a238
  • LLVMParseCommandLineOptions:
  • LLVMInstallFatalErrorHandler: f9902c6
  • LLVMContextSetDiagnosticHandler
  • LLVMContextSetYieldCallback
  • LLVMGetDiagInfoDescription
  • LLVMGetDiagInfoSeverity

@71
Copy link
Contributor Author

71 commented Oct 11, 2017

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
Copy link
Contributor Author

71 commented Oct 11, 2017

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.

@TheDan64
Copy link
Owner

TheDan64 commented Oct 13, 2017

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 TheDan64 modified the milestones: 0.1.0, 0.2.0 Nov 29, 2017
@TheDan64 TheDan64 modified the milestones: 0.6.0, 0.7.0 Jul 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants