From 41afd3af42bd8028a1740c30a2b745105b4063d2 Mon Sep 17 00:00:00 2001 From: Than McIntosh Date: Fri, 30 Apr 2021 11:15:44 -0400 Subject: [PATCH] cmd/compile: fix abbrev selection for output params In Cl 302071 we changed the compiler to use a different recipe for selecting the DWARF frame offset for output parameters, to reflect the fact that registerized output params don't have a stack memory location on entry to the function. In the process, however, we switched from using an abbrev pf DW_ABRV_PARAM to an abbrev of DW_ABRV_AUTO, which means that Delve can't recognize them correctly. To fix the problem, switch back to picking the correct abbrev entry, while leaving the new offset recipe intact. Updates #40724. Updates #45720. Change-Id: If721c9255bcd030177806576cde3450563f7a235 Reviewed-on: https://go-review.googlesource.com/c/go/+/315610 Trust: Than McIntosh Run-TryBot: Than McIntosh TryBot-Result: Go Bot Reviewed-by: Cherry Zhang --- src/cmd/compile/internal/dwarfgen/dwarf.go | 25 +++-- src/cmd/link/internal/ld/dwarf_test.go | 110 +++++++++++++++++++++ 2 files changed, 125 insertions(+), 10 deletions(-) diff --git a/src/cmd/compile/internal/dwarfgen/dwarf.go b/src/cmd/compile/internal/dwarfgen/dwarf.go index 09e1f12a05..422c7e66c4 100644 --- a/src/cmd/compile/internal/dwarfgen/dwarf.go +++ b/src/cmd/compile/internal/dwarfgen/dwarf.go @@ -268,23 +268,28 @@ func createSimpleVar(fnsym *obj.LSym, n *ir.Name) *dwarf.Var { var abbrev int var offs int64 - switch n.Class { - case ir.PPARAM, ir.PPARAMOUT: - if !n.IsOutputParamInRegisters() { - abbrev = dwarf.DW_ABRV_PARAM - offs = n.FrameOffset() + base.Ctxt.FixedFrameSize() - break - } - fallthrough - case ir.PAUTO: + localAutoOffset := func() int64 { offs = n.FrameOffset() - abbrev = dwarf.DW_ABRV_AUTO if base.Ctxt.FixedFrameSize() == 0 { offs -= int64(types.PtrSize) } if buildcfg.FramePointerEnabled { offs -= int64(types.PtrSize) } + return offs + } + + switch n.Class { + case ir.PAUTO: + offs = localAutoOffset() + abbrev = dwarf.DW_ABRV_AUTO + case ir.PPARAM, ir.PPARAMOUT: + abbrev = dwarf.DW_ABRV_PARAM + if n.IsOutputParamInRegisters() { + offs = localAutoOffset() + } else { + offs = n.FrameOffset() + base.Ctxt.FixedFrameSize() + } default: base.Fatalf("createSimpleVar unexpected class %v for node %v", n.Class, n) diff --git a/src/cmd/link/internal/ld/dwarf_test.go b/src/cmd/link/internal/ld/dwarf_test.go index 56dc4753b2..5cc4800e2a 100644 --- a/src/cmd/link/internal/ld/dwarf_test.go +++ b/src/cmd/link/internal/ld/dwarf_test.go @@ -19,6 +19,7 @@ import ( "path/filepath" "reflect" "runtime" + "sort" "strconv" "strings" "testing" @@ -1641,3 +1642,112 @@ func TestIssue42484(t *testing.T) { } f.Close() } + +func TestOutputParamAbbrevAndAttr(t *testing.T) { + testenv.MustHaveGoBuild(t) + + if runtime.GOOS == "plan9" { + t.Skip("skipping on plan9; no DWARF symbol table in executables") + } + t.Parallel() + + // This test verifies that the compiler is selecting the correct + // DWARF abbreviation for output parameters, and that the + // variable parameter attribute is correct for in-params and + // out-params. + + const prog = ` +package main + +//go:noinline +func ABC(p1, p2, p3 int, f1, f2, f3 float32, b1 [1024]int) (r1 int, r2 int, r3 [1024]int, r4 byte) { + b1[0] = 6 + r1, r2, r3, r4 = p3, p2, b1, 'a' + return +} + +func main() { + a := [1024]int{} + v1, v2, v3, v4 := ABC(1, 2, 3, 1.0, 2.0, 1.0, a) + println(v1, v2, v3[0], v4) +} +` + dir := t.TempDir() + f := gobuild(t, dir, prog, NoOpt) + defer f.Close() + + d, err := f.DWARF() + if err != nil { + t.Fatalf("error reading DWARF: %v", err) + } + + rdr := d.Reader() + ex := examiner{} + if err := ex.populate(rdr); err != nil { + t.Fatalf("error reading DWARF: %v", err) + } + + // Locate the main.ABC DIE + abcs := ex.Named("main.ABC") + if len(abcs) == 0 { + t.Fatalf("unable to locate DIE for main.ABC") + } + if len(abcs) != 1 { + t.Fatalf("more than one main.ABC DIE") + } + abcdie := abcs[0] + + // Vet the DIE + if abcdie.Tag != dwarf.TagSubprogram { + t.Fatalf("unexpected tag %v on main.ABC DIE", abcdie.Tag) + } + + // A setting of DW_AT_variable_parameter indicates that the + // param in question is an output parameter; we want to see this + // attribute set to TRUE for all Go return params. It would be + // OK to have it missing for input parameters, but for the moment + // we verify that the attr is present but set to false. + + // Values in this map: + // + // 0: + // -1: varparm attr not found + // 1: varparm found with value false + // 2: varparm found with value true + // + foundParams := make(map[string]int) + + // Walk ABCs's children looking for params. + abcIdx := ex.idxFromOffset(abcdie.Offset) + childDies := ex.Children(abcIdx) + for _, child := range childDies { + if child.Tag == dwarf.TagFormalParameter { + st := -1 + if vp, ok := child.Val(dwarf.AttrVarParam).(bool); ok { + if vp { + st = 2 + } else { + st = 1 + } + } + if name, ok := child.Val(dwarf.AttrName).(string); ok { + foundParams[name] = st + } + } + } + + // Digest the result. + found := make([]string, 0, len(foundParams)) + for k, v := range foundParams { + found = append(found, fmt.Sprintf("%s:%d", k, v)) + } + sort.Strings(found) + + // Make sure we see all of the expected params, that they have + // the varparam attr, and the varparm is set for the returns. + expected := "[b1:1 f1:1 f2:1 f3:1 p1:1 p2:1 p3:1 r1:2 r2:2 r3:2 r4:2]" + if fmt.Sprintf("%+v", found) != expected { + t.Errorf("param check failed, wanted %s got %s\n", + expected, found) + } +} -- 2.48.1