[dev.link] cmd/link: remove implicit reachability setting from SymbolBuilder

The loader's SymbolBuilder Add*/Set* methods include a call to mark
the underlying symbol as reachable (as a convenience, so that callers
would not have to set it explicitly). This code was carried over from
the corresponding sym.Symbol methods; back in the sym.Symbol world
unreachable symbols were never removed from the AllSyms slice, hence
setting and checking reachability was a good deal more important.

With the advent of the loader and the new deadcode implementation,
there is less of a need for this sort of fallback, and in addition the
implicit attr setting introduces data races in the the loader if there
are SymbolBuilder Add*/Set* method calls in parallel threads, as well
as adding overhead to the methods.

This patch gets rid of the implicit reachability setting, and instead
marks reachability in CreateSymForUpdate, as well as adding a few
explicit SetAttrReachable calls where needed.

Change-Id: I029a0c5a4a24237826a7831f9cbe5180d44cbc40
Reviewed-on: https://go-review.googlesource.com/c/go/+/237678
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>

1 files changed, 0 insertions, 8 deletions

diff --git a/src/cmd/link/internal/loader/loader_test.go b/src/cmd/link/internal/loader/loader_test.go
index 8805a1ee16..cf905492ed 100644
--- a/src/cmd/link/internal/loader/loader_test.go
+++ b/src/cmd/link/internal/loader/loader_test.go
@@ -210,11 +210,6 @@ func TestAddMaterializedSymbol(t *testing.T) {
 	if 0 != es1val {
 		t.Errorf("expected IsReflectMethod(es1) value of 0, got %v", irm)
 	}
-
-	// Writing data to a materialized symbol should mark it reachable.
-	if !sb1.Reachable() || !sb2.Reachable() {
-		t.Fatalf("written-to materialized symbols should be reachable")
-	}
 }
 
 func sameRelocSlice(s1 *Relocs, s2 []Reloc) bool {
@@ -347,9 +342,6 @@ func TestAddDataMethods(t *testing.T) {
 			t.Errorf("testing Loader.%s: expected data %v got %v",
 				tp.which, tp.expData, ldr.Data(mi))
 		}
-		if !ldr.AttrReachable(mi) {
-			t.Fatalf("testing Loader.%s: sym updated should be reachable", tp.which)
-		}
 		relocs := ldr.Relocs(mi)
 		if !sameRelocSlice(&relocs, tp.expRel) {
 			t.Fatalf("testing Loader.%s: got relocslice %+v wanted %+v",