cmd/link: always read type data for dynimport symbols

Consider three shared libraries:

 libBase.so -- defines a type T
 lib2.so    -- references type T
 lib3.so    -- also references type T, and something from lib2

lib2.so will contain a type symbol for T in its symbol table, but no
definition. If, when linking lib3.so the linker reads the symbols from lib2.so
before libBase.so, the linker didn't read the type data and later crashed.

The fix is trivial but the test change is a bit messy because the order the
linker reads the shared libraries in ends up depending on the order of the
import statements in the file so I had to rename one of the test packages so
that gofmt doesn't fix the test by accident...

Fixes #15516

Change-Id: I124b058f782c900a3a54c15ed66a0d91d0cde5ce
Reviewed-on: https://go-review.googlesource.com/22744
Run-TryBot: Michael Hudson-Doyle <michael.hudson@canonical.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>

diff --git a/misc/cgo/testshared/shared_test.go b/misc/cgo/testshared/shared_test.go
index 5c063963a0364b7e8ded7037097e7c9a3929fbb3..34d97de526d8328f7e428e02d27dee37f2a73c9c 100644 (file)
--- a/misc/cgo/testshared/shared_test.go
+++ b/misc/cgo/testshared/shared_test.go
@@ -135,7 +135,7 @@ func testMain(m *testing.M) (int, error) {
        goCmd(nil, append([]string{"install", "-buildmode=shared"}, minpkgs...)...)
 
        myContext.InstallSuffix = suffix + "_dynlink"
-       depP, err := myContext.Import("dep", ".", build.ImportComment)
+       depP, err := myContext.Import("depBase", ".", build.ImportComment)
        if err != nil {
                return 0, fmt.Errorf("import failed: %v", err)
        }
@@ -416,11 +416,11 @@ func TestCgoPIE(t *testing.T) {
 // Build a GOPATH package into a shared library that links against the goroot runtime
 // and an executable that links against both.
 func TestGopathShlib(t *testing.T) {
-       goCmd(t, "install", "-buildmode=shared", "-linkshared", "dep")
-       AssertIsLinkedTo(t, filepath.Join(gopathInstallDir, "libdep.so"), soname)
+       goCmd(t, "install", "-buildmode=shared", "-linkshared", "depBase")
+       AssertIsLinkedTo(t, filepath.Join(gopathInstallDir, "libdepBase.so"), soname)
        goCmd(t, "install", "-linkshared", "exe")
        AssertIsLinkedTo(t, "./bin/exe", soname)
-       AssertIsLinkedTo(t, "./bin/exe", "libdep.so")
+       AssertIsLinkedTo(t, "./bin/exe", "libdepBase.so")
        AssertHasRPath(t, "./bin/exe", gorootInstallDir)
        AssertHasRPath(t, "./bin/exe", gopathInstallDir)
        // And check it runs.
@@ -436,7 +436,7 @@ func testPkgListNote(t *testing.T, f *elf.File, note *note) {
        if isOffsetLoaded(f, note.section.Offset) {
                t.Errorf("package list section contained in PT_LOAD segment")
        }
-       if note.desc != "dep\n" {
+       if note.desc != "depBase\n" {
                t.Errorf("incorrect package list %q", note.desc)
        }
 }
@@ -486,7 +486,7 @@ func testDepsNote(t *testing.T, f *elf.File, note *note) {
        if isOffsetLoaded(f, note.section.Offset) {
                t.Errorf("package list section contained in PT_LOAD segment")
        }
-       // libdep.so just links against the lib containing the runtime.
+       // libdepBase.so just links against the lib containing the runtime.
        if note.desc != soname {
                t.Errorf("incorrect dependency list %q", note.desc)
        }
@@ -494,8 +494,8 @@ func testDepsNote(t *testing.T, f *elf.File, note *note) {
 
 // The shared library contains notes with defined contents; see above.
 func TestNotes(t *testing.T) {
-       goCmd(t, "install", "-buildmode=shared", "-linkshared", "dep")
-       f, err := elf.Open(filepath.Join(gopathInstallDir, "libdep.so"))
+       goCmd(t, "install", "-buildmode=shared", "-linkshared", "depBase")
+       f, err := elf.Open(filepath.Join(gopathInstallDir, "libdepBase.so"))
        if err != nil {
                t.Fatal(err)
        }
@@ -543,16 +543,24 @@ func TestNotes(t *testing.T) {
        }
 }
 
-// Build a GOPATH package (dep) into a shared library that links against the goroot
+// Build a GOPATH package (depBase) into a shared library that links against the goroot
 // runtime, another package (dep2) that links against the first, and and an
 // executable that links against dep2.
 func TestTwoGopathShlibs(t *testing.T) {
-       goCmd(t, "install", "-buildmode=shared", "-linkshared", "dep")
+       goCmd(t, "install", "-buildmode=shared", "-linkshared", "depBase")
        goCmd(t, "install", "-buildmode=shared", "-linkshared", "dep2")
        goCmd(t, "install", "-linkshared", "exe2")
        run(t, "executable linked to GOPATH library", "./bin/exe2")
 }
 
+func TestThreeGopathShlibs(t *testing.T) {
+       goCmd(t, "install", "-buildmode=shared", "-linkshared", "depBase")
+       goCmd(t, "install", "-buildmode=shared", "-linkshared", "dep2")
+       goCmd(t, "install", "-buildmode=shared", "-linkshared", "dep3")
+       goCmd(t, "install", "-linkshared", "exe3")
+       run(t, "executable linked to GOPATH library", "./bin/exe3")
+}
+
 // If gccgo is not available or not new enough call t.Skip. Otherwise,
 // return a build.Context that is set up for gccgo.
 func prepGccgo(t *testing.T) build.Context {
@@ -586,16 +594,16 @@ func TestGoPathShlibGccgo(t *testing.T) {
 
        libgoRE := regexp.MustCompile("libgo.so.[0-9]+")
 
-       depP, err := gccgoContext.Import("dep", ".", build.ImportComment)
+       depP, err := gccgoContext.Import("depBase", ".", build.ImportComment)
        if err != nil {
                t.Fatalf("import failed: %v", err)
        }
        gccgoInstallDir := filepath.Join(depP.PkgTargetRoot, "shlibs")
-       goCmd(t, "install", "-compiler=gccgo", "-buildmode=shared", "-linkshared", "dep")
-       AssertIsLinkedToRegexp(t, filepath.Join(gccgoInstallDir, "libdep.so"), libgoRE)
+       goCmd(t, "install", "-compiler=gccgo", "-buildmode=shared", "-linkshared", "depBase")
+       AssertIsLinkedToRegexp(t, filepath.Join(gccgoInstallDir, "libdepBase.so"), libgoRE)
        goCmd(t, "install", "-compiler=gccgo", "-linkshared", "exe")
        AssertIsLinkedToRegexp(t, "./bin/exe", libgoRE)
-       AssertIsLinkedTo(t, "./bin/exe", "libdep.so")
+       AssertIsLinkedTo(t, "./bin/exe", "libdepBase.so")
        AssertHasRPath(t, "./bin/exe", gccgoInstallDir)
        // And check it runs.
        run(t, "gccgo-built", "./bin/exe")
@@ -609,21 +617,21 @@ func TestTwoGopathShlibsGccgo(t *testing.T) {
 
        libgoRE := regexp.MustCompile("libgo.so.[0-9]+")
 
-       depP, err := gccgoContext.Import("dep", ".", build.ImportComment)
+       depP, err := gccgoContext.Import("depBase", ".", build.ImportComment)
        if err != nil {
                t.Fatalf("import failed: %v", err)
        }
        gccgoInstallDir := filepath.Join(depP.PkgTargetRoot, "shlibs")
-       goCmd(t, "install", "-compiler=gccgo", "-buildmode=shared", "-linkshared", "dep")
+       goCmd(t, "install", "-compiler=gccgo", "-buildmode=shared", "-linkshared", "depBase")
        goCmd(t, "install", "-compiler=gccgo", "-buildmode=shared", "-linkshared", "dep2")
        goCmd(t, "install", "-compiler=gccgo", "-linkshared", "exe2")
 
-       AssertIsLinkedToRegexp(t, filepath.Join(gccgoInstallDir, "libdep.so"), libgoRE)
+       AssertIsLinkedToRegexp(t, filepath.Join(gccgoInstallDir, "libdepBase.so"), libgoRE)
        AssertIsLinkedToRegexp(t, filepath.Join(gccgoInstallDir, "libdep2.so"), libgoRE)
-       AssertIsLinkedTo(t, filepath.Join(gccgoInstallDir, "libdep2.so"), "libdep.so")
+       AssertIsLinkedTo(t, filepath.Join(gccgoInstallDir, "libdep2.so"), "libdepBase.so")
        AssertIsLinkedToRegexp(t, "./bin/exe2", libgoRE)
        AssertIsLinkedTo(t, "./bin/exe2", "libdep2")
-       AssertIsLinkedTo(t, "./bin/exe2", "libdep.so")
+       AssertIsLinkedTo(t, "./bin/exe2", "libdepBase.so")
 
        // And check it runs.
        run(t, "gccgo-built", "./bin/exe2")
@@ -690,22 +698,22 @@ func AssertNotRebuilt(t *testing.T, msg, path string) {
 }
 
 func TestRebuilding(t *testing.T) {
-       goCmd(t, "install", "-buildmode=shared", "-linkshared", "dep")
+       goCmd(t, "install", "-buildmode=shared", "-linkshared", "depBase")
        goCmd(t, "install", "-linkshared", "exe")
 
        // If the source is newer than both the .a file and the .so, both are rebuilt.
        resetFileStamps()
-       touch("src/dep/dep.go")
+       touch("src/depBase/dep.go")
        goCmd(t, "install", "-linkshared", "exe")
-       AssertRebuilt(t, "new source", filepath.Join(gopathInstallDir, "dep.a"))
-       AssertRebuilt(t, "new source", filepath.Join(gopathInstallDir, "libdep.so"))
+       AssertRebuilt(t, "new source", filepath.Join(gopathInstallDir, "depBase.a"))
+       AssertRebuilt(t, "new source", filepath.Join(gopathInstallDir, "libdepBase.so"))
 
        // If the .a file is newer than the .so, the .so is rebuilt (but not the .a)
        resetFileStamps()
-       touch(filepath.Join(gopathInstallDir, "dep.a"))
+       touch(filepath.Join(gopathInstallDir, "depBase.a"))
        goCmd(t, "install", "-linkshared", "exe")
-       AssertNotRebuilt(t, "new .a file", filepath.Join(gopathInstallDir, "dep.a"))
-       AssertRebuilt(t, "new .a file", filepath.Join(gopathInstallDir, "libdep.so"))
+       AssertNotRebuilt(t, "new .a file", filepath.Join(gopathInstallDir, "depBase.a"))
+       AssertRebuilt(t, "new .a file", filepath.Join(gopathInstallDir, "libdepBase.so"))
 }
 
 func appendFile(path, content string) {
@@ -726,17 +734,17 @@ func appendFile(path, content string) {
 }
 
 func TestABIChecking(t *testing.T) {
-       goCmd(t, "install", "-buildmode=shared", "-linkshared", "dep")
+       goCmd(t, "install", "-buildmode=shared", "-linkshared", "depBase")
        goCmd(t, "install", "-linkshared", "exe")
 
-       // If we make an ABI-breaking change to dep and rebuild libp.so but not exe,
+       // If we make an ABI-breaking change to depBase and rebuild libp.so but not exe,
        // exe will abort with a complaint on startup.
        // This assumes adding an exported function breaks ABI, which is not true in
        // some senses but suffices for the narrow definition of ABI compatibility the
        // toolchain uses today.
        resetFileStamps()
-       appendFile("src/dep/dep.go", "func ABIBreak() {}\n")
-       goCmd(t, "install", "-buildmode=shared", "-linkshared", "dep")
+       appendFile("src/depBase/dep.go", "func ABIBreak() {}\n")
+       goCmd(t, "install", "-buildmode=shared", "-linkshared", "depBase")
        c := exec.Command("./bin/exe")
        output, err := c.CombinedOutput()
        if err == nil {
@@ -744,7 +752,7 @@ func TestABIChecking(t *testing.T) {
        }
        scanner := bufio.NewScanner(bytes.NewReader(output))
        foundMsg := false
-       const wantLine = "abi mismatch detected between the executable and libdep.so"
+       const wantLine = "abi mismatch detected between the executable and libdepBase.so"
        for scanner.Scan() {
                if scanner.Text() == wantLine {
                        foundMsg = true
@@ -763,10 +771,10 @@ func TestABIChecking(t *testing.T) {
        run(t, "rebuilt exe", "./bin/exe")
 
        // If we make a change which does not break ABI (such as adding an unexported
-       // function) and rebuild libdep.so, exe still works.
+       // function) and rebuild libdepBase.so, exe still works.
        resetFileStamps()
-       appendFile("src/dep/dep.go", "func noABIBreak() {}\n")
-       goCmd(t, "install", "-buildmode=shared", "-linkshared", "dep")
+       appendFile("src/depBase/dep.go", "func noABIBreak() {}\n")
+       goCmd(t, "install", "-buildmode=shared", "-linkshared", "depBase")
        run(t, "after non-ABI breaking change", "./bin/exe")
 }
 

diff --git a/misc/cgo/testshared/src/dep2/dep2.go b/misc/cgo/testshared/src/dep2/dep2.go
index bac1086a4a6760fd19419e613094be989492a70d..c2c812adb9b4311cfba33f87de876d3d738de7ef 100644 (file)
--- a/misc/cgo/testshared/src/dep2/dep2.go
+++ b/misc/cgo/testshared/src/dep2/dep2.go
@@ -1,11 +1,15 @@
 package dep2
 
-import "dep"
+import "depBase"
 
 var W int = 1
 
-var hasProg dep.HasProg
+var hasProg depBase.HasProg
+
+type Dep2 struct {
+       depBase.Dep
+}
 
 func G() int {
-       return dep.F() + 1
+       return depBase.F() + 1
 }

diff --git a/misc/cgo/testshared/src/dep3/dep3.go b/misc/cgo/testshared/src/dep3/dep3.go
new file mode 100644 (file)
index 0000000..7b7c9da
--- /dev/null
+++ b/misc/cgo/testshared/src/dep3/dep3.go
@@ -0,0 +1,22 @@
+package dep3
+
+// The point of this test file is that it references a type from
+// depBase that is also referenced in dep2, but dep2 is loaded by the
+// linker before depBase (because it is earlier in the import list).
+// There was a bug in the linker where it would not correctly read out
+// the type data in this case and later crash.
+
+import (
+       "dep2"
+       "depBase"
+)
+
+type Dep3 struct {
+       dep  depBase.Dep
+       dep2 dep2.Dep2
+}
+
+func D3() int {
+       var x Dep3
+       return x.dep.X + x.dep2.X
+}

diff --git a/misc/cgo/testshared/src/dep/asm.s b/misc/cgo/testshared/src/depBase/asm.s
similarity index 100%
rename from misc/cgo/testshared/src/dep/asm.s
rename to misc/cgo/testshared/src/depBase/asm.s

diff --git a/misc/cgo/testshared/src/dep/dep.go b/misc/cgo/testshared/src/depBase/dep.go
similarity index 74%
rename from misc/cgo/testshared/src/dep/dep.go
rename to misc/cgo/testshared/src/depBase/dep.go
index d3bed3f8ff501c409fb1f468406e08dc49071a8a..f9d3d7ce3a948e6d631537f29d17fe5957592fa7 100644 (file)
--- a/misc/cgo/testshared/src/dep/dep.go
+++ b/misc/cgo/testshared/src/depBase/dep.go
@@ -1,4 +1,4 @@
-package dep
+package depBase
 
 var V int = 1
 
@@ -8,6 +8,10 @@ type HasProg struct {
        array [1024]*byte
 }
 
+type Dep struct {
+       X int
+}
+
 func F() int {
        return V
 }

diff --git a/misc/cgo/testshared/src/dep/gccgo.go b/misc/cgo/testshared/src/depBase/gccgo.go
similarity index 73%
rename from misc/cgo/testshared/src/dep/gccgo.go
rename to misc/cgo/testshared/src/depBase/gccgo.go
index 552ec303faf92dea8576af4ec78559c65ffd7da4..3e2b69b50bc2b0c34fd8ff7b217ac541077deef5 100644 (file)
--- a/misc/cgo/testshared/src/dep/gccgo.go
+++ b/misc/cgo/testshared/src/depBase/gccgo.go
@@ -1,5 +1,5 @@
 //+build gccgo
 
-package dep
+package depBase
 
 func ImplementedInAsm() {}

diff --git a/misc/cgo/testshared/src/dep/stubs.go b/misc/cgo/testshared/src/depBase/stubs.go
similarity index 72%
rename from misc/cgo/testshared/src/dep/stubs.go
rename to misc/cgo/testshared/src/depBase/stubs.go
index 036296a2fc67725deb7710e043ad3efae7c0eb8b..96573c12ececb0950a28ea77e11999a882f1e85b 100644 (file)
--- a/misc/cgo/testshared/src/dep/stubs.go
+++ b/misc/cgo/testshared/src/depBase/stubs.go
@@ -1,5 +1,5 @@
 //+build !gccgo
 
-package dep
+package depBase
 
 func ImplementedInAsm()

diff --git a/misc/cgo/testshared/src/exe/exe.go b/misc/cgo/testshared/src/exe/exe.go
index f64477613a853c74664e190b05c60fa377a42a0d..136803fbc1d496f4cffb788b7773a6cd62a217ab 100644 (file)
--- a/misc/cgo/testshared/src/exe/exe.go
+++ b/misc/cgo/testshared/src/exe/exe.go
@@ -1,12 +1,12 @@
 package main
 
 import (
-       "dep"
+       "depBase"
        "runtime"
 )
 
 func main() {
-       defer dep.ImplementedInAsm()
+       defer depBase.ImplementedInAsm()
        runtime.GC()
-       dep.V = dep.F() + 1
+       depBase.V = depBase.F() + 1
 }

diff --git a/misc/cgo/testshared/src/exe3/exe3.go b/misc/cgo/testshared/src/exe3/exe3.go
new file mode 100644 (file)
index 0000000..643f260
--- /dev/null
+++ b/misc/cgo/testshared/src/exe3/exe3.go
@@ -0,0 +1,7 @@
+package main
+
+import "dep3"
+
+func main() {
+       dep3.D3()
+}

diff --git a/src/cmd/link/internal/ld/lib.go b/src/cmd/link/internal/ld/lib.go
index 0516ff60cf212610ecfac05985764df3d9b1bef9..ffad820aff6c49f8ff0383d42c8ca9ffe6068261 100644 (file)
--- a/src/cmd/link/internal/ld/lib.go
+++ b/src/cmd/link/internal/ld/lib.go
@@ -1522,9 +1522,10 @@ func ldshlibsyms(shlib string) {
                }
                lsym := Linklookup(Ctxt, elfsym.Name, 0)
                // Because loadlib above loads all .a files before loading any shared
-               // libraries, any symbols we find that duplicate symbols already
-               // loaded should be ignored (the symbols from the .a files "win").
-               if lsym.Type != 0 {
+               // libraries, any non-dynimport symbols we find that duplicate symbols
+               // already loaded should be ignored (the symbols from the .a files
+               // "win").
+               if lsym.Type != 0 && lsym.Type != obj.SDYNIMPORT {
                        continue
                }
                lsym.Type = obj.SDYNIMPORT