search

rust-lang / rust-bindgen

Automatically generates Rust FFI bindings to C (and some C++) libraries.
https://rust-lang.github.io/rust-bindgen/
BSD 3-Clause "New" or "Revised" License
4.23k stars 679 forks source link

Workaround for expansion of function-like macros #2779

Closed jbaublitz closed 3 months ago

jbaublitz commented 3 months ago

Related to #753

@ojeda Let me know if you have any thoughts on this. Compared to without the fallback (1s on the contants in the kernel header) I managed to get the compilation times from 2m to 8s to 3.5s. There may be additional optimizations we can do but it seems like it's definitely trending in the right direction.

jbaublitz commented 3 months ago

I've done a quick pass to fix the low hanging fruit. Will try to get back to this later today to resolve the rest of the concerns and follow up on any outstanding discussions.

jbaublitz commented 3 months ago

@ojeda @emilio I think this is ready for a second pass! I think I've addressed all of the concerns raised above.

jbaublitz commented 3 months ago

@emilio What would you like in terms of the test? An integration or unit test or both?

emilio commented 3 months ago

@emilio What would you like in terms of the test? An integration or unit test or both?

Integration should be fine. Just using the new flags from a regular test (see the tests/headers directory and bindgen-flags: directives). I think it should be straight-forward but let me know if you hit any issues or weirdness. Thanks!

jbaublitz commented 3 months ago

Okay I think I've addressed everything!

emilio commented 3 months ago

It seems clang 9.0 doesn't like your test and the new test fails there. Let me know if you have the appetite to chase that down (shouldn't be hard by pulling a pre-built clang for Linux amd64, and pointing to it with LIBCLANG_PATH).

