[llvm-debuginfo-analyzer] Add support for DWARF `DW_AT_byte_size` #139110

jalopezg-git · 2025-05-08T17:16:32Z

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.

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

llvmbot · 2025-05-08T17:17:05Z

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

@llvm/pr-subscribers-debuginfo

Author: Javier Lopez-Gomez (jalopezg-git)

Changes

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.

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/139110.diff

8 Files Affected:

(modified) llvm/include/llvm/DebugInfo/LogicalView/Core/LVElement.h (+7)
(modified) llvm/include/llvm/DebugInfo/LogicalView/Core/LVScope.h (+6)
(modified) llvm/include/llvm/DebugInfo/LogicalView/Core/LVType.h (+7)
(modified) llvm/lib/DebugInfo/LogicalView/Core/LVType.cpp (+4-1)
(modified) llvm/lib/DebugInfo/LogicalView/Readers/LVCodeViewVisitor.cpp (+3)
(modified) llvm/lib/DebugInfo/LogicalView/Readers/LVDWARFReader.cpp (+3)
(modified) llvm/unittests/DebugInfo/LogicalView/CodeViewReaderTest.cpp (+41)
(modified) llvm/unittests/DebugInfo/LogicalView/DWARFReaderTest.cpp (+6)

diff --git a/llvm/include/llvm/DebugInfo/LogicalView/Core/LVElement.h b/llvm/include/llvm/DebugInfo/LogicalView/Core/LVElement.h
index 17fa04040ad77..b2ef6579caf41 100644
--- a/llvm/include/llvm/DebugInfo/LogicalView/Core/LVElement.h
+++ b/llvm/include/llvm/DebugInfo/LogicalView/Core/LVElement.h
@@ -64,6 +64,10 @@ using LVElementKindSet = std::set<LVElementKind>;
 using LVElementDispatch = std::map<LVElementKind, LVElementGetFunction>;
 using LVElementRequest = std::vector<LVElementGetFunction>;
 
