[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.

[ayermolo]

Please add description of what this test does.
I think current convention is to use ## for Comments.

[Jinjie-Huang]

Got it. It seems we don't need to describe how the assembly comes via update_test_body.py, so I've also
removed this part for now.

[ayermolo]

Please add description of what this test does.
I think current convention is to use ## for Comments.

[ayermolo]

Please minimize assembly. Example: #120103