cmd/internal/obj: fix LSym.Type during compilation, not linking

Prior to this CL, the compiler and assembler
were sloppy about the LSym.Type for LSyms
containing static data.

The linker then fixed this up, converting
Sxxx and SBSS to SDATA, and SNOPTRBSS to SNOPTRDATA
if it noticed that the symbol had associated data.

It is preferable to just get this right in cmd/compile
and cmd/asm, because it removes an unnecessary traversal
of the symbol table from the linker (see #14624).
Do this by touching up the LSym.Type fixes in
LSym.prepwrite and Link.Globl.

I have confirmed by instrumenting the linker
that the now-eliminated code paths were unreached.
And an additional check in the object file writing code
will help preserve that invariant.

There was a case in the Windows linker,
with internal linking and cgo,
where we were generating SNOPTRBSS symbols with data.
For now, convert those at the site at which they occur
into SNOPTRDATA, just like they were.

Does not pass toolstash-check,
but does generate identical linked binaries.

No compiler performance changes.

Change-Id: I77b071ab103685ff8e042cee9abb864385488872
Reviewed-on: https://go-review.googlesource.com/40864
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
Reviewed-by: Michael Hudson-Doyle <michael.hudson@canonical.com>

src/cmd/compile/internal/gc/obj.go

@@ -259,10 +259,6 @@ func addGCLocals() {
 }
 
 func duintxx(s *obj.LSym, off int, v uint64, wid int) int {
-	if s.Type == 0 {
-		// TODO(josharian): Do this in obj.prepwrite instead.
-		s.Type = objabi.SDATA
-	}
 	if off&(wid-1) != 0 {
 		Fatalf("duintxxLSym: misaligned: v=%d wid=%d off=%d", v, wid, off)
 	}

src/cmd/internal/obj/data.go

@@ -73,8 +73,13 @@ func (s *LSym) prepwrite(ctxt *Link, off int64, siz int) {
 	if off < 0 || siz < 0 || off >= 1<<30 {
 		ctxt.Diag("prepwrite: bad off=%d siz=%d s=%v", off, siz, s)
 	}
-	if s.Type == objabi.SBSS || s.Type == objabi.STLSBSS {
-		ctxt.Diag("cannot supply data for BSS var")
+	switch s.Type {
+	case objabi.Sxxx, objabi.SBSS:
+		s.Type = objabi.SDATA
+	case objabi.SNOPTRBSS:
+		s.Type = objabi.SNOPTRDATA
+	case objabi.STLSBSS:
+		ctxt.Diag("cannot supply data for %v var %v", s.Type, s.Name)
 	}
 	l := off + int64(siz)
 	s.Grow(l)

src/cmd/internal/obj/objfile.go

@@ -128,6 +128,12 @@ func WriteObjFile(ctxt *Link, b *bufio.Writer) {
 		}
 	}
 	for _, s := range ctxt.Data {
+		if len(s.P) > 0 {
+			switch s.Type {
+			case objabi.SBSS, objabi.SNOPTRBSS, objabi.STLSBSS:
+				ctxt.Diag("cannot provide data for %v sym %v", s.Type, s.Name)
+			}
+		}
 		w.wr.Write(s.P)
 	}
 

src/cmd/internal/obj/plist.go

@@ -171,7 +171,11 @@ func (ctxt *Link) Globl(s *LSym, size int64, flag int) {
 	if flag&RODATA != 0 {
 		s.Type = objabi.SRODATA
 	} else if flag&NOPTR != 0 {
+		if s.Type == objabi.SDATA {
+			s.Type = objabi.SNOPTRDATA
+		} else {
 			s.Type = objabi.SNOPTRBSS
+		}
 	} else if flag&TLSBSS != 0 {
 		s.Type = objabi.STLSBSS
 	}

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

@@ -1141,15 +1141,9 @@ func addinitarrdata(ctxt *Link, s *Symbol) {
 }
 
 func dosymtype(ctxt *Link) {
+	switch Buildmode {
+	case BuildmodeCArchive, BuildmodeCShared:
 		for _, s := range ctxt.Syms.Allsym {
-		if len(s.P) > 0 {
-			if s.Type == SBSS {
-				s.Type = SDATA
-			}
-			if s.Type == SNOPTRBSS {
-				s.Type = SNOPTRDATA
-			}
-		}
 			// Create a new entry in the .init_array section that points to the
 			// library initializer function.
 			switch Buildmode {
@@ -1159,6 +1153,7 @@ func dosymtype(ctxt *Link) {
 				}
 			}
 		}
+	}
 }
 
 // symalign returns the required alignment for the given symbol s.

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

@@ -177,6 +177,18 @@ func ldpeError(ctxt *Link, input *bio.Reader, pkg string, length int64, pn strin
 
 		case IMAGE_SCN_CNT_UNINITIALIZED_DATA | IMAGE_SCN_MEM_READ | IMAGE_SCN_MEM_WRITE: //.bss
 			s.Type = SNOPTRBSS
+			// It seems like this shouldn't happen, but it does, with symbol "runtime/cgo(.bss)".
+			// TODO: Figure out why and either document why it is ok or fix it at the source--
+			// either by eliminating the all-zero data or
+			// by making this SNOPTRDATA (IMAGE_SCN_CNT_INITIALIZED_DATA) to begin with.
+			if len(data) > 0 {
+				for _, x := range data {
+					if x != 0 {
+						Errorf(s, "non-zero data in .bss section: %q", data)
+					}
+				}
+				s.Type = SNOPTRDATA
+			}
 
 		case IMAGE_SCN_CNT_INITIALIZED_DATA | IMAGE_SCN_MEM_READ | IMAGE_SCN_MEM_WRITE: //.data
 			s.Type = SNOPTRDATA

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

@@ -1943,7 +1943,7 @@ func genasmsym(ctxt *Link, put func(*Link, *Symbol, string, SymbolType, int64, *
 				continue
 			}
 			if len(s.P) > 0 {
-				Errorf(s, "should not be bss (size=%d type=%d special=%v)", len(s.P), s.Type, s.Attr.Special())
+				Errorf(s, "should not be bss (size=%d type=%v special=%v)", len(s.P), s.Type, s.Attr.Special())
 			}
 			put(ctxt, s, s.Name, BSSSym, Symaddr(s), s.Gotype)
 

src/debug/pe/file_test.go

@@ -341,7 +341,7 @@ func testDWARF(t *testing.T, linktype int) {
 	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)
+		t.Fatalf("building test executable for linktype %d failed: %s %s", linktype, err, out)
 	}
 	out, err = exec.Command(exe).CombinedOutput()
 	if err != nil {