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

Link crtbegin/crtend on musl to terminate .eh_frame #82534

Merged
merged 1 commit into from
Feb 28, 2021

Conversation

nikic
Copy link
Contributor

@nikic nikic commented Feb 25, 2021

For some targets, rustc uses a "CRT fallback", where it links CRT
object files it ships instead of letting the host compiler link
them.

On musl, rustc currently links crt1, crti and crtn (provided by
libc), but does not link crtbegin and crtend (provided by libgcc).
In particular, crtend is responsible for terminating the .eh_frame
section. Lack of terminator may result in segfaults during
unwinding, as reported in #47551 and encountered by the LLVM 12
update in #81451.

This patch links crtbegin and crtend for musl as well, following
the table at the top of crt_objects.rs.

r? @nagisa

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 25, 2021
@nikic nikic mentioned this pull request Feb 25, 2021
Copy link
Member

@nagisa nagisa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any way we could possibly have a test for this? Fine to not do if its not trivial to construct an example that crashes the way we seen things crash in the LLVM12 PR.

@@ -74,7 +74,7 @@ pub(super) fn pre_musl_fallback() -> CrtObjects {
}

pub(super) fn post_musl_fallback() -> CrtObjects {
all("crtn.o")
all(&["crtn.o", "rsend.o"])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I probably would be significantly more comfortable with distributing and linking crtend.o.

I'm not sure we want to take care of maintaining rsend.rs to undo whatever crt{1,i}.o do. For instance it looks like crtend.o is also responsible for terminating the .ctors and .dtors sections – which is not something rsend.o appears to handle currently.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feel free to r=me if you think this concern is unwarranted.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you're right, it's safer to link the normal crtbegin/crtend objects, rather than reimplementing them. I've adjusted things to copy the object files and link them according to the table at the top of the file.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is exactly what I was going to suggest.

(The comment higher in this file says that we are not linking crtbegin/crtend, but it can probably be kept as is because we are still not linking them on other targets supporting self-contained mode.)

@petrochenkov petrochenkov self-assigned this Feb 26, 2021
For some targets, rustc uses a "CRT fallback", where it links CRT
object files it ships instead of letting the host compiler link
them.

On musl, rustc currently links crt1, crti and crtn (provided by
libc), but does not link crtbegin and crtend (provided by libgcc).
In particular, crtend is responsible for terminating the .eh_frame
section. Lack of terminator may result in segfaults during
unwinding, as reported in rust-lang#47551 and encountered by the LLVM 12
update in rust-lang#81451.

This patch links crtbegin and crtend for musl as well, following
the table at the top of crt_objects.rs.
@nikic nikic changed the title Add .eh_frame terminator on musl Link crtbegin/crtend on musl to terminate .eh_frame Feb 26, 2021
@nagisa
Copy link
Member

nagisa commented Feb 26, 2021

@bors r+

@bors
Copy link
Contributor

bors commented Feb 26, 2021

📌 Commit c7091f5 has been approved by nagisa

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 26, 2021
@petrochenkov petrochenkov removed their assignment Feb 26, 2021
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Feb 27, 2021
Link crtbegin/crtend on musl to terminate .eh_frame

For some targets, rustc uses a "CRT fallback", where it links CRT
object files it ships instead of letting the host compiler link
them.

On musl, rustc currently links crt1, crti and crtn (provided by
libc), but does not link crtbegin and crtend (provided by libgcc).
In particular, crtend is responsible for terminating the .eh_frame
section. Lack of terminator may result in segfaults during
unwinding, as reported in rust-lang#47551 and encountered by the LLVM 12
update in rust-lang#81451.

This patch links crtbegin and crtend for musl as well, following
the table at the top of crt_objects.rs.

r? `@nagisa`
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 28, 2021
Rollup of 11 pull requests

Successful merges:

 - rust-lang#81856 (Suggest character encoding is incorrect when encountering random null bytes)
 - rust-lang#82395 (Add missing "see its documentation for more" stdio)
 - rust-lang#82401 (Remove a redundant macro)
 - rust-lang#82498 (Use log level to control partitioning debug output)
 - rust-lang#82534 (Link crtbegin/crtend on musl to terminate .eh_frame)
 - rust-lang#82537 (Update measureme dependency to the latest version)
 - rust-lang#82561 (doc: cube root, not cubic root)
 - rust-lang#82563 (Fix intra-doc handling of `Self` in enum)
 - rust-lang#82584 (Add ARIA role to sidebar toggle in Rustdoc)
 - rust-lang#82596 (clarify RW lock's priority gotcha)
 - rust-lang#82607 (Add a getter for Frame.loc)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit a7f9aca into rust-lang:master Feb 28, 2021
@rustbot rustbot added this to the 1.52.0 milestone Feb 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants