-
Notifications
You must be signed in to change notification settings - Fork 17.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
cmd/link: inlinedCall entries missing FuncInfo-based fields (FuncID) in -linkshared mode #55954
Comments
Change https://go.dev/cl/429637 mentions this issue: |
I'm not sure it is actually a problem. Most of the special funcIDs are runtime internal. They are probably never inlined outside of the runtime. Then there is FuncID_wrapper. Wrapper functions are dupOK, therefore always have a local definition in the current package (therefore DSO). Also, I'm not sure inlining across DSO boundary is well defined anyway (currently we do it, but maybe it isn't actually desired). One idea of a shared object is that it can be replaced without replacing other DSOs in the same program. If we inline across DSO boundary, the inlined copy may potentially become out of sync with the actually definition. |
I agree it isn't much of a problem right now. I was actually wondering if we can remove funcID from inlinedCall entirely because I'm not sure if wrapper functions are ever inlined (if a wrapper function is used somewhere that it can be inlined, then the compiler might not need to generate a wrapper at all? But I'm not sure about this). I hadn't considered that wrappers should always be local. Anyways, I'm currently planning to add function start line numbers to inlinedCall via FuncInfo for the PGO proposal. In that case it will matter, as we won't find the line number for cross-DSO inlined functions. If we stop cross-DSO inlining entirely then indeed this problem would go away entirely. |
This is at least true for ABI wrappers. I'm not really sure about other type of wrappers. I agree that we probably should do it this way (if not already). |
All inlined functions are Go functions, and thus should be capable of having a FuncInfo. Missing FuncInfo is likely indication of a compiler bug that dropped the symbol too early, failing to add it to the symbol list used for writing output. I believe all existing cases have been fixed; this check will prevent regressions. The exception is -linkshared mode. There symbols are loaded from the shared library, and the FuncInfo is not available. This is a bug, as it can result in incorrect the FuncID in inlinedCall, but it is very involved to fix. For #54959. For #55954. Change-Id: Ib0dc4f1ea62525b55f68604d6013ff33223fdcdd Reviewed-on: https://go-review.googlesource.com/c/go/+/429637 TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Matthew Dempsky <mdempsky@google.com> Run-TryBot: Michael Pratt <mpratt@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
Follow-up to #54959 with another failing case.
When constructing
runtime.inlinedCall
entries, we fetch the inlined function FuncID from the FuncInfo.When building in
-linkshared
mode, if the inlined function is from the standard library (e.g.,sync/atomic.(*Int64).Add
), then it is an "external" symbol andLoader.FuncInfo
will find no FuncInfo because we don't load an auxs for external symbols (that would likely happen here, though I don't think the ELF contains the auxs at all.The result is that the
inlinedCall
is left with the default FuncID (FuncID_normal
), even if that is incorrect.Note that this is not a problem for the other FuncInfo users, notably the pcsp, pcfile, pcline, etc tables. Those ultimately end up in the
moduledata
. There are multiplemoduledata
s: one forlibstd.so
and one for the main binary. Linking oflibstd.so
has access to all theFuncInfo
s it needs to construct themoduledata
forlibstd.so
. It is only a problem forinlinedCall
because we need cross-module data.There are two ways I can see of addressing this:
Ensure
FuncInfo
availability for external symbols. Since the symbol aux data is not directly available inlibstd.so
, this could be done by reverse engineering it from the shared objectmoduledata
symbol (blegh), or (I believe) in addition to to thepackageshlib
pointing tolibstd.so
, the importcfg also includes thepackagefile
with the original package Go object file. We could reach into that file just for the symbol aux data.Eliminate the
FuncInfo
-based data frominlinedCall
. If we ensure that every inlined function has a_func
in the function table, then at runtime we could lookup the_func
from the inlined symbol name and get the metadata from there. For cross-module inlining, this means that the linker never needs to know the metadata at all.cc @mdempsky @cherrymui
The text was updated successfully, but these errors were encountered: