[PATCH] D38719: [llvm-dwarfdump] Verify compatible TAG for attributes.
David Blaikie via llvm-commits
llvm-commits at lists.llvm.org
Tue Oct 10 12:17:29 PDT 2017
On Tue, Oct 10, 2017 at 12:06 PM Jonas Devlieghere <jdevlieghere at apple.com>
wrote:
>
> On Oct 10, 2017, at 6:24 PM, David Blaikie <dblaikie at gmail.com> wrote:
>
>
>
> On Tue, Oct 10, 2017 at 10:06 AM Jonas Devlieghere via Phabricator <
> reviews at reviews.llvm.org> wrote:
>
>> JDevlieghere marked 2 inline comments as done.
>> JDevlieghere added inline comments.
>>
>>
>> ================
>> Comment at: lib/DebugInfo/DWARF/DWARFVerifier.cpp:423
>> + // Make sure specification and abstract origin have compatible tag.
>> + if (auto Offset = AttrValue.Value.getAsReference()) {
>> +
>> ----------------
>> dblaikie wrote:
>> > Can you use DWARFDie::getAttributeValueAsReferencedDie here, perhaps?
>> Unfortunately not, because this makes use of `getUnitForOffset` which in
>> turn uses a list of units that isn't build when doing the verification.
>>
>
> Not sure I understand - this code calls getDIEForOffset, which calls
> getUnitForOffset as well. So why would one path be usable and the other not?
>
>
> I would have to double check with the code in front of me, but it uses an
> upper_bound to find the right unit based on the offset, and this code path
> doesn’t populate that list. I noticed this earlier when I was trying to
> understand why the name of the subroutine is shown when dumping the debug
> info but not when running the verifier. I think this is the result of only
> considering one unit at a time when verifying, but I didn’t dig much deeper
> once I had found the explanation.
>
Yeah, I'm really not understanding the difference between these possible
codepaths.
The code in this review calls:
auto Offset = AttrValue.Value.getAsReference()
DCtx.getDIEForOffset(*Offset)
->CUs.getUnitForOffset(*Offset)->getDIEForOffset(*Offset);
the code in DWARFDie::getAttributeValueAsReferencedDie calls:
auto SpecRef = toReference(find(Attr));
U->getUnitSection().getUnitForOffset(*SpecRef)->getDIEForOffset(*SpecRef);
(where 'getUnitSection' returns DWARFUnitSectionBase, and 'CUs' is a
DWARFUnitSectionBase (well, a class derived from it,
DWARFUnitSection<DWARFCompileUnit>)
So I'm really having a hard time understanding why these codepaths would be
different & it seems subtle/worth fixing if they are.
>
>
>
>> Furthermore, this is (probably) slightly more efficient as we already
>> have the attribute value and therefore do not need to do any indirections
>> via the abbrev declaration.
>>
>>
>> https://reviews.llvm.org/D38719
>>
>>
>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171010/34a67479/attachment.html>
More information about the llvm-commits
mailing list