-
Notifications
You must be signed in to change notification settings - Fork 757
Add linux aarch64 target to ci test #3217
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
base: main
Are you sure you want to change the base?
Conversation
5049277
to
b2c3bb3
Compare
Not sure what to do about the failure:
|
ac7e42b
to
ebb148b
Compare
This needs a rebase but I think the failure is legit. Is it crashing on the valist in that test? If you can reproduce locally, it'd be great to see what bit of the test is failing. |
ebb148b
to
330b518
Compare
I reproduced and got a stacktrace locally:
How to reproduce, something like this:
|
I tried running it manually but it seems to succeed, @emilio how can I trigger the panic by running bindgen?
edit: Managed to reproduce
edit2: Attached debug output edit3: I have never worked on bindgen, so I probably won't be able to figure out what could be wrong here. |
Ok, to reproduce that on x86_64 you can:
It seems bindgen can't reason about the builtin va_list there: https://github.com/Blizzard/clang/blob/6c67f57680c38ccd146786841eeac753ad45bf0f/lib/AST/ASTContext.cpp#L6099 As a workaround for now, you can probably ifdef the tests using va_list and update expectations or so. |
Please do file an issue about the wrap-static-fns not dealing with valists in aarch64 |
1dbee4b
to
ccc363a
Compare
Fixed the failed CI and rebased Maybe it is a bit excessive to run all the combinations on |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. If we see CI taking too long we can restrict in a follow-up.
Would you mind filing an issue for the valist bit that was broken and reference it from the code where we disable that test?
.arg("-DUSE_VA_HEADER"); | ||
|
||
#[cfg(all(target_arch = "aarch64", target_os = "linux"))] | ||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you point to a follow-up issue for this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
5a8d5dd
to
22ede02
Compare
I guess it should be as simple as this now that aarch64 runners have been added.