Skip to content

Give the ppc64le resolver a real ABI frame header - #60

Merged
Samueru-sama merged 1 commit into
pkgforge-dev:mainfrom
talaria0101:fix-ppc64-resolver-frame
Oct 22, 2026
Merged

Samueru-sama merged 1 commit into
pkgforge-dev:mainfrom
talaria0101:fix-ppc64-resolver-frame

Conversation

@talaria0101

@talaria0101 talaria0101 commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

glfwd_resolve_asm saved the argument registers at 0..64(sp), which is the
back chain, CR, LR and TOC save area plus the parameter save area. A C
callee writes its caller's LR slot at 16(sp) and the TOC slot at 24(sp),
so glfwd_resolve_one, which is not a leaf, overwrote the saved r5 and r6
on the first call through every slot. The first call of any GL entry point
taking a third or fourth integer argument was handed garbage. Reported
in issue 59 on the gtk3-demo ppc64le AppImage: the shipped gl-fwd.so exits
139 when preloaded, one built from this change stays alive. Not rerun here.

Registers are now saved above the 96-byte callee area (32-byte ELFv2
header plus a 64-byte parameter area). The frame is 288 bytes.

make gl-fwd-qemu-check gains a first-call test with eight live integer
arguments; it fails on the old frame (ppc64le) and passes on the new one.

The ELFv1 big-endian ppc64 trampoline block is removed. ppc64 is not
built (see docs/limits.md), nothing ran that block, and a ppc64 ELFv1
compile now stops at the existing "needs a tail-jump trampoline" error.


Requested by Samueru via errand.
Conversation: https://discord.com/channels/1313385177703256064/1562751109771300866
Transcript: https://talaria.qaidvoid.dev/?session=q19v2w4cx3o4

@Nemo-010 Nemo-010 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I reproduced the bug on main and verified this branch fixes it, including the regression test. Everything below is from running the code, not reading it.

The bug is real and the diagnosis is exact

glfwd_resolve_one's generated prologue on ppc64le:

	mflr 0
	std 31, -8(1)
	stdu 1, -128(1)
	std 0, 144(1)      # <- 16(r1) of the CALLER = the old r5 save slot

So 144 - 128 = 16 lands on the caller's frame at offset 16, which is where the old std 5, 16(1) put r5. Confirmed against the actual compiler output, not just the ABI text.

Reproduced with an 8-long target under qemu-ppc64le 7.2 and a Debian ppc64el glibc 2.41 sysroot:

gl-fwd.c result
main (3095c15) first=3 second=0 → FIRST CALL WRONG
this branch first=0 second=0 → all passed

The regression test does what it claims

I extracted the tgt.c / tgt-fwd.h / m.c this Makefile generates and ran them exactly as make gl-fwd-qemu-check would:

  • against this branch: all passed, exit 0
  • against main's gl-fwd.c with the same generated sources: exit 5, i.e. t_many returned nonzero on its first call

So the new case is a genuine regression test and not a test that happens to pass both ways. Putting t_many first in main() matters, and it is first.

Frame layout

I checked the arithmetic rather than trusting it:

  • header 0..112, first saved register at 112 → no overlap with the callee's back chain / CR / LR / TOC slots
  • highest used byte is 296 (LR at 288..296), frame is 304 → 8 bytes of slack, no overlaps anywhere
  • 304 is 16-byte aligned, which the bl requires

ELFv1 (BE ppc64) hunk

The body says this is edited the same way and not exercised, which is honest. I checked what I could:

  • it compiles for powerpc64-linux-gnu (zig)
  • the translation is faithful: stdu 1,-304(1), regs from 112, LR at 288, addi 1,1,304

I could not run it. Debian has no ppc64 (BE) port any more, and ArchPOWER's ELFv2 glibc does not define the GLIBC_2.3/GLIBC_2.4 symbols zig's ELFv1 stubs want — the same ELFv1-vs-ELFv2 split as issue #11. So "a one-off run under qemu-ppc64 passed" is not something I can corroborate; I am not disputing it either, just flagging that it is unverified from here.

One observation on that hunk while reading it: ELFv1 never saved r11 in the first place (main has no std 11 in that body, and its ld 11, 16(3) is the descriptor load, not a restore). This branch keeps that behaviour unchanged, so the ELFv1 and ELFv2 paths now differ in whether they preserve r11. That is pre-existing rather than introduced here, and it may well be correct for ELFv1 where r11 comes from the descriptor — but the two bodies now look more similar than they behave, which is worth a comment if it is deliberate.

Minor

docs/building.md and docs/report/10-measured-versus-assumed.md both describe the new case accurately. No other files touched.

Not covered

Still nothing in CI runs gl-fwd-qemu-check — gates.yml builds and runs verify-artifacts.sh, which never executes a trampoline. This PR adds the case that would have caught the bug, but the target it lives in is still not wired into the matrix. That is a separate change and I would rather see it land than block this one, since the fix itself is correct and the AppImage consumers are currently broken on ppc64le.

@Nemo-010

Copy link
Copy Markdown
Contributor

Correction to my review, and it removes the one caveat I raised.

I flagged the ELFv1 (BE ppc64) hunk as "unverified" and noted that ELFv1 and ELFv2 now differ in whether they preserve r11. That was wrong of me: ppc64 BE is not a supported target, dropped in #36 because its artefacts cannot load where they ship — Debian's powerpc64-linux-gnu toolchain targets ELFv1 (_CALL_ELF 1, ld64.so.1) while the AppImage runtimes are ELFv2 throughout, so the loader refuses the object. A process is one ABI from its loader up, so the gap is not closable here.

So:

  • scripts/build.sh rejects --arch ppc64; the valid set is x86_64|aarch64|riscv64|ppc64le|loongarch64|both.
  • v0.2.7 ships no ppc64 asset — only ppc64le.
  • The #elif defined(__powerpc64__) ELFv1 block in gl-fwd.c is therefore unreachable for every shipped architecture.

Which means the r11 observation in my review is about dead code, and my inability to run the ELFv1 path is not a gap in this PR's verification — there is nothing on that path to verify against a release.

Two consequences for the PR itself:

  1. The hunk that edits the ELFv1 body is not needed for this fix. It is harmless and keeps the two bodies in step, which is a reasonable reason to keep it, but it is not part of what fixes ppc64le, and it cannot be tested by any target this repo ships or builds. Worth saying so in the commit message rather than leaving "edited the same way" to carry that weight.
  2. docs/building.md and the report section both say the qemu check "runs the ELFv1 path" for ppc64 — the same claim I repeated. Worth a look, since if ppc64 is not built, that sentence describes something that does not run. I may be reading it too literally; GLFWD_CROSS does not include ppc64, so the target itself is fine — it is the prose I would check.

Nothing else in my review changes. The ppc64le diagnosis, the frame arithmetic, the fix working on main vs this branch, and the regression test failing on old code (exit 5) and passing on new (exit 0) all stand, and those are the parts that matter for merging.

@Nemo-010

Copy link
Copy Markdown
Contributor

Strike point 2 from my previous comment — I checked and the docs are right, not misleading.

docs/building.md says "On ppc64le it runs the ELFv2 global-entry TOC rebuild", which is accurate, and GLFWD_CROSS is aarch64 riscv64 ppc64le loongarch64 with no ppc64 in it. I read the sentence as claiming an ELFv1 run; it does not. Nothing to fix there.

So the only real content of the correction is the first half: ppc64 BE is not a supported target, so the ELFv1 hunk in this PR is dead code for every architecture that ships, my "unverified" caveat about it was not a gap, and the r11 observation I raised was about code no release builds.

@talaria0101

Copy link
Copy Markdown
Contributor Author

Thanks for the review. One correction to the commit message, which I cannot amend without a push.

The line "A one-off run under qemu-ppc64 passed with the change and failed without it" is broader than what was run:

  • powerpc64-linux-gnu-gcc here defaults to _CALL_ELF 2, so a full make gl-fwd-qemu-check style run for ppc64 under qemu-ppc64-static exercised the big-endian ELFv2 branch, not the ELFv1 hunk.
  • The ELFv1 body itself was only run in a cut-down harness: the .opd and resolver asm extracted, built with -mabi=elfv1 -nostdlib, linked statically, with a non-leaf C resolver returning a descriptor for an 8-argument target. Old frame: exit 4 (r5 clobbered). New frame: PASS. No ELFv1 libc or loader was involved.

Agreed that the ELFv1 hunk is not needed for the ppc64le fix and no shipped target builds it. It only keeps the two bodies in step. If you prefer, drop it.

glfwd_resolve_asm saved the argument registers at 0..64(sp), which is the
back chain, CR, LR and TOC save area plus the parameter save area. A C
callee writes its caller's LR slot at 16(sp) and the TOC slot at 24(sp),
so glfwd_resolve_one, which is not a leaf, overwrote the saved r5 and r6
on the first call through every slot. The first call of any GL entry point
taking a third or fourth integer argument was handed garbage. Reported
in issue 59 on the gtk3-demo ppc64le AppImage: the shipped gl-fwd.so exits
139 when preloaded, one built from this change stays alive. Not rerun here.

