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

macho: set minimum default headerpad to 32 bytes #20481

Closed
wants to merge 1 commit into from
Closed

Conversation

kubkon
Copy link
Member

@kubkon kubkon commented Jul 3, 2024

This matches Apple's ld and LLVM's lld.

This matches Apple's ld and LLVM's lld.
@kubkon kubkon enabled auto-merge (rebase) July 3, 2024 12:16
@daurnimator
Copy link
Contributor

This matches Apple's ld and LLVM's lld.

what happens if we don't?

@kubkon
Copy link
Member Author

kubkon commented Jul 3, 2024

One example I know is that LLVM TSAN dylib is malformed. But it is arguably an invalid assumption by LLVM. Having said that if all linkers in the wild assume a nonzero padding for images, who knows what else can break in the wild, and worse some tooling by Apple.

@kubkon kubkon disabled auto-merge July 3, 2024 13:57
@nektro
Copy link
Contributor

nektro commented Jul 3, 2024

did you mean to disable auto-merge?

@kubkon
Copy link
Member Author

kubkon commented Jul 3, 2024

did you mean to disable auto-merge?

I did. Jacob suggested checking if we can further fault LLVM's TSAN which would result in a very clear bug report upstream.

@kubkon
Copy link
Member Author

kubkon commented Jul 3, 2024

@nektro upstream issue: llvm/llvm-project#97627 It was indeed possible to fault TSAN with LLVM's linker.

@kubkon
Copy link
Member Author

kubkon commented Jul 3, 2024

In the light of the above, I am closing this PR as it is worse than the hackiest hack to make another hack work.

@kubkon kubkon closed this Jul 3, 2024
@kubkon kubkon deleted the macos-tsan branch July 3, 2024 19:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants