Skip to content

Both _WIN32 and _WIN64 are defined when targeting 32-bit windows from 64-bit windows #1062

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
liranringel opened this issue Oct 6, 2017 · 11 comments
Labels

Comments

@liranringel
Copy link
Contributor

I tried to investigate #541 and found out that both _WIN32 and _WIN64 are defined. I could see how it makes many troubles, and perhaps related to that issue .
It won't happen if the command has the --target=i686-pc-windows-msvc flag, but it does happen if it has the --target=x86_64-pc-windows-msvc flag.
It also happens when I use bindgen through build.rs script (this way it happens on both cargo run --target=i686-pc-windows-msvc and cargo run --target=x86_64-pc-windows-msvc).

Input C/C++ Header

// example.h
#ifdef _WIN32
typedef int type1;
#endif
#ifdef _WIN64
typedef long long type2;
#endif

Bindgen Invocation

bindgen.exe example.h

Actual Results

pub type type1 = ::std::os::raw::c_int;
pub type type2 = ::std::os::raw::c_longlong;

Expected Results

// when --target=i686-pc-windows-msvc

pub type type1 = ::std::os::raw::c_int;

// when --target=x86_64-pc-windows-msvc

pub type type2 = ::std::os::raw::c_longlong;
@fitzgen
Copy link
Member

fitzgen commented Oct 6, 2017

Thanks for the bug report!

I'm not really familiar with windows development, but to be clear, only one of either _WIN32 or _WIN64 should ever be defined at once, right?

@petrochenkov
Copy link

petrochenkov commented Oct 6, 2017

_WIN32 is always defined these days (on both 32 and 64 bit targets), see https://msdn.microsoft.com/en-us/library/b0084kay.aspx

_WIN32 Defined as 1 when the compilation target is 32-bit ARM, 64-bit ARM, x86, or x64. Otherwise, undefined.
_WIN64 Defined as 1 when the compilation target is 64-bit ARM or x64. Otherwise, undefined.

32 here is supposed to be the opposite to 16, not 64.

@fitzgen
Copy link
Member

fitzgen commented Oct 6, 2017

_WIN32 is always defined these days (on both 32 and 64 bit targets), see https://msdn.microsoft.com/en-us/library/b0084kay.aspx

Does that mean that this issue can be closed as exhibiting expected behavior?

@liranringel
Copy link
Contributor Author

Ah good to know.
Yet there is a problem, because through build.rs script _WIN64 is defined for 32bit.

@fitzgen
Copy link
Member

fitzgen commented Oct 6, 2017

To make sure I understand: the remaining issue is that when invoking bindgen as a library via build.rs on 32 bit host platforms, _WIN64 is still defined? But not when targeting 32-bit from 64 bit?

@liranringel
Copy link
Contributor Author

No.
It happens on my 64 bit machine when targeting to 32 bit.
I don't know what happens on 32 bit hosts.

@fitzgen fitzgen changed the title Both _WIN32 and _WIN64 are defined Both _WIN32 and _WIN64 are defined when targeting 32-bit windows Oct 6, 2017
@fitzgen
Copy link
Member

fitzgen commented Oct 6, 2017

Thanks for the clarification!

@fitzgen fitzgen changed the title Both _WIN32 and _WIN64 are defined when targeting 32-bit windows Both _WIN32 and _WIN64 are defined when targeting 32-bit windows from 64-bit windows Oct 6, 2017
@liranringel
Copy link
Contributor Author

liranringel commented Oct 6, 2017

If I had to guess, I'd say it's related to rust-lang/rust#42587
In other words, I guess that the bindgen library uses conditional compilation that misleads it because it's executed on a process that is 64 bit (like the build host).

@liranringel
Copy link
Contributor Author

Update:
It happens on version 0.30, but it's working as expected on master.

@liranringel
Copy link
Contributor Author

Ok I used git bisect to find the commit that solved it and it's 26da344
Makes sense that it solved it because it implements find_effective_target.

Only need to publish a new version.

@fitzgen
Copy link
Member

fitzgen commented Oct 6, 2017

We'll publish a new version after merging #1042

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

3 participants