[BOLT][DWARF] Fix debug info update issue with dwarf4 dwp

[Jinjie-Huang]

Currently in emitDWOBuilder(), when emitting Compile Units for DWO with dwarf version below 4, the Compile Unit information is obtained through SplitCU.getContext().dwo_compile_units(). This seems to assume that SplitCU is a DWO. However, if it is a DWP, it will at this point fetch and iterate over CUs that do not belong to the current DWO (and have not been registered), which will lead to a crash during emitUnit.

I may not be fully clear about some of the context, but it seems that a better approach here would be to align with the handling in dwarf5 and directly emit the current SplitCU. And this seems to both avoid the crash issue and be more efficient?

[llvmbot]

@llvm/pr-subscribers-bolt

Author: Jinjie Huang (Jinjie-Huang)

Changes

Currently in emitDWOBuilder(), when emitting Compile Units for DWO with dwarf version below 4, the Compile Unit information is obtained through SplitCU.getContext().dwo_compile_units(). This seems to assume that SplitCU is a DWO. However, if it is a DWP, it will at this point fetch and iterate over CUs that do not belong to the current DWO (and have not been registered), which will lead to a crash during emitUnit.

I may not be fully clear about some of the context, but it seems that a better approach here would be to align with the handling in dwarf5 and directly emit the current SplitCU. And this seems to both avoid the crash issue and be more efficient?

---

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

1 Files Affected:

• (modified) bolt/lib/Rewrite/DWARFRewriter.cpp (+1-3)

diff --git a/bolt/lib/Rewrite/DWARFRewriter.cpp b/bolt/lib/Rewrite/DWARFRewriter.cpp
index 0c1a1bac6c72e..6eefa5155298b 100644
--- a/bolt/lib/Rewrite/DWARFRewriter.cpp
+++ b/bolt/lib/Rewrite/DWARFRewriter.cpp
@@ -504,9 +504,7 @@ static void emitDWOBuilder(const std::string &DWOName,
     }
     emitUnit(DWODIEBuilder, *Streamer, SplitCU);
   } else {
-    for (std::unique_ptr<llvm::DWARFUnit> &CU :
-         SplitCU.getContext().dwo_compile_units())
-      emitUnit(DWODIEBuilder, *Streamer, *CU);
+    emitUnit(DWODIEBuilder, *Streamer, SplitCU);
 
     // emit debug_types sections for dwarf4
     for (DWARFUnit *CU : DWODIEBuilder.getDWARF4TUVector())

[Jinjie-Huang]

@ayermolo @kazutakahirata PTAL, thanks!

[ayermolo]

Umm yes, makes sense.
Can you add a test please.

[Jinjie-Huang]

@ayermolo Thanks for review, and the test has been added.

[ayermolo]

Thanks for adding a test. Can you convert it to assembly test, and minimize assembly as much as possible, so it is more robust.

[paschalis-mpeis]

Thanks for improving support for split dwarf.
As this is generic, we’d welcome a test that runs on AArch64 too (if not much effort). :)

[Jinjie-Huang]

I've modified the test to use the minimal assembly, and the test for AArch64 has also been added.

[Jinjie-Huang]

Thanks for the feedback. I've updated the test case accordingly.

[Jinjie-Huang]

@paschalis-mpeis @maksfb Could you help to review? Thanks.

[Sockke]

LGTM, let's see if anyone else has comments in the next few days.

[paschalis-mpeis]

Hey @Jinjie-Huang,

Thanks for your work! :)
Could you amend cflags cxxflags to X86/dwarf4-dwp-x86.s as it currently cross-runs on AArch64 with failures:

Failed Tests (1):
  BOLT :: X86/dwarf4-dwp-x86.s

Tried on a local AArch64 machine with targets: 'AArch64;X86'. The buildbot would hit it too.

Test was fixed