diff --git a/cl/compile.go b/cl/compile.go index 8f99b9a0f5..1c30bde86c 100644 --- a/cl/compile.go +++ b/cl/compile.go @@ -157,6 +157,12 @@ type pkgInfo struct { type none = struct{} +type debugStableParam struct { + home llssa.Expr + value ssa.Value + block *ssa.BasicBlock +} + type context struct { prog llssa.Program pkg llssa.Package @@ -178,6 +184,8 @@ type context struct { anonDefers map[*ssa.Function]bool debugDIVars map[*types.Var]llssa.DIVar debugAllocVars map[*ssa.Alloc]*types.Var + debugAllocObjects map[*types.Var]bool + debugStableParams map[*types.Var]debugStableParam runtimeCallerFuncs map[*ssa.Function]bool pcLineSeq uint64 @@ -619,9 +627,13 @@ func (p *context) compileFuncDecl(pkg llssa.Package, f *ssa.Function) (llssa.Fun if dbgSymsEnabled { p.debugDIVars = make(map[*types.Var]llssa.DIVar) p.debugAllocVars = collectDebugAllocVariables(f) + p.debugAllocObjects = collectDebugAllocObjects(p.debugAllocVars) + p.debugStableParams = make(map[*types.Var]debugStableParam) } else { p.debugDIVars = nil p.debugAllocVars = nil + p.debugAllocObjects = nil + p.debugStableParams = nil } dbgGoSSADump(f) dbgInstrln("==> FuncBody", name) @@ -797,6 +809,31 @@ func (p *context) debugRef(b llssa.Builder, v *ssa.DebugRef) { // avoid generate local variable debug info of global variable in function return } + if p.debugAllocObjects[variable] { + // The variable already has a declaration tied to its real storage. + // A value DebugRef for an aggregate would replace it with a snapshot. + return + } + if stable, ok := p.debugStableParams[variable]; ok { + if stable.value == v.X && stable.block == v.Block() { + return + } + var value llssa.Expr + if iv, ok := v.X.(instrOrValue); ok { + var exists bool + value, exists = p.bvals[iv] + if !exists { + return + } + } else { + value = p.compileValue(b, v.X) + } + b.DIStore(stable.home, value) + stable.value = v.X + stable.block = v.Block() + p.debugStableParams[variable] = stable + return + } pos := p.goProg.Fset.Position(v.Pos()) var value llssa.Expr if iv, ok := v.X.(instrOrValue); ok { @@ -824,7 +861,7 @@ func (p *context) debugRef(b llssa.Builder, v *ssa.DebugRef) { func (p *context) debugParams(b llssa.Builder, f *ssa.Function) { for i, param := range f.Params { variable := param.Object().(*types.Var) - if hasDebugAlloc(p.debugAllocVars, variable) { + if p.debugAllocObjects[variable] { continue } pos := p.goProg.Fset.Position(param.Pos()) @@ -835,7 +872,11 @@ func (p *context) debugParams(b llssa.Builder, f *ssa.Function) { if p.debugDIVars != nil { p.debugDIVars[variable] = div } - b.DIParam(variable, v, div, p.fn, pos, p.fn.Block(0)) + if home := b.DIParamWithHome(variable, v, div, p.fn, pos, p.fn.Block(0)); !home.IsNil() { + p.debugStableParams[variable] = debugStableParam{ + home: home, value: param, block: f.Blocks[0], + } + } } } diff --git a/cl/debug_alloc.go b/cl/debug_alloc.go index 5d05c0b442..a39e610fb9 100644 --- a/cl/debug_alloc.go +++ b/cl/debug_alloc.go @@ -37,6 +37,14 @@ func hasDebugAlloc(variables map[*ssa.Alloc]*types.Var, variable *types.Var) boo return false } +func collectDebugAllocObjects(variables map[*ssa.Alloc]*types.Var) map[*types.Var]bool { + objects := make(map[*types.Var]bool, len(variables)) + for _, variable := range variables { + objects[variable] = true + } + return objects +} + func (p *context) debugAlloc(b llssa.Builder, alloc *ssa.Alloc, addr llssa.Expr) { variable := p.debugAllocVars[alloc] if variable == nil { diff --git a/cl/debug_alloc_test.go b/cl/debug_alloc_test.go index 7e304c0023..278c5596d9 100644 --- a/cl/debug_alloc_test.go +++ b/cl/debug_alloc_test.go @@ -61,6 +61,9 @@ func assertDebugAllocParameter(t *testing.T, fn *ssa.Function, name string) { if !hasDebugAlloc(variables, variable) { t.Fatalf("%s parameter %q has no debug alloca", fn, name) } + if !collectDebugAllocObjects(variables)[variable] { + t.Fatalf("%s parameter %q is missing from the debug alloca object set", fn, name) + } if got := debugParameterArgNo(fn, variable); got != 1 { t.Fatalf("debugParameterArgNo(%s) = %d, want 1", name, got) } diff --git a/cl/debug_compile_test.go b/cl/debug_compile_test.go index 2f177005b2..63399fe977 100644 --- a/cl/debug_compile_test.go +++ b/cl/debug_compile_test.go @@ -8,6 +8,7 @@ import ( "go/parser" "go/token" "go/types" + "regexp" "runtime" "strings" "testing" @@ -30,8 +31,12 @@ func inspect(items [2]item, seed int) int { if x > 0 { items[0].value = x local[0].value = x + seed = x + } else { + seed = x } - return items[0].value + local[0].value + seed = 42 + return items[0].value + local[0].value + seed } var anonymous = func(seed int) int { @@ -92,4 +97,73 @@ var anonymous = func(seed int) int { t.Errorf("debug module is missing %q:\n%s", want, ir) } } + assertDebugRecords(t, ir, `name: "items", arg: 1`, true, false) + assertDebugRecords(t, ir, `name: "seed", arg: 2`, true, false) + assertDebugHomeStores(t, ir, `name: "seed", arg: 2`, 4) + + optimizedProg := newLLSSAProgForTarget(t, &llssa.Target{ + GOOS: runtime.GOOS, + GOARCH: runtime.GOARCH, + OptLevel: optlevel.O2, + }) + defer optimizedProg.Dispose() + optimizedPkg, err := NewPackage(optimizedProg, ssaPkg, []*ast.File{file}) + if err != nil { + t.Fatal(err) + } + if err := llvm.VerifyModule(optimizedPkg.Module(), llvm.ReturnStatusAction); err != nil { + t.Fatalf("optimized debug module is invalid: %v\n%s", err, optimizedPkg.Module().String()) + } + optimizedIR := optimizedPkg.Module().String() + assertDebugRecords(t, optimizedIR, `name: "items", arg: 1`, true, false) + assertDebugRecords(t, optimizedIR, `name: "seed", arg: 2`, false, true) +} + +func assertDebugHomeStores(t *testing.T, ir, variable string, minimum int) { + t.Helper() + variableID := debugVariableID(t, ir, variable) + re := regexp.MustCompile(`#dbg_declare\(ptr ([^,]+), ` + regexp.QuoteMeta(variableID) + `,`) + match := re.FindStringSubmatch(ir) + if len(match) != 2 { + t.Fatalf("debug home for %q not found:\n%s", variable, ir) + } + stores := 0 + for _, line := range strings.Split(ir, "\n") { + if strings.Contains(line, "store ") && strings.Contains(line, ", ptr "+match[1]+",") { + stores++ + if strings.Contains(line, "!dbg") { + t.Fatalf("debug home store for %q has a source location: %s", variable, line) + } + } + } + if stores < minimum { + t.Fatalf("debug home for %q has %d stores, want at least %d\n%s", variable, stores, minimum, ir) + } +} + +func assertDebugRecords(t *testing.T, ir, variable string, wantDeclare, wantValue bool) { + t.Helper() + variableID := debugVariableID(t, ir, variable) + var declare, value bool + for _, line := range strings.Split(ir, "\n") { + if !strings.Contains(line, variableID+",") { + continue + } + declare = declare || strings.Contains(line, "#dbg_declare") + value = value || strings.Contains(line, "#dbg_value") + } + if declare != wantDeclare || value != wantValue { + t.Fatalf("debug records for %q: declare=%v value=%v, want declare=%v value=%v\n%s", + variable, declare, value, wantDeclare, wantValue, ir) + } +} + +func debugVariableID(t *testing.T, ir, variable string) string { + t.Helper() + re := regexp.MustCompile(`(?m)^(![0-9]+) = !DILocalVariable\(` + regexp.QuoteMeta(variable)) + match := re.FindStringSubmatch(ir) + if len(match) != 2 { + t.Fatalf("debug variable %q not found:\n%s", variable, ir) + } + return match[1] } diff --git a/cmd/llgo/lldbtest/test.py b/cmd/llgo/lldbtest/test.py index db42489ea1..637a859e45 100644 --- a/cmd/llgo/lldbtest/test.py +++ b/cmd/llgo/lldbtest/test.py @@ -137,6 +137,14 @@ def test_case(marker: str, expectations: List[tuple[str, str]]) -> TestCase: ("all variables", "i8 i16 i32 i64 i u8 u16 u32 u64 u f32 f64 b " "c64 c128 slice arr arr2 s e f pf pi intr m c err fn currentI32 " "currentI64 currentI currentU32 currentU64 currentU currentF32 currentF64"), + ("i32", "3"), + ("i64", "4"), + ("i", "5"), + ("u32", "8"), + ("u64", "9"), + ("u", "10"), + ("f32", "11"), + ("f64", "12"), ("currentI32", "3"), ("currentI64", "4"), ("currentI", "5"), @@ -162,14 +170,22 @@ def test_case(marker: str, expectations: List[tuple[str, str]]) -> TestCase: test_case("all_params_updated", [ ("i8", r"'\t'"), ("i16", "10"), + ("i32", "11"), + ("i64", "12"), + ("i", "13"), ("currentI32", "11"), ("currentI64", "12"), ("currentI", "13"), ("u8", r"'\x0e'"), ("u16", "15"), + ("u32", "16"), + ("u64", "17"), + ("u", "18"), ("currentU32", "16"), ("currentU64", "17"), ("currentU", "18"), + ("f32", "19"), + ("f64", "20"), ("currentF32", "19"), ("currentF64", "20"), ("b", "false"), @@ -253,6 +269,7 @@ def test_case(marker: str, expectations: List[tuple[str, str]]) -> TestCase: ]), test_case("main_struct_updated", [ ("all variables", "s i err"), + ("s.i8", r"'\x12'"), ("(*globalStructPtr).i8", r"'\x12'"), ]), ] diff --git a/ssa/di.go b/ssa/di.go index f046d8601a..460b68bec7 100644 --- a/ssa/di.go +++ b/ssa/di.go @@ -633,13 +633,12 @@ func (b diBuilder) createExpression(ops []uint64) DIExpression { // ----------------------------------------------------------------------------- // Copy to alloca'd memory to get declareable address. -func (b Builder) constructDebugAddr(v Expr) (dbgPtr Expr, dbgVal Expr, exists bool) { +func (b Builder) constructDebugAddr(v Expr) Expr { t := v.Type.RawType().Underlying() - dbgPtr, dbgVal = b.doConstructDebugAddr(v, t) - return dbgPtr, dbgVal, false + return b.doConstructDebugAddr(v, t) } -func (b Builder) doConstructDebugAddr(v Expr, t types.Type) (dbgPtr Expr, dbgVal Expr) { +func (b Builder) doConstructDebugAddr(v Expr, t types.Type) (dbgPtr Expr) { var ty Type switch t := t.(type) { case *types.Basic: @@ -670,8 +669,7 @@ func (b Builder) doConstructDebugAddr(v Expr, t types.Type) (dbgPtr Expr, dbgVal dbgPtr = b.AllocaT(ty) dbgPtr.Type = b.Prog.Pointer(v.Type) b.Store(dbgPtr, v) - dbgVal = b.Load(dbgPtr) - return dbgPtr, dbgVal + return dbgPtr } func (b Builder) di() diBuilder { @@ -679,7 +677,39 @@ func (b Builder) di() diBuilder { } func (b Builder) DIParam(variable *types.Var, v Expr, dv DIVar, scope DIScope, pos token.Position, blk BasicBlock) { - b.DIValue(variable, v, dv, scope, pos, blk) + b.diParam(variable, v, dv, scope, pos, blk) +} + +// DIParamWithHome returns the stable O0 storage backing the parameter. +func (b Builder) DIParamWithHome(variable *types.Var, v Expr, dv DIVar, scope DIScope, pos token.Position, blk BasicBlock) Expr { + return b.diParam(variable, v, dv, scope, pos, blk) +} + +func (b Builder) diParam(variable *types.Var, v Expr, dv DIVar, scope DIScope, pos token.Position, blk BasicBlock) Expr { + if b.Prog.debugInfoOptimized { + b.DIValue(variable, v, dv, scope, pos, blk) + return Nil + } + var dbgPtr Expr + b.withoutDebugLocation(func() { + dbgPtr = b.constructDebugAddr(v) + }) + b.DIDeclare(variable, dbgPtr, dv, scope, pos, blk) + return dbgPtr +} + +// DIStore updates debug-only storage without creating a source line site. +func (b Builder) DIStore(ptr, value Expr) { + b.withoutDebugLocation(func() { + b.Store(ptr, value) + }) +} + +func (b Builder) withoutDebugLocation(fn func()) { + loc := b.impl.GetCurrentDebugLocation() + b.impl.SetCurrentDebugLocation(0, 0, llvm.Metadata{}, llvm.Metadata{}) + defer b.impl.SetCurrentDebugLocation(loc.Line, loc.Col, loc.Scope, loc.InlinedAt) + fn() } func (b Builder) DIDeclare(variable *types.Var, v Expr, dv DIVar, scope DIScope, pos token.Position, blk BasicBlock) { @@ -693,7 +723,7 @@ func (b Builder) DIValue(variable *types.Var, v Expr, dv DIVar, scope DIScope, p expr := b.di().createExpression(nil) b.di().dbgValue(v, dv, scope, pos, expr, blk) } else { - dbgPtr, _, _ := b.constructDebugAddr(v) + dbgPtr := b.constructDebugAddr(v) expr := b.di().createExpression([]uint64{opDeref}) b.di().dbgValue(dbgPtr, dv, scope, pos, expr, blk) } diff --git a/ssa/di_debug_test.go b/ssa/di_debug_test.go index 5732096564..fb0ca6c2cc 100644 --- a/ssa/di_debug_test.go +++ b/ssa/di_debug_test.go @@ -103,6 +103,96 @@ func inspect() { } } +func TestDebugParameterHomes(t *testing.T) { + for _, tc := range []struct { + name string + opt optlevel.Level + wantHomes bool + }{ + {"O0", optlevel.O0, true}, + {"O2", optlevel.O2, false}, + } { + t.Run(tc.name, func(t *testing.T) { + fset := token.NewFileSet() + file, err := parser.ParseFile(fset, "params.go", `package p +func inspect(first, second int) {} +`, 0) + if err != nil { + t.Fatal(err) + } + typesPkg, err := (&types.Config{}).Check("example.com/p", fset, []*ast.File{file}, nil) + if err != nil { + t.Fatal(err) + } + object := typesPkg.Scope().Lookup("inspect").(*types.Func) + signature := object.Type().(*types.Signature) + + prog := NewProgram(&Target{OptLevel: tc.opt}) + defer prog.Dispose() + prog.TypeSizes(types.SizesFor("gc", runtime.GOARCH)) + pkg := prog.NewPackage("p", "example.com/p") + pkg.InitDebug("p", "example.com/p", fset) + function := pkg.NewFunc("example.com/p.inspect", signature, InGo) + builder := function.MakeBody(1) + defer builder.Dispose() + decl := file.Decls[0].(*ast.FuncDecl) + builder.DebugFunction( + function, + object.Scope(), + fset.Position(object.Pos()), + fset.Position(decl.Body.Lbrace), + ) + + first := signature.Params().At(0) + firstPos := fset.Position(first.Pos()) + firstVar := builder.DIVarParam(function, firstPos, first.Name(), prog.Int(), 1) + home := builder.DIParamWithHome(first, function.Param(0), firstVar, function, firstPos, function.Block(0)) + if got := !home.IsNil(); got != tc.wantHomes { + t.Fatalf("stable parameter home: %v, want %v", got, tc.wantHomes) + } + if !home.IsNil() { + builder.DIStore(home, function.Param(0)) + } + + second := signature.Params().At(1) + secondPos := fset.Position(second.Pos()) + secondVar := builder.DIVarParam(function, secondPos, second.Name(), prog.Int(), 2) + builder.DIParam(second, function.Param(1), secondVar, function, secondPos, function.Block(0)) + builder.Return() + builder.EndBuild() + pkg.FinalizeDebug() + + if err := llvm.VerifyModule(pkg.Module(), llvm.ReturnStatusAction); err != nil { + t.Fatalf("parameter debug metadata is invalid: %v\n%s", err, pkg.Module().String()) + } + ir := pkg.Module().String() + hasDeclare := strings.Contains(ir, "#dbg_declare") + hasValue := strings.Contains(ir, "#dbg_value") + if hasDeclare != tc.wantHomes || hasValue == tc.wantHomes { + t.Fatalf("debug records: declare=%v value=%v, want homes=%v\n%s", + hasDeclare, hasValue, tc.wantHomes, ir) + } + if tc.wantHomes { + stores := 0 + for _, line := range strings.Split(ir, "\n") { + if strings.Contains(line, " load ") { + t.Fatalf("debug home emits an unused load: %s", line) + } + if strings.Contains(line, "store ") { + stores++ + if strings.Contains(line, "!dbg") { + t.Fatalf("debug home store has a source location: %s", line) + } + } + } + if stores < 3 { + t.Fatalf("found %d debug home stores, want at least 3\n%s", stores, ir) + } + } + }) + } +} + func TestDebugGoTypeEncodings(t *testing.T) { fset := token.NewFileSet() file, err := parser.ParseFile(fset, "types.go", `package p