Skip to content

Commit

Permalink
[Cloning] Take another pass at properly cloning debug info
Browse files Browse the repository at this point in the history
Summary:
In rL302576, DISubprograms gained the constraint that a !dbg attachments to functions must
have a 1:1 mapping to DISubprograms. As part of that change, the function cloning support
was adjusted to attempt to enforce this invariant during cloning. However, there
were several problems with the implementation. Part of these were fixed in rL304079.
However, there was a more fundamental problem with these changes, namely that it
bypasses the matadata value map, causing the cloned metadata to be a mix of metadata
pointing to the new suprogram (where manual code was added to fix those up) and the
old suprogram (where this was not the case). This mismatch could cause a number of
different assertion failures in the DWARF emitter. Some of these are given at
JuliaLang/julia#22069, but some others have been observed
as well. Attempt to rectify this by partially reverting the manual DI metadata fixup,
and instead using the standard value map approach. To retain the desired semantics
of not duplicating the compilation unit and inlined subprograms, explicitly freeze
these in the value map.

Reviewers: dblaikie, aprantl, GorNishanov, echristo

Reviewed By: aprantl

Subscribers: llvm-commits

Differential Revision: https://reviews.llvm.org/D33655

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@304226 91177308-0d34-0410-b5e6-96231b3b80d8
  • Loading branch information
Keno committed May 30, 2017
1 parent 1f0488c commit e34d6c6
Show file tree
Hide file tree
Showing 6 changed files with 48 additions and 118 deletions.
6 changes: 0 additions & 6 deletions include/llvm/IR/DebugLoc.h
Original file line number Diff line number Diff line change
Expand Up @@ -90,12 +90,6 @@ namespace llvm {
DenseMap<const MDNode *, MDNode *> &Cache,
bool ReplaceLast = false);

/// Reparent all debug locations referenced by \c I that belong to \c OrigSP
/// to become (possibly indirect) children of \c NewSP.
static void reparentDebugInfo(Instruction &I, DISubprogram *OrigSP,
DISubprogram *NewSP,
DenseMap<const MDNode *, MDNode *> &Cache);

unsigned getLine() const;
unsigned getCol() const;
MDNode *getScope() const;
Expand Down
4 changes: 3 additions & 1 deletion include/llvm/Transforms/Utils/Cloning.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ class BasicBlock;
class BlockFrequencyInfo;
class CallInst;
class CallGraph;
class DebugInfoFinder;
class DominatorTree;
class Function;
class Instruction;
Expand Down Expand Up @@ -110,7 +111,8 @@ struct ClonedCodeInfo {
///
BasicBlock *CloneBasicBlock(const BasicBlock *BB, ValueToValueMapTy &VMap,
const Twine &NameSuffix = "", Function *F = nullptr,
ClonedCodeInfo *CodeInfo = nullptr);
ClonedCodeInfo *CodeInfo = nullptr,
DebugInfoFinder *DIFinder = nullptr);

/// CloneFunction - Return a copy of the specified function and add it to that
/// function's module. Also, any references specified in the VMap are changed
Expand Down
81 changes: 0 additions & 81 deletions lib/IR/DebugLoc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -99,87 +99,6 @@ DebugLoc DebugLoc::appendInlinedAt(DebugLoc DL, DILocation *InlinedAt,
return Last;
}

/// Reparent \c Scope from \c OrigSP to \c NewSP.
static DIScope *reparentScope(LLVMContext &Ctx, DIScope *Scope,
DISubprogram *OrigSP, DISubprogram *NewSP,
DenseMap<const MDNode *, MDNode *> &Cache) {
SmallVector<DIScope *, 3> ScopeChain;
DIScope *Last = NewSP;
DIScope *CurScope = Scope;
do {
if (auto *SP = dyn_cast<DISubprogram>(CurScope)) {
// Don't rewrite this scope chain if it doesn't lead to the replaced SP.
if (SP != OrigSP)
return Scope;
Cache.insert({OrigSP, NewSP});
break;
}
if (auto *Found = Cache[CurScope]) {
Last = cast<DIScope>(Found);
break;
}
ScopeChain.push_back(CurScope);
} while ((CurScope = CurScope->getScope().resolve()));

// Starting from the top, rebuild the nodes to point to the new inlined-at
// location (then rebuilding the rest of the chain behind it) and update the
// map of already-constructed inlined-at nodes.
for (const DIScope *MD : reverse(ScopeChain)) {
if (auto *LB = dyn_cast<DILexicalBlock>(MD))
Cache[MD] = Last = DILexicalBlock::getDistinct(
Ctx, Last, LB->getFile(), LB->getLine(), LB->getColumn());
else if (auto *LB = dyn_cast<DILexicalBlockFile>(MD))
Cache[MD] = Last = DILexicalBlockFile::getDistinct(
Ctx, Last, LB->getFile(), LB->getDiscriminator());
else
llvm_unreachable("illegal parent scope");
}
return Last;
}

