This is an archive of the discontinued LLVM Phabricator instance.

[BOLT][DWARF] Implement monolithic DWARF5
ClosedPublic

Authored by ayermolo on Mar 16 2022, 6:45 PM.

Details

Summary

Added implementation to support DWARF5 in monolithic mode.
Next step DWARF5 split dwarf support.

Diff Detail

Event Timeline

ayermolo created this revision.Mar 16 2022, 6:45 PM
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
Herald added subscribers: hoy, modimo, wenlei and 2 others. · View Herald Transcript
ayermolo requested review of this revision.Mar 16 2022, 6:46 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 16 2022, 6:46 PM
ormris removed a subscriber: ormris.Mar 17 2022, 10:56 AM
ormris added a subscriber: ormris.
ayermolo edited the summary of this revision. (Show Details)Mar 17 2022, 4:17 PM
maksfb added inline comments.Apr 5 2022, 1:51 PM
bolt/include/bolt/Core/DebugData.h
187

nit

214

The comment is out of sync.

217

Why do you need to redefine getKind()?

231

Is uint32_t enough to support DWARF-64 in the future?

285–286

Update the comment.

486

Could this be constexpr and defined here?

487

Likewise.

1093

nit

bolt/lib/Core/BinaryContext.cpp
1538

nit

bolt/lib/Core/DebugData.cpp
168

nit

186
209
226
404
414

nit

446
458
477

Expand auto.

573
603
649
1522–1523
bolt/lib/Rewrite/DWARFRewriter.cpp
83–87
494

No need to introduce the new scope unless you need it.

562–563
1585
ayermolo marked 25 inline comments as done.Apr 5 2022, 5:16 PM
ayermolo added inline comments.
bolt/include/bolt/Core/DebugData.h
231

no, but then rest of the code either.

ayermolo updated this revision to Diff 420668.Apr 5 2022, 5:21 PM

addressing comments

ayermolo updated this revision to Diff 422642.Apr 13 2022, 1:52 PM

addressed comments.

ayermolo marked an inline comment as done.Apr 13 2022, 1:52 PM
ayermolo added inline comments.Apr 13 2022, 3:58 PM
bolt/lib/Core/BinaryContext.cpp
1465

Add a warning if it's dwar5 and legacy

1507–1508

remove

1508

rename BinaryLineTable

bolt/lib/Core/DebugData.cpp
412

add const to this and AddrSize

419

add type

431

remove curly brases

454

lower case

1522

remove .

1555

const

bolt/lib/Rewrite/DWARFRewriter.cpp
375–376

remove space

590

Unit.getVersion()== 5

594

use braces for rest

604–605

const

612

const here and Index

662

rewrite to make sense

667

const

962

use curly

973

use curly

974

llvm_unreachable

ayermolo updated this revision to Diff 422683.Apr 13 2022, 4:00 PM

addressing more comments

ayermolo updated this revision to Diff 423761.Apr 19 2022, 4:27 PM

Change how we handle .debug_line_str

Herald added a project: Restricted Project. · View Herald TranscriptApr 19 2022, 4:27 PM
ayermolo retitled this revision from [BOLT][DWARF] Implementation of monolithic DWARF5. to [BOLT][DWARF] Implement monolithic DWARF5..Apr 19 2022, 4:28 PM
DavidSpickett added inline comments.
lldb/test/Shell/SymbolFile/DWARF/x86/DW_TAG_variable-DW_AT_decl_file-DW_AT_abstract_origin-crosscu1.s
17 ↗(On Diff #423761)

Was this test just using whatever DWARF %clang_host defaulted to? (since I don't see any changes outside of bolt other than this)

ayermolo added inline comments.Apr 20 2022, 11:31 AM
lldb/test/Shell/SymbolFile/DWARF/x86/DW_TAG_variable-DW_AT_decl_file-DW_AT_abstract_origin-crosscu1.s
17 ↗(On Diff #423761)

Yes.

ayermolo retitled this revision from [BOLT][DWARF] Implement monolithic DWARF5. to [BOLT][DWARF] Implement monolithic DWARF5.Apr 21 2022, 2:46 PM
maksfb accepted this revision.Apr 21 2022, 2:57 PM

LGTM

This revision is now accepted and ready to land.Apr 21 2022, 2:57 PM
This revision was automatically updated to reflect the committed changes.
Amir added a comment.May 27 2022, 9:37 AM

Hi,
The test started to fail on Ubuntu buildbot: https://lab.llvm.org/buildbot/#/builders/215/builds/6376

# PRECHECK: include_directories[ 0] = .debug_line_str[0x00000000]
            ^
<stdin>:8:12: note: scanning from here
 version: 5
           ^
<stdin>:30:1: note: possible intended match here
include_directories[ 0] = .debug_line_str[0x00000009] = "."

Can you please take a look?

Hi,
The test started to fail on Ubuntu buildbot: https://lab.llvm.org/buildbot/#/builders/215/builds/6376

# PRECHECK: include_directories[ 0] = .debug_line_str[0x00000000]
            ^
<stdin>:8:12: note: scanning from here
 version: 5
           ^
<stdin>:30:1: note: possible intended match here
include_directories[ 0] = .debug_line_str[0x00000009] = "."

Can you please take a look?

Fixed in: D126733. Was due to LLD change.