Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DWARF] Refactor findDebugNamesOffsets #88064

Merged

Conversation

MaskRay
Copy link
Member

@MaskRay MaskRay commented Apr 8, 2024

Address some post-review comments in #82153 and move the function inside
llvm::dwarf, used by certain free functions.

Created using spr 1.3.5-bogner
@llvmbot
Copy link
Collaborator

llvmbot commented Apr 8, 2024

@llvm/pr-subscribers-debuginfo

Author: Fangrui Song (MaskRay)

Changes

Address some post-review comments in #82153 and move the function inside
llvm::dwarf, used by certain free functions.


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

2 Files Affected:

  • (modified) llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h (+5-3)
  • (modified) llvm/lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp (+8-6)
diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h
index f1d4fc72d5a727..9543b78ea61309 100644
--- a/llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h
+++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h
@@ -804,9 +804,11 @@ class DWARFDebugNames : public DWARFAcceleratorTable {
 
 /// Calculates the starting offsets for various sections within the
 /// .debug_names section.
-void findDebugNamesOffsets(DWARFDebugNames::DWARFDebugNamesOffsets &Offsets,
-                           uint64_t HdrSize, const dwarf::DwarfFormat Format,
-                           const DWARFDebugNames::Header &Hdr);
+namespace dwarf {
+DWARFDebugNames::DWARFDebugNamesOffsets
+findDebugNamesOffsets(uint64_t EndOfHeaderOffset,
+                      const DWARFDebugNames::Header &Hdr);
+}
 
 /// If `Name` is the name of a templated function that includes template
 /// parameters, returns a substring of `Name` containing no template
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp b/llvm/lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp
index 9c65d85985f1bb..3a84784ff967f0 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp
@@ -552,11 +552,12 @@ DWARFDebugNames::NameIndex::extractAbbrev(uint64_t *Offset) {
   return Abbrev(Code, dwarf::Tag(Tag), AbbrevOffset, std::move(*AttrEncOr));
 }
 
-void llvm::findDebugNamesOffsets(
-    DWARFDebugNames::DWARFDebugNamesOffsets &Offsets, uint64_t HdrSize,
-    dwarf::DwarfFormat Format, const DWARFDebugNames::Header &Hdr) {
-  uint32_t DwarfSize = (Format == llvm::dwarf::DwarfFormat::DWARF64) ? 8 : 4;
-  uint64_t Offset = HdrSize;
+DWARFDebugNames::DWARFDebugNamesOffsets
+dwarf::findDebugNamesOffsets(uint64_t EndOfHeaderOffset,
+                             const DWARFDebugNames::Header &Hdr) {
+  DWARFDebugNames::DWARFDebugNamesOffsets Offsets;
+  uint32_t DwarfSize = Hdr.Format == dwarf::DwarfFormat::DWARF64 ? 8 : 4;
+  uint64_t Offset = EndOfHeaderOffset;
   Offsets.CUsBase = Offset;
   Offset += Hdr.CompUnitCount * DwarfSize;
   Offset += Hdr.LocalTypeUnitCount * DwarfSize;
@@ -577,6 +578,7 @@ void llvm::findDebugNamesOffsets(
 
   Offset += Hdr.AbbrevTableSize;
   Offsets.EntriesBase = Offset;
+  return Offsets;
 }
 
 Error DWARFDebugNames::NameIndex::extract() {
@@ -586,7 +588,7 @@ Error DWARFDebugNames::NameIndex::extract() {
     return E;
 
   const unsigned SectionOffsetSize = dwarf::getDwarfOffsetByteSize(Hdr.Format);
-  findDebugNamesOffsets(Offsets, hdrSize, Hdr.Format, Hdr);
+  Offsets = dwarf::findDebugNamesOffsets(hdrSize, Hdr);
 
   uint64_t Offset =
       Offsets.EntryOffsetsBase + (Hdr.NameCount * SectionOffsetSize);

Copy link
Contributor

@felipepiovezan felipepiovezan left a comment

Choose a reason for hiding this comment

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

thanks for following up!

Created using spr 1.3.5-bogner
dwarf::findDebugNamesOffsets(uint64_t EndOfHeaderOffset,
const DWARFDebugNames::Header &Hdr) {
DWARFDebugNames::DWARFDebugNamesOffsets Ret;
uint32_t DwarfSize = Hdr.Format == dwarf::DwarfFormat::DWARF64 ? 8 : 4;
Copy link
Contributor

Choose a reason for hiding this comment

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

In a post-review comment in PR 82153, David suggested using dwarf::gerDwarfOffsetByteSize for the DwarfSize.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks. Adopted gerDwarfOffsetByteSize

Created using spr 1.3.5-bogner
@MaskRay MaskRay merged commit 9797a7e into main Apr 9, 2024
3 of 4 checks passed
@MaskRay MaskRay deleted the users/MaskRay/spr/dwarf-refactor-finddebugnamesoffsets branch April 9, 2024 19:32
MaskRay added a commit to MaskRay/llvm-project that referenced this pull request Apr 15, 2024
The parameter of `findDebugNamesOffsets` has been renamed to
`EndOfHeaderOffset` in llvm#88064 to make it clear it is a section offset
instead of an offset relative to the current name index. Rename the call
site variable as well.
MaskRay added a commit that referenced this pull request Apr 16, 2024
The parameter of `findDebugNamesOffsets` has been renamed to
`EndOfHeaderOffset` in #88064 to make it clear it is a section offset
instead of an offset relative to the current name index. Rename the call
site variable as well.
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.

4 participants