Verify that no compile units share the same line table in "llvm-dwarfdump --verify"

[clayborg]

Summary

Check to make sure no compile units have the same DW_AT_stmt_list values. Report a verification error if they do.

[aprantl]

lib/DebugInfo/DWARF/DWARFContext.cpp

298 ↗

(On Diff #97520)

I think it starts to make sense to create a lib/DebugInfo/DWARF/DWARFVerifier.cpp file.

487 ↗

(On Diff #97520)

The check is fine, but this function is starting to get really long and hard to follow, especially with all the continues. Could you break it up into smaller bool verifyXXX() or visitXXX() helpers?

[clayborg]

It is fine to create a DWARFVerifier.cpp, but can we do that in the next patch? Do you really want to review a patch where everything is moved around? And all functionality is moved around? This patch is nice and self contained.

[aprantl]

Sure. I'm fine with a separate commit that breaks the Verifier out into its own file and also breaks the current HandleDebugLine function (which btw. should start with a lower-case character) into smaller functions.

llvm/trunk/lib/DebugInfo/DWARF/DWARFContext.cpp

for (auto Pair: ReferenceToDIEOffsets) {
         OS << "\n";
       }
       OS << "\n";
     }
     return Success;
   }
 
   bool HandleDebugLine() {
+    std::map<uint64_t, DWARFDie> StmtListToDie;
     bool Success = true;
     OS << "Verifying .debug_line...\n";
     for (const auto &CU : DCtx.compile_units()) {
       uint32_t LineTableOffset = 0;
-      auto StmtFormValue = CU->getUnitDIE().find(DW_AT_stmt_list);
+      auto CUDie = CU->getUnitDIE();
+      auto StmtFormValue = CUDie.find(DW_AT_stmt_list);
       if (!StmtFormValue) {
         // No line table for this compile unit.
         continue;
       }
       // Get the attribute value as a section offset. No need to produce an
       // error here if the encoding isn't correct because we validate this in
       // the .debug_info verifier.
       if (auto StmtSectionOffset = toSectionOffset(StmtFormValue)) {
         LineTableOffset = *StmtSectionOffset;
         if (LineTableOffset >= DCtx.getLineSection().Data.size()) {
           // Make sure we don't get a valid line table back if the offset
           // is wrong.
           assert(DCtx.getLineTableForUnit(CU.get()) == nullptr);
           // Skip this line table as it isn't valid. No need to create an error
           // here because we validate this in the .debug_info verifier.
           continue;
+        } else {
+          auto Iter = StmtListToDie.find(LineTableOffset);
+          if (Iter != StmtListToDie.end()) {
+            Success = false;
+            OS << "error: two compile unit DIEs, "
+               << format("0x%08" PRIx32, Iter->second.getOffset()) << " and "
+               << format("0x%08" PRIx32, CUDie.getOffset())
+               << ", have the same DW_AT_stmt_list section offset:\n";
+            Iter->second.dump(OS, 0);
+            CUDie.dump(OS, 0);
+            OS << '\n';
+            // Already verified this line table before, no need to do it again.
+            continue;
+          }
+          StmtListToDie[LineTableOffset] = CUDie;
         }
       }
       auto LineTable = DCtx.getLineTableForUnit(CU.get());
       if (!LineTable) {
         Success = false;
         OS << "error: .debug_line[" << format("0x%08" PRIx32, LineTableOffset)
            << "] was not able to be parsed for CU:\n";
-        CU->getUnitDIE().dump(OS, 0);
+        CUDie.dump(OS, 0);
         OS << '\n';
         continue;
       }
       uint32_t MaxFileIndex = LineTable->Prologue.FileNames.size();
       uint64_t PrevAddress = 0;
       uint32_t RowIndex = 0;
       for (const auto &Row : LineTable->Rows) {
         if (Row.Address < PrevAddress) {

llvm/trunk/unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp

TEST(DWARFDebugInfo, TestDwarfVerifyInvalidLineFileIndex) {
   )";
   auto ErrOrSections = DWARFYAML::EmitDebugSections(yamldata);
   ASSERT_TRUE((bool)ErrOrSections);
   DWARFContextInMemory DwarfContext(*ErrOrSections, 8);
   VerifyError(DwarfContext, "error: .debug_line[0x00000000][1] has invalid "
                             "file index 5 (valid values are [1,1]):");
 }
 
+TEST(DWARFDebugInfo, TestDwarfVerifyCUDontShareLineTable) {
+  // Create a two compile units where both compile units share the same
+  // DW_AT_stmt_list value and verify we report the error correctly.
+  StringRef yamldata = R"(
+    debug_str:
+      - ''
+      - /tmp/main.c
+      - /tmp/foo.c
+    debug_abbrev:    
+      - Code:            0x00000001
+        Tag:             DW_TAG_compile_unit
+        Children:        DW_CHILDREN_no
+        Attributes:      
+          - Attribute:       DW_AT_name
+            Form:            DW_FORM_strp
+          - Attribute:       DW_AT_stmt_list
+            Form:            DW_FORM_sec_offset
+    debug_info:      
+      - Length:          
+          TotalLength:     16
+        Version:         4
+        AbbrOffset:      0
+        AddrSize:        8
+        Entries:         
+          - AbbrCode:        0x00000001
+            Values:          
+              - Value:           0x0000000000000001
+              - Value:           0x0000000000000000
+      - Length:          
+          TotalLength:     16
+        Version:         4
+        AbbrOffset:      0
+        AddrSize:        8
+        Entries:         
+          - AbbrCode:        0x00000001
+            Values:          
+              - Value:           0x000000000000000D
+              - Value:           0x0000000000000000
+    debug_line:      
+      - Length:          
+          TotalLength:     60
+        Version:         2
+        PrologueLength:  34
+        MinInstLength:   1
+        DefaultIsStmt:   1
+        LineBase:        251
+        LineRange:       14
+        OpcodeBase:      13
+        StandardOpcodeLengths: [ 0, 1, 1, 1, 1, 0, 0, 0, 1, 0, 0, 1 ]
+        IncludeDirs:     
+          - /tmp
+        Files:           
+          - Name:            main.c
+            DirIdx:          1
+            ModTime:         0
+            Length:          0
+        Opcodes:         
+          - Opcode:          DW_LNS_extended_op
+            ExtLen:          9
+            SubOpcode:       DW_LNE_set_address
+            Data:            4096
+          - Opcode:          DW_LNS_advance_line
+            SData:           9
+            Data:            4096
+          - Opcode:          DW_LNS_copy
+            Data:            4096
+          - Opcode:          DW_LNS_advance_pc
+            Data:            256
+          - Opcode:          DW_LNS_extended_op
+            ExtLen:          1
+            SubOpcode:       DW_LNE_end_sequence
+            Data:            256
+  )";
+  auto ErrOrSections = DWARFYAML::EmitDebugSections(yamldata);
+  ASSERT_TRUE((bool)ErrOrSections);
+  DWARFContextInMemory DwarfContext(*ErrOrSections, 8);
+  VerifyError(DwarfContext, "error: two compile unit DIEs, 0x0000000b and "
+                            "0x0000001f, have the same DW_AT_stmt_list section "
+                            "offset:");
+}
+
 } // end anonymous namespace