Linker: Drop function pointers for overridden subprograms

Instead of dropping subprograms that have been overridden, just set
their function pointers to `nullptr`.  This is a minor adjustment to the
stop-gap fix for PR21910 committed in r224487, and fixes the crasher
from PR22792.

The problem that r224487 put a band-aid on: how do we find the canonical
subprogram for a `Function`?  Since the backend currently relies on
`DebugInfoFinder` (which does a naive in-order traversal of compile
units and picks the first subprogram) for this, r224487 tried dropping
non-canonical subprograms.

Dropping subprograms fails because the backend *also* builds up a map
from subprogram to compile unit (`DwarfDebug::SPMap`) based on the
subprogram lists.  A missing subprogram causes segfaults later when an
inlined reference (such as in this testcase) is created.

Instead, just drop the `Function` pointer to `nullptr`, which nicely
mirrors what happens when an already-inlined `Function` is optimized
out.  We can't really be sure that it's the same definition anyway, as
the testcase demonstrates.

This still isn't completely satisfactory.  Two flaws at least that I can
think of:

  - I still haven't found a straightforward way to make this symmetric
    in the IR.  (Interestingly, the DWARF output is already symmetric,
    and I've tested for that to be sure we don't regress.)
  - Using `DebugInfoFinder` to find the canonical subprogram for a
    function is kind of crazy.  We should just attach metadata to the
    function, like this:

        define weak i32 @foo(i32, i32) !dbg !MDSubprogram(...) {

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@233164 91177308-0d34-0410-b5e6-96231b3b80d8
This commit is contained in:
Duncan P. N. Exon Smith
2015-03-25 02:26:32 +00:00
parent ef81f7c255
commit c89369a941
4 changed files with 173 additions and 16 deletions

View File

@@ -1250,9 +1250,10 @@ void ModuleLinker::linkNamedMDNodes() {
/// Drop DISubprograms that have been superseded.
///
/// FIXME: this creates an asymmetric result: we strip losing subprograms from
/// DstM, but leave losing subprograms in SrcM. Instead we should also strip
/// losers from SrcM, but this requires extra plumbing in MapMetadata.
/// FIXME: this creates an asymmetric result: we strip functions from losing
/// subprograms in DstM, but leave losing subprograms in SrcM.
/// TODO: Remove this logic once the backend can correctly determine canonical
/// subprograms.
void ModuleLinker::stripReplacedSubprograms() {
// Avoid quadratic runtime by returning early when there's nothing to do.
if (OverridingFunctions.empty())
@@ -1262,8 +1263,8 @@ void ModuleLinker::stripReplacedSubprograms() {
auto Functions = std::move(OverridingFunctions);
OverridingFunctions.clear();
// Drop subprograms whose functions have been overridden by the new compile
// unit.
// Drop functions from subprograms if they've been overridden by the new
// compile unit.
NamedMDNode *CompileUnits = DstM->getNamedMetadata("llvm.dbg.cu");
if (!CompileUnits)
return;
@@ -1274,19 +1275,15 @@ void ModuleLinker::stripReplacedSubprograms() {
DITypedArray<DISubprogram> SPs(CU.getSubprograms());
assert(SPs && "Expected valid subprogram array");
SmallVector<Metadata *, 16> NewSPs;
NewSPs.reserve(SPs.getNumElements());
for (unsigned S = 0, SE = SPs.getNumElements(); S != SE; ++S) {
DISubprogram SP = SPs.getElement(S);
if (SP && SP.getFunction() && Functions.count(SP.getFunction()))
if (!SP || !SP.getFunction() || !Functions.count(SP.getFunction()))
continue;
NewSPs.push_back(SP);
// Prevent DebugInfoFinder from tagging this as the canonical subprogram,
// since the canonical one is in the incoming module.
SP->replaceFunction(nullptr);
}
// Redirect operand to the overriding subprogram.
if (NewSPs.size() != SPs.getNumElements())
CU.replaceSubprograms(DIArray(MDNode::get(DstM->getContext(), NewSPs)));
}
}