From b51bf4febc32dfa6bb3c1cf467f1513a1d6d8b75 Mon Sep 17 00:00:00 2001 From: Than McIntosh Date: Tue, 8 Jun 2021 20:09:49 -0400 Subject: [release-branch.go1.15] cmd/link: fix handling of dupok mapzero syms The compiler machinery that generates "map.zero" symbols marks them as RODATA and DUPOK, which is problematic when a given application has multiple map zero symbols (from different packages) with varying sizes: the dupok path in the loader assumes that if two symbols have the same name, it is safe to pick any of the versions. In the case of map.zero, the link needs to select the largest symbol, not an arbitrary sym. This patch changes the linker's dupok symbol loading path to detect this problem, and in situations where we're loading a dupok symbol whose name is the same as an existing symbol but whose size is large, select the new dup over the old. Note: this fix differs from the one used in 1.16/1.17, which uses content-addressable symbols instead. Fixes #46656. Change-Id: Iabd2feef01d448670ba795c7eaddc48c191ea276 Reviewed-on: https://go-review.googlesource.com/c/go/+/326211 Trust: Than McIntosh Run-TryBot: Than McIntosh TryBot-Result: Go Bot Reviewed-by: Cherry Mui (cherry picked from commit aa5540cd82170f82c6fe11511e12de96aa58cbc1) Reviewed-on: https://go-review.googlesource.com/c/go/+/326213 --- src/cmd/link/internal/loader/loader.go | 12 +++++- test/fixedbugs/issue46653.dir/bad/bad.go | 64 ++++++++++++++++++++++++++++++++ test/fixedbugs/issue46653.dir/main.go | 27 ++++++++++++++ test/fixedbugs/issue46653.go | 10 +++++ 4 files changed, 112 insertions(+), 1 deletion(-) create mode 100644 test/fixedbugs/issue46653.dir/bad/bad.go create mode 100644 test/fixedbugs/issue46653.dir/main.go create mode 100644 test/fixedbugs/issue46653.go diff --git a/src/cmd/link/internal/loader/loader.go b/src/cmd/link/internal/loader/loader.go index 32c342e545..3df84c5c94 100644 --- a/src/cmd/link/internal/loader/loader.go +++ b/src/cmd/link/internal/loader/loader.go @@ -434,7 +434,17 @@ func (l *Loader) AddSym(name string, ver int, r *oReader, li int, kind int, dupo if l.flags&FlagStrictDups != 0 { l.checkdup(name, r, li, oldi) } - return oldi, false + // Fix for issue #46656 -- given two dupok symbols with + // different sizes, favor symbol with larger size. + szdup := l.SymSize(oldi) + sz := int64(r.Sym(li).Siz()) + if szdup >= sz { + return oldi, false + } else { + // new symbol overwrites old symbol. + l.objSyms[oldi] = objSym{r, li} + return oldi, true + } } oldr, oldli := l.toLocal(oldi) oldsym := oldr.Sym(oldli) diff --git a/test/fixedbugs/issue46653.dir/bad/bad.go b/test/fixedbugs/issue46653.dir/bad/bad.go new file mode 100644 index 0000000000..c1611b8347 --- /dev/null +++ b/test/fixedbugs/issue46653.dir/bad/bad.go @@ -0,0 +1,64 @@ +// Copyright 2021 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 a + +func Bad() { + m := make(map[int64]A) + a := m[0] + if len(a.B.C1.D2.E2.F1) != 0 || + len(a.B.C1.D2.E2.F2) != 0 || + len(a.B.C1.D2.E2.F3) != 0 || + len(a.B.C1.D2.E2.F4) != 0 || + len(a.B.C1.D2.E2.F5) != 0 || + len(a.B.C1.D2.E2.F6) != 0 || + len(a.B.C1.D2.E2.F7) != 0 || + len(a.B.C1.D2.E2.F8) != 0 || + len(a.B.C1.D2.E2.F9) != 0 || + len(a.B.C1.D2.E2.F10) != 0 || + len(a.B.C1.D2.E2.F11) != 0 || + len(a.B.C1.D2.E2.F16) != 0 { + panic("bad") + } +} + +type A struct { + B +} + +type B struct { + C1 C + C2 C +} + +type C struct { + D1 D + D2 D +} + +type D struct { + E1 E + E2 E + E3 E + E4 E +} + +type E struct { + F1 string + F2 string + F3 string + F4 string + F5 string + F6 string + F7 string + F8 string + F9 string + F10 string + F11 string + F12 string + F13 string + F14 string + F15 string + F16 string +} diff --git a/test/fixedbugs/issue46653.dir/main.go b/test/fixedbugs/issue46653.dir/main.go new file mode 100644 index 0000000000..e2a96e54ec --- /dev/null +++ b/test/fixedbugs/issue46653.dir/main.go @@ -0,0 +1,27 @@ +// Copyright 2021 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 main + +import ( + bad "issue46653.dir/bad" +) + +func main() { + bad.Bad() +} + +func neverCalled() L { + m := make(map[string]L) + return m[""] +} + +type L struct { + A Data + B Data +} + +type Data struct { + F1 [22][]string +} diff --git a/test/fixedbugs/issue46653.go b/test/fixedbugs/issue46653.go new file mode 100644 index 0000000000..e6283b1de5 --- /dev/null +++ b/test/fixedbugs/issue46653.go @@ -0,0 +1,10 @@ +// runindir + +// Copyright 2021 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 to verify compiler and linker handling of multiple +// competing map.zero symbol definitions. + +package ignored -- cgit v1.2.3-54-g00ecf