[PATCH] D38719: [llvm-dwarfdump] Verify compatible TAG for attributes.

[David Blaikie via llvm-commits]

On Tue, Sep 21, 2021 at 1:58 PM Adrian Prantl <aprantl at apple.com> wrote:

>
>
> On Sep 21, 2021, at 1:11 PM, David Blaikie <dblaikie at gmail.com> wrote:
>
> Ping
>
> On Thu, Aug 12, 2021 at 7:42 PM David Blaikie <dblaikie at gmail.com> wrote:
>
>> On Wed, Oct 11, 2017 at 10:03 AM Adrian Prantl <aprantl at apple.com> wrote:
>>
>>>
>>>
>>> > On Oct 11, 2017, at 9:45 AM, David Blaikie <dblaikie at gmail.com> wrote:
>>> >
>>> > Adrian - could you take a look here & see why
>>> DWARFVerifier::handleDebugInfo is creating empty TUSection/CUSection,
>>> rather than passing the DCtx's populated sections in? (that's my rough
>>> reading of the code, at least)
>>>
>>> It looks like this was introduced by https://reviews.llvm.org/D35521
>>>
>>> "This patch modifies the handleDebugInfo() function so that we verify
>>> the contents of each unit
>>> in the .debug_info section only if its header has been successfully
>>> verified."
>>>
>>> So it looks like the code is trying to avoid parsing the unit if the
>>> header didn't verify? At the first glance this does look bit redundant to
>>> me but I didn't give it a thorough reading.
>>>
>>> Jonas, can you make sense of it in the context of that commit? I would
>>> not be surprised if this is just compensating for a missing feature in the
>>> codebase at that point in time.
>>>
>>> If we can get away with deleting the redundant code that would
>>> definitely be a win.
>>>
>>
>> Resurrecting this thread, since I just came across this issue again -
>> while trying to improve the verifier (after seeing a lot of "error:
>> invalid DIE reference 0x0000015f. Offset is in between DIEs:" followed
>> by  a series of blank lines - caused by
>> ReferenceToDIEOffsets being kept and used over multiple sections (when
>> it contains section relative offsets without any base section to know which
>> section they are relative to) - so you get invalid references from type
>> units, and then verifyDebugInfoReferences tries to dump the DIEs in the
>> type units that have the "invalid" references and it can't dump those DIEs
>> (just ends up with spooky blank lines) because it's looking in the wrong
>> section for them).
>>
>
> I read through https://reviews.llvm.org/D35521 but it wasn't enough for
> me to understand the problem. Maybe you can help me clarify what's going
> on. For what you are saying it sounds like the Verifier code was written
> without regard for type units, and so if we have a verifier error in a type
> unit verifyDebugInfoReferences returns incorrect results
> because ReferenceToDIEOffsets has no concept of which unit a relative
> offset should be resolved in, but I'm not sure if that's accurate. What
> situations are currently not handled correctly?
>

Thanks for taking a look, sorry it's taken me a bit to come back to this.
I'm trying to page the state back in now..

D35521 changed handleDebugInfo - it previously used `for (const auto &CU :
DCtx.compile_units()) {`, which would populate the DCtx's CU/TUSection
members - but after the change that's no longer used, and DWARFUnit's are
explicitly constructed like this:
        DWARFUnitSection<DWARFCompileUnit> CUSection{};
        Unit.reset(new DWARFCompileUnit(
            DCtx, DCtx.getInfoSection(), DCtx.getDebugAbbrev(),
            &DCtx.getRangeSection(), DCtx.getStringSection(),
            DCtx.getStringOffsetSection(), &DCtx.getAppleObjCSection(),
            DCtx.getLineSection(), DCtx.isLittleEndian(), false, CUSection,
            nullptr));
Which means the DCtxt's CU/TUSection members will be empty (so attempting
to resolve a cross-CU reference in the verifier will break, I think?) and
the UnitVector reference member of DWARFUnit will be left dangling (because
the CUSection (similarly for the TUSection code nearby) variable above goes
out of scope immediately after the construction of the unit).

Does that make sense/accurately describe a/the problem?

- Dave
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20211004/c265d098/attachment.html>