void DebugLoc::reparentDebugInfo(Instruction &I, DISubprogram *OrigSP,
DISubprogram *NewSP,
DenseMap<const MDNode *, MDNode *> &Cache) {
auto DL = I.getDebugLoc();
if (!OrigSP || !NewSP || OrigSP == NewSP || !DL)
return;

// Reparent the debug location.
auto &Ctx = I.getContext();
DILocation *InlinedAt = DL->getInlinedAt();
if (InlinedAt) {
while (auto *IA = InlinedAt->getInlinedAt())
InlinedAt = IA;
auto NewScope =
reparentScope(Ctx, InlinedAt->getScope(), OrigSP, NewSP, Cache);
InlinedAt =
DebugLoc::get(InlinedAt->getLine(), InlinedAt->getColumn(), NewScope);
}
I.setDebugLoc(
DebugLoc::get(DL.getLine(), DL.getCol(),
reparentScope(Ctx, DL->getScope(), OrigSP, NewSP, Cache),
DebugLoc::appendInlinedAt(DL, InlinedAt, Ctx, Cache,
ReplaceLastInlinedAt)));

// Fix up debug variables to point to NewSP.
auto reparentVar = [&](DILocalVariable *Var) {
return DILocalVariable::get(
Ctx,
cast<DILocalScope>(
reparentScope(Ctx, Var->getScope(), OrigSP, NewSP, Cache)),
Var->getName(), Var->getFile(), Var->getLine(), Var->getType(),
Var->getArg(), Var->getFlags(), Var->getAlignInBits());
};
if (auto *DbgValue = dyn_cast<DbgValueInst>(&I)) {
auto *Var = DbgValue->getVariable();
I.setOperand(2, MetadataAsValue::get(Ctx, reparentVar(Var)));
} else if (auto *DbgDeclare = dyn_cast<DbgDeclareInst>(&I)) {
auto *Var = DbgDeclare->getVariable();
I.setOperand(1, MetadataAsValue::get(Ctx, reparentVar(Var)));
}
}


