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

Bindgen doesn't work on with MSVC #67

Closed
upsuper opened this issue Sep 28, 2016 · 16 comments
Closed

Bindgen doesn't work on with MSVC #67

upsuper opened this issue Sep 28, 2016 · 16 comments

Comments

@upsuper
Copy link
Contributor

upsuper commented Sep 28, 2016

I tried to investigate this, but failed. It throws lots of errors. See the attached log: bindgen.zip

This log gives me an impression that it just fails to parse everything.

My Clang version is:

clang version 3.9.0 (branches/release_39)
Target: x86_64-pc-windows-msvc
Thread model: posix
InstalledDir: c:\Program Files\LLVM\bin
@upsuper
Copy link
Contributor Author

upsuper commented Sep 28, 2016

@emilio could you have a look?

@emilio
Copy link
Contributor

emilio commented Sep 28, 2016

It's choking to parse some LLVM3.9-specific type that isn't handled. I'm looking into it, shouldn't be hard to fix.

Seems like a good moment to upgrade to LLVM 3.9 by default.

@upsuper
Copy link
Contributor Author

upsuper commented Sep 28, 2016

I've been using llvm 3.9 for quite a while, so I suppose that may not be relevant.

@emilio
Copy link
Contributor

emilio commented Sep 28, 2016

The rewrite chokes on errors and doesn't just ignore it, so yeah, it is relevant.

@emilio
Copy link
Contributor

emilio commented Sep 28, 2016

Problems found:

  • @upsuper was using llvm 3.9 but compiling with llvm_stable, which was the cause for this concrete bug. That's going away soon since I plan to require llvm 3.9 soon.
  • We found a weird const size_t in nsCSSValueArray that made bindings incorrect. Presumably is parsed as a new type. I'll try to reduce it.
  • Making _Atomic_Base opaque in MSVC wasn't working.
  • Found a libclang crash while getting the mangling of a cursor, so it's probably MSVC-specific libclang bug, though difficult to diagnose.

@upsuper
Copy link
Contributor Author

upsuper commented Sep 28, 2016

The mangling crash looks very much a clang bug. It always crashes at static const nsRunnableMethodTraits<R(C::*)(As...), Owning, Cancelable>::can_cancel in nsThreadUtils.h. However, if I comment out the line for that, it goes well even for the later nsRunnableMethodTraits<R(C::*)(As...) const, Owning, Cancelable>::can_cancel.

I guess we may want to switch off mangling for now, and probably try to file a bug to clang.

@upsuper
Copy link
Contributor Author

upsuper commented Sep 29, 2016

It crashes with this minimal example:

template<class M> struct A;
template<class C> struct A<void(C::*)()> { static int v; };

Wait... but how can a static variable in a template class has a mangling name?! It isn't linkable at all, so it actually makes no sense to try to generate mangling name for it.

@upsuper
Copy link
Contributor Author

upsuper commented Oct 19, 2016

Hmmm, bindgen fails to work again on Windows. It panicked for nonexisting item:

thread 'main' panicked at 'Not an item: ItemId(57262)', src\ir\context.rs:402
stack backtrace:
   0:     0x7ff7502c2b6e - _<std..rand..ThreadRng as rand..Rng>::next_u64::h75a21f4cede609ad
   1:     0x7ff7502c0fe3 - std::rt::lang_start::h53bf99b0829cc03c
   2:     0x7ff7502c1a4d - std::panicking::rust_panic_with_hook::h4cbd7ca63ce1aee9
   3:     0x7ff7502c1896 - std::panicking::begin_panic_fmt::hd0daa02942245d81
   4:     0x7ff7502c17f4 - std::panicking::begin_panic_fmt::hd0daa02942245d81
   5:     0x7ff74fdefe0d - bindgen::ir::context::BindgenContext::resolve_item
                        at c:\mozilla-source\stylo\servo\components\style\binding_tools\rust-bindgen\src\ir\context.rs:8
   6:     0x7ff74fdf8022 - bindgen::ir::item::Item::real_canonical_name
                        at c:\mozilla-source\stylo\servo\components\style\binding_tools\rust-bindgen\src\ir\item.rs:361
   7:     0x7ff74fdfd323 - bindgen::ir::item::{{impl}}::canonical_name
                        at c:\mozilla-source\stylo\servo\components\style\binding_tools\rust-bindgen\src\ir\item.rs:706
   8:     0x7ff74fdf60d6 - bindgen::ir::item::{{impl}}::canonical_name
                        at c:\mozilla-source\stylo\servo\components\style\binding_tools\rust-bindgen\src\ir\item.rs:69
   9:     0x7ff74fdf79bf - bindgen::ir::item::Item::real_canonical_name
                        at c:\mozilla-source\stylo\servo\components\style\binding_tools\rust-bindgen\src\ir\item.rs:292
  10:     0x7ff74fdfd323 - bindgen::ir::item::{{impl}}::canonical_name
                        at c:\mozilla-source\stylo\servo\components\style\binding_tools\rust-bindgen\src\ir\item.rs:706
  11:     0x7ff74fe3e2c5 - bindgen::codegen::codegen::{{closure}}
                        at c:\mozilla-source\stylo\servo\components\style\binding_tools\rust-bindgen\target\debug\build\bindgen-22479e177d6249bd\out\codegen.rs:2615
  12:     0x7ff74fdef5f7 - bindgen::ir::context::BindgenContext::gen<closure,collections::vec::Vec<syntex_syntax::ptr::P<syntex_syntax::ast::Item>>>
                        at c:\mozilla-source\stylo\servo\components\style\binding_tools\rust-bindgen\src\ir\context.rs:357
  13:     0x7ff74fe28a1d - bindgen::codegen::codegen
                        at c:\mozilla-source\stylo\servo\components\style\binding_tools\rust-bindgen\target\debug\build\bindgen-22479e177d6249bd\out\codegen.rs:2601
  14:     0x7ff74fe3714a - bindgen::Bindings::generate
                        at c:\mozilla-source\stylo\servo\components\style\binding_tools\rust-bindgen\src\lib.rs:249
  15:     0x7ff74fd45bc2 - bindgen::main
                        at c:\mozilla-source\stylo\servo\components\style\binding_tools\rust-bindgen\src\bin\bindgen.rs:242
  16:     0x7ff7502c4d31 - _rust_maybe_catch_panic
  17:     0x7ff7502c088a - std::rt::lang_start::h53bf99b0829cc03c
  18:     0x7ff74fd465a9 - main
  19:     0x7ff7502ceba0 - __scrt_common_main_seh
                        at f:\dd\vctools\crt\vcstartup\src\startup\exe_common.inl:253
  20:     0x7fffff088363 - BaseThreadInitThunk

@emilio
Copy link
Contributor

emilio commented Oct 19, 2016

This one looks like a bindgen bug, could you post a RUST_LOG=bindgen
output, and if you can look at what type was "self" in the
real_canonical_name call that fails, that'd be helpful too.

On Oct 19, 2016 7:50 AM, "Xidorn Quan" [email protected] wrote:

Hmmm, bindgen fails to work again on Windows. It panicked for nonexisting
item:

