This is an archive of the discontinued LLVM Phabricator instance.

[BOLT][DWARF] Always use new low_pc for exprloc
ClosedPublic

Authored by ayermolo on Aug 7 2023, 8:32 PM.

Details

Summary

Changed to creating a new index all the time. This code was legacy of when we
couldn't change the size of .debug_info, and led to subtle bugs where index for
new entries was pointing to a wrong address.

Diff Detail

Event Timeline

ayermolo created this revision.Aug 7 2023, 8:32 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
ayermolo requested review of this revision.Aug 7 2023, 8:32 PM
Herald added a project: Restricted Project. · View Herald Transcript
Amir accepted this revision.Aug 8 2023, 2:35 PM

LGTM sans nits.

bolt/lib/Rewrite/DWARFRewriter.cpp
1125–1133

Drop

1154

Is it correct to drop this else? I.e. for all non-DW_OP_{,GNU_}addrx forms is it ok to skip them?
Looks like yes as the other forms are copied above.

This revision is now accepted and ready to land.Aug 8 2023, 2:35 PM
ayermolo marked 2 inline comments as done.Aug 8 2023, 4:25 PM
ayermolo added inline comments.
bolt/lib/Rewrite/DWARFRewriter.cpp
1154

Yes. Previously we copied old data, and then added existing index in to AddressTable. Now we create a new entry.

ayermolo updated this revision to Diff 548400.Aug 8 2023, 4:44 PM
ayermolo marked an inline comment as done.

addressed comments

This revision was automatically updated to reflect the committed changes.