[llvm] [libDebugInfo] Prevent infinite recursion in DWARFDie::getTypeSize() (PR #74681)

Adrian Prantl via llvm-commits llvm-commits at lists.llvm.org
Wed Dec 6 16:53:33 PST 2023


https://github.com/adrian-prantl updated https://github.com/llvm/llvm-project/pull/74681

>From 1642490086b153f0db952ad046795a0c1addd113 Mon Sep 17 00:00:00 2001
From: Adrian Prantl <aprantl at apple.com>
Date: Wed, 6 Dec 2023 16:28:31 -0800
Subject: [PATCH] [libDebugInfo] Prevent infinite recursion in
 DWARFDie::getTypeSize()

when run on invalid input.
---
 llvm/include/llvm/DebugInfo/DWARF/DWARFDie.h  |  5 ++-
 llvm/lib/DebugInfo/DWARF/DWARFContext.cpp     |  6 ++-
 llvm/lib/DebugInfo/DWARF/DWARFDie.cpp         | 14 +++++--
 llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp        |  7 +++-
 .../DebugInfo/DWARF/DWARFDebugInfoTest.cpp    | 37 +++++++++++++++++++
 5 files changed, 60 insertions(+), 9 deletions(-)

diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFDie.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFDie.h
index 421b84d644db6..1b1e3618bd1d6 100644
--- a/llvm/include/llvm/DebugInfo/DWARF/DWARFDie.h
+++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFDie.h
@@ -10,6 +10,7 @@
 #define LLVM_DEBUGINFO_DWARF_DWARFDIE_H
 
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/iterator.h"
 #include "llvm/ADT/iterator_range.h"
 #include "llvm/BinaryFormat/Dwarf.h"
@@ -284,7 +285,9 @@ class DWARFDie {
   /// \param PointerSize the pointer size of the containing CU.
   /// \returns if this is a type DIE, or this DIE contains a DW_AT_type, returns
   /// the size of the type.
-  std::optional<uint64_t> getTypeSize(uint64_t PointerSize);
+  std::optional<uint64_t>
+  getTypeSize(uint64_t PointerSize,
+              llvm::SmallPtrSetImpl<const DWARFDebugInfoEntry *> &Visited);
 
   class iterator;
 
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp b/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp
index c671aedbc9e52..788e665fe6cbe 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp
@@ -1663,8 +1663,10 @@ void DWARFContext::addLocalsForDie(DWARFCompileUnit *CU, DWARFDie Subprogram,
     if (auto NameAttr = Die.find(DW_AT_name))
       if (std::optional<const char *> Name = dwarf::toString(*NameAttr))
         Local.Name = *Name;
-    if (auto Type = Die.getAttributeValueAsReferencedDie(DW_AT_type))
-      Local.Size = Type.getTypeSize(getCUAddrSize());
+    if (auto Type = Die.getAttributeValueAsReferencedDie(DW_AT_type)) {
+      llvm::SmallPtrSet<const DWARFDebugInfoEntry *, 4> Visited;
+      Local.Size = Type.getTypeSize(getCUAddrSize(), Visited);
+    }
     if (auto DeclFileAttr = Die.find(DW_AT_decl_file)) {
       if (const auto *LT = CU->getContext().getLineTableForUnit(CU))
         LT->getFileNameByIndex(
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFDie.cpp b/llvm/lib/DebugInfo/DWARF/DWARFDie.cpp
index 0f01933002c06..fb042fb7b1392 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFDie.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFDie.cpp
@@ -489,7 +489,12 @@ void DWARFDie::getCallerFrame(uint32_t &CallFile, uint32_t &CallLine,
   CallDiscriminator = toUnsigned(find(DW_AT_GNU_discriminator), 0);
 }
 
-std::optional<uint64_t> DWARFDie::getTypeSize(uint64_t PointerSize) {
+std::optional<uint64_t>
+DWARFDie::getTypeSize(uint64_t PointerSize,
+                      SmallPtrSetImpl<const DWARFDebugInfoEntry *> &Visited) {
+  // Cycle detected?
+  if (!Visited.insert(Die).second)
+    return {};
   if (auto SizeAttr = find(DW_AT_byte_size))
     if (std::optional<uint64_t> Size = SizeAttr->getAsUnsignedConstant())
       return Size;
@@ -511,14 +516,15 @@ std::optional<uint64_t> DWARFDie::getTypeSize(uint64_t PointerSize) {
   case DW_TAG_restrict_type:
   case DW_TAG_typedef: {
     if (DWARFDie BaseType = getAttributeValueAsReferencedDie(DW_AT_type))
-      return BaseType.getTypeSize(PointerSize);
+      return BaseType.getTypeSize(PointerSize, Visited);
     break;
   }
   case DW_TAG_array_type: {
     DWARFDie BaseType = getAttributeValueAsReferencedDie(DW_AT_type);
     if (!BaseType)
       return std::nullopt;
-    std::optional<uint64_t> BaseSize = BaseType.getTypeSize(PointerSize);
+    std::optional<uint64_t> BaseSize =
+        BaseType.getTypeSize(PointerSize, Visited);
     if (!BaseSize)
       return std::nullopt;
     uint64_t Size = *BaseSize;
@@ -543,7 +549,7 @@ std::optional<uint64_t> DWARFDie::getTypeSize(uint64_t PointerSize) {
   }
   default:
     if (DWARFDie BaseType = getAttributeValueAsReferencedDie(DW_AT_type))
-      return BaseType.getTypeSize(PointerSize);
+      return BaseType.getTypeSize(PointerSize, Visited);
     break;
   }
   return std::nullopt;
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp b/llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp
index 9f455fa7e96a7..1780eec8fdd87 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp
@@ -817,9 +817,12 @@ void DWARFUnit::updateVariableDieMap(DWARFDie Die) {
   // no type), then we use a size of one to still allow symbolization of the
   // exact address.
   uint64_t GVSize = 1;
-  if (Die.getAttributeValueAsReferencedDie(DW_AT_type))
-    if (std::optional<uint64_t> Size = Die.getTypeSize(getAddressByteSize()))
+  if (Die.getAttributeValueAsReferencedDie(DW_AT_type)) {
+    llvm::SmallPtrSet<const DWARFDebugInfoEntry *, 4> Visited;
+    if (std::optional<uint64_t> Size =
+            Die.getTypeSize(getAddressByteSize(), Visited))
       GVSize = *Size;
+  }
 
   if (Address != UINT64_MAX)
     VariableDieMap[Address] = {Address + GVSize, Die};
diff --git a/llvm/unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp b/llvm/unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp
index 0b7f8f41bc53f..f839e5b91f29a 100644
--- a/llvm/unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp
+++ b/llvm/unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp
@@ -1615,6 +1615,43 @@ TEST(DWARFDebugInfo, TestFindRecurse) {
   EXPECT_EQ(AbsDieName, StringOpt.value_or(nullptr));
 }
 
+TEST(DWARFDebugInfo, TestSelfRecursiveType) {
+  typedef uint32_t AddrType;
+  Triple Triple = getDefaultTargetTripleForAddrSize(sizeof(AddrType));
+  if (!isConfigurationSupported(Triple))
+    GTEST_SKIP();
+
+  auto ExpectedDG = dwarfgen::Generator::create(Triple, 4);
+  ASSERT_THAT_EXPECTED(ExpectedDG, Succeeded());
+  dwarfgen::Generator *DG = ExpectedDG.get().get();
+  dwarfgen::CompileUnit &CU = DG->addCompileUnit();
+  dwarfgen::DIE CUDie = CU.getUnitDIE();
+
+  // Create an invalid self-recursive typedef.
+  dwarfgen::DIE TypedefDie = CUDie.addChild(DW_TAG_typedef);
+  TypedefDie.addAttribute(DW_AT_name, DW_FORM_strp, "illegal");
+  TypedefDie.addAttribute(DW_AT_type, DW_FORM_ref_addr, TypedefDie);
+
+  MemoryBufferRef FileBuffer(DG->generate(), "dwarf");
+  auto Obj = object::ObjectFile::createObjectFile(FileBuffer);
+  EXPECT_TRUE((bool)Obj);
+  std::unique_ptr<DWARFContext> DwarfContext = DWARFContext::create(**Obj);
+
+  // Verify the number of compile units is correct.
+  uint32_t NumCUs = DwarfContext->getNumCompileUnits();
+  EXPECT_EQ(NumCUs, 1u);
+  DWARFCompileUnit *U = cast<DWARFCompileUnit>(DwarfContext->getUnitAtIndex(0));
+  {
+    DWARFDie CUDie = U->getUnitDIE(false);
+    EXPECT_TRUE(CUDie.isValid());
+    DWARFDie TypedefDie = CUDie.getFirstChild();
+
+    // Test that getTypeSize doesn't get into an infinite loop.
+    llvm::SmallPtrSet<const DWARFDebugInfoEntry *, 1> visited;
+    EXPECT_EQ(TypedefDie.getTypeSize(sizeof(AddrType), visited), std::nullopt);
+  }
+}
+
 TEST(DWARFDebugInfo, TestDwarfToFunctions) {
   // Test all of the dwarf::toXXX functions that take a
   // std::optional<DWARFFormValue> and extract the values from it.



More information about the llvm-commits mailing list