Skip to content

Bindgen generates inconsistent enum representations on Windows and Unix #1244

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

Closed
ebkalderon opened this issue Jan 31, 2018 · 5 comments
Closed
Labels

Comments

@ebkalderon
Copy link

ebkalderon commented Jan 31, 2018

I am working on some Rust bindings for RenderDoc, and I am trying to replace my hand-rolled FFI code with bindgen. However, I am running into an issue with incorrect and inconsistent enum representations between Windows and Unix-likes, where on Windows, the resulting code doesn't even compile when integrated into my bindings.

The bug is caused due to a single line in the input code below, and appears to be tangentially related to #1185.

Input C/C++ Header

typedef enum {
  eRENDERDOC_Overlay_Enabled = 0x1,
  eRENDERDOC_Overlay_FrameRate = 0x2,
  eRENDERDOC_Overlay_FrameNumber = 0x4,
  eRENDERDOC_Overlay_CaptureList = 0x8,
  eRENDERDOC_Overlay_Default = (eRENDERDOC_Overlay_Enabled | eRENDERDOC_Overlay_FrameRate |
                                eRENDERDOC_Overlay_FrameNumber | eRENDERDOC_Overlay_CaptureList),
  eRENDERDOC_Overlay_All = ~0U, // <-- This is the culprit!
  eRENDERDOC_Overlay_None = 0,
} RENDERDOC_OverlayBits;

typedef uint32_t(RENDERDOC_CC *pRENDERDOC_GetOverlayBits)();

Bindgen Invocation

bindgen::Builder::default()
    .header("input.h")
    .generate()
    .unwrap()

Actual Results

On Unix, bindgen correctly outputs:

pub const RENDERDOC_OverlayBits_eRENDERDOC_Overlay_Enabled: RENDERDOC_OverlayBits = 1;
pub const RENDERDOC_OverlayBits_eRENDERDOC_Overlay_FrameRate: RENDERDOC_OverlayBits = 2;
pub const RENDERDOC_OverlayBits_eRENDERDOC_Overlay_FrameNumber: RENDERDOC_OverlayBits = 4;
pub const RENDERDOC_OverlayBits_eRENDERDOC_Overlay_CaptureList: RENDERDOC_OverlayBits = 8;
pub const RENDERDOC_OverlayBits_eRENDERDOC_Overlay_Default: RENDERDOC_OverlayBits = 15;
pub const RENDERDOC_OverlayBits_eRENDERDOC_Overlay_All: RENDERDOC_OverlayBits = 4294967295; // Technically correct value, but literal is out of range.
pub const RENDERDOC_OverlayBits_eRENDERDOC_Overlay_None: RENDERDOC_OverlayBits = 0;
pub type RENDERDOC_OverlayBits = u32; // Correct type.
pub type pRENDERDOC_GetOverlayBits = ::std::option::Option<unsafe extern "C" fn() -> u32>;

When cross-compiling on Unix to Windows, bindgen incorrectly outputs:

pub const RENDERDOC_OverlayBits_eRENDERDOC_Overlay_Enabled: RENDERDOC_OverlayBits = 1;
pub const RENDERDOC_OverlayBits_eRENDERDOC_Overlay_FrameRate: RENDERDOC_OverlayBits = 2;
pub const RENDERDOC_OverlayBits_eRENDERDOC_Overlay_FrameNumber: RENDERDOC_OverlayBits = 4;
pub const RENDERDOC_OverlayBits_eRENDERDOC_Overlay_CaptureList: RENDERDOC_OverlayBits = 8;
pub const RENDERDOC_OverlayBits_eRENDERDOC_Overlay_Default: RENDERDOC_OverlayBits = 15;
pub const RENDERDOC_OverlayBits_eRENDERDOC_Overlay_All: RENDERDOC_OverlayBits = -1;  // Incorrect value!
pub const RENDERDOC_OverlayBits_eRENDERDOC_Overlay_None: RENDERDOC_OverlayBits = 0;
pub type RENDERDOC_OverlayBits = i32; // Incorrect type!
pub type pRENDERDOC_GetOverlayBits = ::std::option::Option<unsafe extern "C" fn() -> u32>;

Expected Results

The correct enum representation in this case should have been uint32_t on both platforms. I would like to see better support for ~0U/u32::MAX on the bindgen side of things.

@emilio
Copy link
Contributor

emilio commented Jan 31, 2018

Huh, interesting, which target are you passing to bindgen when cross-compiling? Afaict we just use the type that clang gives us.

@ebkalderon
Copy link
Author

ebkalderon commented Jan 31, 2018

@emilio I'm cross-compiling to the x86_64-pc-windows-msvc target using bindgen 0.32.1 and rustc 1.25.0-nightly (90eb44a58 2018-01-29).

ebkalderon added a commit to ebkalderon/renderdoc-rs that referenced this issue Feb 1, 2018
* Temporary hacks for rust-lang/rust-bindgen#1244
* Implement WindowHandle trait
* Add rudimentary EGL support (see renderdoc/issues#853)
* Fix compilation bugs with Windows and winapi
* Modernize triangle example
* Change replay wrapper filenames to snake case
@emilio
Copy link
Contributor

emilio commented Feb 3, 2018

Sorry I couldn't look at this during the week. clang is returning us the signed integer type, instead of the unsigned one.

I think this is not a bindgen (nor clang) bug, and it's a bogus assumption that the enum ends up unsigned.

In particular, see https://godbolt.org/g/p6tBZP. MSVC is the only compiler that fails to compile that program, but passes if I replace unsigned with int.

I'm closing this because of the above, but please let me know if I got it wrong. Thanks a lot for the report! MSVC never stops to amuse me :)

@emilio emilio closed this as completed Feb 3, 2018
@emilio
Copy link
Contributor

emilio commented Feb 3, 2018

For reference, just in case the godbolt link expires or something, the program is:

#include <type_traits>

extern "C" {

typedef enum {
  eRENDERDOC_Overlay_All = ~0U,
} RENDERDOC_OverlayBits;

}

int main() {
  static_assert(
      std::is_same<std::underlying_type<RENDERDOC_OverlayBits>::type, unsigned>::value,
      "Huh");
}

@emilio
Copy link
Contributor

emilio commented Feb 3, 2018

It may be worth to point this up to upstream renderdoc, just in case it causes some weird behavior on windows.

@emilio emilio added the invalid label Feb 3, 2018
ConnorGray added a commit to WolframResearch/wolfram-library-link-rs that referenced this issue Jun 11, 2022
…ants

This is a fix for:

    #29

The existing code assumed that the Wolfram LibraryLink constants would have
the same size and signedness on all platforms. This assumption was not correct
on Windows targeting MSVC, where constants that would be u32 on other platforms
would be i32.

See also:

    * rust-lang/rust-bindgen#1244
    * rust-lang/rust-bindgen#1361
ConnorGray added a commit to WolframResearch/wolfram-library-link-rs that referenced this issue Jun 11, 2022
…ants (#41)

This is a fix for:

    #29

The existing code assumed that the Wolfram LibraryLink constants would have
the same size and signedness on all platforms. This assumption was not correct
on Windows targeting MSVC, where constants that would be u32 on other platforms
would be i32.

See also:

    * rust-lang/rust-bindgen#1244
    * rust-lang/rust-bindgen#1361
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants