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

[via llvm-commits]

I think it's worth repeating a point that dblaikie made a little while back, which is that we want to allow for a DIE with DW_AT_abstract_origin that points to a DIE that in turn has DW_AT_specification.  The recursive implementation was an implementation convenience, not a statement that arbitrary-length mix-n-match chains are legal.  (If it was such a statement, I disagree with it.)  I don't think I can come up with a case where DW_AT_specification would meaningfully point to a DIE with DW_AT_abstract_origin, or where you'd want chains of DIEs with these attributes.
--paulr

From: David Blaikie [mailto:dblaikie at gmail.com]
Sent: Tuesday, February 13, 2018 11:48 AM
To: Jonas Devlieghere
Cc: Adrian Prantl; reviews+D43092+public+94abba730c8ff6fa at reviews.llvm.org; Robinson, Paul; clayborg at gmail.com; LLVM Commits; hiraditya at msn.com; djordje.todorovic at rt-rk.com; Davis, Matthew; ewan at codeplay.com
Subject: Re: [PATCH] D43092: [DebugInfo] Prevent infinite recursion for malformed DWARF

2.13.2 says:

"A debugging information entry that represents a declaration that completes another (earlier) non-defining declaration may have a DW_AT_specification attribute whose value is a reference to the debugging information entry representing the non-defining declaration"

Seems pretty clear that there's only one defining declaration and that DW_AT_specification is for referencing from that defining declaration to the non-defining declaration.

And the wording around abstract origins I think is pretty clear too - that there's a concrete definition, an abstract definition, and any number of inline definitions.

But as always, real world situations somewhat trump any of this, imho - so if there's a compelling use case/existing producer that produces some interesting DWARF related to declarations/definitions specifications/abstract_origins, it'd be great to see to better understand what would be useful to support (& to document why it's being supported/what to look for when we're evaluating how that support should work/be maintained in the future)

On Tue, Feb 13, 2018 at 11:42 AM Jonas Devlieghere <jdevlieghere at apple.com<mailto:jdevlieghere at apple.com>> wrote:
Is there anything in the DWARF standard that would suggest this shouldn’t be possible? If not I think we should support it as is and implement the more advanced loop detection?



On Feb 9, 2018, at 3:53 PM, David Blaikie <dblaikie at gmail.com<mailto:dblaikie at gmail.com>> wrote:

*nod* Thought so - would be good to know what scenario this is meant to support.
On Fri, Feb 9, 2018 at 2:22 AM Jonas Devlieghere via Phabricator <reviews at reviews.llvm.org<mailto:reviews at reviews.llvm.org>> wrote:
JDevlieghere added a subscriber: clayborg.
JDevlieghere added a comment.

In https://reviews.llvm.org/D43092#1002677, @dblaikie wrote:

> Could you check the revision history here? I'm pretty sure the first version of this I reviewed from Greg wasn't recursive - and then it became recursive at some point to handle something needed, but maybe those decisions need to be reexamined?


It was @clayborg himself that updated the implementation. (https://reviews.llvm.org/D40156 https://reviews.llvm.org/rL319104)

> The previous implementation would only look 1 DW_AT_specification or DW_AT_abstract_origin deep. This means DWARFDie::getName() would fail in certain cases. I ran into such a case while creating a tool that used the LLVM DWARF parser to generate a symbolication format so I have seen this in the wild.


https://reviews.llvm.org/D43092



-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180213/9b52f55e/attachment.html>