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>

diff --git a/src/cmd/compile/internal/gc/obj.go b/src/cmd/compile/internal/gc/obj.go
index 29cdb5684efba0f66041a1b06165a24ac5e1c7d6..83e64e728e957ab9928973f114edb9628a60e983 100644 (file)
--- a/src/cmd/compile/internal/gc/obj.go
+++ b/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)
        }

diff --git a/src/cmd/internal/obj/data.go b/src/cmd/internal/obj/data.go
index ab873123fcb7f8204687ef64cd2d75b0b3dba1b8..8b1bdb1056fc17dcf572557857d5cbec96a3baf6 100644 (file)
--- a/src/cmd/internal/obj/data.go
+++ b/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)

diff --git a/src/cmd/internal/obj/objfile.go b/src/cmd/internal/obj/objfile.go
index 2528064a8263960dfeef325a599a570b296cde6a..c550d43f26e6abdd3e39319a8f2b96d2800e3d7a 100644 (file)
--- a/src/cmd/internal/obj/objfile.go
+++ b/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)
        }
 

diff --git a/src/cmd/internal/obj/plist.go b/src/cmd/internal/obj/plist.go
index c8d282712ba065737f541657d34a15326b7828f8..5c86c20e73db5c75c6453d28a5078e552d3cc0b0 100644 (file)
--- a/src/cmd/internal/obj/plist.go
+++ b/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 {
-               s.Type = objabi.SNOPTRBSS
+               if s.Type == objabi.SDATA {
+                       s.Type = objabi.SNOPTRDATA
+               } else {
+                       s.Type = objabi.SNOPTRBSS
+               }
        } else if flag&TLSBSS != 0 {
                s.Type = objabi.STLSBSS
        }

diff --git a/src/cmd/link/internal/ld/data.go b/src/cmd/link/internal/ld/data.go
index 8aa6cde60376a5aa3a6af60bc66e0cc8ec6c9874..d724c596c138758a53838de6249d43e6f2c0c74b 100644 (file)
--- a/src/cmd/link/internal/ld/data.go
+++ b/src/cmd/link/internal/ld/data.go
@@ -1141,21 +1141,16 @@ func addinitarrdata(ctxt *Link, s *Symbol) {
 }
 
 func dosymtype(ctxt *Link) {
-       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 {
-               case BuildmodeCArchive, BuildmodeCShared:
-                       if s.Name == *flagEntrySymbol {
-                               addinitarrdata(ctxt, s)
+       switch Buildmode {
+       case BuildmodeCArchive, BuildmodeCShared:
+               for _, s := range ctxt.Syms.Allsym {
+                       // Create a new entry in the .init_array section that points to the
+                       // library initializer function.
+                       switch Buildmode {
+                       case BuildmodeCArchive, BuildmodeCShared:
+                               if s.Name == *flagEntrySymbol {
+                                       addinitarrdata(ctxt, s)
+                               }
                        }
                }
        }

diff --git a/src/cmd/link/internal/ld/ldpe.go b/src/cmd/link/internal/ld/ldpe.go
index 3f09b358641941ad4f7d27e7cbbdc79d5ff81d85..fd3664c2ed10a223bf187532b1399b14671a7e65 100644 (file)
--- a/src/cmd/link/internal/ld/ldpe.go
+++ b/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

diff --git a/src/cmd/link/internal/ld/lib.go b/src/cmd/link/internal/ld/lib.go
index 0297eb5b608bb9fa712af6836aa2288891fd369b..0f3b46d972f273268ede132a23cdb8974f90d2e2 100644 (file)
--- a/src/cmd/link/internal/ld/lib.go
+++ b/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)
 

diff --git a/src/debug/pe/file_test.go b/src/debug/pe/file_test.go
index 79570836390194ac707a5fc71196a502fd9b8003..100516f0ba7be55abb41df8876a4e0bbd639c2b2 100644 (file)
--- a/src/debug/pe/file_test.go
+++ b/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 {