Skip to content

Conversation

@jalopezg-git
Copy link
Contributor

@jalopezg-git jalopezg-git commented Apr 24, 2025

This pull request improves LVDWARFReader. Specifically, it

  • Adds support for DW_TAG_module DIEs and recurse over their children. Prior to this patchset, entities hanging below DW_TAG_module were just not visible. This DIE kind is commonly generated by Objective-C modules.
    This patch suggests to add LVScopeModule to represent such entities, which will print as
[0x000000000b][001]              {CompileUnit} '/llvm/tools/clang/test/modules/<stdin>'
[0x000000000b][002]                {Producer} 'LLVM version 3.7.0'
                                   {Directory} '/llvm/tools/clang/test/modules'
                                   {File} '<stdin>'
[0x0000000023][002]                {Module} 'DebugModule'

The minimal test case included is just the result of

$ llc llvm/test/DebugInfo/X86/DIModule.ll -accel-tables=Dwarf -o llvm/unittests/DebugInfo/LogicalView/Inputs/test-dwarf-clang-module.o -filetype=obj

FYI, @CarlosAlbertoEnciso. I belive this patchset is also complete; feel free to start review at your earliest discretion 👍.

@llvmbot
Copy link
Member

llvmbot commented Apr 24, 2025

@llvm/pr-subscribers-llvm-binary-utilities

@llvm/pr-subscribers-debuginfo

Author: Javier Lopez-Gomez (jalopezg-git)

Changes

This pull request improves LVDWARFReader to deal with a couple of unhandled cases. Specifically,

  • Add support for DW_TAG_module DIEs and recurse over their children. Prior to this patchset, entities hanging below DW_TAG_module were just not visible. This DIE kind is commonly generated by Objective-C modules.
    This patch suggests to add LVScopeModule to represent such entities, which will print as
[0x000000000b][001]              {CompileUnit} '/llvm/tools/clang/test/modules/&lt;stdin&gt;'
[0x000000000b][002]                {Producer} 'LLVM version 3.7.0'
                                   {Directory} '/llvm/tools/clang/test/modules'
                                   {File} '&lt;stdin&gt;'
[0x0000000023][002]                {Module} 'debugmodule'

The minimal test case included is just the result of

$ llc llvm/test/DebugInfo/X86/DIModule.ll -accel-tables=Dwarf -o llvm/unittests/DebugInfo/LogicalView/Inputs/test-dwarf-clang-module.o -filetype=obj
  • Introduce handling of DW_AT_byte_size. Most DWARF emitters include this attribute for types to specify the size of an entity of the given type.

FYI, @CarlosAlbertoEnciso. I belive this patchset is also complete; feel free to start review at your earliest discretion 👍.


Full diff: https://github.com/llvm/llvm-project/pull/137228.diff

9 Files Affected:

  • (modified) llvm/include/llvm/DebugInfo/LogicalView/Core/LVElement.h (+2)
  • (modified) llvm/include/llvm/DebugInfo/LogicalView/Core/LVReader.h (+2)
  • (modified) llvm/include/llvm/DebugInfo/LogicalView/Core/LVScope.h (+25)
  • (modified) llvm/include/llvm/DebugInfo/LogicalView/Core/LVType.h (+7)
  • (modified) llvm/lib/DebugInfo/LogicalView/Core/LVScope.cpp (+17-1)
  • (modified) llvm/lib/DebugInfo/LogicalView/Core/LVType.cpp (+4-1)
  • (modified) llvm/lib/DebugInfo/LogicalView/Readers/LVDWARFReader.cpp (+8)
  • (modified) llvm/unittests/DebugInfo/LogicalView/DWARFReaderTest.cpp (+26)
  • (added) llvm/unittests/DebugInfo/LogicalView/Inputs/test-dwarf-clang-module.o ()
