fix(lint): calibrate register-clobber to the Go ABI and add legacy SSE moves

Assisted-by: Qwen 3.8 Max Preview
2026-07-11 17:36:52 +02:00
parent 458cfb626e
commit 1a01870695
14 changed files with 381 additions and 89 deletions
@@ -319,18 +319,27 @@ func lintText(t *ast.Text, tab *arch.Table, archKnown bool, cfg Config, macros m
 		}
 	}

-	// Register liveness: a callee-saved register that is written but never
-	// saved and restored is clobbered across the call.  The check runs over the
-	// control-flow graph and is skipped for macro-using files, where an opaque
-	// macro may perform the save/restore.
+	// Register liveness: a register the Go ABI fixes across calls that is
+	// written but never saved and restored is clobbered.  The check runs over
+	// the control-flow graph and is skipped for macro-using files, where an
+	// opaque macro may perform the save/restore.
 	if doLabelChecks && archKnown && !cfg.Disable[CodeRegisterClobber] {
 		live := analyzeLiveness(t, cfg.Arch)
-		if clobbered := clobberedCalleeSaved(live, cfg.Arch); len(clobbered) > 0 {
+		always, rt := clobberedGoFixed(live, cfg.Arch, reachesRuntime(t))
+		if len(always) > 0 {
 			out = append(out, Diagnostic{
 				Pos:      t.Keyword.Pos,
 				Severity: Warning,
 				Code:     CodeRegisterClobber,
-				Message:  fmt.Sprintf("callee-saved register(s) %s written but never saved/restored", strings.Join(clobbered, ", ")),
+				Message:  fmt.Sprintf("register(s) %s written but never saved/restored: fixed by the Go ABI (frame/goroutine pointer)", strings.Join(always, ", ")),
+			})
+		}
+		if len(rt) > 0 {
+			out = append(out, Diagnostic{
+				Pos:      t.Keyword.Pos,
+				Severity: Warning,
+				Code:     CodeRegisterClobber,
+				Message:  fmt.Sprintf("goroutine-pointer register(s) %s written but never saved/restored in a function that can reach the Go runtime", strings.Join(rt, ", ")),
 			})
 		}
 	}
@@ -341,6 +350,29 @@ func lintText(t *ast.Text, tab *arch.Table, archKnown bool, cfg Config, macros m
 	return out
 }

+// reachesRuntime reports whether a function can reach the Go runtime: it is
+// not NOSPLIT (so the stack-split and traceback machinery runs) or it makes a
+// CALL.  Goroutine-pointer registers must survive such functions; a NOSPLIT
+// leaf may clobber them, since the ABI0 transition restores them (the
+// runtime's own assembly relies on this, e.g. R14 on amd64).
+func reachesRuntime(t *ast.Text) bool {
+	nosplit := false
+	for _, f := range t.Flags {
+		if strings.EqualFold(f, "NOSPLIT") {
+			nosplit = true
+		}
+	}
+	for _, s := range t.Body {
+		if in, ok := s.(*ast.Instr); ok {
+			switch strings.ToUpper(in.Mnemonic.Text) {
+			case "CALL", "BL", "JAL": // amd64, arm64/loong64, riscv64 calls
+				return true
+			}
+		}
+	}
+	return !nosplit
+}
+
 // usesFPArgs reports whether a function references its arguments through the FP
 // pseudo-register — i.e. it uses the stack-based ABI0 layout, where the
 // declared argument size must match the signature.
@@ -48,11 +48,11 @@ func TestFixtureIsClean(t *testing.T) {
 	if len(errs) > 0 {
 		t.Fatalf("parse: %v", errs)
 	}
-	// The fixture mirrors the go-flac kernels, which use callee-saved registers
-	// (BX, R13) without saving them; the register-clobber audit flags that by
-	// design.  This test targets the other rules, so the audit is disabled here
-	// (it is covered by TestRegisterClobber).
-	diags := File(f, Config{Arch: arch.AMD64, Disable: map[string]bool{CodeRegisterClobber: true}})
+	// The fixture mirrors the go-flac kernels, which write the Go ABI0
+	// scratch registers (BX, R13) without saving them — legal under Go's
+	// stack-based ABI, so the register-clobber audit stays silent and the
+	// fixture must lint entirely clean.
+	diags := File(f, Config{Arch: arch.AMD64})
 	if len(diags) != 0 {
 		t.Fatalf("expected no diagnostics on the fixture, got %+v", diags)
 	}
@@ -4,7 +4,6 @@
 package lint

 import (
-	"fmt"
 	"sort"
 	"strings"

@@ -248,7 +247,7 @@ func instrEffect(in *ast.Instr, a arch.Arch) regEffect {
 	}

 	compare := isCompare(mnem)
-	dstIdx := dstIndex(in, a)
+	dstIdx := dstIndex(in)

 	for i, op := range in.Operands {
 		r := gprName(op, a)
@@ -281,13 +280,11 @@ func instrEffect(in *ast.Instr, a arch.Arch) regEffect {
 	return eff
 }

-// dstIndex returns the operand index of the destination register: last for the
-// Plan 9 (amd64) spelling, first for arm64/riscv64/loong64.
-func dstIndex(in *ast.Instr, a arch.Arch) int {
-	if a == arch.AMD64 {
-		return len(in.Operands) - 1
-	}
-	return 0
+// dstIndex returns the operand index of the destination register: in Plan 9
+// notation the destination is the last operand on every architecture Go
+// supports (amd64, arm64, riscv64 and loong64 alike).
+func dstIndex(in *ast.Instr) int {
+	return len(in.Operands) - 1
 }

 // isCompare reports whether the mnemonic only reads its operands (setting flags).
@@ -372,41 +369,37 @@ func sameSet(a, b map[string]bool) bool {
 	return true
 }

-// calleeSavedGPRs returns the general-purpose registers an assembly function
-// must preserve for its caller, using the register names the assembler accepts
-// for each architecture.
-func calleeSavedGPRs(a arch.Arch) map[string]bool {
+// goFixedGPRs returns the general-purpose registers the Go ABI designates as
+// fixed across calls — the ones hand-written assembly must not permanently
+// clobber.  This follows cmd/compile/abi-internal.md, not the platform ABI:
+// Go's stack-based ABI0 (which hand-written assembly uses) has no System V
+// style callee-saved registers, so clobbering the argument and scratch
+// registers (amd64 BX, R12, R13, R15, …) is legal.
+//
+// Two groups are returned.  always holds registers whose loss is never safe.
+// runtime holds registers that survive an ABI0 leaf only because the
+// transition machinery restores them (on amd64 the g pointer is reloaded
+// from TLS): clobbering them is safe exactly in NOSPLIT functions that make
+// no calls, which is how the runtime's own assembly uses them.
+func goFixedGPRs(a arch.Arch) (always, runtime map[string]bool) {
 	switch a {
 	case arch.AMD64:
-		return gprSet("BX", "BP", "R12", "R13", "R14", "R15")
+		// BP maintains the frame chain; R14 holds the current goroutine.
+		// R15 is scratch except in dynamically linked binaries, so it is not
+		// flagged.
+		return gprSet("BP"), gprSet("R14")
 	case arch.ARM64:
-		names := []string{"R29", "R30"} // FP, LR
-		for i := 19; i <= 28; i++ {
-			names = append(names, fmt.Sprintf("R%d", i))
-		}
-		return gprSet(names...)
+		// R18 is reserved for the OS on some platforms, R28 holds the current
+		// goroutine, R29 is the frame pointer.
+		return gprSet("R18", "R28", "R29"), nil
 	case arch.RISCV:
-		// RA (X1) and the S registers (X8, X9, X18–X27) are callee-saved.
-		names := []string{"X1", "RA", "X8", "X9", "S0", "S1", "FP"}
-		for i := 18; i <= 27; i++ {
-			names = append(names, fmt.Sprintf("X%d", i))
-		}
-		for i := 2; i <= 11; i++ {
-			names = append(names, fmt.Sprintf("S%d", i))
-		}
-		return gprSet(names...)
+		// X27 holds the current goroutine.
+		return gprSet("X27"), nil
 	case arch.LOONG64:
-		// RA (R1), FP (R22) and S0–S8 (R23–R31) are callee-saved.
-		names := []string{"R1", "RA", "R22", "FP"}
-		for i := 23; i <= 31; i++ {
-			names = append(names, fmt.Sprintf("R%d", i))
-		}
-		for i := 0; i <= 8; i++ {
-			names = append(names, fmt.Sprintf("S%d", i))
-		}
-		return gprSet(names...)
+		// R22 holds the current goroutine.
+		return gprSet("R22"), nil
 	}
-	return nil
+	return nil, nil
 }

 func gprSet(names ...string) map[string]bool {
@@ -417,15 +410,16 @@ func gprSet(names ...string) map[string]bool {
 	return m
 }

-// clobberedCalleeSaved returns the callee-saved registers a function writes
-// without also saving and restoring them — i.e. registers whose caller-owned
-// value is lost across the call.  It walks the blocks of the liveness analysis
-// (so the control-flow graph is what supplies the instruction set) and
-// aggregates each instruction's register effects.
-func clobberedCalleeSaved(l *liveness, a arch.Arch) []string {
-	callee := calleeSavedGPRs(a)
-	if len(callee) == 0 {
-		return nil
+// clobberedGoFixed returns the Go-ABI-fixed registers a function writes
+// without also saving and restoring them.  The first result lists registers
+// whose loss is never safe; the second lists the goroutine-pointer class,
+// whose loss is reported only when reachesRuntime is true (a non-NOSPLIT
+// function, or one that makes calls — the ABI0 transition machinery restores
+// the g pointer only on such paths).
+func clobberedGoFixed(l *liveness, a arch.Arch, reachesRuntime bool) (always, runtime []string) {
+	alwaysSet, runtimeSet := goFixedGPRs(a)
+	if len(alwaysSet) == 0 && len(runtimeSet) == 0 {
+		return nil, nil
 	}
 	def := map[string]bool{}
 	saved := map[string]bool{}
@@ -444,12 +438,19 @@ func clobberedCalleeSaved(l *liveness, a arch.Arch) []string {
 			}
 		}
 	}
-	var out []string
-	for r := range callee {
-		if def[r] && !(saved[r] && restored[r]) {
-			out = append(out, r)
+	clobbered := func(set map[string]bool) []string {
+		var out []string
+		for r := range set {
+			if def[r] && !(saved[r] && restored[r]) {
+				out = append(out, r)
+			}
 		}
+		sort.Strings(out)
+		return out
 	}
-	sort.Strings(out)
-	return out
+	always = clobbered(alwaysSet)
+	if reachesRuntime {
+		runtime = clobbered(runtimeSet)
+	}
+	return always, runtime
 }
@@ -5,36 +5,132 @@ package lint

 import "testing"

-// TestRegisterClobber detects writes to callee-saved registers that are not
-// saved and restored.
+// TestRegisterClobber checks the register-clobber audit is calibrated to the
+// Go ABI (cmd/compile/abi-internal.md), not the platform ABI: Go's
+// stack-based ABI0 — which hand-written assembly uses — has no System V
+// style callee-saved registers, so argument and scratch registers may be
+// clobbered freely.  Only the registers the ABI fixes across calls (the
+// frame pointer, the goroutine pointer, OS-reserved registers) are audited.
 func TestRegisterClobber(t *testing.T) {
-	// BX (callee-saved on amd64) is written but never saved → clobbered.
-	clob := lintSrc(t, "#include \"textflag.h\"\n"+
+	// amd64: BX, R12, R13 and R15 are argument/permanent-scratch registers in
+	// Go ABI0 — writing them unsaved is legal (a System V calibration would
+	// report all of these).
+	scratch := lintSrc(t, "#include \"textflag.h\"\n"+
 		"TEXT ·f(SB), NOSPLIT, $0\n"+
 		"\tMOVQ CX, BX\n"+
+		"\tXORL R12, R12\n"+
+		"\tXORL R13, R13\n"+
+		"\tXORL R15, R15\n"+
 		"\tRET\n")
-	if codes(clob)[CodeRegisterClobber] != 1 {
-		t.Fatalf("unsaved callee-saved write should be flagged: %+v", clob)
+	if codes(scratch)[CodeRegisterClobber] != 0 {
+		t.Fatalf("Go ABI0 scratch registers must not be flagged: %+v", scratch)
 	}

-	// Saved and restored → preserved.
+	// amd64: R14 (the goroutine pointer) in a NOSPLIT function without calls
+	// is the runtime's own pattern — the ABI0 transition restores it — so it
+	// is not flagged.
+	leaf := lintSrc(t, "#include \"textflag.h\"\n"+
+		"TEXT ·f(SB), NOSPLIT, $0\n"+
+		"\tXORL R14, R14\n"+
+		"\tRET\n")
+	if codes(leaf)[CodeRegisterClobber] != 0 {
+		t.Fatalf("R14 in a NOSPLIT leaf must not be flagged: %+v", leaf)
+	}
+
+	// amd64: R14 in a function that makes a call is a genuine hazard.
+	withCall := lintSrc(t, "#include \"textflag.h\"\n"+
+		"TEXT ·f(SB), NOSPLIT, $0\n"+
+		"\tXORL R14, R14\n"+
+		"\tCALL ·g(SB)\n"+
+		"\tRET\n")
+	if codes(withCall)[CodeRegisterClobber] != 1 {
+		t.Fatalf("unsaved R14 with a call should be flagged: %+v", withCall)
+	}
+
+	// amd64: R14 in a non-NOSPLIT function is a hazard regardless of calls.
+	split := lintSrc(t, "#include \"textflag.h\"\n"+
+		"TEXT ·f(SB), $0\n"+
+		"\tMOVQ CX, R14\n"+
+		"\tRET\n")
+	if codes(split)[CodeRegisterClobber] != 1 {
+		t.Fatalf("unsaved R14 in a non-NOSPLIT function should be flagged: %+v", split)
+	}
+
+	// amd64: R14 saved and restored around the call is preserved.
 	saved := lintSrc(t, "#include \"textflag.h\"\n"+
 		"TEXT ·f(SB), NOSPLIT, $8\n"+
-		"\tPUSHQ BX\n"+
-		"\tMOVQ CX, BX\n"+
-		"\tPOPQ BX\n"+
+		"\tPUSHQ R14\n"+
+		"\tXORL R14, R14\n"+
+		"\tCALL ·g(SB)\n"+
+		"\tPOPQ R14\n"+
 		"\tRET\n")
 	if codes(saved)[CodeRegisterClobber] != 0 {
-		t.Fatalf("saved/restored register must not be flagged: %+v", saved)
+		t.Fatalf("saved/restored R14 must not be flagged: %+v", saved)
 	}

-	// A caller-saved register (CX) is fine to write.
-	caller := lintSrc(t, "#include \"textflag.h\"\n"+
+	// amd64: BP maintains the frame chain and is always audited.
+	bp := lintSrc(t, "#include \"textflag.h\"\n"+
 		"TEXT ·f(SB), NOSPLIT, $0\n"+
-		"\tMOVQ $1, CX\n"+
+		"\tMOVQ CX, BP\n"+
 		"\tRET\n")
-	if codes(caller)[CodeRegisterClobber] != 0 {
-		t.Fatalf("caller-saved register must not be flagged: %+v", caller)
+	if codes(bp)[CodeRegisterClobber] != 1 {
+		t.Fatalf("unsaved BP write should be flagged: %+v", bp)
+	}
+
+	// arm64: R20 is scratch; R28 (goroutine pointer) and R18 (OS-reserved)
+	// are fixed by the Go ABI.
+	armScratch := lintSrcArch(t, "t_arm64.s", "#include \"textflag.h\"\n"+
+		"TEXT ·f(SB), NOSPLIT, $0\n"+
+		"\tMOVD R0, R20\n"+
+		"\tRET\n")
+	if codes(armScratch)[CodeRegisterClobber] != 0 {
+		t.Fatalf("arm64 scratch register must not be flagged: %+v", armScratch)
+	}
+	armG := lintSrcArch(t, "t_arm64.s", "#include \"textflag.h\"\n"+
+		"TEXT ·f(SB), NOSPLIT, $0\n"+
+		"\tMOVD R0, R28\n"+
+		"\tRET\n")
+	if codes(armG)[CodeRegisterClobber] != 1 {
+		t.Fatalf("unsaved arm64 R28 write should be flagged: %+v", armG)
+	}
+	armReserved := lintSrcArch(t, "t_arm64.s", "#include \"textflag.h\"\n"+
+		"TEXT ·f(SB), NOSPLIT, $0\n"+
+		"\tMOVD R0, R18\n"+
+		"\tRET\n")
+	if codes(armReserved)[CodeRegisterClobber] != 1 {
+		t.Fatalf("arm64 R18 write should be flagged: %+v", armReserved)
+	}
+
+	// riscv64: X27 holds the goroutine; X5–X7 are scratch.
+	riscScratch := lintSrcArch(t, "t_riscv64.s", "#include \"textflag.h\"\n"+
+		"TEXT ·f(SB), NOSPLIT, $0\n"+
+		"\tMOV X5, X6\n"+
+		"\tRET\n")
+	if codes(riscScratch)[CodeRegisterClobber] != 0 {
+		t.Fatalf("riscv64 scratch register must not be flagged: %+v", riscScratch)
+	}
+	riscG := lintSrcArch(t, "t_riscv64.s", "#include \"textflag.h\"\n"+
+		"TEXT ·f(SB), NOSPLIT, $0\n"+
+		"\tMOV X5, X27\n"+
+		"\tRET\n")
+	if codes(riscG)[CodeRegisterClobber] != 1 {
+		t.Fatalf("unsaved riscv64 X27 write should be flagged: %+v", riscG)
+	}
+
+	// loong64: R22 holds the goroutine; R5–R19 are argument/scratch.
+	loongScratch := lintSrcArch(t, "t_loong64.s", "#include \"textflag.h\"\n"+
+		"TEXT ·f(SB), NOSPLIT, $0\n"+
+		"\tMOVV R5, R6\n"+
+		"\tRET\n")
+	if codes(loongScratch)[CodeRegisterClobber] != 0 {
+		t.Fatalf("loong64 scratch register must not be flagged: %+v", loongScratch)
+	}
+	loongG := lintSrcArch(t, "t_loong64.s", "#include \"textflag.h\"\n"+
+		"TEXT ·f(SB), NOSPLIT, $0\n"+
+		"\tMOVV R5, R22\n"+
+		"\tRET\n")
+	if codes(loongG)[CodeRegisterClobber] != 1 {
+		t.Fatalf("unsaved loong64 R22 write should be flagged: %+v", loongG)
 	}
 }