From d08e0e444e9c7c24298d8aa92590c7d64607dc0f Mon Sep 17 00:00:00 2001 From: Li Jie Date: Wed, 22 Jul 2026 06:30:32 +0800 Subject: [PATCH 1/2] fix(build): keep DebugRefs with reordered return loads --- internal/build/ssa_order_fix.go | 68 +++++----- internal/build/ssa_order_fix_test.go | 141 ++++++++++++++++++++- test/go/dwarf_semantics_acceptance_test.go | 62 +++++++++ 3 files changed, 231 insertions(+), 40 deletions(-) create mode 100644 test/go/dwarf_semantics_acceptance_test.go diff --git a/internal/build/ssa_order_fix.go b/internal/build/ssa_order_fix.go index 57235740c4..08c7df38d8 100644 --- a/internal/build/ssa_order_fix.go +++ b/internal/build/ssa_order_fix.go @@ -302,11 +302,15 @@ func fixSSAOrderBlock(b *ssa.BasicBlock) { continue } - // If the loaded value is used by any instruction between its current - // position and the return (excluding return itself), moving it may place - // its definition after one of those uses and break SSA form. + // DebugRefs are metadata-only and move with the value they describe. Any + // executable use before Return still makes reordering unsafe. + moving := map[ssa.Instruction]struct{}{u: {}} usedBeforeReturn := false for i := loadIdx + 1; i < retIdx; i++ { + if ref, ok := b.Instrs[i].(*ssa.DebugRef); ok && instrUsesValue(ref, u) { + moving[ref] = struct{}{} + continue + } if instrUsesValue(b.Instrs[i], u) { usedBeforeReturn = true break @@ -316,9 +320,7 @@ func fixSSAOrderBlock(b *ssa.BasicBlock) { continue } - // Move the load right after the last call (but before Return). - b.Instrs = moveInstr(b.Instrs, loadIdx, lastCallIdx+1) - // Adjust retIdx for subsequent moves in this block. + b.Instrs = moveInstrsAfter(b.Instrs, moving, b.Instrs[lastCallIdx]) retIdx = indexOfInstr(b.Instrs, ret) } } @@ -391,41 +393,29 @@ func valueDependsOn(v, target ssa.Value, seen map[ssa.Value]struct{}) bool { return false } -// moveInstr moves instrs[from] to position to (like inserting before to), -// preserving relative order of other elements. -func moveInstr(instrs []ssa.Instruction, from, to int) []ssa.Instruction { - if from < 0 || from >= len(instrs) { +// moveInstrsAfter moves selected instructions as a stable group immediately +// after anchor. +func moveInstrsAfter(instrs []ssa.Instruction, moving map[ssa.Instruction]struct{}, anchor ssa.Instruction) []ssa.Instruction { + if len(moving) == 0 || anchor == nil { return instrs } - if to < 0 { - to = 0 - } - if to > len(instrs) { - to = len(instrs) - } - if from == to || from+1 == to { - return instrs - } - - ins := instrs[from] - // Remove. - copy(instrs[from:], instrs[from+1:]) - instrs = instrs[:len(instrs)-1] - - // Recompute insertion index after removal. - if to > from { - to-- - } - if to < 0 { - to = 0 - } - if to > len(instrs) { - to = len(instrs) + moved := make([]ssa.Instruction, 0, len(moving)) + remaining := make([]ssa.Instruction, 0, len(instrs)) + for _, instr := range instrs { + if _, ok := moving[instr]; ok { + moved = append(moved, instr) + continue + } + remaining = append(remaining, instr) + } + for i, instr := range remaining { + if instr == anchor { + ret := make([]ssa.Instruction, 0, len(instrs)) + ret = append(ret, remaining[:i+1]...) + ret = append(ret, moved...) + ret = append(ret, remaining[i+1:]...) + return ret + } } - - // Insert. - instrs = append(instrs, nil) - copy(instrs[to+1:], instrs[to:]) - instrs[to] = ins return instrs } diff --git a/internal/build/ssa_order_fix_test.go b/internal/build/ssa_order_fix_test.go index 7408ae261c..0cd4c1b514 100644 --- a/internal/build/ssa_order_fix_test.go +++ b/internal/build/ssa_order_fix_test.go @@ -11,6 +11,7 @@ import ( "strings" "testing" + "golang.org/x/tools/go/packages" "golang.org/x/tools/go/ssa" "golang.org/x/tools/go/ssa/ssautil" ) @@ -115,7 +116,145 @@ func f() { } } +func TestFixSSAOrderReturnLoadWithDebugRefs(t *testing.T) { + const src = `package p +type value struct { n int } +func (v *value) mutate() bool { v.n = 1; return true } +func f() (value, bool) { + var v value + return v, v.mutate() +}` + base := ssa.SanityCheckFunctions | ssa.InstantiateGenerics + for _, test := range []struct { + name string + mode ssa.BuilderMode + }{ + {name: "default", mode: base}, + {name: "global-debug", mode: base | ssa.GlobalDebug}, + } { + t.Run(test.name, func(t *testing.T) { + fn := buildSSAOrderTestPackageMode(t, src, test.mode) + checkReturnLoadAfterMutation(t, fn, "mutate") + }) + } +} + +func TestFixSSAOrderCryptoX509ParseOID(t *testing.T) { + fset := token.NewFileSet() + loaded, err := packages.Load(&packages.Config{ + Mode: packages.LoadSyntax, + Fset: fset, + }, "crypto/x509") + if err != nil { + t.Fatal(err) + } + if packages.PrintErrors(loaded) != 0 || len(loaded) != 1 { + t.Fatal("failed to load crypto/x509") + } + mode := ssa.SanityCheckFunctions | ssa.InstantiateGenerics | ssa.GlobalDebug + prog, ssaPackages := ssautil.Packages(loaded, mode) + prog.Build() + pkg := ssaPackages[0] + fixSSAOrder(pkg, loaded[0].Syntax) + checkReturnLoadAfterMutation(t, pkg.Func("ParseOID"), "unmarshalOIDText") +} + +func TestMoveInstrsAfter(t *testing.T) { + const src = `package p +func f() { + println(1) + println(2) + println(3) +}` + fn := buildSSAOrderTestPackage(t, src) + instrs := fn.Blocks[0].Instrs + if len(instrs) < 4 { + t.Fatalf("instructions = %d, want at least 4", len(instrs)) + } + + assertOrder := func(t *testing.T, got, want []ssa.Instruction) { + t.Helper() + if len(got) != len(want) { + t.Fatalf("instruction count = %d, want %d", len(got), len(want)) + } + for i := range want { + if got[i] != want[i] { + t.Fatalf("instruction %d = %v, want %v", i, got[i], want[i]) + } + } + } + + t.Run("empty", func(t *testing.T) { + assertOrder(t, moveInstrsAfter(instrs, nil, instrs[1]), instrs) + }) + t.Run("nil-anchor", func(t *testing.T) { + moving := map[ssa.Instruction]struct{}{instrs[0]: {}} + assertOrder(t, moveInstrsAfter(instrs, moving, nil), instrs) + }) + t.Run("missing-anchor", func(t *testing.T) { + moving := map[ssa.Instruction]struct{}{instrs[0]: {}} + assertOrder(t, moveInstrsAfter(instrs, moving, &ssa.Return{}), instrs) + }) + t.Run("stable", func(t *testing.T) { + moving := map[ssa.Instruction]struct{}{ + instrs[0]: {}, + instrs[2]: {}, + } + want := []ssa.Instruction{instrs[1], instrs[0], instrs[2]} + want = append(want, instrs[3:]...) + assertOrder(t, moveInstrsAfter(instrs, moving, instrs[1]), want) + }) +} + +func checkReturnLoadAfterMutation(t *testing.T, fn *ssa.Function, mutation string) { + t.Helper() + var ret *ssa.Return + var mutationCall ssa.CallInstruction + for _, block := range fn.Blocks { + for _, instr := range block.Instrs { + if candidate, ok := instr.(*ssa.Return); ok { + ret = candidate + } + if call, ok := instr.(ssa.CallInstruction); ok { + callee := call.Common().StaticCallee() + if callee != nil && callee.Name() == mutation { + mutationCall = call + } + } + } + } + if ret == nil || len(ret.Results) != 2 || mutationCall == nil { + t.Fatalf("unexpected return instruction: %v", ret) + } + callInstr := mutationCall.(ssa.Instruction) + callIdx := indexOfInstr(callInstr.Block().Instrs, callInstr) + loadIdx := -1 + debugRefIdx := -1 + var resultLoad *ssa.UnOp + for i, instr := range callInstr.Block().Instrs { + if load, ok := instr.(*ssa.UnOp); ok && load.Op == token.MUL { + if alloc, ok := load.X.(*ssa.Alloc); ok && callUsesValue(mutationCall, alloc) { + loadIdx = i + resultLoad = load + } + } + if ref, ok := instr.(*ssa.DebugRef); ok && resultLoad != nil && instrUsesValue(ref, resultLoad) { + debugRefIdx = i + } + } + if callIdx < 0 || loadIdx <= callIdx { + t.Fatalf("return load index = %d, mutation call index = %d; want load after call\n%s", loadIdx, callIdx, fn) + } + if debugRefIdx >= 0 && debugRefIdx <= loadIdx { + t.Fatalf("DebugRef index = %d, load index = %d; want DebugRef after its load\n%s", debugRefIdx, loadIdx, fn) + } +} + func buildSSAOrderTestPackage(t *testing.T, src string) *ssa.Function { + return buildSSAOrderTestPackageMode(t, src, ssa.SanityCheckFunctions|ssa.InstantiateGenerics) +} + +func buildSSAOrderTestPackageMode(t *testing.T, src string, mode ssa.BuilderMode) *ssa.Function { t.Helper() fset := token.NewFileSet() file, err := parser.ParseFile(fset, "p.go", src, 0) @@ -129,7 +268,7 @@ func buildSSAOrderTestPackage(t *testing.T, src string) *ssa.Function { fset, pkg, files, - ssa.SanityCheckFunctions|ssa.InstantiateGenerics, + mode, ) if err != nil { t.Fatalf("BuildPackage: %v", err) diff --git a/test/go/dwarf_semantics_acceptance_test.go b/test/go/dwarf_semantics_acceptance_test.go new file mode 100644 index 0000000000..29a8c34fc8 --- /dev/null +++ b/test/go/dwarf_semantics_acceptance_test.go @@ -0,0 +1,62 @@ +//go:build !llgo + +package gotest + +import ( + "os" + "os/exec" + "path/filepath" + "strings" + "testing" +) + +const dwarfReturnOrderProbe = `package main + +type value struct { + n int +} + +func (v *value) mutate() bool { + v.n = 1 + return true +} + +func result() (value, bool) { + var v value + return v, v.mutate() +} + +func main() { + v, ok := result() + if !ok || v.n != 1 { + panic("return value was loaded before mutation") + } + println("RETURN_ORDER_OK") +} +` + +func TestDWARFReturnOrderSemantics(t *testing.T) { + repoRoot := findStringConversionRepoRoot(t) + source := filepath.Join(t.TempDir(), "main.go") + if err := os.WriteFile(source, []byte(dwarfReturnOrderProbe), 0o600); err != nil { + t.Fatal(err) + } + cmd := exec.Command( + "go", "run", "./cmd/llgo", "run", "-ldflags=-w=false", source, + ) + cmd.Dir = repoRoot + cmd.Env = append(os.Environ(), + "LLGO_ROOT="+repoRoot, + "LLGO_BUILD_CACHE=off", + "GOMAXPROCS=2", + "GOMEMLIMIT=6GiB", + "GOFLAGS=-p=1", + ) + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("LLGo DWARF return-order acceptance failed: %v\n%s", err, out) + } + if !strings.Contains(string(out), "RETURN_ORDER_OK") { + t.Fatalf("LLGo DWARF return-order acceptance did not report success:\n%s", out) + } +} From 4b03b2f6da62345cae2283d72a12af2ff91e87cc Mon Sep 17 00:00:00 2001 From: Li Jie Date: Wed, 22 Jul 2026 19:03:45 +0800 Subject: [PATCH 2/2] fix(build): preserve DebugRefs in select order repair --- internal/build/ssa_order_fix.go | 27 +++++++++++++++ internal/build/ssa_order_fix_test.go | 49 +++++++++++++++++++--------- 2 files changed, 61 insertions(+), 15 deletions(-) diff --git a/internal/build/ssa_order_fix.go b/internal/build/ssa_order_fix.go index 08c7df38d8..1efd9e46b4 100644 --- a/internal/build/ssa_order_fix.go +++ b/internal/build/ssa_order_fix.go @@ -186,6 +186,9 @@ func moveAssignDepsAfterRecv(b *ssa.BasicBlock, roots []ssa.Value, recv ssa.Valu if len(move) == 0 { return false } + // Metadata uses must follow the definitions they describe rather than + // blocking an otherwise safe source-order repair. + includeDebugRefsForMovedValues(b.Instrs, move, recvIdx) if moveWouldBreakSSA(b.Instrs, move, recvIdx) { return false } @@ -205,6 +208,30 @@ func moveAssignDepsAfterRecv(b *ssa.BasicBlock, roots []ssa.Value, recv ssa.Valu return true } +func includeDebugRefsForMovedValues(instrs []ssa.Instruction, move map[int]struct{}, through int) { + moved := make(map[ssa.Value]struct{}, len(move)) + for i := range move { + if v, ok := instrs[i].(ssa.Value); ok && v != nil { + moved[v] = struct{}{} + } + } + for i := 0; i <= through && i < len(instrs); i++ { + if _, moving := move[i]; moving { + continue + } + ref, ok := instrs[i].(*ssa.DebugRef) + if !ok { + continue + } + for v := range moved { + if instrUsesValue(ref, v) { + move[i] = struct{}{} + break + } + } + } +} + func moveWouldBreakSSA(instrs []ssa.Instruction, move map[int]struct{}, recvIdx int) bool { moved := make(map[ssa.Value]struct{}, len(move)) for i := range move { diff --git a/internal/build/ssa_order_fix_test.go b/internal/build/ssa_order_fix_test.go index 0cd4c1b514..33ff26041f 100644 --- a/internal/build/ssa_order_fix_test.go +++ b/internal/build/ssa_order_fix_test.go @@ -29,11 +29,12 @@ func f() { case *fp(&x, 100) = <-fc(c, 1): } }` - fn := buildSSAOrderTestPackage(t, src) - got := instrOrder(fn, "fc(", "<-", "fp(", "*t") - if !inOrder(got, "fc(", "<-", "fp(") { - t.Fatalf("single-case select receive assignment order = %v, want fc/receive before fp", got) - } + testSSAOrderModes(t, src, func(t *testing.T, fn *ssa.Function) { + got := instrOrder(fn, "fc(", "<-", "fp(", "*t") + if !inOrder(got, "fc(", "<-", "fp(") { + t.Fatalf("single-case select receive assignment order = %v, want fc/receive before fp", got) + } + }) } func TestFixSSAOrderPlainRecvAssignKeepsLeftToRight(t *testing.T) { @@ -67,11 +68,12 @@ func f() { case m[fn(13, 100)] = <-fc(c, 1): } }` - fn := buildSSAOrderTestPackage(t, src) - got := instrOrder(fn, "fc(", "<-", "fn(") - if !inOrder(got, "fc(", "<-", "fn(") { - t.Fatalf("single-case select map receive assignment order = %v, want fc/receive before fn", got) - } + testSSAOrderModes(t, src, func(t *testing.T, fn *ssa.Function) { + got := instrOrder(fn, "fc(", "<-", "fn(") + if !inOrder(got, "fc(", "<-", "fn(") { + t.Fatalf("single-case select map receive assignment order = %v, want fc/receive before fn", got) + } + }) } func TestFixSSAOrderSingleCaseSelectTwoValueRecv(t *testing.T) { @@ -88,11 +90,12 @@ func f() { case *fp(&x, 100), ok = <-fc(c, 1): } }` - fn := buildSSAOrderTestPackage(t, src) - got := instrOrder(fn, "fc(", "<-", "fp(", "*t") - if !inOrder(got, "fc(", "<-", "fp(") { - t.Fatalf("single-case select two-value receive assignment order = %v, want fc/receive before fp", got) - } + testSSAOrderModes(t, src, func(t *testing.T, fn *ssa.Function) { + got := instrOrder(fn, "fc(", "<-", "fp(", "*t") + if !inOrder(got, "fc(", "<-", "fp(") { + t.Fatalf("single-case select two-value receive assignment order = %v, want fc/receive before fp", got) + } + }) } func TestFixSSAOrderMultiCaseSelectKeepsLeftToRight(t *testing.T) { @@ -254,6 +257,22 @@ func buildSSAOrderTestPackage(t *testing.T, src string) *ssa.Function { return buildSSAOrderTestPackageMode(t, src, ssa.SanityCheckFunctions|ssa.InstantiateGenerics) } +func testSSAOrderModes(t *testing.T, src string, check func(*testing.T, *ssa.Function)) { + t.Helper() + base := ssa.SanityCheckFunctions | ssa.InstantiateGenerics + for _, test := range []struct { + name string + mode ssa.BuilderMode + }{ + {name: "default", mode: base}, + {name: "global-debug", mode: base | ssa.GlobalDebug}, + } { + t.Run(test.name, func(t *testing.T) { + check(t, buildSSAOrderTestPackageMode(t, src, test.mode)) + }) + } +} + func buildSSAOrderTestPackageMode(t *testing.T, src string, mode ssa.BuilderMode) *ssa.Function { t.Helper() fset := token.NewFileSet()