[llvm] r362951 - [DebugInfo] Terminate all location-lists at end of block

[Jeremy Morse via llvm-commits]

Hi,

On Fri, Jun 21, 2019 at 1:24 AM David Blaikie <dblaikie at gmail.com> wrote:
> Did you happen to do any measurements of debug info size growth with this change? For one optimized+asan binary at Google I observe a 20% increase in debug_loc.dwo size (& a total 4.7% growth in the corresponding dwp file) in object files, with compression enabled.

I sort-of recall looking at the size of .debug_loc on a clang-3.4
build before hand, IIRC it grew by less than 5% in size which is more
or less what I was expecting. Nothing thorough though, DWARF specifics
are something I'm still learning.

Awkwardly there are several things going on with this commit, as I
didn't want a chain of reversions if the buildbots complained, it's
code in both [0,1] rolled into one. The LiveDebugValues change in [0]
is guaranteed to generate more locations as constant/stack-spilt
locations now get propagated between blocks, which they weren't
before.

> I don't think there's necessarily any bugs in this, but looking at the DWARF even before this change there are some quirks that those interested in optimized debug info that I figured I'd highlight (perhaps you folk already know about these issues)
>
> a) debug_loc descriptions that decsribe all addresses in the surrounding debug_ranges with a single location - and instead should use a direct location description
> b) debug_loc descriptions taht describe addresses outside the range of the surrounding debug_ranges - which is meaningless debug info so far as I know, since a debugger shouldn't be trying to render the value of a variable outside the debug_ranges of the scope the variable is in

Hmn, a) is sort of expected, this change exposed some scenarios where
we can't currently determine a single location can be used, but
previously got it right due to other bugs. More detail in [1]s summary
about inlined-argument.ll. b) sounds genuinely buggy though. Some
range-trimming code was recently deleted [2], perhaps we relied on
that more than expected.

> Here's an example of both from the binary I've been looking at:
>
>   DW_TAG_inlined_subroutine
>     DW_AT_low_pc        (0x000000000eb939f5)
>     DW_AT_high_pc       (0x000000000eb93a03)
>     DW_TAG_formal_parameter
>       DW_AT_location    (0x00001112
>         [0x000000000eb939ce,  0x000000000eb93a03): DW_OP_reg0 RAX)
>
> It seems to me this should not use a debug_loc/loclist, and instead a direct DW_AT_location

If there's more than one basic block involved this could be the
inlined-argument.ll matter; it wouldn't surprise me if there were even
more scenarios where we don't produce single locations though.

> There are also cases where we're using separate ranges in debug_loc even though they're next to each other:
>
>   DW_TAG_inlined_subroutine
>     DW_AT_ranges        (0x0001d680
>       [0x000000000eb93ab7, 0x000000000eb93af4)
>       [0x000000000eb95251, 0x000000000eb9525e))
>     DW_TAG_formal_parameter
>       DW_AT_location    (0x0000143d
>         [0x000000000eb93ab7,  0x000000000eb93b17): DW_OP_reg4 RSI
>         [0x000000000eb95251,  0x000000000eb95259): DW_OP_reg4 RSI
>         [0x000000000eb95259,  0x000000000eb9525e): DW_OP_reg4 RSI)
>
> Note those last two entries are contiguous - (the third entry starts where the second entry ends) - and should've produced only two ranges.

Much more surprising, I would definitely expect that buildLocationList
would catch this, it's its entire purpose.

[...]

> I'm just interested in the cases where these representational quirks cost us debug info size.

Is it worth turning these cases into a `llvm-dwarfdump -verify` error?
(Or some kind of warning?). They look relatively straightforwards to
identify, aren't contributing to the debug experience, but are costing
space. I'm all for eliminating these quirks, it bothers users for
nothing.

(I'm not familiar with `llvm-dwarfdump -verfy`, perhaps this is more a
case of "debuginfo isn't helpful" as opposed to "the DWARF is wrong").

[0] https://reviews.llvm.org/D59431
[1] https://reviews.llvm.org/D61940?id=201204
[2] https://reviews.llvm.org/rL362750

--
Thanks,
Jeremy