This is an archive of the discontinued LLVM Phabricator instance.

[BOLT][DWARF] Fix handling of inlined subroutine with no output PC
ClosedPublic

Authored by ayermolo on Jul 31 2023, 2:09 PM.

Details

Summary

Clang can generate DW_TAG_inlined_subroutine with low_pc 0. With split dwarf
this led to range offset being a negative number.

Diff Detail

Event Timeline

ayermolo created this revision.Jul 31 2023, 2:09 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.Jul 31 2023, 2:09 PM
Herald added a project: Restricted Project. · View Herald Transcript
maksfb added inline comments.Jul 31 2023, 2:29 PM
bolt/lib/Rewrite/DWARFRewriter.cpp
842

Why is this change needed?

868

The condition is always true here.

ayermolo added inline comments.Jul 31 2023, 2:33 PM
bolt/lib/Rewrite/DWARFRewriter.cpp
842

It's noop. If range is empty we will normalize to low_pc/high_pc.
In subsequent diffs I want to remove it completely.

maksfb added inline comments.Jul 31 2023, 3:44 PM
bolt/lib/Rewrite/DWARFRewriter.cpp
868

Remove unnecessary check.

872

Do we expect such range to be ignored by DWARF consumers? Why not push {0, 0} or {0, 1} then?

ayermolo updated this revision to Diff 545839.Jul 31 2023, 3:49 PM

Added test, addressed comments.

ayermolo marked 2 inline comments as done.Jul 31 2023, 3:52 PM
ayermolo added inline comments.
bolt/lib/Rewrite/DWARFRewriter.cpp
868

Removed, but it proved useful. Added extra test that would trigger it.

872

I want to preserve as much of original as possible.

maksfb added inline comments.Jul 31 2023, 3:59 PM
bolt/lib/Rewrite/DWARFRewriter.cpp
868

Sorry, I don't follow. What was useful? I was talking about the check for OutputRanges.empty().

ayermolo added inline comments.Jul 31 2023, 4:02 PM
bolt/lib/Rewrite/DWARFRewriter.cpp
868

oh, I thought you were referring to the low_pc check.

maksfb accepted this revision.Jul 31 2023, 4:04 PM

Rename before commit: "Fix handling inlined subroutine with no output pc." -> "Fix handling of inlined subroutine with no output PC"

This revision is now accepted and ready to land.Jul 31 2023, 4:04 PM
ayermolo retitled this revision from [BOLT][DWARF] Fix handling inlined subroutine with no output pc. to [BOLT][DWARF] Fix handling of inlined subroutine with no output PC.Jul 31 2023, 4:08 PM
ayermolo updated this revision to Diff 545863.Jul 31 2023, 4:42 PM

added how gc assembly files were generated