thread 'main' panicked at 'Not an item: ItemId(57262)', src\ir\context.rs:402
stack backtrace:
0: 0x7ff7502c2b6e - _<std..rand..ThreadRng as rand..Rng>::next_u64::h75a21f4cede609ad
1: 0x7ff7502c0fe3 - std::rt::lang_start::h53bf99b0829cc03c
2: 0x7ff7502c1a4d - std::panicking::rust_panic_with_hook::h4cbd7ca63ce1aee9
3: 0x7ff7502c1896 - std::panicking::begin_panic_fmt::hd0daa02942245d81
4: 0x7ff7502c17f4 - std::panicking::begin_panic_fmt::hd0daa02942245d81
5: 0x7ff74fdefe0d - bindgen::ir::context::BindgenContext::resolve_item
at c:\mozilla-source\stylo\servo\components\style\binding_tools\rust-bindgen\src\ir\context.rs:8
6: 0x7ff74fdf8022 - bindgen::ir::item::Item::real_canonical_name
at c:\mozilla-source\stylo\servo\components\style\binding_tools\rust-bindgen\src\ir\item.rs:361
7: 0x7ff74fdfd323 - bindgen::ir::item::{{impl}}::canonical_name
at c:\mozilla-source\stylo\servo\components\style\binding_tools\rust-bindgen\src\ir\item.rs:706
8: 0x7ff74fdf60d6 - bindgen::ir::item::{{impl}}::canonical_name
at c:\mozilla-source\stylo\servo\components\style\binding_tools\rust-bindgen\src\ir\item.rs:69
9: 0x7ff74fdf79bf - bindgen::ir::item::Item::real_canonical_name
at c:\mozilla-source\stylo\servo\components\style\binding_tools\rust-bindgen\src\ir\item.rs:292
10: 0x7ff74fdfd323 - bindgen::ir::item::{{impl}}::canonical_name
at c:\mozilla-source\stylo\servo\components\style\binding_tools\rust-bindgen\src\ir\item.rs:706
11: 0x7ff74fe3e2c5 - bindgen::codegen::codegen::{{closure}}
at c:\mozilla-source\stylo\servo\components\style\binding_tools\rust-bindgen\target\debug\build\bindgen-22479e177d6249bd\out\codegen.rs:2615
12: 0x7ff74fdef5f7 - bindgen::ir::context::BindgenContext::gen<closure,collections::vec::Vec<syntex_syntax::ptr::P<syntex_syntax::ast::Item>>>
at c:\mozilla-source\stylo\servo\components\style\binding_tools\rust-bindgen\src\ir\context.rs:357
13: 0x7ff74fe28a1d - bindgen::codegen::codegen
at c:\mozilla-source\stylo\servo\components\style\binding_tools\rust-bindgen\target\debug\build\bindgen-22479e177d6249bd\out\codegen.rs:2601
14: 0x7ff74fe3714a - bindgen::Bindings::generate
at c:\mozilla-source\stylo\servo\components\style\binding_tools\rust-bindgen\src\lib.rs:249
15: 0x7ff74fd45bc2 - bindgen::main
at c:\mozilla-source\stylo\servo\components\style\binding_tools\rust-bindgen\src\bin\bindgen.rs:242
16: 0x7ff7502c4d31 - _rust_maybe_catch_panic
17: 0x7ff7502c088a - std::rt::lang_start::h53bf99b0829cc03c
18: 0x7ff74fd465a9 - main
19: 0x7ff7502ceba0 - __scrt_common_main_seh
at f:\dd\vctools\crt\vcstartup\src\startup\exe_common.inl:253
20: 0x7fffff088363 - BaseThreadInitThunk


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#67 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABQwuuXy3xeTVPChWEayPt6zReXCMcyRks5q1a-ogaJpZM4KIbgp
.

@emilio
Copy link
Contributor

emilio commented Oct 19, 2016

I'm somewhat surprised that the latest changes have introduced MSVC-only regressions, but I guess... #85 is the most likely regressor? Either that, or MSVC headers have changed and now have more dark magic.

@upsuper
Copy link
Contributor Author

upsuper commented Oct 19, 2016

I'm not aware of any update to MSVC's headers, so I suppose that is not the reason.

@upsuper
Copy link
Contributor Author

upsuper commented Oct 19, 2016

The log: log.zip

@upsuper
Copy link
Contributor Author

upsuper commented Oct 19, 2016

This is the file in question: xtr1common.zip

Looks like the unrecognized thing is the alias template.

@upsuper
Copy link
Contributor Author

upsuper commented Oct 19, 2016

The file in question itself doesn't actually cause the bindgen to panic. Then I have no idea.

@emilio
Copy link
Contributor

emilio commented Oct 19, 2016

So, after reading the log I think I know what's going on, and I don't like that at all.

So turns out clang can generate duplicated usrs, which they promise not doing, so we choke. usrs are platform-dependent I think, so this might be a MSVC-only issue. In any case this sort-of uncovers a flakyness on bindgen, that should be fixed.

Basically, what I think happens after reading the log is something like:

  • We parse the generic integral_constant<_Ty, _Val>, and store it in the usr map.
  • Life goes on...
  • We arrive at integral_constant<bool>, which is a CXType_Unexposed, with usr: c:@N@std@bool_constant. It's an unexposed type, and we see it has a canonical type, also unexposed, called integral_constant<bool, _Val>. This usr happens to be the same as integral_constant<_Ty, _Val>, so we mark it as found (which isn't too bad), and go on.
  • Something already uses the id we expected for integral_constant<bool>.

In any case, this is fixable (I think), by storing the type we just found as a resolved type refernce to the new type found. I think this is the proper and elegant solution and shouldn't be hard. Another less elegant solution is adding a fallible canonical name to do the whitelist checking, since it seems we choke at that point.

bors-servo pushed a commit that referenced this issue Oct 29, 2016
@emilio
Copy link
Contributor

emilio commented Nov 3, 2016

I think we can close this for now :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants