diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 79d5c3535..a8bd07477 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -354,6 +354,7 @@ jobs: tools/aminet-batch.sh \ tools/check-generated.sh \ tools/check-lvo-matrix.sh \ + tools/check-lvo-clobbers.sh \ tools/check-option-stubs.sh \ tools/check-gitlinks.sh \ tools/check-test-duration.sh \ diff --git a/tests/atf/atf_main.c b/tests/atf/atf_main.c index 9a6b5e83f..a563bf0a9 100644 --- a/tests/atf/atf_main.c +++ b/tests/atf/atf_main.c @@ -60,9 +60,11 @@ static VOID atfc_set_errno_ptr(APTR ptr, LONG size) register LONG d0 __asm("d0") = size; register LONG _d1 __asm("d1"); register LONG _a1 __asm("a1"); + register LONG _a0 __asm("a0"); + register LONG _d0 __asm("d0"); __asm __volatile ("jsr a6@(-168:W)" - : "=r" (_d1), "=r" (_a1) + : "=r" (_d1), "=r" (_a1), "=r" (_a0), "=r" (_d0) : "r" (a6), "r" (a0), "r" (d0) : "cc", "memory"); } diff --git a/tests/compare/tickprobe.c b/tests/compare/tickprobe.c index accd62341..2b62e0cdb 100644 --- a/tests/compare/tickprobe.c +++ b/tests/compare/tickprobe.c @@ -119,7 +119,7 @@ static LONG s_close(LONG s) __asm __volatile ("jsr a6@(-120:W)" : "=r"(r) : "r"(a6), "r"(d0) - : "a0", "a1", "cc", "memory"); + : "d1", "a0", "a1", "cc", "memory"); return r; } diff --git a/tests/endurance/endurance.c b/tests/endurance/endurance.c index 98af29562..03a832b02 100644 --- a/tests/endurance/endurance.c +++ b/tests/endurance/endurance.c @@ -118,7 +118,7 @@ static LONG e_accept(struct Library *base, LONG s) __asm __volatile ("jsr a6@(-48:W)" : "=r" (res), "=r" (_clob_a0), "=r" (_clob_a1) : "r" (a6), "r" (d0), "r" (a0), "r" (a1) - : "cc", "memory"); + : "d1", "cc", "memory"); return res; } diff --git a/tests/ipv6/ipv6_socket_test.c b/tests/ipv6/ipv6_socket_test.c index 228d28717..32fa7779b 100644 --- a/tests/ipv6/ipv6_socket_test.c +++ b/tests/ipv6/ipv6_socket_test.c @@ -667,7 +667,7 @@ BSD_SCRATCH; __asm __volatile ("jsr a6@(-804:W)" : BSD_SCRATCH_OUT : "r" (a6), "r" (a0) - : "cc", "memory"); + : "d0", "cc", "memory"); } /* gai_strerror takes its argument in a0, not d0, pragmas line 141. */ diff --git a/tests/leak/refused_leak_test.c b/tests/leak/refused_leak_test.c index 250e8007e..b44495f5e 100644 --- a/tests/leak/refused_leak_test.c +++ b/tests/leak/refused_leak_test.c @@ -98,7 +98,7 @@ static LONG l_accept(struct Library *base, LONG s) __asm __volatile ("jsr a6@(-48:W)" : "=r" (res), "=r" (_clob_a0), "=r" (_clob_a1) : "r" (a6), "r" (d0), "r" (a0), "r" (a1) - : "cc", "memory"); + : "d1", "cc", "memory"); return res; } diff --git a/tests/tcpdrill/tcpdrill.c b/tests/tcpdrill/tcpdrill.c index 88eec972d..b2bbaf2f4 100644 --- a/tests/tcpdrill/tcpdrill.c +++ b/tests/tcpdrill/tcpdrill.c @@ -155,7 +155,7 @@ static LONG s_accept(LONG s) __asm __volatile ("jsr a6@(-48:W)" : "=r"(r), "=r" (_clob_a0), "=r" (_clob_a1) : "r"(a6), "r"(d0), "r"(a0), "r"(a1) - : "cc", "memory"); + : "d1", "cc", "memory"); return r; } @@ -265,7 +265,7 @@ static LONG s_close(LONG s) __asm __volatile ("jsr a6@(-120:W)" : "=r"(r) : "r"(a6), "r"(d0) - : "a0", "a1", "cc", "memory"); + : "d1", "a0", "a1", "cc", "memory"); return r; } diff --git a/tests/tls/tls_loop.c b/tests/tls/tls_loop.c index 282928ee3..9bb0a6ec3 100644 --- a/tests/tls/tls_loop.c +++ b/tests/tls/tls_loop.c @@ -266,9 +266,11 @@ static LONG bsd_wait_select(struct Library *base, LONG nfds, APTR readfds, register LONG res __asm("d0"); register LONG _clob_a0 __asm("a0"); register LONG _clob_a1 __asm("a1"); + register LONG _clob_d1 __asm("d1"); __asm __volatile ("jsr a6@(-126:W)" - : "=r" (res), "=r" (_clob_a0), "=r" (_clob_a1) + : "=r" (res), "=r" (_clob_a0), "=r" (_clob_a1), + "=r" (_clob_d1) : "r" (a6), "r" (d0), "r" (a0), "r" (a1), "r" (a2), "r" (a3), "r" (d1) : "cc", "memory"); diff --git a/tests/tools/cardgrab.c b/tests/tools/cardgrab.c index aee419b3c..1860bdd15 100644 --- a/tests/tools/cardgrab.c +++ b/tests/tools/cardgrab.c @@ -64,8 +64,8 @@ static struct CardHandle *cg_own_card(struct CardHandle *h) register struct CardHandle *res __asm("d0"); __asm __volatile ("jsr a6@(-0x6)" - : "=r" (res) - : "r" (_a6), "r" (_a1) + : "=r" (res), "+r" (_a1) + : "r" (_a6) : "d1", "a0", "cc", "memory"); return res; @@ -78,8 +78,8 @@ static VOID cg_release_card(struct CardHandle *h, ULONG flags) register ULONG _d0 __asm("d0") = flags; __asm __volatile ("jsr a6@(-0xc)" - : "+r" (_d0) - : "r" (_a6), "r" (_a1) + : "+r" (_d0), "+r" (_a1) + : "r" (_a6) : "d1", "a0", "cc", "memory"); } diff --git a/tests/tools/ifprobe.c b/tests/tools/ifprobe.c index d0120a46d..c4cfd8ccb 100644 --- a/tests/tools/ifprobe.c +++ b/tests/tools/ifprobe.c @@ -825,9 +825,10 @@ static ULONG p_if_nametoindex(struct Library *base, const char *name) register const char *a0 __asm("a0") = name; register ULONG res __asm("d0"); register LONG _clob_d1 __asm("d1"); + register LONG _clob_a0 __asm("a0"); __asm __volatile ("jsr a6@(-882:W)" /* if_nametoindex -0x372 */ - : "=r" (res), "=r" (_clob_d1) + : "=r" (res), "=r" (_clob_d1), "=r" (_clob_a0) : "r" (a6), "r" (a0) : "d2", "d3", "a1", "cc", "memory"); return res; @@ -840,9 +841,10 @@ static char *p_if_indextoname(struct Library *base, ULONG index, char *out) register char *a0 __asm("a0") = out; register char *res __asm("d0"); register LONG _clob_d1 __asm("d1"); + register LONG _clob_a0 __asm("a0"); __asm __volatile ("jsr a6@(-888:W)" /* if_indextoname -0x378 */ - : "=r" (res), "=r" (_clob_d1) + : "=r" (res), "=r" (_clob_d1), "=r" (_clob_a0) : "r" (a6), "r" (d0), "r" (a0) : "d2", "d3", "a1", "cc", "memory"); return res; diff --git a/tests/udpdrill/udpdrill.c b/tests/udpdrill/udpdrill.c index 7b792f7d4..8b5dc55f9 100644 --- a/tests/udpdrill/udpdrill.c +++ b/tests/udpdrill/udpdrill.c @@ -191,9 +191,10 @@ register APTR a2 __asm("a2") = fromlen; register LONG res __asm("d0"); register LONG _s_d1 __asm("d1"); register LONG _s_a0 __asm("a0"); +register LONG _s_a1 __asm("a1"); __asm __volatile ("jsr a6@(-72:W)" - : "=r" (_s_d1), "=r" (_s_a0), "=r" (res) + : "=r" (_s_d1), "=r" (_s_a0), "=r" (_s_a1), "=r" (res) : "r" (a6), "r" (d0), "r" (a0), "r" (d1), "r" (d2), "r" (a1), "r" (a2) : "cc", "memory"); @@ -273,9 +274,10 @@ register APTR a1 __asm("a1") = len; register LONG res __asm("d0"); register LONG _s_d1 __asm("d1"); register LONG _s_a0 __asm("a0"); +register LONG _s_a1 __asm("a1"); __asm __volatile ("jsr a6@(-96:W)" - : "=r" (_s_d1), "=r" (_s_a0), "=r" (res) + : "=r" (_s_d1), "=r" (_s_a0), "=r" (_s_a1), "=r" (res) : "r" (a6), "r" (d0), "r" (d1), "r" (d2), "r" (a0), "r" (a1) : "cc", "memory"); diff --git a/tools/check-gates-wired.sh b/tools/check-gates-wired.sh index 58c1a62ed..4893357b1 100755 --- a/tools/check-gates-wired.sh +++ b/tools/check-gates-wired.sh @@ -74,6 +74,7 @@ want stage-coverage tools/ci.sh 'check-stage-coverage\.sh' want rx-posted tools/ci.sh 'check-rx-posted\.sh' want option-stubs tools/ci.sh 'check-option-stubs\.sh' want lvo-matrix tools/ci.sh 'check-lvo-matrix\.sh' +want lvo-clobbers tools/ci.sh 'check-lvo-clobbers\.sh' want generated .githooks/pre-commit 'check-generated\.sh' # Gate 5: the push itself. .githooks/pre-push refuses a tree the host stage @@ -198,7 +199,7 @@ done # ------------------------------------------ and the gate scripts still run --- for g in check-changelog-prose check-image-size check-ram-size check-rate \ check-stage-coverage check-rx-posted check-option-stubs \ - check-lvo-matrix check-generated \ + check-lvo-matrix check-lvo-clobbers check-generated \ check-hot-calls check-rearm-invariants check-hotpath-budget \ check-gates-wired; do if [ ! -x "tools/$g.sh" ]; then diff --git a/tools/check-lvo-clobbers.py b/tools/check-lvo-clobbers.py new file mode 100755 index 000000000..d2827f5bc --- /dev/null +++ b/tools/check-lvo-clobbers.py @@ -0,0 +1,241 @@ +#!/usr/bin/env python3 +# +# Every inline-asm library call names all four scratch registers. +# +# tools/check-lvo-clobbers.py [--root DIR] [FILE...] +# +# The AmigaOS library ABI lets any LVO destroy d0, d1, a0 and a1. An extended +# asm statement that calls one (`jsr a6@(-N)`) has to list each of the four as +# an output or a clobber; one listed only as an input is a register GCC +# believes still holds its value after the jsr. That was #68: bsd_recvfrom +# left a1 off, the next inlined call passed a stale `from`, and it answered +# EFAULT. The rest of the class is #70. +# +# Declarations are read through object-like macros (BSD_SCRATCH, +# NETDEV_REG_A1) from the file and the headers it includes by "name". +# Top-level asm has no operand lists and is not an inline call; it is skipped. +# +# Output: one `lvo_clobbers=fail` line per site, then a summary line. +# Exit 0 when nothing is flagged, 1 when something is, 2 on a usage error. +# +# SPDX-License-Identifier: MIT + +import os +import re +import sys + +SCRATCH = ("a0", "a1", "d0", "d1") + +LVO_RE = re.compile(r'\bjsr\s+(?:%*a6@|-?\w+\(%*a6\))') +ASM_RE = re.compile(r'\b(?:__asm__|__asm|asm)\s*(?:(?:__volatile__|__volatile|volatile)\s*)?\(') +DECL_RE = re.compile(r'\bregister\b[^;{}()]*?\b([A-Za-z_]\w*)\s*__asm(?:__)?\s*\(\s*"%*([ad][0-7])"\s*\)') +DEFINE_RE = re.compile(r'^[ \t]*#[ \t]*define[ \t]+([A-Za-z_]\w*)(?![\w(])[ \t]*(.*)$', re.M) +INCLUDE_RE = re.compile(r'^[ \t]*#[ \t]*include[ \t]+"([^"]+)"', re.M) +OPERAND_RE = re.compile(r'"([^"]*)"\s*\(\s*([A-Za-z_]\w*)\s*\)') +REG_RE = re.compile(r'"%*([ad][0-7])"') + + +def strip_comments(text): + """Blank comments; keep strings and every newline.""" + out = [] + i, n = 0, len(text) + while i < n: + c = text[i] + if c == '/' and i + 1 < n and text[i + 1] == '*': + j = text.find('*/', i + 2) + j = n if j < 0 else j + 2 + out.append(re.sub(r'[^\n]', ' ', text[i:j])) + i = j + elif c == '/' and i + 1 < n and text[i + 1] == '/': + j = text.find('\n', i) + j = n if j < 0 else j + out.append(' ' * (j - i)) + i = j + elif c in '"\'': + j = i + 1 + while j < n and text[j] != c and text[j] != '\n': + j += 2 if text[j] == '\\' else 1 + out.append(text[i:j + 1]) + i = j + 1 + else: + out.append(c) + i += 1 + return ''.join(out) + + +def balanced(text, i): + """text[i] is '('; return the index of its matching ')'.""" + depth = 0 + n = len(text) + while i < n: + c = text[i] + if c in '"\'': + j = i + 1 + while j < n and text[j] != c: + j += 2 if text[j] == '\\' else 1 + i = j + elif c == '(': + depth += 1 + elif c == ')': + depth -= 1 + if depth == 0: + return i + i += 1 + return -1 + + +def split_colons(body): + """Split an asm body at its top-level colons.""" + parts, depth, start, i, n = [], 0, 0, 0, len(body) + while i < n: + c = body[i] + if c in '"\'': + j = i + 1 + while j < n and body[j] != c: + j += 2 if body[j] == '\\' else 1 + i = j + elif c in '([': + depth += 1 + elif c in ')]': + depth -= 1 + elif c == ':' and depth == 0: + parts.append(body[start:i]) + start = i + 1 + i += 1 + parts.append(body[start:]) + return parts + + +class Scanner: + def __init__(self, root): + self.root = root + self.cache = {} + self.joined = {} + self.by_base = {} + for d in ("include", "src", "tests"): + for dp, _, fs in os.walk(os.path.join(root, d)): + for f in fs: + if f.endswith(".h"): + self.by_base.setdefault(f, []).append(os.path.join(dp, f)) + + def text(self, path): + if path not in self.cache: + with open(path, encoding="latin-1") as fh: + raw = strip_comments(fh.read()) + # A continuation keeps its line for scanning, and is joined for + # reading a #define. + self.cache[path] = raw.replace("\\\n", " \n") + self.joined[path] = raw.replace("\\\n", " ") + return self.cache[path] + + def resolve(self, name, here): + for cand in (os.path.join(os.path.dirname(here), name), + os.path.join(self.root, "include", name), + os.path.join(self.root, "src", name)): + if os.path.isfile(cand): + return os.path.normpath(cand) + hits = self.by_base.get(os.path.basename(name), []) + return hits[0] if len(hits) == 1 else None + + def macros(self, path, seen=None): + seen = set() if seen is None else seen + if path in seen: + return {} + seen.add(path) + self.text(path) + text = self.joined[path] + out = {} + for inc in INCLUDE_RE.findall(text): + p = self.resolve(inc, path) + if p: + out.update(self.macros(p, seen)) + for m in DEFINE_RE.finditer(text): + out[m.group(1)] = m.group(2).strip() + return out + + @staticmethod + def expand(s, macros): + if not macros: + return s + pat = re.compile(r'\b(' + '|'.join(map(re.escape, macros)) + r')\b') + for _ in range(6): + t = pat.sub(lambda m: macros[m.group(1)], s) + if t == s: + break + s = t + return s + + def scan(self, path): + text = self.text(path) + macros = None + rel = os.path.relpath(path, self.root) + stmts, flagged = 0, [] + for m in ASM_RE.finditer(text): + open_at = m.end() - 1 + close_at = balanced(text, open_at) + if close_at < 0: + continue + body = text[open_at + 1:close_at] + if macros is None: + macros = self.macros(path) + parts = split_colons(self.expand(body, macros)) + if len(parts) < 2 or not LVO_RE.search(parts[0]): + continue + stmts += 1 + # Declarations visible here: from the last column-0 brace. + start = text.rfind("\n{", 0, m.start()) + start = 0 if start < 0 else start + regs = {} + for d in DECL_RE.finditer(self.expand(text[start:m.start()], macros)): + regs[d.group(1)] = d.group(2) + covered = set() + for c, var in OPERAND_RE.findall(parts[1]): + if var in regs and ("=" in c or "+" in c): + covered.add(regs[var]) + if len(parts) > 3: + covered.update(REG_RE.findall(parts[3])) + missing = [r for r in SCRATCH if r not in covered] + if missing: + line = text.count("\n", 0, m.start()) + 1 + flagged.append((rel, line, missing)) + return stmts, flagged + + +def main(argv): + root = os.path.normpath(os.path.join(os.path.dirname(os.path.abspath(__file__)), "..")) + files = [] + args = list(argv) + while args: + a = args.pop(0) + if a == "--root" and args: + root = os.path.abspath(args.pop(0)) + elif a.startswith("-"): + print("usage: check-lvo-clobbers.py [--root DIR] [FILE...]", file=sys.stderr) + return 2 + else: + files.append(os.path.abspath(a)) + if not files: + for d in ("src", "tests"): + for dp, dns, fs in os.walk(os.path.join(root, d)): + dns[:] = sorted(x for x in dns if x != "third_party") + files += [os.path.join(dp, f) for f in fs if f.endswith((".c", ".h"))] + + sc = Scanner(root) + total, flagged, nfiles = 0, [], 0 + for f in sorted(files): + s, fl = sc.scan(f) + total += s + nfiles += 1 if s else 0 + flagged += fl + for rel, line, missing in flagged: + print("lvo_clobbers=fail file=%s line=%d missing=%s" % (rel, line, ",".join(missing))) + if total == 0: + print("lvo_clobbers=fail reason=no_statements_found") + return 1 + state = "fail" if flagged else "ok" + print("lvo_clobbers=%s statements=%d files=%d flagged=%d" % (state, total, nfiles, len(flagged))) + return 1 if flagged else 0 + + +if __name__ == "__main__": + sys.exit(main(sys.argv[1:])) diff --git a/tools/check-lvo-clobbers.sh b/tools/check-lvo-clobbers.sh new file mode 100755 index 000000000..082bca9f9 --- /dev/null +++ b/tools/check-lvo-clobbers.sh @@ -0,0 +1,18 @@ +#!/usr/bin/env bash +# +# Every inline-asm library call names all four scratch registers (#70). +# +# tools/check-lvo-clobbers.sh [FILE...] +# +# An LVO may destroy d0, d1, a0 and a1; an asm statement that lists one of +# them only as an input lets GCC reuse a value the call has overwritten. The +# scan is tools/check-lvo-clobbers.py; this is its CI entry point. +# +# Output: `lvo_clobbers=fail file=... line=... missing=...` per site, then +# `lvo_clobbers=ok|fail statements=N files=N flagged=N`. Exit 1 on any site. +# +# SPDX-License-Identifier: MIT + +set -eu +ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +exec python3 "$ROOT/tools/check-lvo-clobbers.py" --root "$ROOT" "$@" diff --git a/tools/ci.sh b/tools/ci.sh index 2a6fa75ee..acf568703 100755 --- a/tools/ci.sh +++ b/tools/ci.sh @@ -931,6 +931,20 @@ ${rlwhy:+ -- }${rlwhy:-, see the log above}" ;; return 1 fi + # An LVO may destroy d0, d1, a0 and a1, and an inline-asm call that lists + # one only as an input lets GCC reuse what the call overwrote: #68 passed + # a stale recvfrom address and got EFAULT. Every such call, src/ and + # tests/ alike, names all four as outputs or clobbers (#70). + if tools/check-lvo-clobbers.sh > "$BUILD/lvo-clobbers.log" 2>&1; then + note "lvo clobbers: $(sed -n 's/^lvo_clobbers=ok statements=\([0-9]*\).*/\1 asm calls/p' \ + "$BUILD/lvo-clobbers.log")" + else + cat "$BUILD/lvo-clobbers.log" + fail "an inline-asm LVO call leaves a scratch register off its\ + outputs and clobbers (tools/check-lvo-clobbers.sh)" + return 1 + fi + # An option's OFF side must define every function its ON side does. The # link only notices on the arm that turns the option off, which is the arm # nobody builds locally; raw.c cost three of them in one sitting.