cmd/link: do not prefix external symbols with underscore on windows/386/cgo

CL 18057 added underscore to most external pe symbols
on windows/386/cgo. The CL changed runtime.epclntab and
runtime.pclntab pe symbols into _runtime.pclntab and
_runtime.epclntab, and now cmd/nm cannot find them.
Revert correspondent CL 18057 changes, because most pe
symbols do not need underscore prefix.

This CL also removes code that added obj.SHOSTOBJ symbols
explicitly, because each of those was also added via
genasmsym call. These created duplicate pe symbols (like
_GetProcAddress@8 and __GetProcAddress@8), and external
linker would complain.

This CL adds new test in cmd/nm to verify go programs
built with cgo.

Fixes #18416

Change-Id: I68b1be8fb631d95ec69bd485c77c79604fb23f26
Reviewed-on: https://go-review.googlesource.com/35076Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

src/cmd/link/internal/ld/pe.go

@@ -941,12 +941,13 @@ func writePESymTableRecords(ctxt *Link) int {
 		case DataSym, BSSSym, TextSym, UndefinedSym:
 		}
 
-		// only windows/386 requires underscore prefix on external symbols
+		// Only windows/386 requires underscore prefix on external symbols.
+		// Include .text symbol as external, because .ctors section relocations refer to it.
 		if SysArch.Family == sys.I386 &&
 			Linkmode == LinkExternal &&
