-
Notifications
You must be signed in to change notification settings - Fork 1.4k
cranelift-object: Make sections read only by default #5619
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
Conversation
The diff itself looks fine here; I'll trust that this section type is what is needed :-) Regarding CI:
|
8f8a375
to
68ac43f
Compare
Did the best I could, we are now only duplicating the Also
👍, You also recently did a similar vet for |
@afonso360 would you be willing to update this with an exemption for the |
68ac43f
to
9424805
Compare
I've rebased this and added the exception, but I think we still have the same issue with cargo vet of missing a few vet's similar to #5550. |
9424805
to
2207d2e
Compare
Rebased this on top of main, and that resolves pretty much all of our vet issues. With the exception of |
This audit is needed for bytecodealliance#5619. I'm going ahead and updating Cargo.toml and Cargo.lock at the same time because no source code changes are required for this update.
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.
Once #5827 merges you should be able to rebase this without any cargo-vet complaints. At that point, go ahead and merge!
This changes the default section type to be `ReadOnlyDataWithRel` instead of `Data`. On COFF types the CRT initializers do not run unless their section is read only. The new SectionKind makes these sections read only for COFF and MachO, but leaves it as Writable as required by ELF.
2207d2e
to
c52a173
Compare
Thanks for unblocking this! 🎉 |
The merge queue failure seems to have been sporadic, I retried the job and it passed. I'm going to retry the merge queue again. |
This changes the default section type to be
ReadOnlyDataWithRel
instead ofData
.On COFF the CRT initializers do not run unless their section is read only. (See: https://github.com/bjorn3/rustc_codegen_cranelift/issues/1249#issuecomment-1382431789)
The new
SectionKind
makes these sections read only for COFF and MachO, but leaves it as Writable as required by ELF.cc: @bjorn3