Skip to content

Conversation

grigorypas
Copy link
Contributor

@grigorypas grigorypas commented Jul 30, 2025

Fix incorrect file index handling that differed between DWARF4 and DWARF5.

DWARF4 file indices start at 1, while DWARF5 starts at 0. The code was
manually adjusting indices with Row.File - 1, which works for DWARF4
but breaks DWARF5.

Replace manual indexing with getFileNameEntry() which abstracts away
the DWARF version differences.

Fixed in:

  • printDebugInfo()
  • addDebugFilenameToUnit()

@llvmbot
Copy link
Member

llvmbot commented Jul 30, 2025

@llvm/pr-subscribers-bolt

Author: Grigory Pastukhov (grigorypas)

Changes

This PR fixes 3 failing tests on the master branch.

  1. FileIndex starts from 1 in DWARF4 and from 0 in DWARF5. getFileNameEntry function abstracts that logic away.
  2. perf_test.lds was crashing because the address of the first section was too low.

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

2 Files Affected:

  • (modified) bolt/lib/Core/BinaryContext.cpp (+7-11)
  • (modified) bolt/test/perf2bolt/Inputs/perf_test.lds (+5-6)
diff --git a/bolt/lib/Core/BinaryContext.cpp b/bolt/lib/Core/BinaryContext.cpp
index 84f1853469709..3f6d27c751556 100644
--- a/bolt/lib/Core/BinaryContext.cpp
+++ b/bolt/lib/Core/BinaryContext.cpp
@@ -1568,23 +1568,19 @@ unsigned BinaryContext::addDebugFilenameToUnit(const uint32_t DestCUID,
   DWARFCompileUnit *SrcUnit = DwCtx->getCompileUnitForOffset(SrcCUID);
   const DWARFDebugLine::LineTable *LineTable =
       DwCtx->getLineTableForUnit(SrcUnit);
-  const std::vector<DWARFDebugLine::FileNameEntry> &FileNames =
-      LineTable->Prologue.FileNames;
-  // Dir indexes start at 1, as DWARF file numbers, and a dir index 0
+  const DWARFDebugLine::FileNameEntry &FileNameEntry =
+      LineTable->Prologue.getFileNameEntry(FileIndex);
+  // Dir indexes start at 1 and a dir index 0
   // means empty dir.
-  assert(FileIndex > 0 && FileIndex <= FileNames.size() &&
-         "FileIndex out of range for the compilation unit.");
   StringRef Dir = "";
-  if (FileNames[FileIndex - 1].DirIdx != 0) {
+  if (FileNameEntry.DirIdx != 0) {
     if (std::optional<const char *> DirName = dwarf::toString(
-            LineTable->Prologue
-                .IncludeDirectories[FileNames[FileIndex - 1].DirIdx - 1])) {
+            LineTable->Prologue.IncludeDirectories[FileNameEntry.DirIdx - 1])) {
       Dir = *DirName;
     }
   }
   StringRef FileName = "";
-  if (std::optional<const char *> FName =
-          dwarf::toString(FileNames[FileIndex - 1].Name))
+  if (std::optional<const char *> FName = dwarf::toString(FileNameEntry.Name))
     FileName = *FName;
   assert(FileName != "");
   DWARFCompileUnit *DstUnit = DwCtx->getCompileUnitForOffset(DestCUID);
@@ -1925,7 +1921,7 @@ static void printDebugInfo(raw_ostream &OS, const MCInst &Instruction,
   const DWARFDebugLine::Row &Row = LineTable->Rows[RowRef.RowIndex - 1];
   StringRef FileName = "";
   if (std::optional<const char *> FName =
-          dwarf::toString(LineTable->Prologue.FileNames[Row.File - 1].Name))
+          dwarf::toString(LineTable->Prologue.getFileNameEntry(Row.File).Name))
     FileName = *FName;
   OS << " # debug line " << FileName << ":" << Row.Line;
   if (Row.Column)
diff --git a/bolt/test/perf2bolt/Inputs/perf_test.lds b/bolt/test/perf2bolt/Inputs/perf_test.lds
index 66d925a05bebc..c2704d73a638c 100644
--- a/bolt/test/perf2bolt/Inputs/perf_test.lds
+++ b/bolt/test/perf2bolt/Inputs/perf_test.lds
@@ -1,13 +1,12 @@
 SECTIONS {
-  . = SIZEOF_HEADERS;
+  . = 0x400000 + SIZEOF_HEADERS;
   .interp : { *(.interp) }
   .note.gnu.build-id : { *(.note.gnu.build-id) }
-  . = 0x212e8;
   .dynsym         : { *(.dynsym) }
-  . = 0x31860;
+  . = 0x801000;
   .text : { *(.text*) }
-  . = 0x41c20;
+  . = 0x803000;
   .fini_array : { *(.fini_array) }
-  . = 0x54e18;
+  . = 0x805000;
   .data : { *(.data) }
-}
\ No newline at end of file
+}

@grigorypas grigorypas changed the title [BOLT] Fix failing tests by refactoring access to FileNameEntry to make it DWAFR version agnostic [BOLT] Fix failing tests by refactoring access to FileNameEntry to make it DWARF version agnostic Jul 30, 2025
@ayermolo
Copy link
Contributor

ayermolo commented Aug 12, 2025

Can you add which tests were failing, and why did they start to fail?
Is test change related to this change? Seems unrelated.

@grigorypas grigorypas force-pushed the bolt_debug_lines_refactor_fix_tests branch from 3d427e6 to aa4926b Compare August 18, 2025 23:37
@grigorypas
Copy link
Contributor Author

Can you add which tests were failing, and why did they start to fail? Is test change related to this change? Seems unrelated.

Moved fix to failing test unrelated to the code changes to a separate PR (#154209).

Copy link

github-actions bot commented Aug 20, 2025

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

@grigorypas grigorypas changed the title [BOLT] Fix failing tests by refactoring access to FileNameEntry to make it DWARF version agnostic [BOLT] Fix DWARF4/5 file index handling in debug info functions Aug 20, 2025
@grigorypas grigorypas force-pushed the bolt_debug_lines_refactor_fix_tests branch from 680f03f to ce6b1f3 Compare August 20, 2025 03:15

// REQUIRES: system-linux

// RUN: %clang -g -gdwarf-5 -O1 -xc++ %s -o %t-main.o -c
Copy link
Contributor

Choose a reason for hiding this comment

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

See if Inputs/dwarf5_main.s and Inputs/dwarf5_helper.s
can be re-used.

@grigorypas grigorypas force-pushed the bolt_debug_lines_refactor_fix_tests branch from ce6b1f3 to 2327070 Compare August 27, 2025 20:40
@ayermolo ayermolo merged commit 8c8d1d4 into llvm:main Aug 29, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants