[dwarfdump] Verify line table prologue

[JDevlieghere]

Summary

This patch adds prologue verification, which is already present in Apple's dwarfdump. It checks for invalid directory indices and warns about duplicate file paths.

[dblaikie]

lib/DebugInfo/DWARF/DWARFVerifier.cpp

455 ↗

(On Diff #113991)

Don't branch-to-unreachable. Use an assert instead.

Is the false return from getFileNameByIndex really impossible? (has the FileIndex been checked for validity? What other error paths does getFileNameByIndex have, if any?)

[JDevlieghere]

lib/DebugInfo/DWARF/DWARFVerifier.cpp

455 ↗

(On Diff #113991)

Thanks, an assert definitely better expresses the intent. The only path that returns false is this:

if (Kind == FileLineInfoKind::None || !hasFileAtIndex(FileIndex))
  return false;

The FileLineInfoKind is hard-coded so that can't trigger it. The index can't really be invalid either as we're iterating over the list of indices. If this returns false here it must be a programmer error.

[JDevlieghere]

lib/DebugInfo/DWARF/DWARFVerifier.cpp

455 ↗

(On Diff #113991)

What about the unused variable though? I need the function call to populate the string, so I'd have to assign it to a bool that's only used in the assert.

[dblaikie]

Looks good (with the change to the unreachable -> assert)

[clayborg]

Why assert and not emit an error???

lib/DebugInfo/DWARF/DWARFVerifier.cpp

455 ↗

(On Diff #113991)

This is a verifier that is trying to validate that the DWARF is valid. A bad file index is quite possible. Why not emit an appropriate error message here?

[JDevlieghere]

Use assert instead of unreachable.

[aprantl]

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

2072

Porlogue? :-)

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

 void DWARFVerifier::verifyDebugLineRows() {
   for (const auto &CU : DCtx.compile_units()) {
     auto Die = CU->getUnitDIE();
     auto LineTable = DCtx.getLineTableForUnit(CU.get());
     // If there is no line table we will have created an error in the
     // .debug_info verifier or in verifyDebugLineStmtOffsets().
     if (!LineTable)
       continue;
+
+    // Verify prologue.
     uint32_t MaxFileIndex = LineTable->Prologue.FileNames.size();
+    uint32_t MaxDirIndex = LineTable->Prologue.IncludeDirectories.size();
+    uint32_t FileIndex = 1;
+    StringMap<uint16_t> FullPathMap;
+    for (const auto &FileName : LineTable->Prologue.FileNames) {
+      // Verify directory index.
+      if (FileName.DirIdx > MaxDirIndex) {
+        ++NumDebugLineErrors;
+        OS << "error: .debug_line["
+           << format("0x%08" PRIx64,
+                     *toSectionOffset(Die.find(DW_AT_stmt_list)))
+           << "].prologue.file_names[" << FileIndex
+           << "].dir_idx contains an invalid index: " << FileName.DirIdx
+           << "\n";
+      }
+
+      // Check file paths for duplicates.
+      std::string FullPath;
+      const bool HasFullPath = LineTable->getFileNameByIndex(
+          FileIndex, CU->getCompilationDir(),
+          DILineInfoSpecifier::FileLineInfoKind::AbsoluteFilePath, FullPath);
+      assert(HasFullPath && "Invalid index?");
+      (void)HasFullPath;
+      auto It = FullPathMap.find(FullPath);
+      if (It == FullPathMap.end())
+        FullPathMap[FullPath] = FileIndex;
+      else if (It->second != FileIndex) {
+        OS << "warning: .debug_line["
+           << format("0x%08" PRIx64,
+                     *toSectionOffset(Die.find(DW_AT_stmt_list)))
+           << "].prologue.file_names[" << FileIndex
+           << "] is a duplicate of file_names[" << It->second << "]\n";
+      }
+
+      FileIndex++;
+    }
+
+    // Verify rows.
     uint64_t PrevAddress = 0;
     uint32_t RowIndex = 0;
     for (const auto &Row : LineTable->Rows) {
+      // Verify row address.
       if (Row.Address < PrevAddress) {
         ++NumDebugLineErrors;
         OS << "error: .debug_line["
            << format("0x%08" PRIx64,
                      *toSectionOffset(Die.find(DW_AT_stmt_list)))
            << "] row[" << RowIndex
            << "] decreases in address from previous row:\n";
 
         DWARFDebugLine::Row::dumpTableHeader(OS);
         if (RowIndex > 0)
           LineTable->Rows[RowIndex - 1].dump(OS);
         Row.dump(OS);
         OS << '\n';
       }
 
+      // Verify file index.
       if (Row.File > MaxFileIndex) {
         ++NumDebugLineErrors;
         OS << "error: .debug_line["
            << format("0x%08" PRIx64,
                      *toSectionOffset(Die.find(DW_AT_stmt_list)))
            << "][" << RowIndex << "] has invalid file index " << Row.File
            << " (valid values are [1," << MaxFileIndex << "]):\n";
         DWARFDebugLine::Row::dumpTableHeader(OS);

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

TEST(DWARFDebugInfo, TestImplicitConstAbbrevs) {
   EXPECT_EQ(DIEs.count(Val1), 2u);
   EXPECT_EQ(DIEs.count(Val2), 1u);
   auto Val1Range = DIEs.equal_range(Val1);
   for (auto it = Val1Range.first; it != Val1Range.second; ++it)
     EXPECT_EQ(it->second, AbbrevPtrVal1);
   EXPECT_EQ(DIEs.find(Val2)->second, AbbrevPtrVal2);
 }
 
+void VerifyWarning(DWARFContext &DwarfContext, StringRef Error) {
+  SmallString<1024> Str;
+  raw_svector_ostream Strm(Str);
+  EXPECT_TRUE(DwarfContext.verify(Strm, DIDT_All));
+  EXPECT_TRUE(Str.str().contains(Error));
+}
+
 void VerifyError(DWARFContext &DwarfContext, StringRef Error) {
   SmallString<1024> Str;
   raw_svector_ostream Strm(Str);
   EXPECT_FALSE(DwarfContext.verify(Strm, DIDT_All));
   EXPECT_TRUE(Str.str().contains(Error));
 }
 
 TEST(DWARFDebugInfo, TestDwarfVerifyInvalidCURef) {
TEST(DWARFDebugInfo, TestDwarfVerifyInvalidLineFileIndex) {
   auto ErrOrSections = DWARFYAML::EmitDebugSections(yamldata);
   ASSERT_TRUE((bool)ErrOrSections);
   std::unique_ptr<DWARFContext> DwarfContext =
       DWARFContext::create(*ErrOrSections, 8);
   VerifyError(*DwarfContext, "error: .debug_line[0x00000000][1] has invalid "
                              "file index 5 (valid values are [1,1]):");
 }
 
+TEST(DWARFDebugInfo, TestDwarfVerifyInvalidLineTablePorlogueDirIndex) {
+  // Create a single compile unit whose line table has a prologue with an
+  // invalid dir index.
+  StringRef yamldata = R"(
+    debug_str:
+      - ''
+      - /tmp/main.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
+    debug_line:
+      - Length:
+          TotalLength:     61
+        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:          2
+            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:            16
+          - Opcode:          DW_LNS_set_file
+            Data:            1
+          - Opcode:          DW_LNS_extended_op
+            ExtLen:          1
+            SubOpcode:       DW_LNE_end_sequence
+            Data:            1
+  )";
+  auto ErrOrSections = DWARFYAML::EmitDebugSections(yamldata);
+  ASSERT_TRUE((bool)ErrOrSections);
+  std::unique_ptr<DWARFContext> DwarfContext =
+      DWARFContext::create(*ErrOrSections, 8);
+  VerifyError(*DwarfContext,
+              "error: .debug_line[0x00000000].prologue."
+              "file_names[1].dir_idx contains an invalid index: 2");
+}
+
+TEST(DWARFDebugInfo, TestDwarfVerifyDuplicateFileWarning) {
+  // Create a single compile unit whose line table has a prologue with an
+  // invalid dir index.
+  StringRef yamldata = R"(
+    debug_str:
+      - ''
+      - /tmp/main.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
+    debug_line:
+      - Length:
+          TotalLength:     71
+        Version:         2
+        PrologueLength:  44
+        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
+          - 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:            16
+          - Opcode:          DW_LNS_set_file
+            Data:            1
+          - Opcode:          DW_LNS_extended_op
+            ExtLen:          1
+            SubOpcode:       DW_LNE_end_sequence
+            Data:            2
+  )";
+  auto ErrOrSections = DWARFYAML::EmitDebugSections(yamldata);
+  ASSERT_TRUE((bool)ErrOrSections);
+  std::unique_ptr<DWARFContext> DwarfContext =
+      DWARFContext::create(*ErrOrSections, 8);
+  VerifyWarning(*DwarfContext,
+                "warning: .debug_line[0x00000000].prologue.file_names[2] is "
+                "a duplicate of file_names[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