From 1a0187069595a4a039719f3069958531808fdb5b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20Balv=C3=ADn?= Date: Sat, 11 Jul 2026 17:36:52 +0200 Subject: [PATCH] fix(lint): calibrate register-clobber to the Go ABI and add legacy SSE moves Assisted-by: Qwen 3.8 Max Preview --- asm/encode.go | 2 + asm/encode_test.go | 46 +++++++++++++++ asm/evex.go | 2 + asm/evex_test.go | 4 ++ asm/instrs.go | 60 ++++++++++++++++++++ cmd/gasm/main.go | 2 +- docs/ARCHITECTURE.md | 19 ++++--- format/format.go | 6 ++ format/format_test.go | 38 +++++++++++++ justfile | 2 +- lint/lint.go | 44 +++++++++++++-- lint/lint_test.go | 10 ++-- lint/liveness.go | 107 ++++++++++++++++++----------------- lint/liveness_test.go | 128 ++++++++++++++++++++++++++++++++++++------ 14 files changed, 381 insertions(+), 89 deletions(-) diff --git a/asm/encode.go b/asm/encode.go index a83354d..edc9f78 100644 --- a/asm/encode.go +++ b/asm/encode.go @@ -93,6 +93,8 @@ func (e *enc) encode(mnem string, ops []Operand) error { return e.encodeMovExtend(base, ops) case "CVTSL2SD", "CVTSQ2SD": return e.encodeCvtsi2sd(base == "CVTSQ2SD", ops) + case "MOVOU", "MOVO", "MOVUPS", "MOVAPS", "MOVUPD", "MOVAPD", "MOVSD", "MOVSS": + return e.encodeSSEMove(sseMoveTable[base], ops) } return fmt.Errorf("unsupported instruction %q", mnem) } diff --git a/asm/encode_test.go b/asm/encode_test.go index 9d1e6e2..dc74b04 100644 --- a/asm/encode_test.go +++ b/asm/encode_test.go @@ -127,6 +127,52 @@ func TestControl(t *testing.T) { checkOp(t, x86asm.JBE, "JLS", Imm(0)) } +// TestSSEMoveGroundTruth checks the legacy (non-VEX) SSE moves byte for byte +// against the Go assembler. wantOp is the decoder's name, which differs from +// the Plan 9 spelling for the octa moves (MOVOU = MOVDQU, MOVO = MOVDQA). +func TestSSEMoveGroundTruth(t *testing.T) { + cases := []struct { + name string + mnem string + ops []Operand + want string + wantOp string + }{ + {"MOVOU (SI),X1", "MOVOU", []Operand{Ptr(SI, 0, 16), vreg(t, "X1")}, "f30f6f0e", "MOVDQU"}, + {"MOVOU X3,(DI)", "MOVOU", []Operand{vreg(t, "X3"), Ptr(DI, 0, 16)}, "f30f7f1f", "MOVDQU"}, + {"MOVOU X1,X2", "MOVOU", []Operand{vreg(t, "X1"), vreg(t, "X2")}, "f30f6fd1", "MOVDQU"}, + {"MOVOU (SI)(BX*4),X9", "MOVOU", []Operand{Idx(SI, BX, 4, 0, 16), vreg(t, "X9")}, "f3440f6f0c9e", "MOVDQU"}, + {"MOVO (SI),X1", "MOVO", []Operand{Ptr(SI, 0, 16), vreg(t, "X1")}, "660f6f0e", "MOVDQA"}, + {"MOVO X3,(DI)", "MOVO", []Operand{vreg(t, "X3"), Ptr(DI, 0, 16)}, "660f7f1f", "MOVDQA"}, + {"MOVUPS (SI),X1", "MOVUPS", []Operand{Ptr(SI, 0, 16), vreg(t, "X1")}, "0f100e", "MOVUPS"}, + {"MOVAPS X3,(DI)", "MOVAPS", []Operand{vreg(t, "X3"), Ptr(DI, 0, 16)}, "0f291f", "MOVAPS"}, + {"MOVUPD (SI),X1", "MOVUPD", []Operand{Ptr(SI, 0, 16), vreg(t, "X1")}, "660f100e", "MOVUPD"}, + {"MOVAPD X3,(DI)", "MOVAPD", []Operand{vreg(t, "X3"), Ptr(DI, 0, 16)}, "660f291f", "MOVAPD"}, + {"MOVSD (SI),X1", "MOVSD", []Operand{Ptr(SI, 0, 8), vreg(t, "X1")}, "f20f100e", "MOVSD_XMM"}, + {"MOVSD X1,X2", "MOVSD", []Operand{vreg(t, "X1"), vreg(t, "X2")}, "f20f10d1", "MOVSD_XMM"}, + {"MOVSS X3,(DI)", "MOVSS", []Operand{vreg(t, "X3"), Ptr(DI, 0, 4)}, "f30f111f", "MOVSS"}, + } + for _, c := range cases { + code, err := Encode(c.mnem, c.ops...) + if err != nil { + t.Errorf("%s: Encode: %v", c.name, err) + continue + } + if got := hexCompact(code); got != c.want { + t.Errorf("%s: bytes %s, want %s", c.name, got, c.want) + continue + } + inst, err := x86asm.Decode(code, 64) + if err != nil { + t.Errorf("%s: Decode(%x): %v", c.name, code, err) + continue + } + if inst.Op.String() != c.wantOp { + t.Errorf("%s: decoded as %s", c.name, inst.Op.String()) + } + } +} + // TestGoFlacScalarTail encodes the scalar tail of an analyze kernel to confirm // the encoder handles a realistic instruction sequence. func TestGoFlacScalarTail(t *testing.T) { diff --git a/asm/evex.go b/asm/evex.go index 516e8f4..543219c 100644 --- a/asm/evex.go +++ b/asm/evex.go @@ -115,6 +115,8 @@ type evexMoveSpec struct { var evexMoveTable = map[string]evexMoveSpec{ // EVEX.128/256/512.F3.0F.W0 — unaligned integer move. "VMOVDQU32": {1, 2, 0x6F, 0x7F, 0, [3]int{16, 32, 64}}, + // EVEX.128/256/512.F3.0F.W1 — unaligned qword move. + "VMOVDQU64": {1, 2, 0x6F, 0x7F, 1, [3]int{16, 32, 64}}, // EVEX.128/256/512.66.0F.W1 — unaligned packed double move. "VMOVUPD": {1, 1, 0x10, 0x11, 1, [3]int{16, 32, 64}}, } diff --git a/asm/evex_test.go b/asm/evex_test.go index 22664a9..4dae01a 100644 --- a/asm/evex_test.go +++ b/asm/evex_test.go @@ -61,6 +61,10 @@ func TestEvexGroundTruth(t *testing.T) { {"VMOVDQU32 16(SI)(R15*4),Z4", "VMOVDQU32", []Operand{Idx(SI, vreg(t, "R15"), 4, 16, 64), vreg(t, "Z4")}, "62b17e486fa4be10000000"}, {"VMOVDQU32 Z0,4(SI)(AX*1)", "VMOVDQU32", []Operand{vreg(t, "Z0"), Idx(SI, AX, 1, 4, 64)}, "62f17e487f840604000000"}, {"VMOVDQU32 Z3,(DI)(R15*4)", "VMOVDQU32", []Operand{vreg(t, "Z3"), Idx(DI, vreg(t, "R15"), 4, 0, 64)}, "62b17e487f1cbf"}, + // VMOVDQU64 — the W1 qword variant. + {"VMOVDQU64 (SI)(R15*4),Z3", "VMOVDQU64", []Operand{Idx(SI, vreg(t, "R15"), 4, 0, 64), vreg(t, "Z3")}, "62b1fe486f1cbe"}, + {"VMOVDQU64 Z0,4(SI)(AX*1)", "VMOVDQU64", []Operand{vreg(t, "Z0"), Idx(SI, AX, 1, 4, 64)}, "62f1fe487f840604000000"}, + {"VMOVDQU64 Z1,Z2", "VMOVDQU64", []Operand{vreg(t, "Z1"), vreg(t, "Z2")}, "62f1fe487fca"}, {"VMOVUPD (DI),Z14", "VMOVUPD", []Operand{Ptr(DI, 0, 64), vreg(t, "Z14")}, "6271fd481037"}, {"VMOVUPD 64(DI),Z14", "VMOVUPD", []Operand{Ptr(DI, 64, 64), vreg(t, "Z14")}, "6271fd48107701"}, // Conversions and narrowing stores (reg = wide source). diff --git a/asm/instrs.go b/asm/instrs.go index ac7dc4d..479043f 100644 --- a/asm/instrs.go +++ b/asm/instrs.go @@ -661,6 +661,66 @@ func (e *enc) encodeMovExtend(base string, ops []Operand) error { return e.emit(i) } +// --- legacy SSE moves -------------------------------------------------------- + +// sseMove describes a legacy (non-VEX) SSE move: a mandatory prefix plus a +// load opcode (reg = destination, rm = source) and a store opcode (the +// reverse). The Plan 9 names MOVOU/MOVO are the integer unaligned/aligned +// octa moves (MOVDQU/MOVDQA), not the packed-single ones. +type sseMove struct { + prefix byte // 0, 0x66, 0xF2 or 0xF3 + load byte + store byte +} + +var sseMoveTable = map[string]sseMove{ + "MOVOU": {0xF3, 0x6F, 0x7F}, // MOVDQU — unaligned octa + "MOVO": {0x66, 0x6F, 0x7F}, // MOVDQA — aligned octa + "MOVUPS": {0x00, 0x10, 0x11}, // unaligned packed single + "MOVAPS": {0x00, 0x28, 0x29}, // aligned packed single + "MOVUPD": {0x66, 0x10, 0x11}, // unaligned packed double + "MOVAPD": {0x66, 0x28, 0x29}, // aligned packed double + "MOVSD": {0xF2, 0x10, 0x11}, // scalar double + "MOVSS": {0xF3, 0x10, 0x11}, // scalar single +} + +// encodeSSEMove encodes a legacy SSE move: a vector-to-vector move uses the +// load form (reg = destination), matching the Go assembler. +func (e *enc) encodeSSEMove(m sseMove, ops []Operand) error { + if len(ops) != 2 { + return fmt.Errorf("SSE move expects 2 operands, got %d", len(ops)) + } + src, dst := ops[0], ops[1] + srcReg, srcVec := vecReg(src) + dstReg, dstVec := vecReg(dst) + op := m.store + var reg Reg + var rm Operand + switch { + case srcVec && dstVec: + op = m.load + reg, rm = dstReg, src + case srcVec: + if _, ok := dst.(Mem); !ok { + return fmt.Errorf("SSE move: invalid destination operand") + } + reg, rm = srcReg, dst + case dstVec: + if _, ok := src.(Mem); !ok { + return fmt.Errorf("SSE move: invalid source operand") + } + op = m.load + reg, rm = dstReg, src + default: + return fmt.Errorf("SSE move needs a vector register operand") + } + i := &instr{prefix: m.prefix, opcode: []byte{0x0F, op}, modrm: -1, sib: -1} + if err := setRM(i, reg, rm, 8); err != nil { + return err + } + return e.emit(i) +} + // --- CVTSL2SD / CVTSQ2SD ----------------------------------------------------- // encodeCvtsi2sd encodes a signed integer to scalar double conversion diff --git a/cmd/gasm/main.go b/cmd/gasm/main.go index 7632700..b3c3e98 100644 --- a/cmd/gasm/main.go +++ b/cmd/gasm/main.go @@ -26,7 +26,7 @@ import ( // version is the release version, stamped at build time via // -ldflags "-X main.version=…" (defaulting to the current release). -var version = "0.5.0" +var version = "0.6.0" func main() { if len(os.Args) < 2 { diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index f721f36..04fc1ce 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -136,13 +136,18 @@ Two deeper analyses sit on top of the AST: control-flow graph (basic blocks split at labels and after branches, with fall-through and jump-target edges), computes a conservative per-instruction register def/use, and runs the standard backward liveness iteration to a fixed - point. On top of that it flags a **callee-saved register that is written but - never saved and restored** — the per-architecture callee-saved set is amd64 - `BX/BP/R12–R15`, arm64 `R19–R30`, riscv64 `X1/X8/X9/X18–X27`, loong64 - `R1/R22–R31`. This is an *audit*: the runtime's own assembly clobbers these - registers freely (it controls both sides of the call), so the rule is - advisory there, but in hand-written kernels called from ordinary Go code a - clobber is a genuine ABI violation. It runs only on macro-free files, where + point. On top of that it flags writes to the registers the **Go ABI** fixes + across calls that are never saved and restored — calibrated from + `cmd/compile/abi-internal.md`, *not* the platform ABI: Go's stack-based ABI0 + has no System V style callee-saved registers (amd64 `BX`, `R12`–`R15` and + the like are caller-saved or permanent scratch, and hand-written kernels may + clobber them freely). The audited set is the frame pointer and the + goroutine pointer per architecture (amd64 `BP`/`R14`, arm64 `R18`/`R28`/ + `R29`, riscv64 `X27`, loong64 `R22`); the goroutine pointer is reported only + when the function can reach the runtime — it is not `NOSPLIT` or makes a + call — since the ABI0 transition machinery restores it on those paths, and + NOSPLIT call-free leaves may use it (the runtime's own assembly does). It + runs only on macro-free files, where no opaque macro can perform the save/restore. - **`funcdata-pcdata`.** `FUNCDATA $idx, sym(SB)` and `PCDATA $idx, $val` are checked for well-formed operands (arity, immediate index and value, symbol diff --git a/format/format.go b/format/format.go index d3235ad..9deb840 100644 --- a/format/format.go +++ b/format/format.go @@ -99,6 +99,12 @@ func Source(path, src string) string { } case kInstr: out = renderInstr(line, maxWidth[inf.funcID]) + // A RET ends the body for indentation purposes: comments that + // follow it — typically the next function's doc comment — belong + // at column 0, not inside the finished function. + if strings.EqualFold(line[0].Text, "RET") { + inBody = false + } } b.WriteString(strings.TrimRight(out, " \t")) b.WriteByte('\n') diff --git a/format/format_test.go b/format/format_test.go index 2817dd2..ffbe801 100644 --- a/format/format_test.go +++ b/format/format_test.go @@ -39,6 +39,44 @@ func TestGolden(t *testing.T) { } } +// TestDocCommentIndent checks that a doc comment preceding a TEXT directive +// sits at column 0 even when another function (ending in RET) precedes it — +// the RET must terminate the previous body for indentation purposes. +func TestDocCommentIndent(t *testing.T) { + in := "#include \"textflag.h\"\n" + + "\n" + + "// func first()\n" + + "TEXT ·first(SB), NOSPLIT, $0\n" + + "XORQ AX, AX\n" + + "RET\n" + + "\n" + + "// func second()\n" + + "TEXT ·second(SB), NOSPLIT, $0\n" + + "RET\n" + + want := "#include \"textflag.h\"\n" + + "\n" + + "// func first()\n" + + "TEXT ·first(SB), NOSPLIT, $0\n" + + "\tXORQ AX, AX\n" + + "\tRET\n" + + "\n" + + "// func second()\n" + + "TEXT ·second(SB), NOSPLIT, $0\n" + + "\tRET\n" + + got := Source("d_amd64.s", in) + if got != want { + t.Fatalf("formatting mismatch:\n--- got ---\n%q\n--- want ---\n%q", got, want) + } + // Body comments stay indented. + body := "#include \"textflag.h\"\nTEXT ·f(SB), NOSPLIT, $0\n// inside the body\nXORQ AX, AX\nRET\n" + gotBody := Source("b_amd64.s", body) + if !strings.Contains(gotBody, "\t// inside the body\n") { + t.Fatalf("body comment must stay indented:\n%q", gotBody) + } +} + func TestOperandSpacing(t *testing.T) { cases := map[string]string{ "4(SI)": "4(SI)", diff --git a/justfile b/justfile index 5ac0b64..9216642 100644 --- a/justfile +++ b/justfile @@ -3,7 +3,7 @@ # gasm-devkit — developer tooling for Go's Plan 9 assembler (GAsm). -version := "0.5.0" +version := "0.6.0" default: @just --list diff --git a/lint/lint.go b/lint/lint.go index c0511a6..c3845c3 100644 --- a/lint/lint.go +++ b/lint/lint.go @@ -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. diff --git a/lint/lint_test.go b/lint/lint_test.go index 920d852..1482d94 100644 --- a/lint/lint_test.go +++ b/lint/lint_test.go @@ -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) } diff --git a/lint/liveness.go b/lint/liveness.go index 5a83477..5d1c5f9 100644 --- a/lint/liveness.go +++ b/lint/liveness.go @@ -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 } diff --git a/lint/liveness_test.go b/lint/liveness_test.go index d6f591c..5964399 100644 --- a/lint/liveness_test.go +++ b/lint/liveness_test.go @@ -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) } }