Page MenuHomePhabricator

[DWARFDebug] Fix another case of overlapping ranges
ClosedPublic

Authored by loladiro on Jan 29 2016, 4:15 PM.

Details

Summary

In r257979, I added code to ensure that we wouldn't merge DebugLocEntries if the pieces they describe overlap. Unfortunately, I failed to cover the case, where there may have multiple active Expressions in the entry, in which case we need to make sure that no two values overlap before we can perform the merge.

This fixed PR26148.

Diff Detail

Repository
rL LLVM

Event Timeline

loladiro updated this revision to Diff 46451.Jan 29 2016, 4:15 PM
loladiro retitled this revision from to [DWARFDebug] Fix another case of overlapping ranges.
loladiro updated this object.
loladiro added a reviewer: aprantl.
loladiro set the repository for this revision to rL LLVM.
loladiro added a subscriber: llvm-commits.
aprantl added inline comments.Jan 29 2016, 4:24 PM
lib/CodeGen/AsmPrinter/DwarfDebug.cpp
821 ↗(On Diff #46451)

Do we have to do n^2 comparisons? I vaguely remember the pieces being sorted by offset. Or was this only later?

loladiro updated this revision to Diff 46456.Jan 29 2016, 5:25 PM

Take advantage of the fact that pieces are sorted.

Ka-Ka added a subscriber: Ka-Ka.Jan 31 2016, 4:29 AM

We hit this bug too and your fix works for our test cases. LGTM, thanks!

Ping...

Can this go in please? This is breaking debug builds for a number of critical codes for us.

aprantl accepted this revision.Feb 3 2016, 9:00 AM
aprantl edited edge metadata.

Thanks! Minor comments inline.

lib/CodeGen/AsmPrinter/DwarfDebug.cpp
836 ↗(On Diff #46456)

pieceCmp will crash if passed a nullpointer so we might as well use a regular cast here.

841 ↗(On Diff #46456)

Can you add comments to the conditions that explain the conditions we're testing for here?
if (res == -1) break; needs some detective work to decode :-)

This revision is now accepted and ready to land.Feb 3 2016, 9:00 AM
This revision was automatically updated to reflect the committed changes.