Repository navigation
LLVM bindings have become incorrect in places. #17795
Description
Because of x86/x86_64 specifics, certain LLVM functions that used to return unsigned int and now return unsigned long long continue to be usable, ending up in truncated return values.
pub fn LLVMABISizeOfType(TD: TargetDataRef, Ty: TypeRef) -> c_uint;unsigned long long LLVMABISizeOfType(LLVMTargetDataRef TD, LLVMTypeRef Ty);This is just one example of such incorrectness arising over time, there could be more.
Would be nice if we could use bindgen every LLVM upgrade to verify our bindings (or just generate new ones).
Reacted by Luis Gerhorst
Activity
ahmedcharles commentedon Oct 6, 2014
Just curious, what's bindgen? Are there docs on how to use it?
eddyb commentedon Oct 6, 2014
@ahmedcharles I've updated the description with a link to bindgen.
steveklabnik commentedon Dec 31, 2015
I spoke about this with @eddyb on IRC today. This is especially pernicious, because we could be accidentally getting UB in corner cases.
A tactic to close this bug:
Humans looking at it will probably be needed, given that bindgen isn't perfect.
tamird commentedon Sep 13, 2017
I took a look at this, starting with the rather naive:
which produces the output:
tamird commentedon Sep 13, 2017
Using
--cflagsrather than--cxxflagsproduces c++ parsing failures:tamird commentedon Sep 13, 2017
Ah, renaming
rustllvm.htorustllvm.hppgets this further, but we hit rust-lang/rust-bindgen#492:gist containing bindgen.sh & bindgen.stderr
FreezyLemon commentedon Dec 10, 2022
If someone's interested in this (like me, originally), I'll save you some time:
The above PR #46353 was rejected due to A) bindgen needing an unwanted dependency and B) the team preferring a validation of the bindings over re-generating the bindings automatically via bindgen. (This is understandable, as the hand-written bindings are way nicer than bindgens output)
ctestwas proposed as a crate that allows testing the existing bindings. However, after some research,ctestis pretty much abandoned and even the fork ctest2 suffers from outdated dependencies, which results in the crate practically being unable to handlerustc_codegen_llvmwhich is the crate that would need to be parsed. I tried.The way I see it, the validation can't be reliably automated at the moment unless
ctest2gets a major update/rewrite or an alternative pops up (maybe with syn-based parsing).Edit: Actually, it might be possible. For example. libz-sys uses
ctest2. I guess the code in libz-sys is simple enough to be parsed by the outdated syntex crate. Technically, the bindings could be moved to a separate crate (e.g.rustc_llvm_bindings) and if the code is simple/"old" enough, syntex and ctest should be able to handle it.cuviper commentedon Dec 10, 2022
I think the bindings should probably move to
rustc_llvm, since that's the one that actually prepares the link library, like a typical*-syscrate. That's also where we define our C++ shims, then we would have all LLVM ffi in one place.FreezyLemon commentedon Dec 11, 2022
Oh, I didn't know a crate like that already existed. I'll take a look, maybe it's possible to get ctest to work with it.
FreezyLemon commentedon Dec 11, 2022
ctest still fails at parsing even after moving ffi.rs to
rustc_llvm. It might be possible by refactoring the code, but it's not trivial. It's honestly not something I'm willing to spend more time on at the moment. It might make more sense to "just wait" and hope ctest gets an updateAuto merge of rust-lang#17795 - Veykril:library-dep-loading, r=Veykril