Registers are now saved above the 96-byte callee area (32-byte ELFv2
header plus a 64-byte parameter area). The frame is 288 bytes.

make gl-fwd-qemu-check gains a first-call test with eight live integer
arguments; it fails on the old frame (ppc64le) and passes on the new one.

The ELFv1 big-endian ppc64 trampoline block is removed. ppc64 is not
built (see docs/limits.md), nothing ran that block, and a ppc64 ELFv1
compile now stops at the existing "needs a tail-jump trampoline" error.
@talaria0101
talaria0101 force-pushed the fix-ppc64-resolver-frame branch from a5dd48f to b42238b Compare October 22, 2026 10:16
@Nemo-010

Copy link
Copy Markdown
Contributor

Re-verified against the new head b42238b, since the frame changed from 304/112 to 288/96 and the ELFv1 block is now gone. Both changes hold.

New frame arithmetic. Header 0..96, first saved register at 96, highest used byte 280 (std 0 LR at 272..280), frame 288 → 8 bytes slack, no overlaps, 16-byte aligned for the bl. Same shape as before, just a smaller header.

Fix still works. 8-long target, qemu-ppc64le 7.2, Debian ppc64el glibc 2.41:

gl-fwd.c result
main (3095c15) segfault, exit 139
this head first=0 second=0 → all passed

The PR's own regression test still discriminates. I extracted the tgt.c/tgt-fwd.h/m.c the Makefile generates and ran them as make gl-fwd-qemu-check would: this head all passed (exit 0), main's gl-fwd.c with the same sources exit 5. So t_many still catches it.

The ELFv1 removal does what the message says. With a genuine ELFv1 compile (-mabi=elfv1) the build stops at:

#error "gl-fwd needs a tail-jump trampoline for this architecture"

which is the right outcome — an unsupported ABI now fails loudly at build time instead of shipping a body nothing runs.

One note on your correction to the commit message: it is accurate and more precise than the original line, and I appreciate you pinning down that powerpc64-linux-gnu-gcc defaults to _CALL_ELF 2 so the full check was exercising the big-endian ELFv2 branch rather than the ELFv1 hunk. I hit the same thing from the other direction with zig: -dM -E on an empty input reports _CALL_ELF 1 for powerpc64-linux-gnu, but compiling an actual file with that target defines _CALL_ELF 2 and emits an abiv2 object. That is exactly the kind of mismatch that makes the "which ABI did I just test" question hard to answer by inspection, and it is a good argument for the ELFv1 block being gone rather than sitting there untested.

No objection to dropping the ELFv1 hunk, and no further comments from me. From the ppc64le side this is correct and I would merge it.

@talaria0101 talaria0101 changed the title Give the ppc64 resolver a real ABI frame header Give the ppc64le resolver a real ABI frame header Oct 22, 2026

@Nemo-010 Nemo-010 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Approving. Verified on this head (b42238b), not just read:

  • main segfaults on the 8-long target under qemu-ppc64le; this head gives first=0 second=0
  • the new t_many case passes here (exit 0) and fails on main (exit 5), so it is a real regression test
  • frame arithmetic: header 0..96, highest used byte 280, frame 288, no overlaps, 16-byte aligned
  • a genuine -mabi=elfv1 compile now stops at the #error, which is the right outcome for an unsupported ABI

CI is green on all five architectures. Thanks for turning the ELFv1 block into a loud build failure rather than leaving an untested body in place — that is the difference between a shim that works and one that only looks like it does everywhere.

@Samueru-sama
Samueru-sama merged commit 900a393 into pkgforge-dev:main Oct 22, 2026
22 checks passed
Samueru-sama added a commit that referenced this pull request Oct 23, 2026
Both are in PROGRESS.md, and both were caught by asking what had been
asserted without being run.

900a393 is a squashed commit with one parent, not the PR #60 merge commit.
The property that matters is unchanged and is now stated as the property:
the tree CI tested (62985da) is the tree the tag carries, so the commits
differ and the trees do not.

The gtk3-demo ppc64le reproduction came from issue #59 and the PR body,
not from this session. What was measured here is that
`make gl-fwd-qemu-check` fails on the pre-fix gl-fwd.c and passes on the
shipped one, and that the published ppc64le-gl-fwd.so carries the
288-byte frame. Nothing here ran a GTK3 application, so the entry now
says so instead of repeating the report as though it were a result.
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