+// Assume 8-bit bytes; this is consistent, e.g. with
+// lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp.
+constexpr unsigned int DWARF_CHAR_BIT = 8u;
+
 class LVElement : public LVObject {
   enum class Property {
     IsLine,   // A logical line.
@@ -239,6 +243,9 @@ class LVElement : public LVObject {
   virtual bool isBase() const { return false; }
   virtual bool isTemplateParam() const { return false; }
 
+  uint32_t getStorageSizeInBytes() const {
+    return (getBitSize() + (DWARF_CHAR_BIT - 1)) / DWARF_CHAR_BIT;
+  }
   virtual uint32_t getBitSize() const { return 0; }
   virtual void setBitSize(uint32_t Size) {}
 
diff --git a/llvm/include/llvm/DebugInfo/LogicalView/Core/LVScope.h b/llvm/include/llvm/DebugInfo/LogicalView/Core/LVScope.h
index 1b3c377cd7dbb..1cf61d085d84f 100644
--- a/llvm/include/llvm/DebugInfo/LogicalView/Core/LVScope.h
+++ b/llvm/include/llvm/DebugInfo/LogicalView/Core/LVScope.h
@@ -93,6 +93,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;
 
@@ -269,6 +272,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;
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/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/LVCodeViewVisitor.cpp b/llvm/lib/DebugInfo/LogicalView/Readers/LVCodeViewVisitor.cpp
index 97214948d014a..4a1eb888095e2 100644
--- a/llvm/lib/DebugInfo/LogicalView/Readers/LVCodeViewVisitor.cpp
+++ b/llvm/lib/DebugInfo/LogicalView/Readers/LVCodeViewVisitor.cpp
@@ -1987,6 +1987,7 @@ Error LVLogicalVisitor::visitKnownRecord(CVType &Record, ClassRecord &Class,
   Scope->setName(Class.getName());
   if (Class.hasUniqueName())
     Scope->setLinkageName(Class.getUniqueName());
+  Scope->setBitSize(Class.getSize() * DWARF_CHAR_BIT);
 
   if (Class.isNested()) {
     Scope->setIsNested();
@@ -2455,6 +2456,7 @@ Error LVLogicalVisitor::visitKnownRecord(CVType &Record, UnionRecord &Union,
   Scope->setName(Union.getName());
   if (Union.hasUniqueName())
     Scope->setLinkageName(Union.getUniqueName());
+  Scope->setBitSize(Union.getSize() * DWARF_CHAR_BIT);
 
   if (Union.isNested()) {
     Scope->setIsNested();
@@ -3208,6 +3210,7 @@ LVType *LVLogicalVisitor::createBaseType(TypeIndex TI, StringRef TypeName) {
 
   if (createElement(TIR, SimpleKind)) {
     CurrentType->setName(TypeName);
+    CurrentType->setBitSize(getSizeInBytesForTypeIndex(TIR) * DWARF_CHAR_BIT);
     Reader->getCompileUnit()->addElement(CurrentType);
   }
   return CurrentType;
diff --git a/llvm/lib/DebugInfo/LogicalView/Readers/LVDWARFReader.cpp b/llvm/lib/DebugInfo/LogicalView/Readers/LVDWARFReader.cpp
index 42da957233667..269630896e077 100644
--- a/llvm/lib/DebugInfo/LogicalView/Readers/LVDWARFReader.cpp
+++ b/llvm/lib/DebugInfo/LogicalView/Readers/LVDWARFReader.cpp
@@ -307,6 +307,9 @@ void LVDWARFReader::processOneAttribute(const DWARFDie &Die,
   case dwarf::DW_AT_bit_size:
     CurrentElement->setBitSize(GetAsUnsignedConstant());
     break;
+  case dwarf::DW_AT_byte_size:
+    CurrentElement->setBitSize(GetAsUnsignedConstant() * DWARF_CHAR_BIT);
+    break;
   case dwarf::DW_AT_call_file:
     CurrentElement->setCallFilenameIndex(IncrementFileIndex
                                              ? GetAsUnsignedConstant() + 1
diff --git a/llvm/unittests/DebugInfo/LogicalView/CodeViewReaderTest.cpp b/llvm/unittests/DebugInfo/LogicalView/CodeViewReaderTest.cpp
index c93a79094dce9..0549aa2976b46 100644
--- a/llvm/unittests/DebugInfo/LogicalView/CodeViewReaderTest.cpp
+++ b/llvm/unittests/DebugInfo/LogicalView/CodeViewReaderTest.cpp
@@ -21,6 +21,7 @@
 #include "llvm/Testing/Support/Error.h"
 
 #include "gtest/gtest.h"
+#include <algorithm>
 
 using namespace llvm;
 using namespace llvm::logicalview;
@@ -128,6 +129,26 @@ void checkElementPropertiesClangCodeview(LVReader *Reader) {
   const LVLines *Lines = Foo->getLines();
   ASSERT_NE(Lines, nullptr);
   EXPECT_EQ(Lines->size(), 0x10u);
+
+  // Check size of types in CompileUnit.
+  const LVTypes *Types = CompileUnit->getTypes();
+  ASSERT_NE(Types, nullptr);
+  EXPECT_EQ(Types->size(), 6u);
+
+  const auto BoolType =
+      std::find_if(Types->begin(), Types->end(), [](const LVElement *elt) {
+        return elt->getName() == "bool";
+      });
+  ASSERT_NE(BoolType, Types->end());
+  const auto IntType =
+      std::find_if(Types->begin(), Types->end(), [](const LVElement *elt) {
+        return elt->getName() == "int";
+      });
+  ASSERT_NE(IntType, Types->end());
+  EXPECT_EQ(static_cast<LVType *>(*BoolType)->getBitSize(), 8u);
+  EXPECT_EQ(static_cast<LVType *>(*BoolType)->getStorageSizeInBytes(), 1u);
+  EXPECT_EQ(static_cast<LVType *>(*IntType)->getBitSize(), 32u);
+  EXPECT_EQ(static_cast<LVType *>(*IntType)->getStorageSizeInBytes(), 4u);
 }
 
 // Check the logical elements basic properties (MSVC - Codeview).
@@ -194,6 +215,26 @@ void checkElementPropertiesMsvcCodeview(LVReader *Reader) {
   const LVLines *Lines = Foo->getLines();
   ASSERT_NE(Lines, nullptr);
   EXPECT_EQ(Lines->size(), 0x0eu);
+
+  // Check size of types in CompileUnit.
+  const LVTypes *Types = CompileUnit->getTypes();
+  ASSERT_NE(Types, nullptr);
+  EXPECT_EQ(Types->size(), 8u);
+
+  const auto BoolType =
+      std::find_if(Types->begin(), Types->end(), [](const LVElement *elt) {
+        return elt->getName() == "bool";
+      });
+  ASSERT_NE(BoolType, Types->end());
+  const auto IntType =
+      std::find_if(Types->begin(), Types->end(), [](const LVElement *elt) {
+        return elt->getName() == "int";
+      });
+  ASSERT_NE(IntType, Types->end());
+  EXPECT_EQ(static_cast<LVType *>(*BoolType)->getBitSize(), 8u);
+  EXPECT_EQ(static_cast<LVType *>(*BoolType)->getStorageSizeInBytes(), 1u);
+  EXPECT_EQ(static_cast<LVType *>(*IntType)->getBitSize(), 32u);
+  EXPECT_EQ(static_cast<LVType *>(*IntType)->getStorageSizeInBytes(), 4u);
 }
 
 // Check the logical elements basic properties (MSVC library - Codeview).
diff --git a/llvm/unittests/DebugInfo/LogicalView/DWARFReaderTest.cpp b/llvm/unittests/DebugInfo/LogicalView/DWARFReaderTest.cpp
index c062c15481da9..6c53be959d951 100644
--- a/llvm/unittests/DebugInfo/LogicalView/DWARFReaderTest.cpp
+++ b/llvm/unittests/DebugInfo/LogicalView/DWARFReaderTest.cpp
@@ -155,6 +155,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);

llvm/lib/DebugInfo/LogicalView/Core/LVType.cpp

llvm/lib/DebugInfo/LogicalView/Readers/LVCodeViewVisitor.cpp

jalopezg-git · 2025-05-12T12:59:34Z

I have also updated tools/llvm-debuginfo-analyzer/... tests to check that Size = is printed for types.

llvm/lib/DebugInfo/LogicalView/Core/LVScope.cpp

llvm/test/tools/llvm-debuginfo-analyzer/DWARF/06-dwarf-full-logical-view.test

github-actions · 2025-05-13T14:05:41Z

✅ With the latest revision this PR passed the C/C++ code formatter.

llvm/lib/DebugInfo/LogicalView/Core/LVScope.cpp

llvm/lib/DebugInfo/LogicalView/Core/LVType.cpp

CarlosAlbertoEnciso

LGTM with the minor change (rearrange if conditions).

CarlosAlbertoEnciso · 2025-05-15T06:51:18Z

An additional change: Update the unit test CommandLineOptionsTest.cpp to include the new option in CheckExtendedAttributes.

jalopezg-git · 2025-05-16T14:12:26Z

An additional change: Update the unit test CommandLineOptionsTest.cpp to include the new option in CheckExtendedAttributes.

Done too 👍. Thanks for the review! I believe this is now ready to merge.

jalopezg-git · 2025-05-19T11:39:37Z

@jmorse, @SLTozer, @OCHyams, this is also ready to merge. 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 🙂.

OCHyams

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

Hi, thanks for checking. Gave it a quick scan (one nit inline), but Carlos is the expert here so I don't think additional review is required (unless you'd appreciate another pair of eyes @CarlosAlbertoEnciso, ofc).

(Same applies to the other recent patches)

I'll leave it to @CarlosAlbertoEnciso to merge away if/when he's happy.

llvm/include/llvm/DebugInfo/LogicalView/Core/LVElement.h

CarlosAlbertoEnciso · 2025-05-22T05:38:28Z

Missing changes:

As the patch introduces the --attribute=size option, the LVOptions::print(raw_ostream &OS) function must be updated to include it. That function produces a 4 column print of all attributes.
The cmdline.test should be updated: --attribute=<value> to include the new option and --select-scopes=<value> to include =Module kind. (Missing from the DW_AT_module patch).

CarlosAlbertoEnciso · 2025-05-22T10:59:22Z

@jalopezg-git In relation to the write access, have a look at:
https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access

jalopezg-git · 2025-05-22T11:28:25Z

@jalopezg-git In relation to the write access, have a look at: https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access

Thank you! Taking a look at the requested changes now.

jalopezg-git · 2025-05-22T11:50:10Z

Missing changes:

* As the patch introduces the `--attribute=size` option, the `LVOptions::print(raw_ostream &OS)` function must be updated to include it. That function produces a 4 column print of all attributes.

* The `cmdline.test` should be updated: `--attribute=<value>` to include the new option and `--select-scopes=<value>` to include `=Module` kind. (Missing from the DW_AT_module patch).

Done - thanks for the review! Let's wait for test results, but I guess it should be okay.

CarlosAlbertoEnciso · 2025-05-22T13:21:04Z

@jalopezg-git Thanks for adding that support.

…vm#139110) This PR was split from llvm#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.

llvmbot added the debuginfo label May 8, 2025

jalopezg-git mentioned this pull request May 8, 2025

[llvm-debuginfo-analyzer] Support DW_TAG_module #137228

Merged

CarlosAlbertoEnciso assigned jalopezg-git May 12, 2025

CarlosAlbertoEnciso added llvm:binary-utilities tools:llvm-debuginfo-analyzer labels May 12, 2025

CarlosAlbertoEnciso requested review from jmorse, SLTozer, OCHyams and CarlosAlbertoEnciso May 12, 2025 09:22

CarlosAlbertoEnciso reviewed May 12, 2025

View reviewed changes

llvm/lib/DebugInfo/LogicalView/Core/LVType.cpp Show resolved Hide resolved

CarlosAlbertoEnciso reviewed May 12, 2025

View reviewed changes

llvm/lib/DebugInfo/LogicalView/Readers/LVCodeViewVisitor.cpp Show resolved Hide resolved

jalopezg-git force-pushed the jalopezg-logicalview-AT_byte_size branch from 4c697cf to 1dd824c Compare May 12, 2025 12:33

jalopezg-git requested a review from CarlosAlbertoEnciso May 12, 2025 12:33

CarlosAlbertoEnciso reviewed May 12, 2025

View reviewed changes

llvm/lib/DebugInfo/LogicalView/Core/LVScope.cpp Outdated Show resolved Hide resolved

CarlosAlbertoEnciso reviewed May 12, 2025

View reviewed changes

llvm/test/tools/llvm-debuginfo-analyzer/DWARF/06-dwarf-full-logical-view.test Show resolved Hide resolved

jalopezg-git force-pushed the jalopezg-logicalview-AT_byte_size branch 2 times, most recently from 9996060 to 544e95c Compare May 12, 2025 14:36

jalopezg-git requested a review from CarlosAlbertoEnciso May 12, 2025 14:41

jalopezg-git force-pushed the jalopezg-logicalview-AT_byte_size branch 4 times, most recently from 16af1fb to 39bb89c Compare May 13, 2025 14:05

CarlosAlbertoEnciso reviewed May 15, 2025

View reviewed changes

llvm/lib/DebugInfo/LogicalView/Core/LVScope.cpp Outdated Show resolved Hide resolved

CarlosAlbertoEnciso reviewed May 15, 2025

View reviewed changes

llvm/lib/DebugInfo/LogicalView/Core/LVType.cpp Outdated Show resolved Hide resolved

CarlosAlbertoEnciso approved these changes May 15, 2025

View reviewed changes

jalopezg-git force-pushed the jalopezg-logicalview-AT_byte_size branch from 39bb89c to 453f882 Compare May 16, 2025 14:12

OCHyams reviewed May 20, 2025

View reviewed changes

llvm/include/llvm/DebugInfo/LogicalView/Core/LVElement.h Outdated Show resolved Hide resolved

jalopezg-git force-pushed the jalopezg-logicalview-AT_byte_size branch 2 times, most recently from 55acfaf to 49baead Compare May 21, 2025 10:27

jalopezg-git force-pushed the jalopezg-logicalview-AT_byte_size branch from 49baead to 378fddd Compare May 22, 2025 11:45

jalopezg-git added 2 commits May 22, 2025 13:46

[llvm-debuginfo-analyzer] Add support for DWARF DW_AT_byte_size

629d4d3

[llvm-debuginfo-analyzer] Update cmdline.test

7c6d1a4

jalopezg-git force-pushed the jalopezg-logicalview-AT_byte_size branch from 378fddd to 7c6d1a4 Compare May 22, 2025 11:47

CarlosAlbertoEnciso merged commit 9cac4bf into llvm:main May 22, 2025
12 checks passed

jalopezg-git deleted the jalopezg-logicalview-AT_byte_size branch May 22, 2025 14:20

[llvm-debuginfo-analyzer] Add support for DWARF DW_AT_byte_size #139110

[llvm-debuginfo-analyzer] Add support for DWARF DW_AT_byte_size #139110

Uh oh!

Conversation

jalopezg-git commented May 8, 2025

Uh oh!

llvmbot commented May 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jalopezg-git commented May 12, 2025

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented May 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

CarlosAlbertoEnciso left a comment

Choose a reason for hiding this comment

Uh oh!

CarlosAlbertoEnciso commented May 15, 2025

Uh oh!

jalopezg-git commented May 16, 2025

Uh oh!

jalopezg-git commented May 19, 2025

Uh oh!

OCHyams left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

CarlosAlbertoEnciso commented May 22, 2025

Uh oh!

CarlosAlbertoEnciso commented May 22, 2025

Uh oh!

jalopezg-git commented May 22, 2025

Uh oh!

jalopezg-git commented May 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

CarlosAlbertoEnciso commented May 22, 2025

Uh oh!

Uh oh!

[llvm-debuginfo-analyzer] Add support for DWARF `DW_AT_byte_size` #139110

[llvm-debuginfo-analyzer] Add support for DWARF `DW_AT_byte_size` #139110

llvmbot commented May 8, 2025 •

edited

Loading

github-actions bot commented May 13, 2025 •

edited

Loading

jalopezg-git commented May 22, 2025 •

edited

Loading