This is an archive of the discontinued LLVM Phabricator instance.

[BOLT][DWARF] Numerous fixes for a new DWARFRewriter
ClosedPublic

Authored by ayermolo on Jun 1 2023, 10:30 AM.

Details

Summary
  • Some cleanup and minor fixes for the new debug information re-writer before moving on to productatization.
  • The new rewriter wasn't handling binary with DWARF5 and DWARF4 with -fdebug-types-sections.
  • Removed dead cross cu reference code.
  • Added support for DW_AT_sibling.
  • With the new re-writer abbrev number can change which can lead to offset of Type Units changing. Before we would just copy raw data. Changed to write out Type Unit List. This is generated by gdb-add-index.
  • Fixed how bolt handles gdb-index generated by gdb-11 with types sections. Simplified logic that handles variations of gdb-index.
  • Clang can generate two type units with the same hash, but different content. LLD does not de-duplicate when ThinLTO is involved. Changed so that TU hash and offset are used to make TU's unique.
  • It is possible to have references within location expression to another DIE. Fixed it so that relative offset is updated correctly.
  • Removed all the code related to patching.
  • Removed dead code. Changed how we handling writting out TUs and TU Index. It now should fully work for DWARF4 and DWARF5.
  • Removed unused arguments from some APIs, changed return type to void, and other small cleanups.

Depends on D130315

Diff Detail

Event Timeline

ayermolo created this revision.Jun 1 2023, 10:30 AM
Herald added a reviewer: rafauler. · View Herald Transcript
Herald added a reviewer: Amir. · View Herald Transcript
Herald added a reviewer: maksfb. · View Herald Transcript
Herald added a project: Restricted Project. · View Herald Transcript
ayermolo requested review of this revision.Jun 1 2023, 10:30 AM
Herald added a project: Restricted Project. · View Herald Transcript
ayermolo edited the summary of this revision. (Show Details)Jun 1 2023, 10:31 AM
maksfb added inline comments.Jun 16 2023, 4:49 PM
bolt/include/bolt/Core/DIEBuilder.h
18–19

Do we need this?

47–48

Add comment with a description.

47–59
60–61

Add a comment.

171–172
172–173
191–197

Make it an internal DWARF errror?

bolt/include/bolt/Core/DebugData.h
607–608

Remove now or in follow up diffs.

bolt/lib/Core/CMakeLists.txt
8 ↗(On Diff #527984)

Check if we really need it?

bolt/lib/Core/DIEBuilder.cpp
50–51
598

Use braces.

bolt/lib/Rewrite/DWARFRewriter.cpp
75

Could you please add comments with the description of the class?

365
367
1864
ayermolo marked 15 inline comments as done.Jun 20 2023, 2:47 PM
ayermolo updated this revision to Diff 533056.Jun 20 2023, 3:12 PM

rebase + addressed comments

maksfb added inline comments.Jun 22 2023, 10:43 AM
bolt/include/bolt/Core/DIEBuilder.h
1

Update the path.

15

Update the name of the macro too.

31

Are you actually using mutexes?

122
bolt/lib/Core/DIEBuilder.cpp
9

This file needs a license header.

maksfb added inline comments.Jun 22 2023, 10:46 AM
bolt/include/bolt/Core/DIEBuilder.h
57

I think it's better to stick to the common nomenclature and use CU/CompilationUnit for the name.

ayermolo updated this revision to Diff 533715.Jun 22 2023, 11:56 AM
ayermolo marked 6 inline comments as done.

addressed comments

ayermolo added inline comments.Jun 22 2023, 1:19 PM
bolt/include/bolt/Core/DIEBuilder.h
31

Removed this, and cleaned up in general.

maksfb added inline comments.Jun 22 2023, 2:18 PM
bolt/lib/Core/DIEBuilder.cpp
81–88

Use braces.

443–444
444
545–550

Braces.

607–608
bolt/lib/Rewrite/DWARFRewriter.cpp
1850
maksfb added inline comments.Jun 22 2023, 2:39 PM
bolt/lib/Core/DIEBuilder.cpp
81–88

I take it back - you don't really have to here.

ayermolo updated this revision to Diff 533790.Jun 22 2023, 2:48 PM

fixed unit tests to work on build bots

ayermolo retitled this revision from [BOLT][DWARF] Fix new debug re-writer to [BOLT][DWARF] Numerous fixes for a new DWARFRewriter.Jun 30 2023, 2:11 PM
ayermolo edited the summary of this revision. (Show Details)Jul 5 2023, 6:06 PM
maksfb accepted this revision.Jul 6 2023, 11:40 AM

LGTM. Thanks!

This revision is now accepted and ready to land.Jul 6 2023, 11:40 AM
ayermolo closed this revision.Jul 6 2023, 4:59 PM
ayermolo reopened this revision.Jul 7 2023, 4:56 PM
This revision is now accepted and ready to land.Jul 7 2023, 4:56 PM
This revision was landed with ongoing or failed builds.Jul 10 2023, 2:42 PM
This revision was automatically updated to reflect the committed changes.