cmd/compile: fix DWARF inline debug issue with dead local vars

Fix a problem in DWARF inline debug generation relating to handling of
statically unreachable local variables. For a function such as:

    const always = true

    func HasDeadLocal() int {
      if always {
        return 9
      }
      x := new(Something)
      ...
      return x.y
    }

the variable "x" is placed onto the Dcl list for the function during
parsing, but the actual declaration node is deleted later on when
gc.Main invokes "deadcode". Later in the compile the DWARF code emits
an abstract function with "x" (since "x" was on the Dcl list at the
point of the inline), but the export data emitted does not contain
"x". This then creates clashing/inconsistant DWARF abstract function
DIEs later on if HasDeadLocal is inlined in somewhere else.

As a fix, the inliner now pruned away variables such as "x" when
creating a copy of the Dcl list as part of the inlining; this means
that both the export data generator and the DWARF emitter wind up
seeing a consistent picture.

Fixes #25459

Change-Id: I753dc4e9f9ec694340adba5f43c907ba8cc9badc
Reviewed-on: https://go-review.googlesource.com/114090
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>

diff --git a/src/cmd/compile/internal/gc/inl.go b/src/cmd/compile/internal/gc/inl.go
index 9001cd2e0d6731b85552aacb4ace0ea4ec72ce92..46fe87e8c3ddaa5d2796e69786bb940135414568 100644 (file)
--- a/src/cmd/compile/internal/gc/inl.go
+++ b/src/cmd/compile/internal/gc/inl.go
@@ -169,7 +169,20 @@ func caninl(fn *Node) {
                cc = 1 // this appears to yield better performance than 0.
        }
 
