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>

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

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() {

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()