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

start: Avoid going through the argc_argv_ptr global variable #20718

Merged
merged 3 commits into from
Jul 22, 2024

Conversation

alexrp
Copy link
Member

@alexrp alexrp commented Jul 21, 2024

This is problematic for PIE. There's nothing but luck preventing the accesses to this global variable from requiring relocations. I've observed this being an issue on MIPS and PowerPC personally, but others may be affected.

This is problematic for PIE. There's nothing but luck preventing the accesses to
this global variable from requiring relocations. I've observed this being an
issue on MIPS and PowerPC personally, but others may be affected.

Besides, we're really just passing the initial stack pointer value to
posixCallMainAndExit(), so... just do that.
@alexrp
Copy link
Member Author

alexrp commented Jul 21, 2024

Regresses PPC64, marking as draft temporarily.

@alexrp alexrp marked this pull request as draft July 21, 2024 22:11
The previous version of this function referenced the argc_argv_ptr global
variable as an inline asm operand. This caused LLVM to generate prologue code to
initialize the ToC so that the global variable can actually be accessed.

Ordinarily, there's nothing wrong with that. But _start() is a naked function!
This makes it actually super surprising that LLVM did this. It also means that
the old version only really worked by accident.

Once the reference to the global variable was removed, no ToC was set up, thus
violating the calling convention once we got to posixCallMainAndExit(). This
then caused any attempt to access global variables here to crash - namely when
setting std.os.linux.elf_aux_maybe.

The fix is to just initialize the ToC manually in _start().
@alexrp
Copy link
Member Author

alexrp commented Jul 21, 2024

Ok, PPC64 is working again. Turns out the old code only worked by accident and my changes exposed that fact.

@alexrp alexrp marked this pull request as ready for review July 21, 2024 23:24
@andrewrk andrewrk enabled auto-merge July 22, 2024 00:42
@andrewrk
Copy link
Member

Nice work.

I remember why I originally made it write to a global variable. It used to be that an optimized "hello world" or other minimal program would be equivalent to the hand-crafted assembly. Now that can no longer be the case because there will always be a call to posixCallMainAndExit due to the inline asm.

It's not a really important use case, but was nice and now that beauty is lost. Not with your patch, which is a strict improvement, but with the patch that added the function call to the inline asm.

@andrewrk andrewrk merged commit b149d8f into ziglang:master Jul 22, 2024
10 checks passed
@alexrp alexrp deleted the start-gv branch July 22, 2024 09:28
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.

2 participants