cmd/link: crash in testshared

[rsc]

cd go
git fetch origin refs/changes/35/588235/4 && git checkout FETCH_HEAD
cd src
./make.bash
go test cmd/cgo/internal/testshared

crashes with:

goroutine 1 [running]:
cmd/link/internal/loader.(*Loader).SymSize(0x85fda0?, 0x121440?)
	cmd/link/internal/loader/loader.go:804 +0x3c
cmd/link/internal/ld.(*pclntab).generatePctab.func1(0x0)
	cmd/link/internal/ld/pcln.go:466 +0x56
cmd/link/internal/ld.(*pclntab).generatePctab(0xc000848640, 0xc0001ba200, {0xc000dcca00, 0x139c, 0x40?})
	cmd/link/internal/ld/pcln.go:489 +0x373
cmd/link/internal/ld.(*Link).pclntab(0xc0001ba200, {0xc000940000?, 0xc0001405f0?, 0xf?})
	cmd/link/internal/ld/pcln.go:803 +0x1ad
cmd/link/internal/ld.Main(_, {0x20, 0x20, 0x1, 0x7, 0x10, 0x0, {0xc000140461, 0x1, 0x1}, ...})
	cmd/link/internal/ld/main.go:419 +0x17fc
main.main()
	cmd/link/main.go:72 +0xddb

Note that the linker is dying in generatePctab calling SymSize on the line that says return int64(r.Sym(li).Siz()). Clearly r.Sym(li) is nil. The call site claims SymSize will return 0 for a bad symbol but that appears not to be the case.

Applying this diff makes the problem go away:

diff --git a/src/internal/runtime/exithook/hooks.go b/src/internal/runtime/exithook/hooks.go
index 2e7e010d34..eb8aa1ce0a 100644
--- a/src/internal/runtime/exithook/hooks.go
+++ b/src/internal/runtime/exithook/hooks.go
@@ -76,9 +76,7 @@ func Run(code int) {
 		if code != 0 && !h.RunOnFailure {
 			continue
 		}
-		func() {
-			h.F()
-		}()
+		h.F()
 	}
 }

That func wrapping was a dreg that needed to be removed, so no harm done, but we should probably understand the linker crash anyway.

[cherrymui]

My guess is that the closure is completely inlined and dead. But the symbol is still around. In all other build modes the linker will deadcode the symbol, and won't look at it. But for shared build mode we keep all symbols alive.

This is probably the same issue that @dr2chase run into, for early deadcode closures in the compiler.

Possible fixes are

• make the compiler not leave a symbol in this half-dead state: either completely remove the symbol, or compile it as usual
• or, make the linker run deadcode in shared build mode, maybe mark all top-level symbols as root, then go from there. This adds complexity, as it needs some way to know which symbols are top-level.
• or, make the linker more resilient to this kind of half-dead symbols, without slowing down the linker in normal build modes. I haven't looked into how easy/hard that is.

[gopherbot]

Change https://go.dev/cl/588316 mentions this issue: cmd/link: don't include deadcoded function symbols in shared build mode

[dr2chase]

I agree with Cherry's analysis. Working quickly before the freeze, what I came up with was, in dead code where it grabs all the symbols in the shared buildmode case, add a test for "actually not there" and skipping those. Cherry's proposed fix is probably more efficient, if it passes all the tests, a better choice.

[gopherbot]

Change https://go.dev/cl/588455 mentions this issue: cmd/link, ir, cmd/compile, test: combo CL for testing and benchmarking