Otherwise I guess we can document the option as best effort / not working on older libclang versions (totally fine), and silence the failure there by adding the empty (well with the #[allow(...)]) expectation file to bindgen-tests/tests/expectations/tests/libclang-9/.

jbaublitz commented 3 months ago

I may need a little bit of help in terms of the explanation but I can reproduce this reusing a precompiled header from 9.x on the latest version and vice versa. I'm wondering if there's a way there's a lingering precompiled header in the test suite somehow. This also raises the question of whether we should also be deleting the precompiled headers because debugging this would be incredibly difficult for users unfamiliar with the code and I expect you'll bump into in increase in issues with this feature if we don't.

jbaublitz commented 3 months ago

Okay, I managed to track it down to Clang_Cursor_Evaluate. In Clang 9, this returns a NULL pointer when trying to evaluate a Cursor over Cursors with kinds IntegerLiteral and BinaryOperator. I'm not sure if this is a bug or not, but I think it may be best to ignore for the time being. I'll add the file you recommended.

SeleDreams commented 2 months ago

@jbaublitz I was able to test this workaround, but it doesn't seem to be working in my case. I am trying to make a wrapper around the blocksds libnds library (meant for making DS homebrews)

this is a part of the macros that register the interrupts

#include <nds/ndstypes.h>

// Values allowed for REG_IE and REG_IF
#define IRQ_VBLANK          BIT(0)  ///< Vertical blank interrupt mask
#define IRQ_HBLANK          BIT(1)  ///< Horizontal blank interrupt mask
#define IRQ_VCOUNT          BIT(2)  ///< Vcount match interrupt mask
#define IRQ_TIMER0          BIT(3)  ///< Timer 0 interrupt mask
#define IRQ_TIMER1          BIT(4)  ///< Timer 1 interrupt mask
#define IRQ_TIMER2          BIT(5)  ///< Timer 2 interrupt mask
#define IRQ_TIMER3          BIT(6)  ///< Timer 3 interrupt mask
#ifdef ARM7
#define IRQ_NETWORK         BIT(7)  ///< Serial/RTC interrupt mask (ARM7) (deprecated name)
#define IRQ_RTC             BIT(7)  ///< Serial/RTC interrupt mask (ARM7)
#endif
#define IRQ_DMA0            BIT(8)  ///< DMA 0 interrupt mask
#define IRQ_DMA1            BIT(9)  ///< DMA 1 interrupt mask
#define IRQ_DMA2            BIT(10) ///< DMA 2 interrupt mask
#define IRQ_DMA3            BIT(11) ///< DMA 3 interrupt mask
#define IRQ_KEYS            BIT(12) ///< Keypad interrupt mask
#define IRQ_CART            BIT(13) ///< GBA cartridge interrupt mask
#define IRQ_IPC_SYNC        BIT(16) ///< IPC sync interrupt mask

and BIT is a macro defined in ndstypes

/// Returns a number with the nth bit set.
#define BIT(n) (1 << (n))

none of the IRQ defines end up set in the wrapper

let bindings = bindgen::Builder::default()
        // The input header we would like to generate
        // bindings for.
        .header("wrapper.h")
        .wrap_static_fns(true)
        .wrap_static_fns_path("src/arm7_bindings.c")
        .use_core()
        .trust_clang_mangling(false)
        .layout_tests(false)
        .ctypes_prefix("::libc")
        .prepend_enum_name(false)
        .blocklist_type("__builtin_va_list")
        .blocklist_type("__va_list")
        .derive_default(true)
        // Tell cargo to invalidate the built crate whenever any of the
        // included header files changed.
        .parse_callbacks(Box::new(bindgen::CargoCallbacks::new()))
        .clang_arg(format!("-I{}/libs/libnds/include",blocksds_path))
        .clang_arg(format!("-I{}/libs/dswifi/include",blocksds_path))
        .clang_arg(format!("-I{}/libs/maxmod/include",blocksds_path))
        .clang_arg(format!("-isystem{}/toolchain/gcc-arm-none-eabi/arm-none-eabi/include",wonderful_path))
        .clang_arg("-DARM7")
        .clang_macro_fallback()
        // Finish the builder and generate the bindings.
        .generate()
        // Unwrap the Result and panic on failure.
        .expect("Unable to generate bindings");
jbaublitz commented 2 months ago

@SeleDreams I'm not able to reproduce the behavior you're seeing. With:

#define BIT(n) (1 << (n))

#define IRQ_VBLANK          BIT(0)  ///< Vertical blank interrupt mask
#define IRQ_HBLANK          BIT(1)  ///< Horizontal blank interrupt mask
#define IRQ_VCOUNT          BIT(2)  ///< Vcount match interrupt mask
#define IRQ_TIMER0          BIT(3)  ///< Timer 0 interrupt mask
#define IRQ_TIMER1          BIT(4)  ///< Timer 1 interrupt mask
#define IRQ_TIMER2          BIT(5)  ///< Timer 2 interrupt mask
#define IRQ_TIMER3          BIT(6)  ///< Timer 3 interrupt mask
#ifdef ARM7
#define IRQ_NETWORK         BIT(7)  ///< Serial/RTC interrupt mask (ARM7) (deprecated name)
#define IRQ_RTC             BIT(7)  ///< Serial/RTC interrupt mask (ARM7)
#endif
#define IRQ_DMA0            BIT(8)  ///< DMA 0 interrupt mask
#define IRQ_DMA1            BIT(9)  ///< DMA 1 interrupt mask
#define IRQ_DMA2            BIT(10) ///< DMA 2 interrupt mask
#define IRQ_DMA3            BIT(11) ///< DMA 3 interrupt mask
#define IRQ_KEYS            BIT(12) ///< Keypad interrupt mask
#define IRQ_CART            BIT(13) ///< GBA cartridge interrupt mask
#define IRQ_IPC_SYNC        BIT(16) ///< IPC sync interrupt mask

in testheader.h and

./target/debug/bindgen --clang-macro-fallback $HOME/testheader.h

I get:

/* automatically generated by rust-bindgen 0.69.4 */

pub const IRQ_VBLANK: u32 = 1;
pub const IRQ_HBLANK: u32 = 2;
pub const IRQ_VCOUNT: u32 = 4;
pub const IRQ_TIMER0: u32 = 8;
pub const IRQ_TIMER1: u32 = 16;
pub const IRQ_TIMER2: u32 = 32;
pub const IRQ_TIMER3: u32 = 64;
pub const IRQ_DMA0: u32 = 256;
pub const IRQ_DMA1: u32 = 512;
pub const IRQ_DMA2: u32 = 1024;
pub const IRQ_DMA3: u32 = 2048;
pub const IRQ_KEYS: u32 = 4096;
pub const IRQ_CART: u32 = 8192;
pub const IRQ_IPC_SYNC: u32 = 65536;

What version of Clang are you using? Another thing to consider is that if the directory that contains the headers aren't writable, you may need to specify --clang-macro-fallback-build-dir as a temporary writable directory to store the precompiled headers in.

SeleDreams commented 2 months ago

The directory is writable since I do see the temp file get created. I wonder if the issue is caused by BIT being defined in another header

Le dim. 28 avr. 2024 à 05:42, John Baublitz @.***> a écrit :

@SeleDreams https://github.com/SeleDreams I'm not able to reproduce the behavior you're seeing. With:

define BIT(n) (1 << (n))

define IRQ_VBLANK BIT(0) ///< Vertical blank interrupt mask

define IRQ_HBLANK BIT(1) ///< Horizontal blank interrupt mask

define IRQ_VCOUNT BIT(2) ///< Vcount match interrupt mask

define IRQ_TIMER0 BIT(3) ///< Timer 0 interrupt mask

define IRQ_TIMER1 BIT(4) ///< Timer 1 interrupt mask

define IRQ_TIMER2 BIT(5) ///< Timer 2 interrupt mask

define IRQ_TIMER3 BIT(6) ///< Timer 3 interrupt mask

ifdef ARM7

define IRQ_NETWORK BIT(7) ///< Serial/RTC interrupt mask (ARM7) (deprecated name)

define IRQ_RTC BIT(7) ///< Serial/RTC interrupt mask (ARM7)

endif

define IRQ_DMA0 BIT(8) ///< DMA 0 interrupt mask

define IRQ_DMA1 BIT(9) ///< DMA 1 interrupt mask

define IRQ_DMA2 BIT(10) ///< DMA 2 interrupt mask

define IRQ_DMA3 BIT(11) ///< DMA 3 interrupt mask

define IRQ_KEYS BIT(12) ///< Keypad interrupt mask

define IRQ_CART BIT(13) ///< GBA cartridge interrupt mask

define IRQ_IPC_SYNC BIT(16) ///< IPC sync interrupt mask

in testheader.h and

./target/debug/bindgen --clang-macro-fallback $HOME/testheader.h

I get:

/ automatically generated by rust-bindgen 0.69.4 /

pub const IRQ_VBLANK: u32 = 1; pub const IRQ_HBLANK: u32 = 2; pub const IRQ_VCOUNT: u32 = 4; pub const IRQ_TIMER0: u32 = 8; pub const IRQ_TIMER1: u32 = 16; pub const IRQ_TIMER2: u32 = 32; pub const IRQ_TIMER3: u32 = 64; pub const IRQ_DMA0: u32 = 256; pub const IRQ_DMA1: u32 = 512; pub const IRQ_DMA2: u32 = 1024; pub const IRQ_DMA3: u32 = 2048; pub const IRQ_KEYS: u32 = 4096; pub const IRQ_CART: u32 = 8192; pub const IRQ_IPC_SYNC: u32 = 65536;

What version of Clang are you using? Another thing to consider is that if the directory that contains the headers aren't writable, you may need to specify --clang-macro-fallback-build-dir as a temporary writable directory to store the precompiled headers in.

— Reply to this email directly, view it on GitHub https://github.com/rust-lang/rust-bindgen/pull/2779#issuecomment-2081312474, or unsubscribe https://github.com/notifications/unsubscribe-auth/AD4UF4LW6RDIYNOM6D4DWNLY7RV2TAVCNFSM6AAAAABEQVC3USVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAOBRGMYTENBXGQ . You are receiving this because you were mentioned.Message ID: @.***>

SeleDreams commented 2 months ago

Also, does it use the system wide clang installation ? if that's the case

clang --version clang version 17.0.6 Target: x86_64-pc-linux-gnu Thread model: posix InstalledDir: /usr/bin

SeleDreams commented 2 months ago

@jbaublitz after testing, I found something interesting. If I directly include nds.h like

let bindings = bindgen::Builder::default()
        // The input header we would like to generate
        // bindings for.
        .header(format!("{}/libs/libnds/include/nds.h",blocksds_path))
        .wrap_static_fns(true)
        .wrap_static_fns_path("src/arm9_bindings.c")
        .use_core()
        .trust_clang_mangling(false)
        .prepend_enum_name(false)
        .clang_arg("-mfloat-abi=soft")
        // Tell cargo to invalidate the built crate whenever any of the
        // included header files changed.
        .clang_arg(format!("-I{}/libs/libnds/include",blocksds_path))
        .clang_arg(format!("-I{}/libs/dswifi/include",blocksds_path))
        .clang_arg(format!("-I{}/libs/maxmod/include",blocksds_path))
        .clang_arg(format!("-isystem{}/toolchain/gcc-arm-none-eabi/arm-none-eabi/include",wonderful_path))
        .clang_arg("-DARM9")
        .clang_arg("-D__NDS__")
        .clang_macro_fallback()
        .wrap_unsafe_ops(true)
        // Finish the builder and generate the bindings.
        .generate()
        // Unwrap the Result and panic on failure.
        .expect("Unable to generate bindings");

it works, however, if I include a wrapper.h with the content

#include <nds.h>
#ifdef ARM7
#include <dswifi7.h>
#include <maxmod7.h>
#elif ARM9
#include <dswifi9.h>
#include <maxmod9.h>
#endif

it doesn't work, although it did include the rest of nds.h, it didn't import the macros that might be a bug

edit: that seems to occur also when I do that

let bindings = bindgen::Builder::default()
        // The input header we would like to generate
        // bindings for.
        .header(format!("{}/libs/libnds/include/nds.h",blocksds_path))
        .header(format!("{}/libs/dswifi/include/dswifi9.h",blocksds_path))
        .header(format!("{}/libs/maxmod/include/maxmod9.h",blocksds_path))

so the issue seems caused by the fact to import other headers than just nds.h

it also occurs when wrapper.h only includes nds.h this is the library for reference https://github.com/blocksds/libnds/tree/master

jbaublitz commented 2 months ago

@SeleDreams I just did some research and what I'm seeing is that Clang only supports one precompiled header at a time. I'm working on a fix now.