-
Notifications
You must be signed in to change notification settings - Fork 14k
error out when repr(align) exceeds COFF limit
#142638
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
|
rustbot has assigned @WaffleLapkin. Use |
|
Some changes occurred in diagnostic error codes Some changes occurred in compiler/rustc_passes/src/check_attr.rs |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
It only needs to error if we generate a |
|
Could you include testing alignment on functions here in this PR (if relevant, but I suspect it is). I think you can just add it to the test files you already modify. #![feature(fn_align)]
// ...
// and then you can annotate a function with `#[repr(align(N))]`
#[repr(align(16))]
fn foo() {} |
|
@nthery This simple version that rejects |
|
...wait, we can't crater for Windows. |
|
fbbth. |
|
☔ The latest upstream changes (presumably #138165) made this pull request unmergeable. Please resolve the merge conflicts. |
It is relevant indeed. I reproduced the original bug with a function alignment exceeding 0x2000 bytes. Thanks for spotting this. I will add some test points. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
🔔 This is now entering its final comment period, as per the review above. 🔔 |
|
I'm working on rebasing the PR. Thanks for all the comments. |
|
This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
This comment has been minimized.
This comment has been minimized.
The PE-COFF binary format limits section alignment to 8192 bytes. Emit error when alignment exceeds this limit to avoid crash in llvm.
This document the following change to the compiler that enforces the limit: rust-lang/rust#142638.
Here is the PR: rust-lang/reference#2081. |
|
Thanks for that. @rfcbot reviewed cc @rust-lang/fls |
|
The final comment period, with a disposition to merge, as per the review above, is now complete. As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed. This will be merged soon. |
|
I suspect we'll get such feedback soon enough, but I also think that the fastest way to find out is the old "throw it over the wall and see who screams" tactic, so! @bors r+ |
error out when `repr(align)` exceeds COFF limit The PE-COFF binary format limits section alignment to 8192 bytes. Emit error when alignment exceeds this limit to avoid crash in llvm. Closes #142386. Reference PR: - rust-lang/reference#2081
|
The job Click to see the possible cause of the failure (guessed by this bot) |
|
💔 Test failed - checks-actions |
The PE-COFF binary format limits section alignment to 8192 bytes. Emit error when alignment exceeds this limit to avoid crash in llvm.
Closes #142386.
Reference PR: