[llvm] [RemoveDIs][DebugInfo] Verifier and printing fixes for DPLabel (PR #83242)
Stephen Tozer via llvm-commits
llvm-commits at lists.llvm.org
Mon Mar 4 04:57:46 PST 2024
https://github.com/SLTozer updated https://github.com/llvm/llvm-project/pull/83242
>From 85b695a4cc4557f6aa9f540a3225a56ccce65807 Mon Sep 17 00:00:00 2001
From: Stephen Tozer <stephen.tozer at sony.com>
Date: Wed, 28 Feb 2024 09:17:59 +0000
Subject: [PATCH 1/3] Verifier and printing fixes for DPLabel
---
.../include/llvm/IR/DebugProgramInstruction.h | 12 ++--
llvm/lib/IR/AsmWriter.cpp | 2 +
llvm/lib/IR/DebugProgramInstruction.cpp | 12 +++-
llvm/lib/IR/Verifier.cpp | 62 +++++++++++++++----
.../print-non-instruction-debug-info.ll | 4 +-
5 files changed, 69 insertions(+), 23 deletions(-)
diff --git a/llvm/include/llvm/IR/DebugProgramInstruction.h b/llvm/include/llvm/IR/DebugProgramInstruction.h
index 2dd546ce709c76..3b35c7bd45631a 100644
--- a/llvm/include/llvm/IR/DebugProgramInstruction.h
+++ b/llvm/include/llvm/IR/DebugProgramInstruction.h
@@ -174,13 +174,10 @@ inline raw_ostream &operator<<(raw_ostream &OS, const DbgRecord &R) {
/// llvm.dbg.label intrinsic.
/// FIXME: Rename DbgLabelRecord when DPValue is renamed to DbgVariableRecord.
class DPLabel : public DbgRecord {
- DILabel *Label;
+ TrackingMDNodeRef Label;
public:
- DPLabel(DILabel *Label, DebugLoc DL)
- : DbgRecord(LabelKind, DL), Label(Label) {
- assert(Label && "Unexpected nullptr");
- }
+ DPLabel(DILabel *Label, DebugLoc DL);
DPLabel *clone() const;
void print(raw_ostream &O, bool IsForDebug = false) const;
@@ -188,8 +185,9 @@ class DPLabel : public DbgRecord {
DbgLabelInst *createDebugIntrinsic(Module *M,
Instruction *InsertBefore) const;
- void setLabel(DILabel *NewLabel) { Label = NewLabel; }
- DILabel *getLabel() const { return Label; }
+ void setLabel(DILabel *NewLabel);
+ DILabel *getLabel() const;
+ MDNode *getRawLabel() const { return Label; };
/// Support type inquiry through isa, cast, and dyn_cast.
static bool classof(const DbgRecord *E) {
diff --git a/llvm/lib/IR/AsmWriter.cpp b/llvm/lib/IR/AsmWriter.cpp
index 4e1e48b4ad4a35..9c209c7fb4d46e 100644
--- a/llvm/lib/IR/AsmWriter.cpp
+++ b/llvm/lib/IR/AsmWriter.cpp
@@ -4660,6 +4660,8 @@ void AssemblyWriter::printDPLabel(const DPLabel &Label) {
auto WriterCtx = getContext();
Out << "#dbg_label(";
WriteAsOperandInternal(Out, Label.getLabel(), WriterCtx, true);
+ Out << ", ";
+ WriteAsOperandInternal(Out, Label.getDebugLoc(), WriterCtx, true);
Out << ")";
}
diff --git a/llvm/lib/IR/DebugProgramInstruction.cpp b/llvm/lib/IR/DebugProgramInstruction.cpp
index 3a8b94a87bbcf0..987f1c8f636e6a 100644
--- a/llvm/lib/IR/DebugProgramInstruction.cpp
+++ b/llvm/lib/IR/DebugProgramInstruction.cpp
@@ -123,6 +123,14 @@ DbgRecord::createDebugIntrinsic(Module *M, Instruction *InsertBefore) const {
llvm_unreachable("unsupported DbgRecord kind");
}
+DPLabel::DPLabel(DILabel *Label, DebugLoc DL)
+ : DbgRecord(LabelKind, DL), Label(Label) {
+ assert(Label && "Unexpected nullptr");
+}
+
+void DPLabel::setLabel(DILabel *NewLabel) { Label.reset(NewLabel); }
+DILabel *DPLabel::getLabel() const { return cast<DILabel>(Label); }
+
DPValue *DPValue::createDPValue(Value *Location, DILocalVariable *DV,
DIExpression *Expr, const DILocation *DI) {
return new DPValue(ValueAsMetadata::get(Location), DV, Expr, DI,
@@ -337,7 +345,9 @@ DbgRecord *DbgRecord::clone() const {
DPValue *DPValue::clone() const { return new DPValue(*this); }
-DPLabel *DPLabel::clone() const { return new DPLabel(Label, getDebugLoc()); }
+DPLabel *DPLabel::clone() const {
+ return new DPLabel(getLabel(), getDebugLoc());
+}
DbgVariableIntrinsic *
DPValue::createDebugIntrinsic(Module *M, Instruction *InsertBefore) const {
diff --git a/llvm/lib/IR/Verifier.cpp b/llvm/lib/IR/Verifier.cpp
index 3741e5deaa4cd1..2a93e47fb9c7df 100644
--- a/llvm/lib/IR/Verifier.cpp
+++ b/llvm/lib/IR/Verifier.cpp
@@ -547,6 +547,7 @@ class Verifier : public InstVisitor<Verifier>, VerifierSupport {
void visitTemplateParams(const MDNode &N, const Metadata &RawParams);
+ void visit(DPLabel &DPL);
void visit(DPValue &DPV);
// InstVisitor overrides...
using InstVisitor<Verifier>::visit;
@@ -683,10 +684,17 @@ void Verifier::visitDbgRecords(Instruction &I) {
&I);
CheckDI(!isa<PHINode>(&I) || !I.hasDbgValues(),
"PHI Node must not have any attached DbgRecords", &I);
- for (DPValue &DPV : DPValue::filter(I.getDbgValueRange())) {
- CheckDI(DPV.getMarker() == I.DbgMarker, "DbgRecord had invalid DbgMarker",
- &I, &DPV);
- visit(DPV);
+ for (DbgRecord &DR : I.getDbgValueRange()) {
+ CheckDI(DR.getMarker() == I.DbgMarker, "DbgRecord had invalid DbgMarker",
+ &I, &DR);
+ if (MDNode *N = DR.getDebugLoc().getAsMDNode()) {
+ CheckDI(isa<DILocation>(N), "invalid #dbg record location", &DR, N);
+ visitDILocation(*cast<DILocation>(N));
+ }
+ if (auto *DPV = dyn_cast<DPValue>(&DR))
+ visit(*DPV);
+ else if (auto *DPL = dyn_cast<DPLabel>(&DR))
+ visit(*DPL);
}
}
@@ -6187,6 +6195,34 @@ static DISubprogram *getSubprogram(Metadata *LocalScope) {
return nullptr;
}
+void Verifier::visit(DPLabel &DPL) {
+ CheckDI(isa<DILabel>(DPL.getRawLabel()),
+ "invalid #dbg_label intrinsic variable", &DPL, DPL.getRawLabel());
+
+ // Ignore broken !dbg attachments; they're checked elsewhere.
+ if (MDNode *N = DPL.getDebugLoc().getAsMDNode())
+ if (!isa<DILocation>(N))
+ return;
+
+ BasicBlock *BB = DPL.getParent();
+ Function *F = BB ? BB->getParent() : nullptr;
+
+ // The scopes for variables and !dbg attachments must agree.
+ DILabel *Label = DPL.getLabel();
+ DILocation *Loc = DPL.getDebugLoc();
+ Check(Loc, "#dbg_label record requires a !dbg attachment", &DPL, BB, F);
+
+ DISubprogram *LabelSP = getSubprogram(Label->getRawScope());
+ DISubprogram *LocSP = getSubprogram(Loc->getRawScope());
+ if (!LabelSP || !LocSP)
+ return;
+
+ CheckDI(LabelSP == LocSP,
+ "mismatched subprogram between #dbg_label label and !dbg attachment",
+ &DPL, BB, F, Label, Label->getScope()->getSubprogram(), Loc,
+ Loc->getScope()->getSubprogram());
+}
+
void Verifier::visit(DPValue &DPV) {
CheckDI(DPV.getType() == DPValue::LocationType::Value ||
DPV.getType() == DPValue::LocationType::Declare ||
@@ -6223,16 +6259,20 @@ void Verifier::visit(DPValue &DPV) {
"inst not in same function as #dbg_assign", I, &DPV);
}
- if (MDNode *N = DPV.getDebugLoc().getAsMDNode()) {
- CheckDI(isa<DILocation>(N), "invalid #dbg record location", &DPV, N);
- visitDILocation(*cast<DILocation>(N));
- }
+ // This check is redundant with one in visitLocalVariable().
+ DILocalVariable *Var = DPV.getVariable();
+ CheckDI(isType(Var->getRawType()), "invalid type ref", Var,
+ Var->getRawType());
+
+ // Ignore broken !dbg attachments; they're checked elsewhere.
+ if (MDNode *N = DPV.getDebugLoc().getAsMDNode())
+ if (!isa<DILocation>(N))
+ return;
BasicBlock *BB = DPV.getParent();
Function *F = BB ? BB->getParent() : nullptr;
// The scopes for variables and !dbg attachments must agree.
- DILocalVariable *Var = DPV.getVariable();
DILocation *Loc = DPV.getDebugLoc();
CheckDI(Loc, "missing #dbg record DILocation", &DPV, BB, F);
@@ -6245,10 +6285,6 @@ void Verifier::visit(DPValue &DPV) {
"mismatched subprogram between #dbg record variable and DILocation",
&DPV, BB, F, Var, Var->getScope()->getSubprogram(), Loc,
Loc->getScope()->getSubprogram());
-
- // This check is redundant with one in visitLocalVariable().
- CheckDI(isType(Var->getRawType()), "invalid type ref", Var,
- Var->getRawType());
}
void Verifier::visitVPIntrinsic(VPIntrinsic &VPI) {
diff --git a/llvm/test/DebugInfo/print-non-instruction-debug-info.ll b/llvm/test/DebugInfo/print-non-instruction-debug-info.ll
index f8271df146fe96..2e765619fcb896 100644
--- a/llvm/test/DebugInfo/print-non-instruction-debug-info.ll
+++ b/llvm/test/DebugInfo/print-non-instruction-debug-info.ll
@@ -21,8 +21,8 @@
; CHECK-NEXT: {{^}} %[[VAL_ADD:[0-9a-zA-Z]+]] = add i32 %[[VAL_A]], 5
; OLDDBG-NEXT: call void @llvm.dbg.value(metadata !DIArgList(i32 %[[VAL_A]], i32 %[[VAL_ADD]]), metadata ![[VAR_A]], metadata !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_arg, 1, DW_OP_plus)), !dbg ![[LOC_3:[0-9]+]]
; NEWDBG-NEXT: {{^}} #dbg_value(!DIArgList(i32 %[[VAL_A]], i32 %[[VAL_ADD]]), ![[VAR_A]], !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_arg, 1, DW_OP_plus), ![[LOC_3:[0-9]+]])
-; OLDDBG-NEXT: call void @llvm.dbg.label(metadata ![[LABEL_ID:[0-9]+]])
-; NEWDBG-NEXT: {{^}} #dbg_label(![[LABEL_ID:[0-9]+]])
+; OLDDBG-NEXT: call void @llvm.dbg.label(metadata ![[LABEL_ID:[0-9]+]]), !dbg ![[LOC_3]]
+; NEWDBG-NEXT: {{^}} #dbg_label(![[LABEL_ID:[0-9]+]], ![[LOC_3]])
; CHECK-NEXT: {{^}} store i32 %[[VAL_ADD]]{{.+}}, !DIAssignID ![[ASSIGNID:[0-9]+]]
; OLDDBG-NEXT: call void @llvm.dbg.assign(metadata i32 %[[VAL_ADD]], metadata ![[VAR_B]], metadata !DIExpression(), metadata ![[ASSIGNID]], metadata ptr %[[VAL_B]], metadata !DIExpression()), !dbg ![[LOC_4:[0-9]+]]
; NEWDBG-NEXT: {{^}} #dbg_assign(i32 %[[VAL_ADD]], ![[VAR_B]], !DIExpression(), ![[ASSIGNID]], ptr %[[VAL_B]], !DIExpression(), ![[LOC_4:[0-9]+]])
>From 165c4e2b3c4d64dbef27b55dbf039a3cb237c860 Mon Sep 17 00:00:00 2001
From: Stephen Tozer <stephen.tozer at sony.com>
Date: Wed, 28 Feb 2024 18:09:02 +0000
Subject: [PATCH 2/3] Further printer/verifier fixes
---
.../include/llvm/IR/DebugProgramInstruction.h | 79 +++++++--
llvm/lib/IR/AsmWriter.cpp | 12 +-
llvm/lib/IR/DebugProgramInstruction.cpp | 26 +--
llvm/lib/IR/Verifier.cpp | 162 ++++++++++++++----
4 files changed, 215 insertions(+), 64 deletions(-)
diff --git a/llvm/include/llvm/IR/DebugProgramInstruction.h b/llvm/include/llvm/IR/DebugProgramInstruction.h
index 3b35c7bd45631a..f471ec07c69e82 100644
--- a/llvm/include/llvm/IR/DebugProgramInstruction.h
+++ b/llvm/include/llvm/IR/DebugProgramInstruction.h
@@ -69,6 +69,53 @@ class DPMarker;
class DPValue;
class raw_ostream;
+/// A typed tracking MDNode reference that does not require a definition for its
+/// parameter type. Necessary to avoid including DebugInfoMetadata.h, which has
+/// a significant impact on compile times if included in this file.
+template <typename T> class DbgRecordParamRef {
+ TrackingMDNodeRef Ref;
+
+public:
+public:
+ DbgRecordParamRef() = default;
+
+ /// Construct from the templated type.
+ DbgRecordParamRef(const T *Param);
+
+ /// Construct from an \a MDNode.
+ ///
+ /// Note: if \c N does not have the template type, a verifier check will
+ /// fail, and accessors will crash. However, construction from other nodes
+ /// is supported in order to handle forward references when reading textual
+ /// IR.
+ explicit DbgRecordParamRef(const MDNode *Param);
+
+ /// Get the underlying type.
+ ///
+ /// \pre !*this or \c isa<T>(getAsMDNode()).
+ /// @{
+ T *get() const;
+ operator T *() const { return get(); }
+ T *operator->() const { return get(); }
+ T &operator*() const { return *get(); }
+ /// @}
+
+ /// Check for null.
+ ///
+ /// Check for null in a way that is safe with broken debug info.
+ explicit operator bool() const { return Ref; }
+
+ /// Return \c this as a \a MDNode.
+ MDNode *getAsMDNode() const { return Ref; }
+
+ bool operator==(const DbgRecordParamRef &Other) const {
+ return Ref == Other.Ref;
+ }
+ bool operator!=(const DbgRecordParamRef &Other) const {
+ return Ref != Other.Ref;
+ }
+};
+
/// Base class for non-instruction debug metadata records that have positions
/// within IR. Features various methods copied across from the Instruction
/// class to aid ease-of-use. DbgRecords should always be linked into a
@@ -174,7 +221,7 @@ inline raw_ostream &operator<<(raw_ostream &OS, const DbgRecord &R) {
/// llvm.dbg.label intrinsic.
/// FIXME: Rename DbgLabelRecord when DPValue is renamed to DbgVariableRecord.
class DPLabel : public DbgRecord {
- TrackingMDNodeRef Label;
+ DbgRecordParamRef<DILabel> Label;
public:
DPLabel(DILabel *Label, DebugLoc DL);
@@ -185,9 +232,9 @@ class DPLabel : public DbgRecord {
DbgLabelInst *createDebugIntrinsic(Module *M,
Instruction *InsertBefore) const;
- void setLabel(DILabel *NewLabel);
- DILabel *getLabel() const;
- MDNode *getRawLabel() const { return Label; };
+ void setLabel(DILabel *NewLabel) { Label = NewLabel; }
+ DILabel *getLabel() const { return Label.get(); }
+ MDNode *getRawLabel() const { return Label.getAsMDNode(); };
/// Support type inquiry through isa, cast, and dyn_cast.
static bool classof(const DbgRecord *E) {
@@ -222,9 +269,9 @@ class DPValue : public DbgRecord, protected DebugValueUser {
// DebugValueUser superclass instead. The referred to Value can either be a
// ValueAsMetadata or a DIArgList.
- TrackingMDNodeRef Variable;
- DIExpression *Expression;
- DIExpression *AddressExpression;
+ DbgRecordParamRef<DILocalVariable> Variable;
+ DbgRecordParamRef<DIExpression> Expression;
+ DbgRecordParamRef<DIExpression> AddressExpression;
public:
/// Create a new DPValue representing the intrinsic \p DVI, for example the
@@ -329,10 +376,6 @@ class DPValue : public DbgRecord, protected DebugValueUser {
void addVariableLocationOps(ArrayRef<Value *> NewValues,
DIExpression *NewExpr);
- void setVariable(DILocalVariable *NewVar);
-
- void setExpression(DIExpression *NewExpr) { Expression = NewExpr; }
-
unsigned getNumVariableLocationOps() const;
bool hasArgList() const { return isa<DIArgList>(getRawLocation()); }
@@ -347,10 +390,13 @@ class DPValue : public DbgRecord, protected DebugValueUser {
void setKillLocation();
bool isKillLocation() const;
- DILocalVariable *getVariable() const;
- MDNode *getRawVariable() const { return Variable; }
+ void setVariable(DILocalVariable *NewVar) { Variable = NewVar; }
+ DILocalVariable *getVariable() const { return Variable.get(); };
+ MDNode *getRawVariable() const { return Variable.getAsMDNode(); }
- DIExpression *getExpression() const { return Expression; }
+ void setExpression(DIExpression *NewExpr) { Expression = NewExpr; }
+ DIExpression *getExpression() const { return Expression.get(); }
+ MDNode *getRawExpression() const { return Expression.getAsMDNode(); }
/// Returns the metadata operand for the first location description. i.e.,
/// dbg intrinsic dbg.value,declare operand and dbg.assign 1st location
@@ -399,7 +445,10 @@ class DPValue : public DbgRecord, protected DebugValueUser {
}
Metadata *getRawAssignID() const { return DebugValues[2]; }
DIAssignID *getAssignID() const;
- DIExpression *getAddressExpression() const { return AddressExpression; }
+ DIExpression *getAddressExpression() const { return AddressExpression.get(); }
+ MDNode *getRawAddressExpression() const {
+ return AddressExpression.getAsMDNode();
+ }
void setAddressExpression(DIExpression *NewExpr) {
AddressExpression = NewExpr;
}
diff --git a/llvm/lib/IR/AsmWriter.cpp b/llvm/lib/IR/AsmWriter.cpp
index 9c209c7fb4d46e..f2562c926e3bac 100644
--- a/llvm/lib/IR/AsmWriter.cpp
+++ b/llvm/lib/IR/AsmWriter.cpp
@@ -1147,7 +1147,7 @@ void SlotTracker::processDbgRecordMetadata(const DbgRecord &DR) {
if (DPV->isDbgAssign())
CreateMetadataSlot(cast<MDNode>(DPV->getRawAssignID()));
} else if (const DPLabel *DPL = dyn_cast<const DPLabel>(&DR)) {
- CreateMetadataSlot(DPL->getLabel());
+ CreateMetadataSlot(DPL->getRawLabel());
} else {
llvm_unreachable("unsupported DbgRecord kind");
}
@@ -4631,16 +4631,16 @@ void AssemblyWriter::printDPValue(const DPValue &DPV) {
Out << "(";
WriteAsOperandInternal(Out, DPV.getRawLocation(), WriterCtx, true);
Out << ", ";
- WriteAsOperandInternal(Out, DPV.getVariable(), WriterCtx, true);
+ WriteAsOperandInternal(Out, DPV.getRawVariable(), WriterCtx, true);
Out << ", ";
- WriteAsOperandInternal(Out, DPV.getExpression(), WriterCtx, true);
+ WriteAsOperandInternal(Out, DPV.getRawExpression(), WriterCtx, true);
Out << ", ";
if (DPV.isDbgAssign()) {
- WriteAsOperandInternal(Out, DPV.getAssignID(), WriterCtx, true);
+ WriteAsOperandInternal(Out, DPV.getRawAssignID(), WriterCtx, true);
Out << ", ";
WriteAsOperandInternal(Out, DPV.getRawAddress(), WriterCtx, true);
Out << ", ";
- WriteAsOperandInternal(Out, DPV.getAddressExpression(), WriterCtx, true);
+ WriteAsOperandInternal(Out, DPV.getRawAddressExpression(), WriterCtx, true);
Out << ", ";
}
WriteAsOperandInternal(Out, DPV.getDebugLoc().getAsMDNode(), WriterCtx, true);
@@ -4659,7 +4659,7 @@ void AssemblyWriter::printDbgRecordLine(const DbgRecord &DR) {
void AssemblyWriter::printDPLabel(const DPLabel &Label) {
auto WriterCtx = getContext();
Out << "#dbg_label(";
- WriteAsOperandInternal(Out, Label.getLabel(), WriterCtx, true);
+ WriteAsOperandInternal(Out, Label.getRawLabel(), WriterCtx, true);
Out << ", ";
WriteAsOperandInternal(Out, Label.getDebugLoc(), WriterCtx, true);
Out << ")";
diff --git a/llvm/lib/IR/DebugProgramInstruction.cpp b/llvm/lib/IR/DebugProgramInstruction.cpp
index 987f1c8f636e6a..a8d64024e1797b 100644
--- a/llvm/lib/IR/DebugProgramInstruction.cpp
+++ b/llvm/lib/IR/DebugProgramInstruction.cpp
@@ -13,11 +13,26 @@
namespace llvm {
+template <typename T>
+DbgRecordParamRef<T>::DbgRecordParamRef(const T *Param)
+ : Ref(const_cast<T *>(Param)) {}
+template <typename T>
+DbgRecordParamRef<T>::DbgRecordParamRef(const MDNode *Param)
+ : Ref(const_cast<MDNode *>(Param)) {}
+
+template <typename T> T *DbgRecordParamRef<T>::get() const {
+ return cast<T>(Ref);
+}
+
+template class DbgRecordParamRef<DIExpression>;
+template class DbgRecordParamRef<DILabel>;
+template class DbgRecordParamRef<DILocalVariable>;
+
DPValue::DPValue(const DbgVariableIntrinsic *DVI)
: DbgRecord(ValueKind, DVI->getDebugLoc()),
DebugValueUser({DVI->getRawLocation(), nullptr, nullptr}),
Variable(DVI->getVariable()), Expression(DVI->getExpression()),
- AddressExpression(nullptr) {
+ AddressExpression() {
switch (DVI->getIntrinsicID()) {
case Intrinsic::dbg_value:
Type = LocationType::Value;
@@ -128,9 +143,6 @@ DPLabel::DPLabel(DILabel *Label, DebugLoc DL)
assert(Label && "Unexpected nullptr");
}
-void DPLabel::setLabel(DILabel *NewLabel) { Label.reset(NewLabel); }
-DILabel *DPLabel::getLabel() const { return cast<DILabel>(Label); }
-
DPValue *DPValue::createDPValue(Value *Location, DILocalVariable *DV,
DIExpression *Expr, const DILocation *DI) {
return new DPValue(ValueAsMetadata::get(Location), DV, Expr, DI,
@@ -183,8 +195,6 @@ DPValue *DPValue::createLinkedDPVAssign(Instruction *LinkedInstr, Value *Val,
return NewDPVAssign;
}
-void DPValue::setVariable(DILocalVariable *NewVar) { Variable.reset(NewVar); }
-
iterator_range<DPValue::location_op_iterator> DPValue::location_ops() const {
auto *MD = getRawLocation();
// If a Value has been deleted, the "location" for this DPValue will be
@@ -323,10 +333,6 @@ bool DPValue::isKillLocation() const {
any_of(location_ops(), [](Value *V) { return isa<UndefValue>(V); });
}
-DILocalVariable *DPValue::getVariable() const {
- return cast<DILocalVariable>(Variable.get());
-}
-
std::optional<uint64_t> DPValue::getFragmentSizeInBits() const {
if (auto Fragment = getExpression()->getFragmentInfo())
return Fragment->SizeInBits;
diff --git a/llvm/lib/IR/Verifier.cpp b/llvm/lib/IR/Verifier.cpp
index 2a93e47fb9c7df..5ef7523f546962 100644
--- a/llvm/lib/IR/Verifier.cpp
+++ b/llvm/lib/IR/Verifier.cpp
@@ -174,13 +174,10 @@ struct VerifierSupport {
}
void Write(const DbgRecord *DR) {
- if (DR)
+ if (DR) {
DR->print(*OS, MST, false);
- }
-
- void Write(const DPValue *V) {
- if (V)
- V->print(*OS, MST, false);
+ *OS << '\n';
+ }
}
void Write(DPValue::LocationType Type) {
@@ -635,12 +632,15 @@ class Verifier : public InstVisitor<Verifier>, VerifierSupport {
void verifySiblingFuncletUnwinds();
void verifyFragmentExpression(const DbgVariableIntrinsic &I);
+ void verifyFragmentExpression(const DPValue &I);
template <typename ValueOrMetadata>
void verifyFragmentExpression(const DIVariable &V,
DIExpression::FragmentInfo Fragment,
ValueOrMetadata *Desc);
void verifyFnArgs(const DbgVariableIntrinsic &I);
+ void verifyFnArgs(const DPValue &DPV);
void verifyNotEntryValue(const DbgVariableIntrinsic &I);
+ void verifyNotEntryValue(const DPValue &I);
/// Module-level debug info verification...
void verifyCompileUnits();
@@ -687,14 +687,18 @@ void Verifier::visitDbgRecords(Instruction &I) {
for (DbgRecord &DR : I.getDbgValueRange()) {
CheckDI(DR.getMarker() == I.DbgMarker, "DbgRecord had invalid DbgMarker",
&I, &DR);
- if (MDNode *N = DR.getDebugLoc().getAsMDNode()) {
- CheckDI(isa<DILocation>(N), "invalid #dbg record location", &DR, N);
- visitDILocation(*cast<DILocation>(N));
- }
- if (auto *DPV = dyn_cast<DPValue>(&DR))
+ if (auto *Loc =
+ dyn_cast_or_null<DILocation>(DR.getDebugLoc().getAsMDNode()))
+ visitMDNode(*Loc, AreDebugLocsAllowed::Yes);
+ if (auto *DPV = dyn_cast<DPValue>(&DR)) {
visit(*DPV);
- else if (auto *DPL = dyn_cast<DPLabel>(&DR))
+ // These have to appear after `visit` for consistency with existing
+ // intrinsic behaviour.
+ verifyFragmentExpression(*DPV);
+ verifyNotEntryValue(*DPV);
+ } else if (auto *DPL = dyn_cast<DPLabel>(&DR)) {
visit(*DPL);
+ }
}
}
@@ -3026,9 +3030,10 @@ void Verifier::visitBasicBlock(BasicBlock &BB) {
}
// Confirm that no issues arise from the debug program.
- if (BB.IsNewDbgInfoFormat)
+ if (BB.IsNewDbgInfoFormat) {
CheckDI(!BB.getTrailingDPValues(), "Basic Block has trailing DbgRecords!",
&BB);
+ }
}
void Verifier::visitTerminator(Instruction &I) {
@@ -6210,7 +6215,7 @@ void Verifier::visit(DPLabel &DPL) {
// The scopes for variables and !dbg attachments must agree.
DILabel *Label = DPL.getLabel();
DILocation *Loc = DPL.getDebugLoc();
- Check(Loc, "#dbg_label record requires a !dbg attachment", &DPL, BB, F);
+ CheckDI(Loc, "#dbg_label record requires a !dbg attachment", &DPL, BB, F);
DISubprogram *LabelSP = getSubprogram(Label->getRawScope());
DISubprogram *LocSP = getSubprogram(Loc->getRawScope());
@@ -6224,24 +6229,39 @@ void Verifier::visit(DPLabel &DPL) {
}
void Verifier::visit(DPValue &DPV) {
+ BasicBlock *BB = DPV.getParent();
+ Function *F = BB->getParent();
+
CheckDI(DPV.getType() == DPValue::LocationType::Value ||
DPV.getType() == DPValue::LocationType::Declare ||
DPV.getType() == DPValue::LocationType::Assign,
"invalid #dbg record type", &DPV, DPV.getType());
+
// The location for a DPValue must be either a ValueAsMetadata, DIArgList, or
// an empty MDNode (which is a legacy representation for an "undef" location).
auto *MD = DPV.getRawLocation();
- CheckDI(isa<ValueAsMetadata>(MD) || isa<DIArgList>(MD) ||
- (isa<MDNode>(MD) && !cast<MDNode>(MD)->getNumOperands()),
+ CheckDI(MD && (isa<ValueAsMetadata>(MD) || isa<DIArgList>(MD) ||
+ (isa<MDNode>(MD) && !cast<MDNode>(MD)->getNumOperands())),
"invalid #dbg record address/value", &DPV, MD);
- CheckDI(isa<DILocalVariable>(DPV.getRawVariable()),
+ if (auto *VAM = dyn_cast<ValueAsMetadata>(MD))
+ visitValueAsMetadata(*VAM, F);
+ else if (auto *AL = dyn_cast<DIArgList>(MD))
+ visitDIArgList(*AL, F);
+
+ CheckDI(isa_and_nonnull<DILocalVariable>(DPV.getRawVariable()),
"invalid #dbg record variable", &DPV, DPV.getRawVariable());
- CheckDI(DPV.getExpression(), "missing #dbg record expression", &DPV,
- DPV.getExpression());
+ visitMDNode(*DPV.getRawVariable(), AreDebugLocsAllowed::No);
+
+ CheckDI(isa_and_nonnull<DIExpression>(DPV.getRawExpression()),
+ "invalid #dbg record expression", &DPV, DPV.getRawExpression());
+ visitMDNode(*DPV.getExpression(), AreDebugLocsAllowed::No);
if (DPV.isDbgAssign()) {
- CheckDI(isa<DIAssignID>(DPV.getRawAssignID()),
+ CheckDI(isa_and_nonnull<DIAssignID>(DPV.getRawAssignID()),
"invalid #dbg_assign DIAssignID", &DPV, DPV.getRawAssignID());
+ visitMDNode(*cast<DIAssignID>(DPV.getRawAssignID()),
+ AreDebugLocsAllowed::No);
+
const auto *RawAddr = DPV.getRawAddress();
// Similarly to the location above, the address for an assign DPValue must
// be a ValueAsMetadata or an empty MDNode, which represents an undef
@@ -6250,9 +6270,14 @@ void Verifier::visit(DPValue &DPV) {
isa<ValueAsMetadata>(RawAddr) ||
(isa<MDNode>(RawAddr) && !cast<MDNode>(RawAddr)->getNumOperands()),
"invalid #dbg_assign address", &DPV, DPV.getRawAddress());
- CheckDI(DPV.getAddressExpression(),
- "missing #dbg_assign address expression", &DPV,
- DPV.getAddressExpression());
+ if (auto *VAM = dyn_cast<ValueAsMetadata>(RawAddr))
+ visitValueAsMetadata(*VAM, F);
+
+ CheckDI(isa_and_nonnull<DIExpression>(DPV.getRawAddressExpression()),
+ "invalid #dbg_assign address expression", &DPV,
+ DPV.getRawAddressExpression());
+ visitMDNode(*DPV.getAddressExpression(), AreDebugLocsAllowed::No);
+
// All of the linked instructions should be in the same function as DPV.
for (Instruction *I : at::getAssignmentInsts(&DPV))
CheckDI(DPV.getFunction() == I->getFunction(),
@@ -6264,18 +6289,12 @@ void Verifier::visit(DPValue &DPV) {
CheckDI(isType(Var->getRawType()), "invalid type ref", Var,
Var->getRawType());
- // Ignore broken !dbg attachments; they're checked elsewhere.
- if (MDNode *N = DPV.getDebugLoc().getAsMDNode())
- if (!isa<DILocation>(N))
- return;
-
- BasicBlock *BB = DPV.getParent();
- Function *F = BB ? BB->getParent() : nullptr;
-
- // The scopes for variables and !dbg attachments must agree.
+ auto *DLNode = DPV.getDebugLoc().getAsMDNode();
+ CheckDI(isa_and_nonnull<DILocation>(DLNode), "invalid #dbg record location",
+ &DPV, DLNode);
DILocation *Loc = DPV.getDebugLoc();
- CheckDI(Loc, "missing #dbg record DILocation", &DPV, BB, F);
+ // The scopes for variables and !dbg attachments must agree.
DISubprogram *VarSP = getSubprogram(Var->getRawScope());
DISubprogram *LocSP = getSubprogram(Loc->getRawScope());
if (!VarSP || !LocSP)
@@ -6285,6 +6304,8 @@ void Verifier::visit(DPValue &DPV) {
"mismatched subprogram between #dbg record variable and DILocation",
&DPV, BB, F, Var, Var->getScope()->getSubprogram(), Loc,
Loc->getScope()->getSubprogram());
+
+ verifyFnArgs(DPV);
}
void Verifier::visitVPIntrinsic(VPIntrinsic &VPI) {
@@ -6645,6 +6666,30 @@ void Verifier::verifyFragmentExpression(const DbgVariableIntrinsic &I) {
verifyFragmentExpression(*V, *Fragment, &I);
}
+void Verifier::verifyFragmentExpression(const DPValue &DPV) {
+ DILocalVariable *V = dyn_cast_or_null<DILocalVariable>(DPV.getRawVariable());
+ DIExpression *E = dyn_cast_or_null<DIExpression>(DPV.getRawExpression());
+
+ // We don't know whether this intrinsic verified correctly.
+ if (!V || !E || !E->isValid())
+ return;
+
+ // Nothing to do if this isn't a DW_OP_LLVM_fragment expression.
+ auto Fragment = E->getFragmentInfo();
+ if (!Fragment)
+ return;
+
+ // The frontend helps out GDB by emitting the members of local anonymous
+ // unions as artificial local variables with shared storage. When SROA splits
+ // the storage for artificial local variables that are smaller than the entire
+ // union, the overhang piece will be outside of the allotted space for the
+ // variable and this check fails.
+ // FIXME: Remove this check as soon as clang stops doing this; it hides bugs.
+ if (V->isArtificial())
+ return;
+
+ verifyFragmentExpression(*V, *Fragment, &DPV);
+}
template <typename ValueOrMetadata>
void Verifier::verifyFragmentExpression(const DIVariable &V,
@@ -6691,6 +6736,34 @@ void Verifier::verifyFnArgs(const DbgVariableIntrinsic &I) {
CheckDI(!Prev || (Prev == Var), "conflicting debug info for argument", &I,
Prev, Var);
}
+void Verifier::verifyFnArgs(const DPValue &DPV) {
+ // This function does not take the scope of noninlined function arguments into
+ // account. Don't run it if current function is nodebug, because it may
+ // contain inlined debug intrinsics.
+ if (!HasDebugInfo)
+ return;
+
+ // For performance reasons only check non-inlined ones.
+ if (DPV.getDebugLoc()->getInlinedAt())
+ return;
+
+ DILocalVariable *Var = DPV.getVariable();
+ CheckDI(Var, "#dbg record without variable");
+
+ unsigned ArgNo = Var->getArg();
+ if (!ArgNo)
+ return;
+
+ // Verify there are no duplicate function argument debug info entries.
+ // These will cause hard-to-debug assertions in the DWARF backend.
+ if (DebugFnArgs.size() < ArgNo)
+ DebugFnArgs.resize(ArgNo, nullptr);
+
+ auto *Prev = DebugFnArgs[ArgNo - 1];
+ DebugFnArgs[ArgNo - 1] = Var;
+ CheckDI(!Prev || (Prev == Var), "conflicting debug info for argument", &DPV,
+ Prev, Var);
+}
void Verifier::verifyNotEntryValue(const DbgVariableIntrinsic &I) {
DIExpression *E = dyn_cast_or_null<DIExpression>(I.getRawExpression());
@@ -6715,6 +6788,29 @@ void Verifier::verifyNotEntryValue(const DbgVariableIntrinsic &I) {
"swiftasync Argument",
&I);
}
+void Verifier::verifyNotEntryValue(const DPValue &DPV) {
+ DIExpression *E = dyn_cast_or_null<DIExpression>(DPV.getRawExpression());
+
+ // We don't know whether this intrinsic verified correctly.
+ if (!E || !E->isValid())
+ return;
+
+ if (isa<ValueAsMetadata>(DPV.getRawLocation())) {
+ Value *VarValue = DPV.getVariableLocationOp(0);
+ if (isa<UndefValue>(VarValue) || isa<PoisonValue>(VarValue))
+ return;
+ // We allow EntryValues for swift async arguments, as they have an
+ // ABI-guarantee to be turned into a specific register.
+ if (auto *ArgLoc = dyn_cast_or_null<Argument>(VarValue);
+ ArgLoc && ArgLoc->hasAttribute(Attribute::SwiftAsync))
+ return;
+ }
+
+ CheckDI(!E->isEntryValue(),
+ "Entry values are only allowed in MIR unless they target a "
+ "swiftasync Argument",
+ &DPV);
+}
void Verifier::verifyCompileUnits() {
// When more than one Module is imported into the same context, such as during
>From bd0e9202dced35a158d2a113c4b645721a729178 Mon Sep 17 00:00:00 2001
From: Stephen Tozer <stephen.tozer at sony.com>
Date: Mon, 4 Mar 2024 12:57:07 +0000
Subject: [PATCH 3/3] Fix nits
---
llvm/include/llvm/IR/DebugProgramInstruction.h | 2 +-
llvm/lib/IR/Verifier.cpp | 3 +--
2 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/llvm/include/llvm/IR/DebugProgramInstruction.h b/llvm/include/llvm/IR/DebugProgramInstruction.h
index f471ec07c69e82..cf30b4d0b0aaf0 100644
--- a/llvm/include/llvm/IR/DebugProgramInstruction.h
+++ b/llvm/include/llvm/IR/DebugProgramInstruction.h
@@ -84,7 +84,7 @@ template <typename T> class DbgRecordParamRef {
/// Construct from an \a MDNode.
///
- /// Note: if \c N does not have the template type, a verifier check will
+ /// Note: if \c Param does not have the template type, a verifier check will
/// fail, and accessors will crash. However, construction from other nodes
/// is supported in order to handle forward references when reading textual
/// IR.
diff --git a/llvm/lib/IR/Verifier.cpp b/llvm/lib/IR/Verifier.cpp
index 5ef7523f546962..bf3f7c9d9e8b9f 100644
--- a/llvm/lib/IR/Verifier.cpp
+++ b/llvm/lib/IR/Verifier.cpp
@@ -3030,10 +3030,9 @@ void Verifier::visitBasicBlock(BasicBlock &BB) {
}
// Confirm that no issues arise from the debug program.
- if (BB.IsNewDbgInfoFormat) {
+ if (BB.IsNewDbgInfoFormat)
CheckDI(!BB.getTrailingDPValues(), "Basic Block has trailing DbgRecords!",
&BB);
- }
}
void Verifier::visitTerminator(Instruction &I) {
More information about the llvm-commits
mailing list