Pin every calling-convention crossing and gate new ones - #128
Merged
Merged
Conversation
c68k_p256.S reads r, a and b at 8..16(sp). In a 68020 or 68040 tree
(AMINETXDUO_CPU=68020/68040, C68K_ASM without C68K_MV) c68k_p256.c reaches
the assembly by name through c68k_p256.h, which had no pin, so under
-mregparm=3 every P-256 field add, subtract and reduction handed the
operands in a0/a1/a2 and the routine read the stack. The `any` build was
not affected: it calls the _mv0/_mv20 twins through pinned vectors.
Measured, build/c020 (CPU=68020, -fno-lto -S), c68k_p256_fe_add:
before: move.l a0,d2 / jsr _c68k_p256_add_raw
after: move.l a2,-(sp) / move.l a1,-(sp) / move.l a0,-(sp) /
jsr _c68k_p256_add_raw / lea (12,sp),sp
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
cpucal.S and c68k_bulk_kernels.S read reps (and the table) at 4(sp) and
up. cpucal.c declared the kernels with no pin and c68k_bulk_bench.c
declared them and the two pointer types it times them through without
one, so under -mregparm=3 every kernel got reps in d0 and looped for
whatever the stack held. Both programs build in every tree whose CPU is
not 68000 or `any', the cpu68060 CI arm among them.
Measured, build/c020 (CPU=68020, -fno-lto -S):
cpucal.c before: move.l d2,d0 / jsr _cal_mulu
after: move.l d2,-(sp) / jsr _cal_mulu
b_time_reg() before: move.l #40000,d0 / jsr (a2)
after: move.l #40000,-(sp) / jsr (a2)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
sumcheck.c, copycheck.c and sumbench.c are built by hand, and each
declared stack-reading assembly (n68k_copy_sum_longwords, n68k_copy_bytes,
the sumvar*.S variants) with no pin, so a build with the tree's -mregparm=3
passed the arguments in a0/a1/d0. sumbench also times the variants and
its C reference through an unpinned function pointer; the pointer and the
reference are pinned with them. __stdargs, the compiler's own spelling,
because these files include nothing from include/.
Measured, m68k-amigaos-gcc -m68020 -mregparm=3 -Os -S:
sumcheck before: move.l a3,a1 / move.l a4,a0 / jsr _n68k_copy_sum_longwords
after: move.l a3,-(sp) / move.l a4,-(sp) / jsr ... / lea (12,sp),sp
copycheck before: move.l a3,a0 / jsr _n68k_copy_bytes
after: move.l a3,-(sp) / jsr _n68k_copy_bytes / lea (12,sp),sp
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
anxwifipi.device is built with the tree's -mregparm=3.
wifipi e88de3b UnitTask(), PacketReceiver() and PacketPoller() are
AddTask() entries whose arguments are pushed onto the
new task's stack; under -mregparm they read unit/sdio
from a0 instead. Every anxwifipi.device since b6d86db
starts its unit and receiver tasks on garbage pointers.
wifipi 3c56ee9 the fork's own copy of aminetxduo/anxs2ext.h kept the
unpinned RxDirect/RxFilled typedefs #126 fixed here; not
reached today only because that copy accepts extension
version 2 and the library offers 3.
netxduo d765298 _nx_crypto_memset_ptr/_nx_crypto_memcpy_ptr hold libc's
__stdargs memset/memcpy behind an unpinned type; nothing
calls through them yet.
These are local branches in the submodules (fix/regparm-stack-entries,
fix/regparm-crypto-mem-ptrs). tools/check-gitlinks.sh refuses both pins
until those branches are pushed and merged into maint/beta4 and master.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ings -mregparm=3 (b6d86db) passes our own arguments in registers, and GCC accepts a stack-pinned function pointer for an unpinned one, and the reverse, without a diagnostic. Six crossings shipped and were found one at a time (#123, 482f7b1, nx_crypto_mem.h, #124, #125, #126). This reads the source -- ours and the fork code the build compiles with our flags -- and fails on any call across a boundary that does not say its convention: asm-decl C declaration of an assembly routine with no visible pin asm-call C function called from assembly that reads registers rt-helper __udivsi3 and friends, memcpy-family definitions slot function stored in, or passed for, a pointer of the other convention (vectors, typedefs, parameters) escape function with arguments cast to APTR/ULONG/HOOKFUNC or handed to AddTask/SetFunction/... with no pin or registers shared-type function-pointer type in include/aminetxduo, developer/ include or a fork's copy of one, with no pin libc-decl our declaration of a libc/amiga.lib routine, unpinned varargs `...' in one declaration and not another Every #if branch is read, so a configuration nobody builds is covered. A pin counts where GCC would merge it: the same file or one it includes. tools/check-call-abi-allow.txt holds the crossings that are right unpinned, keyed by rule, file and name, each with its reason; a stale entry fails. --selftest holds the six shipped instances and one case per remaining rule in miniature, broken and fixed, and runs before every scan. Wired into tools/ci.sh stage_host after the LVO-clobber check, asserted by tools/check-gates-wired.sh, shellchecked in ci.yml. ~3 s. Proof: this tree call_abi=ok findings=0 d6026ab as it stands call_abi=fail findings=128 (c68k_p256.h, cpucal, crypto68k_bulk, bench, the wifipi task entries and anxs2ext.h copy, the nx_crypto.h pointers) each of the six pins reverted rc=1, the finding on the reverted line Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
A checkout without submodules has empty third_party/ directories, and the scan skipped them: findings=0 for a fork nobody had read (611 files instead of 641, found by deepseek-flash). Every SCAN root holds sources in a full checkout, so an empty one now fails the gate. wifipi/src moved away: exit 1, reason=empty_scan_root; restored: exit 0. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The test for a function definition head used \)\s*(?:__attribute__\s*\(\(.*\)\)\s*)*$, a repeat around a greedy .* that backtracks exponentially on a line of repeated attributes (code scanning on #128). strip_trailing_attributes() removes trailing __attribute__((...)) groups by matching parentheses from the end, then the head is a definition if what is left ends in ')'. Same scan result (files=1830 functions=8522 findings=0, self-test 11/11); 2000 repeated attributes take under a millisecond. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
t is already rstripped, so the attribute regex could only match a string
ending in ')' and was exactly t.endswith(')') (deepseek-flash: 200k
fuzzed strings, no counterexample). The scanning helper from 1c20ae4
was not equivalent: it stripped a trailing attribute from a struct head
and changed the answer there. The plain test keeps the old behaviour
with no regex.
Same scan result: files=1830 functions=8522 findings=0, self-test 11/11.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Audit of every calling-convention crossing after
-mregparm=3(#121), the class behind #123, #124, #125 and #126, plus a gate so the next one fails the build instead of reaching hardware.Fixed (each commit carries before/after
-fno-lto -Sdisassembly with the tree's flags):StartUnitTask()/the receiver push their arguments on the new task's stack andAddTask()it;UnitTask,PacketReceiver,PacketPollerread a0/a1. Pinned withSTACKARGS(WiFiPimaint/beta4e88de3b).anxs2ext.h: unpinned receive callbacks, latent only because its extension version check skips the path (3c56ee9).c68k_p256.hprototypes vsc68k_p256.S(live in CPU=68020/68040 trees)._nx_crypto_memset_ptr/memcpy_ptr(latent; netxduomasterd7652988).Gate:
tools/check-call-abi.sh(+check-call-abi.py, allow-list with reasons) scans the source the build compiles, forks included, for asm externs, runtime helpers, function pointers of the other convention, function addresses handed to AddTask/SetFunction/HOOKFUNC/APTR, shared-header pointer typedefs, libc redeclarations and varargs mismatches, across every#ifbranch. Wired intotools/ci.sh host. Proof: reverting each of the six known pins, one at a time, fails it on that line; d6026ab reports 136 findings without the allow-list (116 with this branch's list, measured by deepseek-flash); this branch 0 (files=1830 functions=8522 findings=0).Not proven: the wifipi entry fault is shown by disassembly, not on hardware.
🤖 Generated with Claude Code