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>

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")
 }
 

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
 }

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
+}

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
 }

misc/cgo/testshared/src/depBase/gccgo.go

@@ -1,5 +1,5 @@
 //+build gccgo
 
-package dep
+package depBase
 
 func ImplementedInAsm() {}

misc/cgo/testshared/src/depBase/stubs.go

@@ -1,5 +1,5 @@
 //+build !gccgo
 
-package dep
+package depBase
 
 func ImplementedInAsm()

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
 }

misc/cgo/testshared/src/exe3/exe3.go

@@ -0,0 +1,7 @@
+package main
+
+import "dep3"
+
+func main() {
+	dep3.D3()
+}

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