diff --git a/llvm/include/llvm/DebugInfo/LogicalView/Core/LVElement.h b/llvm/include/llvm/DebugInfo/LogicalView/Core/LVElement.h
index 17fa04040ad77..36f90e0b6a6d8 100644
--- a/llvm/include/llvm/DebugInfo/LogicalView/Core/LVElement.h
+++ b/llvm/include/llvm/DebugInfo/LogicalView/Core/LVElement.h
@@ -45,6 +45,7 @@ enum class LVSubclassID : unsigned char {
   LV_SCOPE_NAMESPACE,
   LV_SCOPE_ROOT,
   LV_SCOPE_TEMPLATE_PACK,
+  LV_SCOPE_MODULE,
   LV_SCOPE_LAST,
   LV_SYMBOL_FIRST,
   LV_SYMBOL,
@@ -239,6 +240,7 @@ class LVElement : public LVObject {
   virtual bool isBase() const { return false; }
   virtual bool isTemplateParam() const { return false; }
 
+  uint32_t getStorageSizeInBytes() const { return (getBitSize() + 7u) / 8u; }
   virtual uint32_t getBitSize() const { return 0; }
   virtual void setBitSize(uint32_t Size) {}
 
diff --git a/llvm/include/llvm/DebugInfo/LogicalView/Core/LVReader.h b/llvm/include/llvm/DebugInfo/LogicalView/Core/LVReader.h
index 9ce26398e48df..b0d8ff4952d79 100644
--- a/llvm/include/llvm/DebugInfo/LogicalView/Core/LVReader.h
+++ b/llvm/include/llvm/DebugInfo/LogicalView/Core/LVReader.h
@@ -107,6 +107,7 @@ class LVReader {
   LV_OBJECT_ALLOCATOR(ScopeNamespace)
   LV_OBJECT_ALLOCATOR(ScopeRoot)
   LV_OBJECT_ALLOCATOR(ScopeTemplatePack)
+  LV_OBJECT_ALLOCATOR(ScopeModule)
 
   // Symbols allocator.
   LV_OBJECT_ALLOCATOR(Symbol)
@@ -213,6 +214,7 @@ class LVReader {
   LV_CREATE_OBJECT(ScopeNamespace)
   LV_CREATE_OBJECT(ScopeRoot)
   LV_CREATE_OBJECT(ScopeTemplatePack)
+  LV_CREATE_OBJECT(ScopeModule)
 
   // Symbols creation.
   LV_CREATE_OBJECT(Symbol)
diff --git a/llvm/include/llvm/DebugInfo/LogicalView/Core/LVScope.h b/llvm/include/llvm/DebugInfo/LogicalView/Core/LVScope.h
index 1b3c377cd7dbb..23553297e54c2 100644
--- a/llvm/include/llvm/DebugInfo/LogicalView/Core/LVScope.h
+++ b/llvm/include/llvm/DebugInfo/LogicalView/Core/LVScope.h
@@ -57,6 +57,7 @@ enum class LVScopeKind {
   IsTemplatePack,
   IsTryBlock,
   IsUnion,
+  IsModule,
   LastEntry
 };
 using LVScopeKindSet = std::set<LVScopeKind>;
@@ -93,6 +94,9 @@ class LVScope : public LVElement {
   LVProperties<Property> Properties;
   static LVScopeDispatch Dispatch;
 
+  // Size in bits if this scope represents also a compound type.
+  uint32_t BitSize = 0;
+
   // Coverage factor in units (bytes).
   unsigned CoverageFactor = 0;
 
@@ -181,6 +185,7 @@ class LVScope : public LVElement {
   KIND(LVScopeKind, IsTemplatePack);
   KIND_1(LVScopeKind, IsTryBlock, IsBlock);
   KIND_1(LVScopeKind, IsUnion, IsAggregate);
+  KIND_3(LVScopeKind, IsModule, CanHaveRanges, CanHaveLines, TransformName);
 
   PROPERTY(Property, HasDiscriminator);
   PROPERTY(Property, CanHaveRanges);
@@ -269,6 +274,9 @@ class LVScope : public LVElement {
   bool removeElement(LVElement *Element) override;
   void updateLevel(LVScope *Parent, bool Moved) override;
 
+  uint32_t getBitSize() const override { return BitSize; }
+  void setBitSize(uint32_t Size) override { BitSize = Size; }
+
   void resolve() override;
   void resolveName() override;
   void resolveReferences() override;
@@ -826,6 +834,23 @@ class LVScopeTemplatePack final : public LVScope {
   void printExtra(raw_ostream &OS, bool Full = true) const override;
 };
 
+// Class to represent a DWARF Module.
+class LVScopeModule final : public LVScope {
+public:
+  LVScopeModule() : LVScope() {
+    setIsModule();
+    setIsLexicalBlock();
+  }
+  LVScopeModule(const LVScopeModule &) = delete;
+  LVScopeModule &operator=(const LVScopeModule &) = delete;
+  ~LVScopeModule() = default;
+
+  // Returns true if current scope is logically equal to the given 'Scope'.
+  bool equals(const LVScope *Scope) const override;
+
+  void printExtra(raw_ostream &OS, bool Full = true) const override;
+};
+
 } // end namespace logicalview
 } // end namespace llvm
 
diff --git a/llvm/include/llvm/DebugInfo/LogicalView/Core/LVType.h b/llvm/include/llvm/DebugInfo/LogicalView/Core/LVType.h
index 28881b3c95b17..58d5bc48c3a72 100644
--- a/llvm/include/llvm/DebugInfo/LogicalView/Core/LVType.h
+++ b/llvm/include/llvm/DebugInfo/LogicalView/Core/LVType.h
@@ -56,6 +56,9 @@ class LVType : public LVElement {
   LVProperties<Property> Properties;
   static LVTypeDispatch Dispatch;
 
+  // Size in bits of a symbol of this type.
+  uint32_t BitSize = 0;
+
   // Find the current type in the given 'Targets'.
   LVType *findIn(const LVTypes *Targets) const;
 
@@ -109,6 +112,10 @@ class LVType : public LVElement {
   virtual LVElement *getUnderlyingType() { return nullptr; }
   virtual void setUnderlyingType(LVElement *Element) {}
 
+  // Return the size in bits of an entity of this type.
+  uint32_t getBitSize() const override { return BitSize; }
+  void setBitSize(uint32_t Size) override { BitSize = Size; }
+
   void resolveName() override;
   void resolveReferences() override;
 
diff --git a/llvm/lib/DebugInfo/LogicalView/Core/LVScope.cpp b/llvm/lib/DebugInfo/LogicalView/Core/LVScope.cpp
index 8bbaf93db0caa..7ffeed8f18f8a 100644
--- a/llvm/lib/DebugInfo/LogicalView/Core/LVScope.cpp
+++ b/llvm/lib/DebugInfo/LogicalView/Core/LVScope.cpp
@@ -40,6 +40,7 @@ const char *const KindTemplateAlias = "TemplateAlias";
 const char *const KindTemplatePack = "TemplatePack";
 const char *const KindUndefined = "Undefined";
 const char *const KindUnion = "Union";
+const char *const KindModule = "Module";
 } // end anonymous namespace
 
 //===----------------------------------------------------------------------===//
@@ -50,6 +51,8 @@ const char *LVScope::kind() const {
   const char *Kind = KindUndefined;
   if (getIsArray())
     Kind = KindArray;
+  else if (getIsModule())
+    Kind = KindModule;
   else if (getIsBlock())
     Kind = KindBlock;
   else if (getIsCallSite())
@@ -101,7 +104,8 @@ LVScopeDispatch LVScope::Dispatch = {
     {LVScopeKind::IsTemplateAlias, &LVScope::getIsTemplateAlias},
     {LVScopeKind::IsTemplatePack, &LVScope::getIsTemplatePack},
     {LVScopeKind::IsTryBlock, &LVScope::getIsTryBlock},
-    {LVScopeKind::IsUnion, &LVScope::getIsUnion}};
+    {LVScopeKind::IsUnion, &LVScope::getIsUnion},
+    {LVScopeKind::IsModule, &LVScope::getIsModule}};
 
 void LVScope::addToChildren(LVElement *Element) {
   if (!Children)
@@ -2107,3 +2111,15 @@ bool LVScopeTemplatePack::equals(const LVScope *Scope) const {
 void LVScopeTemplatePack::printExtra(raw_ostream &OS, bool Full) const {
   OS << formattedKind(kind()) << " " << formattedName(getName()) << "\n";
 }
+
+//===----------------------------------------------------------------------===//
+// DWARF module (DW_TAG_module).
+//===----------------------------------------------------------------------===//
+bool LVScopeModule::equals(const LVScope *Scope) const {
+  // For lexical blocks, LVScope::equals() compares the parent scope.
+  return LVScope::equals(Scope) && (Scope->getName() == getName());
+}
+
+void LVScopeModule::printExtra(raw_ostream &OS, bool Full) const {
+  OS << formattedKind(kind()) << " " << formattedName(getName()) << "\n";
+}
diff --git a/llvm/lib/DebugInfo/LogicalView/Core/LVType.cpp b/llvm/lib/DebugInfo/LogicalView/Core/LVType.cpp
index 28bccadce598c..e5c9936c008e2 100644
--- a/llvm/lib/DebugInfo/LogicalView/Core/LVType.cpp
+++ b/llvm/lib/DebugInfo/LogicalView/Core/LVType.cpp
@@ -292,7 +292,10 @@ void LVType::print(raw_ostream &OS, bool Full) const {
 }
 
 void LVType::printExtra(raw_ostream &OS, bool Full) const {
-  OS << formattedKind(kind()) << " " << formattedName(getName()) << "\n";
+  OS << formattedKind(kind()) << " " << formattedName(getName());
+  if (uint32_t Size = getStorageSizeInBytes())
+    OS << " [Size = " << Size << "]";
+  OS << "\n";
 }
 
 //===----------------------------------------------------------------------===//
diff --git a/llvm/lib/DebugInfo/LogicalView/Readers/LVDWARFReader.cpp b/llvm/lib/DebugInfo/LogicalView/Readers/LVDWARFReader.cpp
index 42da957233667..fbac5ddcc8a06 100644
--- a/llvm/lib/DebugInfo/LogicalView/Readers/LVDWARFReader.cpp
+++ b/llvm/lib/DebugInfo/LogicalView/Readers/LVDWARFReader.cpp
@@ -233,6 +233,9 @@ LVElement *LVDWARFReader::createElement(dwarf::Tag Tag) {
   case dwarf::DW_TAG_GNU_template_parameter_pack:
     CurrentScope = createScopeTemplatePack();
     return CurrentScope;
+  case dwarf::DW_TAG_module:
+    CurrentScope = createScopeModule();
+    return CurrentScope;
   default:
     // Collect TAGs not implemented.
     if (options().getInternalTag() && Tag)
@@ -307,6 +310,11 @@ void LVDWARFReader::processOneAttribute(const DWARFDie &Die,
   case dwarf::DW_AT_bit_size:
     CurrentElement->setBitSize(GetAsUnsignedConstant());
     break;
+  case dwarf::DW_AT_byte_size:
+    // Assume 8-bit bytes; this is consistent, e.g. with
+    // lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp.
+    CurrentElement->setBitSize(GetAsUnsignedConstant() * 8u);
+    break;
   case dwarf::DW_AT_call_file:
     CurrentElement->setCallFilenameIndex(IncrementFileIndex
                                              ? GetAsUnsignedConstant() + 1
diff --git a/llvm/unittests/DebugInfo/LogicalView/DWARFReaderTest.cpp b/llvm/unittests/DebugInfo/LogicalView/DWARFReaderTest.cpp
index c062c15481da9..7362ea6468b61 100644
--- a/llvm/unittests/DebugInfo/LogicalView/DWARFReaderTest.cpp
+++ b/llvm/unittests/DebugInfo/LogicalView/DWARFReaderTest.cpp
@@ -30,6 +30,7 @@ extern const char *TestMainArgv0;
 namespace {
 
 const char *DwarfClang = "test-dwarf-clang.o";
+const char *DwarfClangModule = "test-dwarf-clang-module.o";
 const char *DwarfGcc = "test-dwarf-gcc.o";
 
 // Helper function to get the first compile unit.
@@ -124,6 +125,22 @@ void checkElementProperties(LVReader *Reader) {
   ASSERT_EQ(Lines->size(), 0x12u);
 }
 
+// Check the basic properties on parsed DW_TAG_module.
+void checkScopeModule(LVReader *Reader) {
+  LVScopeRoot *Root = Reader->getScopesRoot();
+  LVScopeCompileUnit *CompileUnit = getFirstCompileUnit(Root);
+
+  EXPECT_EQ(Root->getFileFormatName(), "Mach-O 64-bit x86-64");
+  EXPECT_EQ(Root->getName(), DwarfClangModule);
+
+  ASSERT_NE(CompileUnit->getChildren(), nullptr);
+  LVElement *FirstChild = *(CompileUnit->getChildren()->begin());
+  EXPECT_EQ(FirstChild->getIsScope(), 1);
+  LVScopeModule *Module = static_cast<LVScopeModule *>(FirstChild);
+  EXPECT_EQ(Module->getIsModule(), 1);
+  EXPECT_EQ(Module->getName(), "debugmodule");
+}
+
 // Check the logical elements selection.
 void checkElementSelection(LVReader *Reader) {
   LVScopeRoot *Root = Reader->getScopesRoot();
@@ -155,6 +172,12 @@ void checkElementSelection(LVReader *Reader) {
   ASSERT_NE(Element, nullptr);
   EXPECT_NE(Element->getName().find("INTEGER"), StringRef::npos);
   EXPECT_EQ(Element->getIsType(), 1);
+  // Underlying type is `int`
+  const LVElement *UnderlyingType =
+      static_cast<LVType *>(Element)->getUnderlyingType();
+  EXPECT_EQ(UnderlyingType->getIsType(), 1);
+  EXPECT_EQ(UnderlyingType->getBitSize(), 32u);
+  EXPECT_EQ(UnderlyingType->getStorageSizeInBytes(), 4u);
 
   Element = MapElements[0x000000000f]; // 'movl	%edx, %eax'
   ASSERT_NE(Element, nullptr);
@@ -264,6 +287,9 @@ void elementProperties(SmallString<128> &InputsDir) {
   std::unique_ptr<LVReader> Reader =
       createReader(ReaderHandler, InputsDir, DwarfClang);
   checkElementProperties(Reader.get());
+
+  Reader = createReader(ReaderHandler, InputsDir, DwarfClangModule);
+  checkScopeModule(Reader.get());
 }
 
 // Logical elements selection.
diff --git a/llvm/unittests/DebugInfo/LogicalView/Inputs/test-dwarf-clang-module.o b/llvm/unittests/DebugInfo/LogicalView/Inputs/test-dwarf-clang-module.o
new file mode 100644
index 0000000000000..0cded9bfb37d1
Binary files /dev/null and b/llvm/unittests/DebugInfo/LogicalView/Inputs/test-dwarf-clang-module.o differ

@jalopezg-git jalopezg-git changed the title [llvm-debuginfo-analyzer] Improve DWARF parsing capabilities [llvm-debuginfo-analyzer] Support DW_AT_byte_size and DW_TAG_module Apr 25, 2025
@jalopezg-git jalopezg-git force-pushed the jalopezg-logicalview-improve-LVDWARFReader branch from 2c8b270 to f1b83dd Compare April 25, 2025 14:28
@CarlosAlbertoEnciso
Copy link
Member

Adding support for DW_TAG_module is great.

@CarlosAlbertoEnciso
Copy link
Member

Can you split the patch into:

  • DW_AT_module support
  • DW_AT_byte_size support

@dwblaikie dwblaikie removed their request for review May 1, 2025 20:19
@jalopezg-git jalopezg-git force-pushed the jalopezg-logicalview-improve-LVDWARFReader branch from f1b83dd to 07cb3d0 Compare May 8, 2025 17:07
@jalopezg-git jalopezg-git changed the title [llvm-debuginfo-analyzer] Support DW_AT_byte_size and DW_TAG_module [llvm-debuginfo-analyzer] Support DW_TAG_module May 8, 2025
@jalopezg-git
Copy link
Contributor Author

Can you split the patch into:

* DW_AT_module support

* DW_AT_byte_size support

Done! I left this one for DW_TAG_module support. The DW_AT_byte_size counterpart is now at #139110.
Feel free to re-review when you have the time!

@CarlosAlbertoEnciso
Copy link
Member

LGTM: After fixing those alphabetical issues.

@CarlosAlbertoEnciso
Copy link
Member

Have you look at the failing test case?

@jalopezg-git
Copy link
Contributor Author

Thanks for the prompt review, @CarlosAlbertoEnciso! I'll address these simple changes either today / tomorrow.

Have you look at the failing test case?

It's totally unrelated - it was failing on main branch too.

@jalopezg-git jalopezg-git force-pushed the jalopezg-logicalview-improve-LVDWARFReader branch 2 times, most recently from 6522085 to 1da3dcc Compare May 12, 2025 12:08
@jalopezg-git
Copy link
Contributor Author

jalopezg-git commented May 12, 2025

It's totally unrelated - it was failing on main branch too.

I have addressed all comments above + rebased atop current main; let's see if test is fixed now.

Other than that, feel free to resolve all comments above and approve if you are happy w/ the current state 🙂!

Copy link
Member

@CarlosAlbertoEnciso CarlosAlbertoEnciso left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just move those LVScopeModule to their alphabetical place. LGTM.

@jalopezg-git jalopezg-git force-pushed the jalopezg-logicalview-improve-LVDWARFReader branch from 1da3dcc to 1948e9f Compare May 13, 2025 11:25
@jalopezg-git
Copy link
Contributor Author

For this one, @jmorse, @SLTozer, @OCHyams, I think this is also ready. Do you have any additional comments that you would like to see applied before merging?

For the moment, I do not have write access to the repository, so someone would have to do the merge for me 🙂.

@jalopezg-git jalopezg-git force-pushed the jalopezg-logicalview-improve-LVDWARFReader branch from 1948e9f to f1d4348 Compare May 21, 2025 10:24
@CarlosAlbertoEnciso CarlosAlbertoEnciso merged commit cb57578 into llvm:main May 21, 2025
12 checks passed
@CarlosAlbertoEnciso
Copy link
Member

@jalopezg-git Committed for you. Thanks for adding that functionality.

@jalopezg-git jalopezg-git deleted the jalopezg-logicalview-improve-LVDWARFReader branch May 21, 2025 16:23
CarlosAlbertoEnciso pushed a commit that referenced this pull request May 22, 2025
…39110)

This PR was split from #137228
(which introduced support for `DW_TAG_module` and `DW_AT_byte_size`).

This PR improves `LVDWARFReader` by introducing handling of
`DW_AT_byte_size`. Most DWARF emitters include this attribute for types
to specify the size of an entity of the given type.
llvm-sync bot pushed a commit to arm/arm-toolchain that referenced this pull request May 22, 2025
…e_size` (#139110)

This PR was split from llvm/llvm-project#137228
(which introduced support for `DW_TAG_module` and `DW_AT_byte_size`).

This PR improves `LVDWARFReader` by introducing handling of
`DW_AT_byte_size`. Most DWARF emitters include this attribute for types
to specify the size of an entity of the given type.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants