[llvm] Add support for verifying .debug_names in split DWARF for CUs and TUs. (PR #101775)

Greg Clayton via llvm-commits llvm-commits at lists.llvm.org
Mon Aug 5 11:11:00 PDT 2024


https://github.com/clayborg updated https://github.com/llvm/llvm-project/pull/101775

>From 8c84ff2d858a3cd492d451505e6ecea2e7f10407 Mon Sep 17 00:00:00 2001
From: Greg Clayton <clayborg at gmail.com>
Date: Fri, 2 Aug 2024 16:44:05 -0700
Subject: [PATCH 1/2] Add support for verifying .debug_names in split DWARF for
 CUs and TUs.

This patch fixes .debug_names verification for split DWARF with no type units. It will print out an error for any name entries where we can't locate the .dwo file. It finds the non skeleton unit and correctly figures out the DIE offset in the .dwo file. If the non skeleton unit is found and yet the skeleton unit has a DWO ID, an error will be emitted showing we couldn't access the non-skeleton compile unit.
---
 .../llvm/DebugInfo/DWARF/DWARFContext.h       |  3 ++
 llvm/lib/DebugInfo/DWARF/DWARFContext.cpp     |  7 ++-
 llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp    | 47 +++++++++++++++++--
 3 files changed, 52 insertions(+), 5 deletions(-)

diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFContext.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFContext.h
index d800850450eb3..455ccaab74d7e 100644
--- a/llvm/include/llvm/DebugInfo/DWARF/DWARFContext.h
+++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFContext.h
@@ -264,6 +264,9 @@ class DWARFContext : public DIContext {
   DWARFCompileUnit *getDWOCompileUnitForHash(uint64_t Hash);
   DWARFTypeUnit *getTypeUnitForHash(uint16_t Version, uint64_t Hash, bool IsDWO);
 
+  /// Return the DWARF unit that includes an offset (relative to .debug_info).
+  DWARFUnit *getUnitForOffset(uint64_t Offset);
+
   /// Return the compile unit that includes an offset (relative to .debug_info).
   DWARFCompileUnit *getCompileUnitForOffset(uint64_t Offset);
 
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp b/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp
index f36399ed00a6e..045f5e219d34a 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp
@@ -1510,9 +1510,12 @@ DWARFUnitVector &DWARFContext::getDWOUnits(bool Lazy) {
   return State->getDWOUnits(Lazy);
 }
 
+DWARFUnit *DWARFContext::getUnitForOffset(uint64_t Offset) {
+  return State->getNormalUnits().getUnitForOffset(Offset);
+}
+
 DWARFCompileUnit *DWARFContext::getCompileUnitForOffset(uint64_t Offset) {
-  return dyn_cast_or_null<DWARFCompileUnit>(
-      State->getNormalUnits().getUnitForOffset(Offset));
+  return dyn_cast_or_null<DWARFCompileUnit>(getUnitForOffset(Offset));
 }
 
 DWARFCompileUnit *DWARFContext::getCompileUnitForCodeAddress(uint64_t Address) {
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp b/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
index 8ff6ebd230025..7c6da712d1276 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
@@ -1626,8 +1626,44 @@ unsigned DWARFVerifier::verifyNameIndexEntries(
       UnitOffset = NI.getCUOffset(*CUIndex);
     if (!UnitOffset)
       continue;
-    uint64_t DIEOffset = *UnitOffset + *EntryOr->getDIEUnitOffset();
-    DWARFDie DIE = DCtx.getDIEForOffset(DIEOffset);
+    // For split DWARF entries we need to make sure we find the non skeleton
+    // DWARF unit that is needed and use that's DWARF unit offset as the
+    // DIE offset to add the DW_IDX_die_offset to.
+    DWARFUnit *DU = DCtx.getUnitForOffset(*UnitOffset);
+    if (DU == nullptr || DU->getOffset() != *UnitOffset) {
+      // If we didn't find a DWARF Unit from the UnitOffset, or if the offset
+      // of the unit doesn't match exactly, report an error.
+      ErrorCategory.Report(
+          "Name Index entry contains invalid CU or TU offset", [&]() {
+            error() << formatv("Name Index @ {0:x}: Entry @ {1:x} contains an "
+                               "invalid CU or TU offset {1:x}.\n",
+                               NI.getUnitOffset(), EntryID, *UnitOffset);
+          });
+      ++NumErrors;
+      continue;
+    }
+    // This function will try to get the non skeleton unit DIE, but if it is
+    // unable to load the .dwo file from the .dwo or .dwp, it will return the
+    // unit DIE of the DWARFUnit in "DU". So we need to check if the DWARFUnit
+    // has a .dwo file, but we couldn't load it.
+    DWARFDie NonSkeletonUnitDie = DU->getNonSkeletonUnitDIE();
+    if (DU->getDWOId() && DU->getUnitDIE() == NonSkeletonUnitDie) {
+      ErrorCategory.Report("Unable to get load .dwo file", [&]() {
+        error() << formatv("Name Index @ {0:x}: Entry @ {1:x} unable to load "
+                           ".dwo file \"{2}\" for DWARF unit @ {3:x}.\n",
+                           NI.getUnitOffset(), EntryID,
+                           dwarf::toString(DU->getUnitDIE().find(
+                               {DW_AT_dwo_name, DW_AT_GNU_dwo_name})),
+                           *UnitOffset);
+      });
+      ++NumErrors;
+      continue;
+    }
+    DWARFUnit *NonSkeletonUnit = NonSkeletonUnitDie.getDwarfUnit();
+    uint64_t DIEOffset =
+        NonSkeletonUnit->getOffset() + *EntryOr->getDIEUnitOffset();
+    DWARFDie DIE = NonSkeletonUnit->getDIEForOffset(DIEOffset);
+
     if (!DIE) {
       ErrorCategory.Report("NameIndex references nonexistent DIE", [&]() {
         error() << formatv("Name Index @ {0:x}: Entry @ {1:x} references a "
@@ -1637,7 +1673,12 @@ unsigned DWARFVerifier::verifyNameIndexEntries(
       ++NumErrors;
       continue;
     }
-    if (DIE.getDwarfUnit()->getOffset() != *UnitOffset) {
+    // Only compare the DIE we found's DWARFUnit offset if the DIE lives in
+    // the DWARFUnit from the DW_IDX_comp_unit or DW_IDX_type_unit. If we are
+    // using split DWARF, then the DIE's DWARFUnit doesn't need to match the
+    // skeleton unit.
+    if (DIE.getDwarfUnit() == DU &&
+        DIE.getDwarfUnit()->getOffset() != *UnitOffset) {
       ErrorCategory.Report("Name index contains mismatched CU of DIE", [&]() {
         error() << formatv(
             "Name Index @ {0:x}: Entry @ {1:x}: mismatched CU of "

>From c6febf01ddd48f159f9dd8eda0a0edf13d59391d Mon Sep 17 00:00:00 2001
From: Greg Clayton <clayborg at gmail.com>
Date: Mon, 5 Aug 2024 11:10:00 -0700
Subject: [PATCH 2/2] Add tests.

This adds a test and fixes another test to check for DIE offsets being out of range for a compile unit as this was triggered by some of the changes made for the non skeleton compile unit.
---
 llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp           | 12 ++++++++++++
 .../llvm-dwarfdump/X86/debug-names-verify-entries.s  |  4 ++--
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp b/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
index 7c6da712d1276..c6bd8ac8419ac 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
@@ -1662,6 +1662,18 @@ unsigned DWARFVerifier::verifyNameIndexEntries(
     DWARFUnit *NonSkeletonUnit = NonSkeletonUnitDie.getDwarfUnit();
     uint64_t DIEOffset =
         NonSkeletonUnit->getOffset() + *EntryOr->getDIEUnitOffset();
+    const uint64_t NextUnitOffset = NonSkeletonUnit->getNextUnitOffset();
+    // DIE offsets are relative to the specified CU or TU. Make sure the DIE
+    // offsets is a valid relative offset.
+    if (DIEOffset >= NextUnitOffset) {
+      ErrorCategory.Report("NameIndex relative DIE offset too large", [&]() {
+        error() << formatv("Name Index @ {0:x}: Entry @ {1:x} references a "
+                           "DIE @ {2:x} when CU or TU ends at {3:x}.\n",
+                           NI.getUnitOffset(), EntryID, DIEOffset,
+                           NextUnitOffset);
+      });
+      continue;
+    }
     DWARFDie DIE = NonSkeletonUnit->getDIEForOffset(DIEOffset);
 
     if (!DIE) {
diff --git a/llvm/test/tools/llvm-dwarfdump/X86/debug-names-verify-entries.s b/llvm/test/tools/llvm-dwarfdump/X86/debug-names-verify-entries.s
index 4ee2b0a40bf01..d447678e44610 100644
--- a/llvm/test/tools/llvm-dwarfdump/X86/debug-names-verify-entries.s
+++ b/llvm/test/tools/llvm-dwarfdump/X86/debug-names-verify-entries.s
@@ -2,8 +2,8 @@
 
 # CHECK: error: Name Index @ 0x0: Unable to get string associated with name 1.
 # CHECK: error: Name Index @ 0x0: Entry @ 0x73 contains an invalid CU index (47).
-# CHECK: error: Name Index @ 0x0: Entry @ 0x79 references a non-existing DIE @ 0x3fa.
-# CHECK: error: Name Index @ 0x0: Entry @ 0x85: mismatched CU of DIE @ 0x30: index - 0x0; debug_info - 0x1e.
+# CHECK: error: Name Index @ 0x0: Entry @ 0x79 references a DIE @ 0x3fa when CU or TU ends at 0x1e.
+# CHECK: error: Name Index @ 0x0: Entry @ 0x85 references a DIE @ 0x30 when CU or TU ends at 0x1e.
 # CHECK: error: Name Index @ 0x0: Entry @ 0x8b: mismatched Tag of DIE @ 0x17: index - DW_TAG_subprogram; debug_info - DW_TAG_variable.
 # CHECK: error: Name Index @ 0x0: Entry @ 0x91: mismatched Name of DIE @ 0x35: index - foo; debug_info - bar, _Z3bar.
 # CHECK: error: Name Index @ 0x0: Name 2 (foo): Incorrectly terminated entry list.



More information about the llvm-commits mailing list