[PATCH] D43092: [DebugInfo] Prevent infinite recursion for malformed DWARF

[David Blaikie via llvm-commits]

Tolerate in the sense of not crash on weird user inputs (including
object/executable files as input to linkers, debuggers, etc) - I'm all for
that.

It doesn't seem unreasonable to ask for an example if there are examples
out there that are intending to be supported, though, which was the
motivation for the original change.

But, sure, I'm not massively invested in this if everyone else feels
otherwise - but I think it'd be useful to have a practical example before
supporting it to check that we're supporting whatever the example is and
that it's reasonable/sane.

- Dave

On Tue, May 1, 2018 at 8:08 AM <paul.robinson at sony.com> wrote:

> The Robustness Principle suggests we should tolerate whatever we
> reasonably can, even if we know that Clang itself would never produce such
> oddball DWARF.  We do have a long-term goal of having LLDB use LLVM's DWARF
> parser, and a debugger's parser needs to be *way* more tolerant than a
> compiler verification tool.  While I can't say the DWARF spec rigorously
> defines what a chain of abstract-origin and specification attributes means,
> intuitively it can be interpreted, so long as there aren't any cycles.
>
> --paulr
>
>
>
> I indeed did see a DIE that had a DW_AT_specification and pointed to
> another die that also had a DW_AT_specification. Regardless of what the
> DWARF spec says, compilers or post production DWARF tools are are producing
> DWARF that has these multiple chaining of specifications or abstract
> origins. There is also no enforcement of the DWARF spec anywhere, so
> compilers produce a wide variety of different DWARF that may or may not be
> legal. We have "llvm-dwarfdump --verify" but it isn't exhaustive in what it
> reports. So I would vote that we have a DWARF parser that can handle what
> ever is thrown at it when possible. Just my 2 cents.
>
>
> I'm not suggesting the DWARF parser should crash or be unhelpful on such
> inputs - but the convenience of following abstract origin/specification DIE
> references to print names seems OK if it's limited to the situations we
> know make sense until we see something else?
>
> So if we could see/figure out what producer is generating such DWARF and
> look an some examples, decide if it's meaningful/useful to support name
> printing in that case, I'd be all for it. Without that, I'd rather err on
> the side of simplicity and not support the recursive case (& then have to
> handle/defend against cycles, etc).
>
>
>
> We have many tools here at Facebook that use LLVM's DWARF parser to
> symbolicate information and rely on the getting the names correctly from
> any producer of DWARF.
>
>
> "correctly" is what I'm wondering about - without seeing the example DWARF
> it's hard to know if this is the correct answer or not.
>
>
> I can't tell you exactly which compilers generate DWARF with multiple
> indirections, but I do know it happens. There are also many post production
> tools that we have here and take an existing linked binary and produce a
> new binary and fix the DWARF up. So in these cases, it isn't the compiler
> itself that ends up vending the final DWARF. So even if we identify the
> compiler that might produce this DWARF, there is no guarantee that the
> DWARF won't get changed. Granted, none of these tools will change
> abbreviation declarations that I know of.
>
>
>
> That being said I would be fine not dealing with recursion as the case
> that I fixed didn't have recursion. And if the DWARF is invalid I don't
> know how much we can/should do to try and recover.
>
>
> Sorry, when I say "recursion" I mean "more than one specification or more
> than one abstract_origin in the chain" - which I think is what your patch
> that switched to recursion was implementing, right?
>
> I'd like to see an example of that, to know what it is, why it exists/what
> it means & so we can know why we're supporting it, before we do.
>
> Yeah, even if it's a chain of weird tools - if it's producing dwarf that's
> not really meaningful (perhaps the tools are buggy, etc) then it's probably
> best not to add support for it in Clang, for example.
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180501/23883ff1/attachment.html>