cmd/link: apply -X options after loading symbols

The linker has been applying -X options before loading symbols,
meaning that when it sees -X y=z it creates a symbol named y
and initializes its string data to z. The symbol named y is marked
"DUPOK" so that when the actual packages are loaded, no error is
emitted when the real y is seen. The predefined y's data is used
instead of whatever the real y says.

If we define -X y=z and we never load y, then the predefined symbol
is dropped during dead code elimination, but not in shared library
builds. Shared library builds must include all symbols, so we have to
be more careful about not defining symbols that wouldn't have
appeared anyway.

To be more careful, save the -X settings until after all the symbols
are loaded from the packages, and then apply the string changes
to whatever symbols are known (but ignore the ones that were not
loaded at all). This ends up being simpler anyway, since it doesn't
depend on DUPOK magic.

Makes CL 86835 safe.

Fixes #23273.

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

diff --git a/src/cmd/link/internal/ld/data.go b/src/cmd/link/internal/ld/data.go
index e0541c435bd8860b565b7c0e9c6f49ce2304d1ca..7450dea6d531971c597e0ff4649fcfa88f3286e0 100644 (file)
--- a/src/cmd/link/internal/ld/data.go
+++ b/src/cmd/link/internal/ld/data.go
@@ -789,7 +789,10 @@ func Dwarfblk(ctxt *Link, addr int64, size int64) {
 
 var zeros [512]byte
 
-var strdata []*sym.Symbol
+var (
+       strdata  = make(map[string]string)
+       strnames []string
+)
 
 func addstrdata1(ctxt *Link, arg string) {
        eq := strings.Index(arg, "=")
@@ -802,19 +805,37 @@ func addstrdata1(ctxt *Link, arg string) {
                pkg = *flagPluginPath
        }
        pkg = objabi.PathToPrefix(pkg)
-       addstrdata(ctxt, pkg+arg[dot:eq], arg[eq+1:])
+       name := pkg + arg[dot:eq]
+       value := arg[eq+1:]
+       if _, ok := strdata[name]; !ok {
+               strnames = append(strnames, name)
+       }
+       strdata[name] = value
 }
 
-func addstrdata(ctxt *Link, name string, value string) {
-       p := fmt.Sprintf("%s.str", name)
+func addstrdata(ctxt *Link, name, value string) {
+       s := ctxt.Syms.ROLookup(name, 0)
+       if s == nil || s.Gotype == nil {
+               // Not defined in the loaded packages.
+               return
+       }
+       if s.Gotype.Name != "type.string" {
+               Errorf(s, "cannot set with -X: not a var of type string (%s)", s.Gotype.Name)
+               return
+       }
+       if s.Type == sym.SBSS {
+               s.Type = sym.SDATA
+       }
+
+       p := fmt.Sprintf("%s.str", s.Name)
        sp := ctxt.Syms.Lookup(p, 0)
 
        Addstring(sp, value)
        sp.Type = sym.SRODATA
 
-       s := ctxt.Syms.Lookup(name, 0)
        s.Size = 0
-       s.Attr |= sym.AttrDuplicateOK
+       s.P = s.P[:0]
+       s.R = s.R[:0]
        reachable := s.Attr.Reachable()
        s.AddAddr(ctxt.Arch, sp)
        s.AddUint(ctxt.Arch, uint64(len(value)))
@@ -824,18 +845,12 @@ func addstrdata(ctxt *Link, name string, value string) {
        // we know before entering this function.
        s.Attr.Set(sym.AttrReachable, reachable)
 
-       strdata = append(strdata, s)
-
        sp.Attr.Set(sym.AttrReachable, reachable)
 }
 
-func (ctxt *Link) checkstrdata() {
-       for _, s := range strdata {
-               if s.Type == sym.STEXT {
-                       Errorf(s, "cannot use -X with text symbol")
-               } else if s.Gotype != nil && s.Gotype.Name != "type.string" {
-                       Errorf(s, "cannot use -X with non-string symbol")
-               }
+func (ctxt *Link) dostrdata() {
+       for _, name := range strnames {
+               addstrdata(ctxt, name, strdata[name])
        }
 }
 

diff --git a/src/cmd/link/internal/ld/go.go b/src/cmd/link/internal/ld/go.go
index 6cfbaebb73c05500349d45e7458c64460d036cb0..7210ebbf382738f26f8167f3abf1601effed3c11 100644 (file)
--- a/src/cmd/link/internal/ld/go.go
+++ b/src/cmd/link/internal/ld/go.go
@@ -349,12 +349,12 @@ func fieldtrack(ctxt *Link) {
        if *flagFieldTrack == "" {
                return
        }
-       s := ctxt.Syms.Lookup(*flagFieldTrack, 0)
-       if !s.Attr.Reachable() {
+       s := ctxt.Syms.ROLookup(*flagFieldTrack, 0)
+       if s == nil || !s.Attr.Reachable() {
                return
        }
-       addstrdata(ctxt, *flagFieldTrack, buf.String())
        s.Type = sym.SDATA
+       addstrdata(ctxt, *flagFieldTrack, buf.String())
 }
 
 func (ctxt *Link) addexport() {

diff --git a/src/cmd/link/internal/ld/main.go b/src/cmd/link/internal/ld/main.go
index 42e1ef7f47235907fe04a88c359e78ed41555c87..42125626779b8d00d84d513203d6b4ebaf46871c 100644 (file)
--- a/src/cmd/link/internal/ld/main.go
+++ b/src/cmd/link/internal/ld/main.go
@@ -196,7 +196,7 @@ func Main(arch *sys.Arch, theArch Arch) {
        }
        ctxt.loadlib()
 
-       ctxt.checkstrdata()
+       ctxt.dostrdata()
        deadcode(ctxt)
        fieldtrack(ctxt)
        ctxt.callgraph()