-       visitor := hairyVisitor{budget: inlineMaxBudget, extraCallCost: cc}
+       // At this point in the game the function we're looking at may
+       // have "stale" autos, vars that still appear in the Dcl list, but
+       // which no longer have any uses in the function body (due to
+       // elimination by deadcode). We'd like to exclude these dead vars
+       // when creating the "Inline.Dcl" field below; to accomplish this,
+       // the hairyVisitor below builds up a map of used/referenced
+       // locals, and we use this map to produce a pruned Inline.Dcl
+       // list. See issue 25249 for more context.
+
+       visitor := hairyVisitor{
+               budget:        inlineMaxBudget,
+               extraCallCost: cc,
+               usedLocals:    make(map[*Node]bool),
+       }
        if visitor.visitList(fn.Nbody) {
                reason = visitor.reason
                return
@@ -181,7 +194,7 @@ func caninl(fn *Node) {
 
        n.Func.Inl = &Inline{
                Cost: inlineMaxBudget - visitor.budget,
-               Dcl:  inlcopylist(n.Name.Defn.Func.Dcl),
+               Dcl:  inlcopylist(pruneUnusedAutos(n.Name.Defn.Func.Dcl, &visitor)),
                Body: inlcopylist(fn.Nbody.Slice()),
        }
 
@@ -245,6 +258,7 @@ type hairyVisitor struct {
        budget        int32
        reason        string
        extraCallCost int32
+       usedLocals    map[*Node]bool
 }
 
 // Look for anything we want to punt on.
@@ -374,6 +388,12 @@ func (v *hairyVisitor) visit(n *Node) bool {
                        return v.visitList(n.Ninit) || v.visitList(n.Nbody) ||
                                v.visitList(n.Rlist)
                }
+
+       case ONAME:
+               if n.Class() == PAUTO {
+                       v.usedLocals[n] = true
+               }
+
        }
 
        v.budget--
@@ -1250,3 +1270,16 @@ func (subst *inlsubst) updatedPos(xpos src.XPos) src.XPos {
        pos.SetBase(newbase)
        return Ctxt.PosTable.XPos(pos)
 }
+
+func pruneUnusedAutos(ll []*Node, vis *hairyVisitor) []*Node {
+       s := make([]*Node, 0, len(ll))
+       for _, n := range ll {
+               if n.Class() == PAUTO {
+                       if _, found := vis.usedLocals[n]; !found {
+                               continue
+                       }
+               }
+               s = append(s, n)
+       }
+       return s
+}

diff --git a/src/cmd/link/internal/ld/dwarf_test.go b/src/cmd/link/internal/ld/dwarf_test.go
index 17deec1735eb7f11e3505d31b741fdd589b2e41c..b4e328bc2a3136dbb273ce943414c442608c03e6 100644 (file)
--- a/src/cmd/link/internal/ld/dwarf_test.go
+++ b/src/cmd/link/internal/ld/dwarf_test.go
@@ -22,9 +22,9 @@ import (
 )
 
 const (
-       NoOpt        = "-gcflags=-l -N"
-       OptInl4      = "-gcflags=all=-l=4"
-       OptInl4DwLoc = "-gcflags=all=-l=4 -dwarflocationlists"
+       DefaultOpt = "-gcflags="
+       NoOpt      = "-gcflags=-l -N"
+       OptInl4    = "-gcflags=all=-l=4"
 )
 
 func TestRuntimeTypesPresent(t *testing.T) {
@@ -111,6 +111,38 @@ func gobuild(t *testing.T, dir string, testfile string, gcflags string) *builtFi
        return &builtFile{f, dst}
 }
 
+func envWithGoPathSet(gp string) []string {
+       env := os.Environ()
+       for i := 0; i < len(env); i++ {
+               if strings.HasPrefix(env[i], "GOPATH=") {
+                       env[i] = "GOPATH=" + gp
+                       return env
+               }
+       }
+       env = append(env, "GOPATH="+gp)
+       return env
+}
+
+// Similar to gobuild() above, but runs off a separate GOPATH environment
+
+func gobuildTestdata(t *testing.T, tdir string, gopathdir string, packtobuild string, gcflags string) *builtFile {
+       dst := filepath.Join(tdir, "out.exe")
+
+       // Run a build with an updated GOPATH
+       cmd := exec.Command(testenv.GoToolPath(t), "build", gcflags, "-o", dst, packtobuild)
+       cmd.Env = envWithGoPathSet(gopathdir)
+       if b, err := cmd.CombinedOutput(); err != nil {
+               t.Logf("build: %s\n", b)
+               t.Fatalf("build error: %v", err)
+       }
+
+       f, err := objfilepkg.Open(dst)
+       if err != nil {
+               t.Fatal(err)
+       }
+       return &builtFile{f, dst}
+}
+
 func TestEmbeddedStructMarker(t *testing.T) {
        testenv.MustHaveGoBuild(t)
 
@@ -684,29 +716,8 @@ func main() {
        }
 }
 
-func abstractOriginSanity(t *testing.T, flags string) {
-       // Nothing special about net/http here, this is just a convenient
-       // way to pull in a lot of code.
-       const prog = `
-package main
-
-import (
-       "net/http"
-       "net/http/httptest"
-)
-
-type statusHandler int
+func abstractOriginSanity(t *testing.T, gopathdir string, flags string) {
 
-func (h *statusHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
-       w.WriteHeader(int(*h))
-}
-
-func main() {
-       status := statusHandler(http.StatusNotFound)
-       s := httptest.NewServer(&status)
-       defer s.Close()
-}
-`
        dir, err := ioutil.TempDir("", "TestAbstractOriginSanity")
        if err != nil {
                t.Fatalf("could not create directory: %v", err)
@@ -714,7 +725,7 @@ func main() {
        defer os.RemoveAll(dir)
 
        // Build with inlining, to exercise DWARF inlining support.
-       f := gobuild(t, dir, prog, flags)
+       f := gobuildTestdata(t, dir, gopathdir, "main", flags)
 
        d, err := f.DWARF()
        if err != nil {
@@ -790,10 +801,15 @@ func TestAbstractOriginSanity(t *testing.T) {
                t.Skip("skipping on solaris and darwin, pending resolution of issue #23168")
        }
 
-       abstractOriginSanity(t, OptInl4)
+       if wd, err := os.Getwd(); err == nil {
+               gopathdir := filepath.Join(wd, "testdata", "httptest")
+               abstractOriginSanity(t, gopathdir, OptInl4)
+       } else {
+               t.Fatalf("os.Getwd() failed %v", err)
+       }
 }
 
-func TestAbstractOriginSanityWithLocationLists(t *testing.T) {
+func TestAbstractOriginSanityIssue25459(t *testing.T) {
        testenv.MustHaveGoBuild(t)
 
        if runtime.GOOS == "plan9" {
@@ -806,7 +822,12 @@ func TestAbstractOriginSanityWithLocationLists(t *testing.T) {
                t.Skip("skipping on not-amd64 not-x86; location lists not supported")
        }
 
-       abstractOriginSanity(t, OptInl4DwLoc)
+       if wd, err := os.Getwd(); err == nil {
+               gopathdir := filepath.Join(wd, "testdata", "issue25459")
+               abstractOriginSanity(t, gopathdir, DefaultOpt)
+       } else {
+               t.Fatalf("os.Getwd() failed %v", err)
+       }
 }
 
 func TestRuntimeTypeAttr(t *testing.T) {

diff --git a/src/cmd/link/internal/ld/testdata/httptest/src/main/main.go b/src/cmd/link/internal/ld/testdata/httptest/src/main/main.go
new file mode 100644 (file)
index 0000000..1bce301
--- /dev/null
+++ b/src/cmd/link/internal/ld/testdata/httptest/src/main/main.go
@@ -0,0 +1,22 @@
+// A small test program that uses the net/http package. There is
+// nothing special about net/http here, this is just a convenient way
+// to pull in a lot of code.
+
+package main
+
+import (
+       "net/http"
+       "net/http/httptest"
+)
+
+type statusHandler int
+
+func (h *statusHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
+       w.WriteHeader(int(*h))
+}
+
+func main() {
+       status := statusHandler(http.StatusNotFound)
+       s := httptest.NewServer(&status)
+       defer s.Close()
+}

diff --git a/src/cmd/link/internal/ld/testdata/issue25459/src/a/a.go b/src/cmd/link/internal/ld/testdata/issue25459/src/a/a.go
new file mode 100644 (file)
index 0000000..6032d76
--- /dev/null
+++ b/src/cmd/link/internal/ld/testdata/issue25459/src/a/a.go
@@ -0,0 +1,27 @@
+package a
+
+const Always = true
+
+var Count int
+
+type FuncReturningInt func() int
+
+var PointerToConstIf FuncReturningInt
+
+func ConstIf() int {
+       if Always {
+               return 1
+       }
+       var imdead [4]int
+       imdead[Count] = 1
+       return imdead[0]
+}
+
+func CallConstIf() int {
+       Count += 3
+       return ConstIf()
+}
+
+func Another() {
+       defer func() { PointerToConstIf = ConstIf; Count += 1 }()
+}

diff --git a/src/cmd/link/internal/ld/testdata/issue25459/src/main/main.go b/src/cmd/link/internal/ld/testdata/issue25459/src/main/main.go
new file mode 100644 (file)
index 0000000..be05f59
--- /dev/null
+++ b/src/cmd/link/internal/ld/testdata/issue25459/src/main/main.go
@@ -0,0 +1,10 @@
+package main
+
+import "a"
+
+var Glob int
+
+func main() {
+       a.Another()
+       Glob += a.ConstIf() + a.CallConstIf()
+}