From 96e83664378918980bd8f60822c4bc39befcb668 Mon Sep 17 00:00:00 2001 From: Cherry Zhang Date: Tue, 30 Jun 2020 19:09:38 +0000 Subject: [PATCH] Revert "cmd/link: fix GC data reading from shared library" This reverts CL 240462. Reason for revert: test fails on PPC64LE. Updates #39927. Change-Id: I4f14fd0c36e604a80ae9f2f86d1e643e28945e93 Reviewed-on: https://go-review.googlesource.com/c/go/+/240616 Reviewed-by: Austin Clements Reviewed-by: Than McIntosh --- misc/cgo/testshared/shared_test.go | 16 -------- .../testshared/testdata/gcdata/main/main.go | 37 ------------------- misc/cgo/testshared/testdata/gcdata/p/p.go | 7 ---- src/cmd/link/internal/ld/decodesym.go | 27 ++++---------- src/cmd/link/internal/ld/lib.go | 4 +- 5 files changed, 9 insertions(+), 82 deletions(-) delete mode 100644 misc/cgo/testshared/testdata/gcdata/main/main.go delete mode 100644 misc/cgo/testshared/testdata/gcdata/p/p.go diff --git a/misc/cgo/testshared/shared_test.go b/misc/cgo/testshared/shared_test.go index f8dabbe7a01..fda3d2ce76f 100644 --- a/misc/cgo/testshared/shared_test.go +++ b/misc/cgo/testshared/shared_test.go @@ -38,15 +38,7 @@ var testWork = flag.Bool("testwork", false, "if true, log and do not delete the // run runs a command and calls t.Errorf if it fails. func run(t *testing.T, msg string, args ...string) { - runWithEnv(t, msg, nil, args...) -} - -// runWithEnv runs a command under the given environment and calls t.Errorf if it fails. -func runWithEnv(t *testing.T, msg string, env []string, args ...string) { c := exec.Command(args[0], args[1:]...) - if len(env) != 0 { - c.Env = append(os.Environ(), env...) - } if output, err := c.CombinedOutput(); err != nil { t.Errorf("executing %s (%s) failed %s:\n%s", strings.Join(args, " "), msg, err, output) } @@ -1042,11 +1034,3 @@ func TestGeneratedHash(t *testing.T) { func TestPackageOrder(t *testing.T) { goCmd(t, "install", "-buildmode=shared", "-linkshared", "./issue39777/a", "./issue39777/b") } - -// Test that GC data are generated correctly by the linker when it needs a type defined in -// a shared library. See issue 39927. -func TestGCData(t *testing.T) { - goCmd(t, "install", "-buildmode=shared", "-linkshared", "./gcdata/p") - goCmd(t, "build", "-linkshared", "./gcdata/main") - runWithEnv(t, "running gcdata/main", []string{"GODEBUG=clobberfree=1"}, "./main") -} diff --git a/misc/cgo/testshared/testdata/gcdata/main/main.go b/misc/cgo/testshared/testdata/gcdata/main/main.go deleted file mode 100644 index 394862fd94c..00000000000 --- a/misc/cgo/testshared/testdata/gcdata/main/main.go +++ /dev/null @@ -1,37 +0,0 @@ -// Copyright 2020 The Go Authors. All rights reserved. -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file. - -// Test that GC data is generated correctly for global -// variables with types defined in a shared library. -// See issue 39927. - -// This test run under GODEBUG=clobberfree=1. The check -// *x[i] == 12345 depends on this debug mode to clobber -// the value if the object is freed prematurely. - -package main - -import ( - "fmt" - "runtime" - "testshared/gcdata/p" -) - -var x p.T - -func main() { - for i := range x { - x[i] = new(int) - *x[i] = 12345 - } - runtime.GC() - runtime.GC() - runtime.GC() - for i := range x { - if *x[i] != 12345 { - fmt.Printf("x[%d] == %d, want 12345\n", i, *x[i]) - panic("FAIL") - } - } -} diff --git a/misc/cgo/testshared/testdata/gcdata/p/p.go b/misc/cgo/testshared/testdata/gcdata/p/p.go deleted file mode 100644 index 1fee75429ef..00000000000 --- a/misc/cgo/testshared/testdata/gcdata/p/p.go +++ /dev/null @@ -1,7 +0,0 @@ -// Copyright 2020 The Go Authors. All rights reserved. -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file. - -package p - -type T [10]*int diff --git a/src/cmd/link/internal/ld/decodesym.go b/src/cmd/link/internal/ld/decodesym.go index 21bcc3a726a..e9c87efe379 100644 --- a/src/cmd/link/internal/ld/decodesym.go +++ b/src/cmd/link/internal/ld/decodesym.go @@ -10,7 +10,6 @@ import ( "cmd/link/internal/loader" "cmd/link/internal/sym" "debug/elf" - "log" ) // Decoding the type.* symbols. This has to be in sync with @@ -230,15 +229,8 @@ func decodetypeGcmask(ctxt *Link, s loader.Sym) []byte { ptrdata := decodetypePtrdata(ctxt.Arch, symData) sect := findShlibSection(ctxt, ctxt.loader.SymPkg(s), addr) if sect != nil { - bits := ptrdata / int64(ctxt.Arch.PtrSize) - r := make([]byte, (bits+7)/8) - // ldshlibsyms avoids closing the ELF file so sect.ReadAt works. - // If we remove this read (and the ones in decodetypeGcprog), we - // can close the file. - _, err := sect.ReadAt(r, int64(addr-sect.Addr)) - if err != nil { - log.Fatal(err) - } + r := make([]byte, ptrdata/int64(ctxt.Arch.PtrSize)) + sect.ReadAt(r, int64(addr-sect.Addr)) return r } Exitf("cannot find gcmask for %s", ctxt.loader.SymName(s)) @@ -259,15 +251,9 @@ func decodetypeGcprog(ctxt *Link, s loader.Sym) []byte { // A gcprog is a 4-byte uint32 indicating length, followed by // the actual program. progsize := make([]byte, 4) - _, err := sect.ReadAt(progsize, int64(addr-sect.Addr)) - if err != nil { - log.Fatal(err) - } + sect.ReadAt(progsize, int64(addr-sect.Addr)) progbytes := make([]byte, ctxt.Arch.ByteOrder.Uint32(progsize)) - _, err = sect.ReadAt(progbytes, int64(addr-sect.Addr+4)) - if err != nil { - log.Fatal(err) - } + sect.ReadAt(progbytes, int64(addr-sect.Addr+4)) return append(progsize, progbytes...) } Exitf("cannot find gcmask for %s", ctxt.loader.SymName(s)) @@ -282,7 +268,7 @@ func decodetypeGcprog(ctxt *Link, s loader.Sym) []byte { func findShlibSection(ctxt *Link, path string, addr uint64) *elf.Section { for _, shlib := range ctxt.Shlibs { if shlib.Path == path { - for _, sect := range shlib.File.Sections[1:] { // skip the NULL section + for _, sect := range shlib.File.Sections { if sect.Addr <= addr && addr <= sect.Addr+sect.Size { return sect } @@ -293,5 +279,8 @@ func findShlibSection(ctxt *Link, path string, addr uint64) *elf.Section { } func decodetypeGcprogShlib(ctxt *Link, data []byte) uint64 { + if ctxt.Arch.Family == sys.ARM64 { + return 0 + } return decodeInuxi(ctxt.Arch, data[2*int32(ctxt.Arch.PtrSize)+8+1*int32(ctxt.Arch.PtrSize):], ctxt.Arch.PtrSize) } diff --git a/src/cmd/link/internal/ld/lib.go b/src/cmd/link/internal/ld/lib.go index 802bfb2b24e..61ccc28a1d5 100644 --- a/src/cmd/link/internal/ld/lib.go +++ b/src/cmd/link/internal/ld/lib.go @@ -2124,9 +2124,7 @@ func ldshlibsyms(ctxt *Link, shlib string) { Errorf(nil, "cannot open shared library: %s", libpath) return } - // Keep the file open as decodetypeGcprog needs to read from it. - // TODO: fix. Maybe mmap the file. - //defer f.Close() + defer f.Close() hash, err := readnote(f, ELF_NOTE_GO_NAME, ELF_NOTE_GOABIHASH_TAG) if err != nil {