Skip to content

Commit

Permalink
cmd/internal/obj: use correct symbol size for Hashed64 classification
Browse files Browse the repository at this point in the history
Use sym.Size, instead of len(sym.P), to decide whether a
content-addressable symbol is "short" and hashed as Hashed64.
So we don't dedup a small symbol with a gigantic almost-zero
symbol.

Fixes golang#42140.

Change-Id: Ic65869e1eaf51947517b3ece49c8b0be1b94bb75
Reviewed-on: https://go-review.googlesource.com/c/go/+/264337
Trust: Cherry Zhang <[email protected]>
Trust: Cuong Manh Le <[email protected]>
Run-TryBot: Cherry Zhang <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Cuong Manh Le <[email protected]>
Reviewed-by: Than McIntosh <[email protected]>
  • Loading branch information
cherrymui committed Oct 22, 2020
1 parent 431d58d commit 2ad4415
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 1 deletion.
2 changes: 1 addition & 1 deletion src/cmd/internal/obj/sym.go
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ func (ctxt *Link) NumberSyms() {
// if Pkgpath is unknown, cannot hash symbols with relocations, as it
// may reference named symbols whose names are not fully expanded.
if s.ContentAddressable() && (ctxt.Pkgpath != "" || len(s.R) == 0) {
if len(s.P) <= 8 && len(s.R) == 0 && !strings.HasPrefix(s.Name, "type.") {
if s.Size <= 8 && len(s.R) == 0 && !strings.HasPrefix(s.Name, "type.") {
// We can use short hash only for symbols without relocations.
// Don't use short hash for type symbols, as they need special handling.
s.PkgIdx = goobj.PkgIdxHashed64
Expand Down
53 changes: 53 additions & 0 deletions src/cmd/link/link_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -820,3 +820,56 @@ func TestReadOnly(t *testing.T) {
t.Errorf("running test program did not fail. output:\n%s", out)
}
}

const testIssue38554Src = `
package main
type T [10<<20]byte
//go:noinline
func f() T {
return T{} // compiler will make a large stmp symbol, but not used.
}
func main() {
x := f()
println(x[1])
}
`

func TestIssue38554(t *testing.T) {
testenv.MustHaveGoBuild(t)

t.Parallel()

tmpdir, err := ioutil.TempDir("", "TestIssue38554")
if err != nil {
t.Fatal(err)
}
defer os.RemoveAll(tmpdir)

src := filepath.Join(tmpdir, "x.go")
err = ioutil.WriteFile(src, []byte(testIssue38554Src), 0666)
if err != nil {
t.Fatalf("failed to write source file: %v", err)
}
exe := filepath.Join(tmpdir, "x.exe")
cmd := exec.Command(testenv.GoToolPath(t), "build", "-o", exe, src)
out, err := cmd.CombinedOutput()
if err != nil {
t.Fatalf("build failed: %v\n%s", err, out)
}

fi, err := os.Stat(exe)
if err != nil {
t.Fatalf("failed to stat output file: %v", err)
}

// The test program is not much different from a helloworld, which is
// typically a little over 1 MB. We allow 5 MB. If the bad stmp is live,
// it will be over 10 MB.
const want = 5 << 20
if got := fi.Size(); got > want {
t.Errorf("binary too big: got %d, want < %d", got, want)
}
}

0 comments on commit 2ad4415

Please sign in to comment.