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

[builtins] Disable COMPILER_RT_CRT_USE_EH_FRAME_REGISTRY by default #83201

Conversation

MaskRay
Copy link
Member

@MaskRay MaskRay commented Feb 27, 2024

Most of GCC's Linux targets have a link spec
%{!static|static-pie:--eh-frame-hdr} that doesn't pass --eh-frame-hdr
for -static links. -static links are supposed to utilize
__register_frame_info (called by crtbeginT.o, not by crtbegin.o or
crtbeginS.o) as a replacement.

compiler-rt crtbegin (not used with GCC) has some ehframe code, which is
not utilized because Clang driver unconditionally passes --eh-frame-hdr
for Linux targets, even for -static. In addition, LLVM libunwind
implements __register_frame_info as an empty stub.

Furthermore, in a non-static link, the __register_frame_info
references can cause an undesired weak dynamic symbol.

For now, just disable the config by default.

Created using spr 1.3.4
@MaskRay
Copy link
Member Author

MaskRay commented Feb 27, 2024

@xarblu

Copy link
Member

@thesamesam thesamesam left a comment

Choose a reason for hiding this comment

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

Thank you!

@MaskRay MaskRay merged commit 062cfad into main Feb 28, 2024
3 of 4 checks passed
@MaskRay MaskRay deleted the users/MaskRay/spr/builtins-disable-compiler_rt_crt_use_eh_frame_registry-by-default branch February 28, 2024 00:32
@mysterymath
Copy link
Contributor

Hey, it looks like this breaks LLDB's test suite on Fuchsia's AArch64 builders: https://luci-milo.appspot.com/ui/p/fuchsia/builders/toolchain.ci/clang-linux-arm64/b8754880571862677073/overview

Reverting this change from the blamelist succeeds, and it's the only likely candidate change with that property.

@petrhosek
Copy link
Member

Can we revert this change and then dig into the reason why these tests started failing? Our builders have been broken for an entire week and I'd like to avoid leaving them broken over the weekend.

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

Successfully merging this pull request may close these issues.

None yet

5 participants