cmd/compile/internal/abi: fix ComputePadding

Fixes the ComputePadding calculation to take into account
the padding added for the current offset. This fixes an issue
where padding can be added incorrectly for certain structs.

Related: https://github.com/go-delve/delve/issues/3923

Fixes #72053

Change-Id: I277629799168c6b44bc9ed03df4345e0318064ce
GitHub-Last-Rev: 9478b29a137e20421ad348bb93a54406b1977008
GitHub-Pull-Request: golang/go#72805
Reviewed-on: https://go-review.googlesource.com/c/go/+/656736
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Keith Randall <khr@google.com>

diff --git a/src/cmd/compile/internal/abi/abiutils.go b/src/cmd/compile/internal/abi/abiutils.go
index e88a80d564623d2030af0c812e2dcc3583bbb755..c013aba19c41a61ef9d14432139f50fe91a0b0e0 100644 (file)
--- a/src/cmd/compile/internal/abi/abiutils.go
+++ b/src/cmd/compile/internal/abi/abiutils.go
@@ -673,10 +673,9 @@ func (pa *ABIParamAssignment) ComputePadding(storage []uint64) []uint64 {
                panic("internal error")
        }
        offsets, _ := appendParamOffsets([]int64{}, 0, pa.Type)
-       off := int64(0)
        for idx, t := range types {
                ts := t.Size()
-               off += int64(ts)
+               off := offsets[idx] + ts
                if idx < len(types)-1 {
                        noff := offsets[idx+1]
                        if noff != off {

diff --git a/src/cmd/compile/internal/ssa/debug.go b/src/cmd/compile/internal/ssa/debug.go
index 59d029426443d44ee0689ee9a835bb18d1dc609c..6faef7c25592e66582406014a145de50edd3757f 100644 (file)
--- a/src/cmd/compile/internal/ssa/debug.go
+++ b/src/cmd/compile/internal/ssa/debug.go
@@ -557,7 +557,7 @@ func PopulateABIInRegArgOps(f *Func) {
        f.Entry.Values = append(newValues, f.Entry.Values...)
 }
 
-// BuildFuncDebug debug information for f, placing the results
+// BuildFuncDebug builds debug information for f, placing the results
 // in "rval". f must be fully processed, so that each Value is where it
 // will be when machine code is emitted.
 func BuildFuncDebug(ctxt *obj.Link, f *Func, loggingLevel int, stackOffset func(LocalSlot) int32, rval *FuncDebug) {

diff --git a/src/cmd/compile/internal/test/abiutils_test.go b/src/cmd/compile/internal/test/abiutils_test.go
index b500de9f18a872aa0e2535e942ad0333571ffac3..da807f5a0aab4fee51ace7a58055cd905692ecde 100644 (file)
--- a/src/cmd/compile/internal/test/abiutils_test.go
+++ b/src/cmd/compile/internal/test/abiutils_test.go
@@ -390,9 +390,9 @@ func TestABIUtilsComputePadding(t *testing.T) {
        padding := make([]uint64, 32)
        parm := regRes.InParams()[1]
        padding = parm.ComputePadding(padding)
-       want := "[1 1 1 0]"
+       want := "[1 0 0 0]"
        got := fmt.Sprintf("%+v", padding)
        if got != want {
-               t.Errorf("padding mismatch: wanted %q got %q\n", got, want)
+               t.Errorf("padding mismatch: wanted %q got %q\n", want, got)
        }
 }

diff --git a/src/cmd/link/internal/ld/dwarf_test.go b/src/cmd/link/internal/ld/dwarf_test.go
index 28b5ddf74c70caad4f26165f10332c9bd29dee12..99bf1a177852cd2776526c9c8cff97ad44ce4ffd 100644 (file)
--- a/src/cmd/link/internal/ld/dwarf_test.go
+++ b/src/cmd/link/internal/ld/dwarf_test.go
@@ -5,7 +5,9 @@
 package ld
 
 import (
+       "bytes"
        "debug/dwarf"
+       "debug/elf"
        "debug/pe"
        "fmt"
        "internal/platform"
@@ -2042,3 +2044,114 @@ func TestConsistentGoKindAndRuntimeType(t *testing.T) {
                t.Logf("%d types checked\n", typesChecked)
        }
 }
+
+func TestIssue72053(t *testing.T) {
+       if runtime.GOOS != "linux" || runtime.GOARCH != "amd64" {
+               t.Skip("skipping test: requires ELF binary and amd64 arch")
+       }
+
+       testenv.MustHaveGoBuild(t)
+
+       mustHaveDWARF(t)
+
+       t.Parallel()
+
+       dir := t.TempDir()
+
+       const prog = `package main
+
+import (
+               "fmt"
+               "strings"
+)
+
+func main() {
+               u := Address{Addr: "127.0.0.1"}
+               fmt.Println(u) // line 10
+}
+
+type Address struct {
+               TLS  bool
+               Addr string
+}
+
+func (a Address) String() string {
+               sb := new(strings.Builder)
+               sb.WriteString(a.Addr)
+               return sb.String()
+}
+`
+
+       bf := gobuild(t, dir, prog, NoOpt)
+
+       defer bf.Close()
+
+       f, err := elf.Open(bf.path)
+       if err != nil {
+               t.Fatal(err)
+       }
+
+       dwrf, err := f.DWARF()
+       if err != nil {
+               t.Fatal(err)
+       }
+
+       rdr := dwrf.Reader()
+
+       found := false
+       for {
+               e, err := rdr.Next()
+               if err != nil {
+                       t.Fatal(err)
+               }
+               if e == nil {
+                       break
+               }
+
+               name, _ := e.Val(dwarf.AttrName).(string)
+
+               if e.Tag == dwarf.TagSubprogram && name == "main.Address.String" {
+                       found = true
+                       continue
+               }
+
+               if found && name == "a" {
+                       loc := e.AttrField(dwarf.AttrLocation)
+                       if loc != nil {
+                               switch loc.Class {
+                               case dwarf.ClassLocListPtr:
+                                       offset := loc.Val.(int64)
+                                       buf := make([]byte, 32)
+                                       s := f.Section(".debug_loc")
+                                       if s == nil {
+                                               t.Fatal("could not find debug_loc section")
+                                       }
+                                       d := s.Open()
+                                       // Seek past the first 16 bytes which establishes the base address and
+                                       // can be brittle and unreliable in the test due to compiler changes or DWARF
+                                       // version used.
+                                       d.Seek(offset+16, io.SeekStart)
+                                       d.Read(buf)
+
+                                       // DW_OP_reg0 DW_OP_piece 0x1 DW_OP_piece 0x7 DW_OP_reg3 DW_OP_piece 0x8 DW_OP_reg2 DW_OP_piece 0x8
+                                       expected := []byte{
+                                               0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+                                               0x1f, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+                                               0x0b, 0x00, 0x50, 0x93, 0x01, 0x93, 0x07, 0x53,
+                                               0x93, 0x08, 0x52, 0x93, 0x08, 0x1f, 0x00, 0x00,
+                                       }
+
+                                       if !bytes.Equal(buf, expected) {
+                                               t.Fatalf("unexpected DWARF sequence found, expected:\n%#v\nfound:\n%#v\n", expected, buf)
+                                       }
+                               }
+                       } else {
+                               t.Fatal("unable to find expected DWARF location list")
+                       }
+                       break
+               }
+       }
+       if !found {
+               t.Fatal("unable to find expected DWARF location list")
+       }
+}