Skip to content

Commit

Permalink
Make it illegal for two Functions to point to the same DISubprogram
Browse files Browse the repository at this point in the history
As recently discussed on llvm-dev [1], this patch makes it illegal for
two Functions to point to the same DISubprogram and updates
FunctionCloner to also clone the debug info of a function to conform
to the new requirement. To simplify the implementation it also factors
out the creation of inlineAt locations from the Inliner into a
general-purpose utility in DILocation.

[1] http://lists.llvm.org/pipermail/llvm-dev/2017-May/112661.html
<rdar://problem/31926379>

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

This reapplies r302469 with a fix for a bot failure (reparentDebugInfo
now checks for the case the orig and new function are identical).

llvm-svn: 302576
  • Loading branch information
adrian-prantl committed May 9, 2017
1 parent d979c1f commit c10d0e5
Show file tree
Hide file tree
Showing 7 changed files with 220 additions and 79 deletions.
16 changes: 16 additions & 0 deletions llvm/include/llvm/IR/DebugLoc.h
Expand Up @@ -80,6 +80,22 @@ namespace llvm {
static DebugLoc get(unsigned Line, unsigned Col, const MDNode *Scope,
const MDNode *InlinedAt = nullptr);

enum { ReplaceLastInlinedAt = true };
/// Rebuild the entire inlined-at chain for this instruction so that the top of
/// the chain now is inlined-at the new call site.
/// \param InlinedAt The new outermost inlined-at in the chain.
/// \param ReplaceLast Replace the last location in the inlined-at chain.
static DebugLoc appendInlinedAt(DebugLoc DL, DILocation *InlinedAt,
LLVMContext &Ctx,
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
114 changes: 114 additions & 0 deletions llvm/lib/IR/DebugLoc.cpp
Expand Up @@ -8,6 +8,7 @@
//===----------------------------------------------------------------------===//

#include "llvm/IR/DebugLoc.h"
#include "llvm/IR/IntrinsicInst.h"
#include "LLVMContextImpl.h"
#include "llvm/IR/DebugInfo.h"
using namespace llvm;
Expand Down Expand Up @@ -66,6 +67,119 @@ DebugLoc DebugLoc::get(unsigned Line, unsigned Col, const MDNode *Scope,
const_cast<MDNode *>(InlinedAt));
}

DebugLoc DebugLoc::appendInlinedAt(DebugLoc DL, DILocation *InlinedAt,
LLVMContext &Ctx,
DenseMap<const MDNode *, MDNode *> &Cache,
bool ReplaceLast) {
SmallVector<DILocation *, 3> InlinedAtLocations;
DILocation *Last = InlinedAt;
DILocation *CurInlinedAt = DL;

// Gather all the inlined-at nodes.
while (DILocation *IA = CurInlinedAt->getInlinedAt()) {
// Skip any we've already built nodes for.
if (auto *Found = Cache[IA]) {
Last = cast<DILocation>(Found);
break;
}

if (ReplaceLast && !IA->getInlinedAt())
break;
InlinedAtLocations.push_back(IA);
CurInlinedAt = IA;
}

// 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 DILocation *MD : reverse(InlinedAtLocations))
Cache[MD] = Last = DILocation::getDistinct(
Ctx, MD->getLine(), MD->getColumn(), MD->getScope(), Last);

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::getDistinct(
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
13 changes: 11 additions & 2 deletions llvm/lib/IR/Verifier.cpp
Expand Up @@ -267,6 +267,9 @@ class Verifier : public InstVisitor<Verifier>, VerifierSupport {
/// \brief Keep track of the metadata nodes that have been checked already.
SmallPtrSet<const Metadata *, 32> MDNodes;

/// Keep track which DISubprogram is attached to which function.
DenseMap<const DISubprogram *, const Function *> DISubprogramAttachments;

/// Track all DICompileUnits visited.
SmallPtrSet<const Metadata *, 2> CUVisited;

Expand Down Expand Up @@ -386,7 +389,7 @@ class Verifier : public InstVisitor<Verifier>, VerifierSupport {
verifyCompileUnits();

verifyDeoptimizeCallingConvs();

DISubprogramAttachments.clear();
return !Broken;
}

Expand Down Expand Up @@ -2085,13 +2088,19 @@ void Verifier::visitFunction(const Function &F) {
switch (I.first) {
default:
break;
case LLVMContext::MD_dbg:
case LLVMContext::MD_dbg: {
++NumDebugAttachments;
AssertDI(NumDebugAttachments == 1,
"function must have a single !dbg attachment", &F, I.second);
AssertDI(isa<DISubprogram>(I.second),
"function !dbg attachment must be a subprogram", &F, I.second);
auto *SP = cast<DISubprogram>(I.second);
const Function *&AttachedTo = DISubprogramAttachments[SP];
AssertDI(!AttachedTo || AttachedTo == &F,
"DISubprogram attached to more than one function", SP, &F);
AttachedTo = &F;
break;
}
case LLVMContext::MD_prof:
++NumProfAttachments;
Assert(NumProfAttachments == 1,
Expand Down
32 changes: 26 additions & 6 deletions llvm/lib/Transforms/Utils/CloneFunction.cpp
Expand Up @@ -41,6 +41,7 @@ BasicBlock *llvm::CloneBasicBlock(const BasicBlock *BB,
ValueToValueMapTy &VMap,
const Twine &NameSuffix, Function *F,
ClonedCodeInfo *CodeInfo) {
DenseMap<const MDNode *, MDNode *> Cache;
BasicBlock *NewBB = BasicBlock::Create(BB->getContext(), "", F);
if (BB->hasName()) NewBB->setName(BB->getName()+NameSuffix);

Expand All @@ -50,6 +51,9 @@ BasicBlock *llvm::CloneBasicBlock(const BasicBlock *BB,
for (BasicBlock::const_iterator II = BB->begin(), IE = BB->end();
II != IE; ++II) {
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 @@ -120,12 +124,28 @@ void llvm::CloneFunctionInto(Function *NewFunc, const Function *OldFunc,

SmallVector<std::pair<unsigned, MDNode *>, 1> MDs;
OldFunc->getAllMetadata(MDs);
for (auto MD : MDs)
NewFunc->addMetadata(
MD.first,
*MapMetadata(MD.second, VMap,
ModuleLevelChanges ? RF_None : RF_NoModuleLevelChanges,
TypeMapper, Materializer));
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(),
NewFunc->getName(), 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);
}

// 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
Expand Down
43 changes: 5 additions & 38 deletions llvm/lib/Transforms/Utils/InlineFunction.cpp
Expand Up @@ -1302,41 +1302,6 @@ static bool hasLifetimeMarkers(AllocaInst *AI) {
return false;
}

/// Rebuild the entire inlined-at chain for this instruction so that the top of
/// the chain now is inlined-at the new call site.
static DebugLoc
updateInlinedAtInfo(const DebugLoc &DL, DILocation *InlinedAtNode,
LLVMContext &Ctx,
DenseMap<const DILocation *, DILocation *> &IANodes) {
SmallVector<DILocation *, 3> InlinedAtLocations;
DILocation *Last = InlinedAtNode;
DILocation *CurInlinedAt = DL;

// Gather all the inlined-at nodes
while (DILocation *IA = CurInlinedAt->getInlinedAt()) {
// Skip any we've already built nodes for
if (DILocation *Found = IANodes[IA]) {
Last = Found;
break;
}

InlinedAtLocations.push_back(IA);
CurInlinedAt = IA;
}

// 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 DILocation *MD : reverse(InlinedAtLocations)) {
Last = IANodes[MD] = DILocation::getDistinct(
Ctx, MD->getLine(), MD->getColumn(), MD->getScope(), Last);
}

// And finally create the normal location for this instruction, referring to
// the new inlined-at chain.
return DebugLoc::get(DL.getLine(), DL.getCol(), DL.getScope(), Last);
}

/// Return the result of AI->isStaticAlloca() if AI were moved to the entry
/// block. Allocas used in inalloca calls and allocas of dynamic array size
/// cannot be static.
Expand Down Expand Up @@ -1364,14 +1329,16 @@ static void fixupLineNumbers(Function *Fn, Function::iterator FI,
// Cache the inlined-at nodes as they're built so they are reused, without
// this every instruction's inlined-at chain would become distinct from each
// other.
DenseMap<const DILocation *, DILocation *> IANodes;
DenseMap<const MDNode *, MDNode *> IANodes;

for (; FI != Fn->end(); ++FI) {
for (BasicBlock::iterator BI = FI->begin(), BE = FI->end();
BI != BE; ++BI) {
if (DebugLoc DL = BI->getDebugLoc()) {
BI->setDebugLoc(
updateInlinedAtInfo(DL, InlinedAtNode, BI->getContext(), IANodes));
auto IA = DebugLoc::appendInlinedAt(DL, InlinedAtNode, BI->getContext(),
IANodes);
auto IDL = DebugLoc::get(DL.getLine(), DL.getCol(), DL.getScope(), IA);
BI->setDebugLoc(IDL);
continue;
}

Expand Down
16 changes: 11 additions & 5 deletions llvm/test/Verifier/metadata-function-dbg.ll
Expand Up @@ -3,20 +3,26 @@
; CHECK: function declaration may not have a !dbg attachment
declare !dbg !4 void @f1()

define void @f2() !dbg !4 {
; CHECK: function must have a single !dbg attachment
define void @f2() !dbg !4 !dbg !4 {
unreachable
}

; CHECK: function must have a single !dbg attachment
define void @f3() !dbg !4 !dbg !4 {
; CHECK: DISubprogram attached to more than one function
define void @f3() !dbg !4 {
unreachable
}

; CHECK: DISubprogram attached to more than one function
define void @f4() !dbg !4 {
unreachable
}

; CHECK-NOT: !dbg
; CHECK: function !dbg attachment must be a subprogram
; CHECK-NEXT: void ()* @bar
; CHECK-NEXT: !{{[0-9]+}} = !{}
define void @bar() !dbg !6 {
define void @bar() !dbg !3 {
unreachable
}

Expand All @@ -26,5 +32,5 @@ define void @bar() !dbg !6 {
!llvm.dbg.cu = !{!1}
!1 = distinct !DICompileUnit(language: DW_LANG_C99, file: !2)
!2 = !DIFile(filename: "t.c", directory: "/path/to/dir")
!3 = !{}
!4 = distinct !DISubprogram(name: "foo", scope: !1, file: !2, unit: !1)
!6 = !{}

0 comments on commit c10d0e5

Please sign in to comment.