Parser fixes for DPLabel

Author: SLTozer

llvm/include/llvm/AsmParser/LLParser.h

@@ -576,7 +576,7 @@ namespace llvm {
     bool parseMDNodeTail(MDNode *&N);
     bool parseMDNodeVector(SmallVectorImpl<Metadata *> &Elts);
     bool parseMetadataAttachment(unsigned &Kind, MDNode *&MD);
-    bool parseDebugProgramValue(DPValue *&DPV, PerFunctionState &PFS);
+    bool parseDebugRecord(DbgRecord *&DR, PerFunctionState &PFS);
     bool parseInstructionMetadata(Instruction &Inst);
     bool parseGlobalObjectMetadataAttachment(GlobalObject &GO);
     bool parseOptionalFunctionMetadata(Function &F);

llvm/include/llvm/IR/DebugProgramInstruction.h

@@ -223,9 +223,18 @@ inline raw_ostream &operator<<(raw_ostream &OS, const DbgRecord &R) {
 class DPLabel : public DbgRecord {
   DbgRecordParamRef<DILabel> Label;
 
+  /// This constructor intentionally left private, so that it is only called via
+  /// "createUnresolvedDPLabel", which clearly expresses that it is for parsing
+  /// only.
+  DPLabel(MDNode *Label, MDNode *DL);
 public:
   DPLabel(DILabel *Label, DebugLoc DL);
 
+  /// For use during parsing; creates a DPLabel from as-of-yet unresolved
+  /// MDNodes. Trying to access the resulting DPLabel's fields before they are
+  /// resolved, or if they resolve to the wrong type, will result in a crash.
+  static DPLabel *createUnresolvedDPLabel(MDNode *Label, MDNode *DL);
+
   DPLabel *clone() const;
   void print(raw_ostream &O, bool IsForDebug = false) const;
   void print(raw_ostream &ROS, ModuleSlotTracker &MST, bool IsForDebug) const;

llvm/lib/AsmParser/LLLexer.cpp

@@ -938,6 +938,7 @@ lltok::Kind LLLexer::LexIdentifier() {
   DBGRECORDTYPEKEYWORD(value);
   DBGRECORDTYPEKEYWORD(declare);
   DBGRECORDTYPEKEYWORD(assign);
+  DBGRECORDTYPEKEYWORD(label);
 #undef DBGRECORDTYPEKEYWORD
 
   if (Keyword.starts_with("DIFlag")) {

llvm/lib/AsmParser/LLParser.cpp

@@ -6422,7 +6422,11 @@ bool LLParser::parseBasicBlock(PerFunctionState &PFS) {
   // Parse the instructions and debug values in this block until we get a
   // terminator.
   Instruction *Inst;
-  SmallVector<std::unique_ptr<DPValue>> TrailingDPValues;
+  auto DeleteDbgRecord = [](DbgRecord *DR) {
+    DR->deleteRecord();
+  };
+  using DbgRecordPtr = std::unique_ptr<DbgRecord, decltype(DeleteDbgRecord)>;
+  SmallVector<DbgRecordPtr> TrailingDbgRecord;
   do {
     // Handle debug records first - there should always be an instruction
     // following the debug records, i.e. they cannot appear after the block
@@ -6436,10 +6440,10 @@ bool LLParser::parseBasicBlock(PerFunctionState &PFS) {
       if (!M->IsNewDbgInfoFormat)
         M->convertToNewDbgValues();
 
-      DPValue *DPV;
-      if (parseDebugProgramValue(DPV, PFS))
+      DbgRecord *DR;
+      if (parseDebugRecord(DR, PFS))
         return true;
-      TrailingDPValues.emplace_back(DPV);
+      TrailingDbgRecord.emplace_back(DR, DeleteDbgRecord);
     }
 
     // This instruction may have three possibilities for a name: a) none
@@ -6488,35 +6492,58 @@ bool LLParser::parseBasicBlock(PerFunctionState &PFS) {
       return true;
 
     // Attach any preceding debug values to this instruction.
-    for (std::unique_ptr<DPValue> &DPV : TrailingDPValues)
-      BB->insertDPValueBefore(DPV.release(), Inst->getIterator());
-    TrailingDPValues.clear();
+    for (DbgRecordPtr &DR : TrailingDbgRecord)
+      BB->insertDPValueBefore(DR.release(), Inst->getIterator());
+    TrailingDbgRecord.clear();
   } while (!Inst->isTerminator());
 
-  assert(TrailingDPValues.empty() &&
+  assert(TrailingDbgRecord.empty() &&
          "All debug values should have been attached to an instruction.");
 
   return false;
 }
 
-/// parseDebugProgramValue
+/// parseDebugRecord
+///   ::= #dbg_label '(' MDNode ')'
 ///   ::= #dbg_type '(' Metadata ',' MDNode ',' Metadata ','
 ///                 (MDNode ',' Metadata ',' Metadata ',')? MDNode ')'
-bool LLParser::parseDebugProgramValue(DPValue *&DPV, PerFunctionState &PFS) {
+bool LLParser::parseDebugRecord(DbgRecord *&DR, PerFunctionState &PFS) {
+  using RecordKind = DbgRecord::Kind;
   using LocType = DPValue::LocationType;
   LocTy DPVLoc = Lex.getLoc();
   if (Lex.getKind() != lltok::DbgRecordType)
     return error(DPVLoc, "expected debug record type here");
-  auto Type = StringSwitch<LocType>(Lex.getStrVal())
+  RecordKind RecordType = StringSwitch<RecordKind>(Lex.getStrVal())
+                  .Case("declare", RecordKind::ValueKind)
+                  .Case("value", RecordKind::ValueKind)
+                  .Case("assign", RecordKind::ValueKind)
+                  .Case("label", RecordKind::LabelKind);
+
+  // Parsing labels is trivial; parse here and early exit, otherwise go into the
+  // full DPValue processing stage.
+  if (RecordType == RecordKind::LabelKind) {
+    Lex.Lex();
+    if (parseToken(lltok::lparen, "Expected '(' here"))
+      return true;
+    MDNode *Label;
+    if (parseMDNode(Label))
+      return true;
+    if (parseToken(lltok::comma, "Expected ',' here"))
+      return true;
+    MDNode *DbgLoc;
+    if (parseMDNode(DbgLoc))
+      return true;
+    if (parseToken(lltok::rparen, "Expected ')' here"))
+      return true;
+    DR = DPLabel::createUnresolvedDPLabel(Label, DbgLoc);
+    return false;
+  }
+
+  LocType ValueType = StringSwitch<LocType>(Lex.getStrVal())
                   .Case("declare", LocType::Declare)
                   .Case("value", LocType::Value)
-                  .Case("assign", LocType::Assign)
-                  .Default(LocType::End);
-  // If the file contained an invalid debug record type then parsing should fail
-  // above; the assert here should only fire if the Lexer gives us an invalid
-  // value.
-  assert(Type != LocType::End &&
-         "Lexer returned an invalid DbgRecordType string.");
+                  .Case("assign", LocType::Assign);
+
   Lex.Lex();
   if (parseToken(lltok::lparen, "Expected '(' here"))
     return true;
@@ -6549,7 +6576,7 @@ bool LLParser::parseDebugProgramValue(DPValue *&DPV, PerFunctionState &PFS) {
   MDNode *AssignID = nullptr;
   Metadata *AddressLocation = nullptr;
   Metadata *AddressExpression = nullptr;
-  if (Type == LocType::Assign) {
+  if (ValueType == LocType::Assign) {
     // Parse DIAssignID.
     if (parseMDNode(AssignID))
       return true;
@@ -6579,8 +6606,8 @@ bool LLParser::parseDebugProgramValue(DPValue *&DPV, PerFunctionState &PFS) {
 
   if (parseToken(lltok::rparen, "Expected ')' here"))
     return true;
-  DPV = DPValue::createUnresolvedDPValue(
-      Type, ValLocMD, Variable, cast<DIExpression>(Expression), AssignID,
+  DR = DPValue::createUnresolvedDPValue(
+      ValueType, ValLocMD, Variable, cast<DIExpression>(Expression), AssignID,
       AddressLocation, cast_or_null<DIExpression>(AddressExpression), DebugLoc);
   return false;
 }

llvm/lib/IR/DebugProgramInstruction.cpp

@@ -138,16 +138,26 @@ DbgRecord::createDebugIntrinsic(Module *M, Instruction *InsertBefore) const {
   llvm_unreachable("unsupported DbgRecord kind");
 }
 
+
+DPLabel::DPLabel(MDNode *Label, MDNode *DL)
+    : DbgRecord(LabelKind, DebugLoc(DL)), Label(Label) {
+  assert(Label && "Unexpected nullptr");
+  assert(isa<DILabel>(Label) || Label->isTemporary() && "Label type must be or resolve to a DILabel");
+}
 DPLabel::DPLabel(DILabel *Label, DebugLoc DL)
     : DbgRecord(LabelKind, DL), Label(Label) {
   assert(Label && "Unexpected nullptr");
 }
 
+DPLabel *DPLabel::createUnresolvedDPLabel(MDNode *Label, MDNode *DL) {
+  return new DPLabel(Label, DL);
+}
+
 DPValue::DPValue(DPValue::LocationType Type, Metadata *Val, MDNode *Variable,
                  MDNode *Expression, MDNode *AssignID, Metadata *Address,
                  MDNode *AddressExpression, MDNode *DI)
-    : DebugValueUser({Val, Address, AssignID}), Variable(Variable),
-      Expression(Expression), DbgLoc(DI), AddressExpression(AddressExpression),
+    : DbgRecord(ValueKind, DebugLoc(DI)), DebugValueUser({Val, Address, AssignID}), Variable(Variable),
+      Expression(Expression), AddressExpression(AddressExpression),
       Type(Type) {}
 
 DPValue *DPValue::createUnresolvedDPValue(DPValue::LocationType Type,

llvm/lib/IR/Verifier.cpp

@@ -684,6 +684,7 @@ 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())) {
   for (DbgRecord &DR : I.getDbgValueRange()) {
     CheckDI(DR.getMarker() == I.DbgMarker, "DbgRecord had invalid DbgMarker",
             &I, &DR);

llvm/test/DebugInfo/roundtrip-non-instruction-debug-info.ll

@@ -28,6 +28,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]+]]), !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]+]])
@@ -36,6 +38,7 @@
 ; OLDDBG-DAG: declare void @llvm.dbg.value
 ; OLDDBG-DAG: declare void @llvm.dbg.declare
 ; OLDDBG-DAG: declare void @llvm.dbg.assign
+; OLDDBG-DAG: declare void @llvm.dbg.label
 
 ; CHECK-DAG: llvm.dbg.cu
 ; CHECK-DAG: ![[VAR_A]] = !DILocalVariable(name: "a"
@@ -44,6 +47,7 @@
 ; CHECK-DAG: ![[LOC_2]] = !DILocation(line: 3, column: 20
 ; CHECK-DAG: ![[LOC_3]] = !DILocation(line: 3, column: 25
 ; CHECK-DAG: ![[LOC_4]] = !DILocation(line: 3, column: 30
+; CHECK-DAG: ![[LABEL_ID]] = !DILabel(
 
 define dso_local i32 @f(i32 %a) !dbg !7 {
 entry:
@@ -52,6 +56,7 @@ entry:
   call void @llvm.dbg.declare(metadata ptr %b, metadata !21, metadata !DIExpression()), !dbg !31
   %add = add i32 %a, 5, !dbg !31
   call void @llvm.dbg.value(metadata !DIArgList(i32 %a, i32 %add), metadata !20, metadata !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_arg, 1, DW_OP_plus)), !dbg !32
+  call void @llvm.dbg.label(metadata !50), !dbg !32
   store i32 %add, ptr %b, !dbg !32, !DIAssignID !40
   call void @llvm.dbg.assign(metadata i32 %add, metadata !21, metadata !DIExpression(), metadata !40, metadata ptr %b, metadata !DIExpression()), !dbg !33
   ret i32 %add, !dbg !33
@@ -61,6 +66,7 @@ entry:
 declare void @llvm.dbg.value(metadata, metadata, metadata)
 declare void @llvm.dbg.declare(metadata, metadata, metadata)
 declare void @llvm.dbg.assign(metadata, metadata, metadata, metadata, metadata, metadata)
+declare void @llvm.dbg.label(metadata)
 
 !llvm.dbg.cu = !{!0}
 !llvm.module.flags = !{!3, !4, !5}
@@ -84,4 +90,5 @@ declare void @llvm.dbg.assign(metadata, metadata, metadata, metadata, metadata,
 !31 = !DILocation(line: 3, column: 20, scope: !7)
 !32 = !DILocation(line: 3, column: 25, scope: !7)
 !33 = !DILocation(line: 3, column: 30, scope: !7)
-!40 = distinct !DIAssignID()
+!40 = distinct !DIAssignID()
+!50 = !DILabel(scope: !7, name: "label", file: !1, line: 3)