From 00723603eb1e183e010371fc5aa76a3d8efda8d1 Mon Sep 17 00:00:00 2001 From: Than McIntosh Date: Wed, 22 Apr 2020 13:40:22 -0400 Subject: [PATCH] [dev.link] cmd/link/internal/loader: fix AttrSubSymbol The code that runs as a part of loadlibfull converts the linker's outer/sub state and sets the sym.Symbol AttrSubSymbol if a symbol has both A) an outer sym, and B) is listed as a sub-symbol by some other symbol. Make sure that we have the same logic in the original loader method, since we need to use it as part of dodata() prior to loadlibfull. Change-Id: I200adab741d778a6ba821419e8ea131ad19375bc Reviewed-on: https://go-review.googlesource.com/c/go/+/229440 Run-TryBot: Than McIntosh TryBot-Result: Gobot Gobot Reviewed-by: Cherry Zhang --- src/cmd/link/internal/loader/loader.go | 36 ++++++++++++++++---------- 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/src/cmd/link/internal/loader/loader.go b/src/cmd/link/internal/loader/loader.go index fd329f5608..987feeb284 100644 --- a/src/cmd/link/internal/loader/loader.go +++ b/src/cmd/link/internal/loader/loader.go @@ -950,10 +950,30 @@ func (l *Loader) SetAttrReadOnly(i Sym, v bool) { // become regular linker symbols and symbols go on the Sub list of // their section) and for constructing the global offset table when // internally linking a dynamic executable. +// +// Note that in later stages of the linker, we set Outer(S) to some +// container symbol C, but don't set Sub(C). Thus we have two +// distinct scenarios: +// +// - Outer symbol covers the address ranges of its sub-symbols. +// Outer.Sub is set in this case. +// - Outer symbol doesn't conver the address ranges. It is zero-sized +// and doesn't have sub-symbols. In the case, the inner symbol is +// not actually a "SubSymbol". (Tricky!) +// +// This method returns TRUE only for sub-symbols in the first scenario. +// +// FIXME: would be better to do away with this and have a better way +// to represent container symbols. + func (l *Loader) AttrSubSymbol(i Sym) bool { // we don't explicitly store this attribute any more -- return // a value based on the sub-symbol setting. - return l.OuterSym(i) != 0 + o := l.OuterSym(i) + if o == 0 { + return false + } + return l.SubSym(o) != 0 } // Note that we don't have a 'SetAttrSubSymbol' method in the loader; @@ -2409,18 +2429,8 @@ func (l *Loader) migrateAttributes(src Sym, dst *sym.Symbol) { dst.Sub = l.Syms[sub] } - // Set sub-symbol attribute. - // - // In sym.Symbols world, it uses Outer to record container symbols. - // Currently there are two kinds - // - Outer symbol covers the address ranges of its sub-symbols. - // Outer.Sub is set in this case. - // - Outer symbol doesn't conver the address ranges. It is zero-sized - // and doesn't have sub-symbols. In the case, the inner symbol is - // not actually a "SubSymbol". (Tricky!) - // - // FIXME: would be better to do away with this and have a better way - // to represent container symbols. + // Set sub-symbol attribute. See the comment on the AttrSubSymbol + // method for more on this, there is some tricky stuff here. dst.Attr.Set(sym.AttrSubSymbol, l.outer[src] != 0 && l.sub[l.outer[src]] != 0) // Copy over dynimplib, dynimpvers, extname.