#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
LLVM_DUMP_METHOD void DebugLoc::dump() const {
if (!Loc)
Expand Down
2 changes: 1 addition & 1 deletion lib/Transforms/Coroutines/CoroSplit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ static Function *createClone(Function &F, Twine Suffix, coro::Shape &Shape,

SmallVector<ReturnInst *, 4> Returns;

CloneFunctionInto(NewF, &F, VMap, /*ModuleLevelChanges=*/false, Returns);
CloneFunctionInto(NewF, &F, VMap, /*ModuleLevelChanges=*/true, Returns);

// Remove old returns.
for (ReturnInst *Return : Returns)
Expand Down
71 changes: 43 additions & 28 deletions lib/Transforms/Utils/CloneFunction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,10 @@
using namespace llvm;

/// See comments in Cloning.h.
BasicBlock *llvm::CloneBasicBlock(const BasicBlock *BB,
ValueToValueMapTy &VMap,
BasicBlock *llvm::CloneBasicBlock(const BasicBlock *BB, ValueToValueMapTy &VMap,
const Twine &NameSuffix, Function *F,
ClonedCodeInfo *CodeInfo) {
ClonedCodeInfo *CodeInfo,
DebugInfoFinder *DIFinder) {
DenseMap<const MDNode *, MDNode *> Cache;
BasicBlock *NewBB = BasicBlock::Create(BB->getContext(), "", F);
if (BB->hasName()) NewBB->setName(BB->getName()+NameSuffix);
Expand All @@ -50,10 +50,11 @@ BasicBlock *llvm::CloneBasicBlock(const BasicBlock *BB,
// Loop over all instructions, and copy them over.
for (BasicBlock::const_iterator II = BB->begin(), IE = BB->end();
II != IE; ++II) {

if (DIFinder && F->getParent() && II->getDebugLoc())
DIFinder->processLocation(*F->getParent(), II->getDebugLoc().get());

Instruction *NewInst = II->clone();
if (F && F->getSubprogram())
DebugLoc::reparentDebugInfo(*NewInst, BB->getParent()->getSubprogram(),
F->getSubprogram(), Cache);
if (II->hasName())
NewInst->setName(II->getName()+NameSuffix);
NewBB->getInstList().push_back(NewInst);
Expand Down Expand Up @@ -122,31 +123,38 @@ void llvm::CloneFunctionInto(Function *NewFunc, const Function *OldFunc,
AttributeList::get(NewFunc->getContext(), OldAttrs.getFnAttributes(),
OldAttrs.getRetAttributes(), NewArgAttrs));

bool MustCloneSP =
OldFunc->getParent() && OldFunc->getParent() == NewFunc->getParent();
DISubprogram *SP = OldFunc->getSubprogram();
if (SP) {
assert(!MustCloneSP || ModuleLevelChanges);
// Add mappings for some DebugInfo nodes that we don't want duplicated
// even if they're distinct.
auto &MD = VMap.MD();
MD[SP->getUnit()].reset(SP->getUnit());
MD[SP->getType()].reset(SP->getType());
MD[SP->getFile()].reset(SP->getFile());
// If we're not cloning into the same module, no need to clone the
// subprogram
if (!MustCloneSP)
MD[SP].reset(SP);
}

SmallVector<std::pair<unsigned, MDNode *>, 1> MDs;
OldFunc->getAllMetadata(MDs);
for (auto MD : MDs) {
MDNode *NewMD;
bool MustCloneSP =
(MD.first == LLVMContext::MD_dbg && OldFunc->getParent() &&
OldFunc->getParent() == NewFunc->getParent());
if (MustCloneSP) {
auto *SP = cast<DISubprogram>(MD.second);
NewMD = DISubprogram::getDistinct(
NewFunc->getContext(), SP->getScope(), SP->getName(),
SP->getLinkageName(), SP->getFile(), SP->getLine(), SP->getType(),
SP->isLocalToUnit(), SP->isDefinition(), SP->getScopeLine(),
SP->getContainingType(), SP->getVirtuality(), SP->getVirtualIndex(),
SP->getThisAdjustment(), SP->getFlags(), SP->isOptimized(),
SP->getUnit(), SP->getTemplateParams(), SP->getDeclaration(),
SP->getVariables(), SP->getThrownTypes());
} else
NewMD =
MapMetadata(MD.second, VMap,
ModuleLevelChanges ? RF_None : RF_NoModuleLevelChanges,
TypeMapper, Materializer);
NewFunc->addMetadata(MD.first, *NewMD);
NewFunc->addMetadata(
MD.first,
*MapMetadata(MD.second, VMap,
ModuleLevelChanges ? RF_None : RF_NoModuleLevelChanges,
TypeMapper, Materializer));
}

// When we remap instructions, we want to avoid duplicating inlined
// DISubprograms, so record all subprograms we find as we duplicate
// instructions and then freeze them in the MD map.
DebugInfoFinder DIFinder;

// Loop over all of the basic blocks in the function, cloning them as
// appropriate. Note that we save BE this way in order to handle cloning of
// recursive functions into themselves.
Expand All @@ -156,7 +164,8 @@ void llvm::CloneFunctionInto(Function *NewFunc, const Function *OldFunc,
const BasicBlock &BB = *BI;

// Create a new basic block and copy instructions into it!
BasicBlock *CBB = CloneBasicBlock(&BB, VMap, NameSuffix, NewFunc, CodeInfo);
BasicBlock *CBB = CloneBasicBlock(&BB, VMap, NameSuffix, NewFunc, CodeInfo,
SP ? &DIFinder : nullptr);

// Add basic block mapping.
VMap[&BB] = CBB;
Expand All @@ -178,6 +187,12 @@ void llvm::CloneFunctionInto(Function *NewFunc, const Function *OldFunc,
Returns.push_back(RI);
}

for (DISubprogram *ISP : DIFinder.subprograms()) {
if (ISP != SP) {
VMap.MD()[ISP].reset(ISP);
}
}

// Loop over all of the instructions in the function, fixing up operand
// references as we go. This uses VMap to do all the hard work.
for (Function::iterator BB =
Expand Down Expand Up @@ -226,7 +241,7 @@ Function *llvm::CloneFunction(Function *F, ValueToValueMapTy &VMap,
}

SmallVector<ReturnInst*, 8> Returns; // Ignore returns cloned.
CloneFunctionInto(NewF, F, VMap, /*ModuleLevelChanges=*/false, Returns, "",
CloneFunctionInto(NewF, F, VMap, F->getSubprogram() != nullptr, Returns, "",
CodeInfo);

return NewF;
Expand Down
2 changes: 1 addition & 1 deletion unittests/Transforms/Utils/Cloning.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,7 @@ TEST_F(CloneFunc, NewFunctionCreated) {
// Test that a new subprogram entry was added and is pointing to the new
// function, while the original subprogram still points to the old one.
TEST_F(CloneFunc, Subprogram) {
EXPECT_FALSE(verifyModule(*M));
EXPECT_FALSE(verifyModule(*M, &errs()));
EXPECT_EQ(3U, Finder->subprogram_count());
EXPECT_NE(NewFunc->getSubprogram(), OldFunc->getSubprogram());
}
Expand Down

0 comments on commit e34d6c6

Please sign in to comment.