From 8f6c083d7bf68a766073c50ceb8ea405a3fe7bed Mon Sep 17 00:00:00 2001 From: Cherry Mui Date: Tue, 25 Mar 2025 16:55:54 -0400 Subject: cmd/link: choose one with larger size for duplicated BSS symbols When two packages declare a variable with the same name (with linkname at least on one side), the linker will choose one as the actual definition of the symbol if one has content (i.e. a DATA symbol) and the other does not (i.e. a BSS symbol). When both have content, it is redefinition error. When neither has content, currently the choice is sort of arbitrary (depending on symbol loading order, etc. which are subject to change). One use case for that is that one wants to reference a symbol defined in another package, and the reference side just wants to see some of the fields, so it may be declared with a smaller type. In this case, we want to choose the one with the larger size as the true definition. Otherwise the code accessing the larger sized one may read/write out of bounds, corrupting the next variable. This CL makes the linker do so. Fixes #72032. Change-Id: I160aa9e0234702066cb8f141c186eaa89d0fcfed Reviewed-on: https://go-review.googlesource.com/c/go/+/660696 LUCI-TryBot-Result: Go LUCI Reviewed-by: David Chase Reviewed-by: Than McIntosh --- src/cmd/link/internal/loader/loader.go | 29 +++++++++++++++++++++-------- 1 file changed, 21 insertions(+), 8 deletions(-) (limited to 'src/cmd/link/internal/loader') diff --git a/src/cmd/link/internal/loader/loader.go b/src/cmd/link/internal/loader/loader.go index 0c234e8975..182379f0df 100644 --- a/src/cmd/link/internal/loader/loader.go +++ b/src/cmd/link/internal/loader/loader.go @@ -432,16 +432,16 @@ func (st *loadState) addSym(name string, ver int, r *oReader, li uint32, kind in return i } // symbol already exists + // Fix for issue #47185 -- given two dupok or BSS symbols with + // different sizes, favor symbol with larger size. See also + // issue #46653 and #72032. + oldsz := l.SymSize(oldi) + sz := int64(r.Sym(li).Siz()) if osym.Dupok() { if l.flags&FlagStrictDups != 0 { l.checkdup(name, r, li, oldi) } - // Fix for issue #47185 -- given two dupok symbols with - // different sizes, favor symbol with larger size. See - // also issue #46653. - szdup := l.SymSize(oldi) - sz := int64(r.Sym(li).Siz()) - if szdup < sz { + if oldsz < sz { // new symbol overwrites old symbol. l.objSyms[oldi] = objSym{r.objidx, li} } @@ -452,11 +452,24 @@ func (st *loadState) addSym(name string, ver int, r *oReader, li uint32, kind in if oldsym.Dupok() { return oldi } - overwrite := r.DataSize(li) != 0 + // If one is a DATA symbol (i.e. has content, DataSize != 0) + // and the other is BSS, the one with content wins. + // If both are BSS, the one with larger size wins. + // Specifically, the "overwrite" variable and the final result are + // + // new sym old sym overwrite + // --------------------------------------------- + // DATA DATA true => ERROR + // DATA lg/eq BSS sm/eq true => new wins + // DATA small BSS large true => ERROR + // BSS large DATA small true => ERROR + // BSS large BSS small true => new wins + // BSS sm/eq D/B lg/eq false => old wins + overwrite := r.DataSize(li) != 0 || oldsz < sz if overwrite { // new symbol overwrites old symbol. oldtyp := sym.AbiSymKindToSymKind[objabi.SymKind(oldsym.Type())] - if !(oldtyp.IsData() && oldr.DataSize(oldli) == 0) { + if !(oldtyp.IsData() && oldr.DataSize(oldli) == 0) || oldsz > sz { log.Fatalf("duplicated definition of symbol %s, from %s and %s", name, r.unit.Lib.Pkg, oldr.unit.Lib.Pkg) } l.objSyms[oldi] = objSym{r.objidx, li} -- cgit v1.3-5-g9baa