-			(s.Type != obj.SDYNIMPORT || s.Attr.CgoExport()) &&
-			s.Name == s.Extname &&
-			s.Name != "_main" {
+			(s.Type == obj.SHOSTOBJ ||
+				s.Attr.CgoExport() ||
+				s.Name == ".text") {
 			s.Name = "_" + s.Name
 		}
 
@@ -997,13 +998,6 @@ func writePESymTableRecords(ctxt *Link) int {
 	}
 
 	if Linkmode == LinkExternal {
-		for d := dr; d != nil; d = d.next {
-			for m := d.ms; m != nil; m = m.next {
-				s := m.s.R[0].Xsym
-				put(ctxt, s, s.Name, UndefinedSym, 0, nil)
-			}
-		}
-
 		s := ctxt.Syms.Lookup(".text", 0)
 		if s.Type == obj.STEXT {
 			put(ctxt, s, s.Name, TextSym, s.Value, nil)

src/cmd/nm/nm_cgo_test.go

+// Copyright 2017 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// +build cgo
+
+package main
+
+import (
+	"testing"
+)
+
+func TestInternalLinkerCgoFile(t *testing.T) {
+	testGoFile(t, true, false)
+}
+
+func TestExternalLinkerCgoFile(t *testing.T) {
+	testGoFile(t, true, true)
+}

src/cmd/nm/nm_test.go

@@ -16,60 +16,45 @@ import (
 	"runtime"
 	"strings"
 	"testing"
+	"text/template"
 )
 
-var testData uint32
+var testnmpath string // path to nm command created for testing purposes
 
-func checkSymbols(t *testing.T, nmoutput []byte) {
-	var checkSymbolsFound, testDataFound bool
-	scanner := bufio.NewScanner(bytes.NewBuffer(nmoutput))
-	for scanner.Scan() {
-		f := strings.Fields(scanner.Text())
-		if len(f) < 3 {
-			continue
-		}
-		switch f[2] {
-		case "cmd/nm.checkSymbols":
-			checkSymbolsFound = true
-			addr := "0x" + f[0]
-			if addr != fmt.Sprintf("%p", checkSymbols) {
-				t.Errorf("nm shows wrong address %v for checkSymbols (%p)", addr, checkSymbols)
-			}
-		case "cmd/nm.testData":
-			testDataFound = true
-			addr := "0x" + f[0]
-			if addr != fmt.Sprintf("%p", &testData) {
-				t.Errorf("nm shows wrong address %v for testData (%p)", addr, &testData)
-			}
-		}
-	}
-	if err := scanner.Err(); err != nil {
-		t.Errorf("error while reading symbols: %v", err)
-		return
-	}
-	if !checkSymbolsFound {
-		t.Error("nm shows no checkSymbols symbol")
-	}
-	if !testDataFound {
-		t.Error("nm shows no testData symbol")
-	}
+// The TestMain function creates a nm command for testing purposes and
+// deletes it after the tests have been run.
+func TestMain(m *testing.M) {
+	os.Exit(testMain(m))
 }
 
-func TestNM(t *testing.T) {
-	testenv.MustHaveGoBuild(t)
+func testMain(m *testing.M) int {
+	if !testenv.HasGoBuild() {
+		return 0
+	}
 
 	tmpDir, err := ioutil.TempDir("", "TestNM")
 	if err != nil {
-		t.Fatal("TempDir failed: ", err)
+		fmt.Printf("TempDir failed: ", err)
+		return 2
 	}
 	defer os.RemoveAll(tmpDir)
 
-	testnmpath := filepath.Join(tmpDir, "testnm.exe")
-	out, err := exec.Command(testenv.GoToolPath(t), "build", "-o", testnmpath, "cmd/nm").CombinedOutput()
+	testnmpath = filepath.Join(tmpDir, "testnm.exe")
+	gotool, err := testenv.GoTool()
+	if err != nil {
+		fmt.Printf("GoTool failed: ", err)
+		return 2
+	}
+	out, err := exec.Command(gotool, "build", "-o", testnmpath, "cmd/nm").CombinedOutput()
 	if err != nil {
-		t.Fatalf("go build -o %v cmd/nm: %v\n%s", testnmpath, err, string(out))
+		fmt.Printf("go build -o %v cmd/nm: %v\n%s", testnmpath, err, string(out))
+		return 2
 	}
 
+	return m.Run()
+}
+
+func TestNonGoFiles(t *testing.T) {
 	testfiles := []string{
 		"elf/testdata/gcc-386-freebsd-exec",
 		"elf/testdata/gcc-amd64-linux-exec",
@@ -88,11 +73,109 @@ func TestNM(t *testing.T) {
 			t.Errorf("go tool nm %v: %v\n%s", exepath, err, string(out))
 		}
 	}
+}
+
+func testGoFile(t *testing.T, iscgo, isexternallinker bool) {
+	tmpdir, err := ioutil.TempDir("", "TestGoFile")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(tmpdir)
+
+	src := filepath.Join(tmpdir, "a.go")
+	file, err := os.Create(src)
+	if err != nil {
+		t.Fatal(err)
+	}
+	err = template.Must(template.New("main").Parse(testprog)).Execute(file, iscgo)
+	if err != nil {
+		file.Close()
+		t.Fatal(err)
+	}
+	file.Close()
+
+	exe := filepath.Join(tmpdir, "a.exe")
+	args := []string{"build", "-o", exe}
+	if iscgo {
+		linkmode := "internal"
+		if isexternallinker {
+			linkmode = "external"
+		}
+		args = append(args, "-ldflags", "-linkmode="+linkmode)
+	}
+	args = append(args, src)
+	out, err := exec.Command(testenv.GoToolPath(t), args...).CombinedOutput()
+	if err != nil {
+		t.Fatalf("building test executable failed: %s %s", err, out)
+	}
+
+	out, err = exec.Command(exe).CombinedOutput()
+	if err != nil {
+		t.Fatalf("running test executable failed: %s %s", err, out)
+	}
+	names := make(map[string]string)
+	for _, line := range strings.Split(string(out), "\n") {
+		if line == "" {
+			continue
+		}
+		f := strings.Split(line, "=")
+		if len(f) != 2 {
+			t.Fatalf("unexpected output line: %q", line)
+		}
+		names["main."+f[0]] = f[1]
+	}
 
-	cmd := exec.Command(testnmpath, os.Args[0])
-	out, err = cmd.CombinedOutput()
+	out, err = exec.Command(testnmpath, exe).CombinedOutput()
+	if err != nil {
+		t.Fatalf("go tool nm: %v\n%s", err, string(out))
+	}
+	scanner := bufio.NewScanner(bytes.NewBuffer(out))
+	dups := make(map[string]bool)
+	for scanner.Scan() {
+		f := strings.Fields(scanner.Text())
+		if len(f) < 3 {
+			continue
+		}
+		name := f[2]
+		if addr, found := names[name]; found {
+			if want, have := addr, "0x"+f[0]; have != want {
+				t.Errorf("want %s address for %s symbol, but have %s", want, name, have)
+			}
+			delete(names, name)
+		}
+		if _, found := dups[name]; found {
+			t.Errorf("duplicate name of %q is found", name)
+		}
+	}
+	err = scanner.Err()
 	if err != nil {
-		t.Fatalf("go tool nm %v: %v\n%s", os.Args[0], err, string(out))
+		t.Fatal("error reading nm output: %v", err)
 	}
-	checkSymbols(t, out)
+	if len(names) > 0 {
+		t.Errorf("executable is missing %v symbols", names)
+	}
+}
+
+func TestGoFile(t *testing.T) {
+	testGoFile(t, false, false)
+}
+
+const testprog = `
+package main
+
+import "fmt"
+{{if .}}import "C"
+{{end}}
+
+func main() {
+	testfunc()
+}
+
+var testdata uint32
+
+func testfunc() {
+	fmt.Printf("main=%p\n", main)
+	fmt.Printf("testfunc=%p\n", testfunc)
+	fmt.Printf("testdata=%p\n", &testdata)
 }
+`