[llvm] r369529 - [DWARF] Adjust return type of DWARFUnit::getLength().

Fri Sep 6 07:43:23 PDT 2019

No, it would not. As long as DWARFUnitHeader::extract() returns 'false', DWARFUnitVector::addUnitsImpl() does not create the classes to represent the unit.

Best Regards,
Igor Kudrin
C++ Developer, Access Softek, Inc.

________________________________________
From: paul.robinson at sony.com <paul.robinson at sony.com>
Sent: Friday, September 6, 2019 21:14
To: dblaikie at gmail.com
Cc: Igor Kudrin; llvm-commits at lists.llvm.org
Subject: RE: [llvm] r369529 - [DWARF] Adjust return type of DWARFUnit::getLength().

> -----Original Message-----
> From: David Blaikie [mailto:dblaikie at gmail.com]
> Sent: Thursday, September 05, 2019 6:23 PM
> To: Robinson, Paul
> Cc: Igor Kudrin; llvm-commits
> Subject: Re: [llvm] r369529 - [DWARF] Adjust return type of
> DWARFUnit::getLength().
>
> Yeah, I'd be OK with a unit test, I think.
>
> Though might be able to be lit-tested too. Building a test from
> assembly which could specify a huge nop or zero bytes to pad out the
> section - though that'd require a lot of disk space to run the test,
> which might be problematic.

Hm.  Disk space was my concern.  But, if we had an assembler test
along the lines of dwarfdump-header-64.s, but just gave the header a
bogus length, would llvm-dwarfdump still dump the header? (GNU readelf
does...) that would be enough to demonstrate this patch works.

# DWARF-64 v5 normal CU header, with bogus length to show printing
# the 64-bit fields works correctly.
        .long -1
Lset64 = CU64_5_start-CU64_5_end
        .long Lset64
CU64_5_start:
        .long 1 # make the length wider than 32 bits
        .short 5 # version
        .byte 1 # unit type
        .byte 8 # address size
        .quad abbrev    # offset into .debug_abbrev
CU64_5_end:
# no DIE

Worth a shot.
--paulr

>
> On Thu, Sep 5, 2019 at 11:45 AM <paul.robinson at sony.com> wrote:
> >
> > I said much the same thing in my review.  Although it occurs to me now,
> perhaps a unittest?  Conjure up a correctly formed unit header with a 64-
> bit length (that is >32-bits wide), but actually has no DIEs in it; call
> the parser (which will fail, but I think still fill in the header info),
> then call dump() with a raw_ostream pointing to a local buffer; then you
> can validate the content of the buffer, which should have the 64-bit
> length in it.
> >
> >
> >
> > Worth a shot.
> >
> > --paulr
> >
> >
> >
> > From: llvm-commits [mailto:llvm-commits-bounces at lists.llvm.org] On
> Behalf Of Igor Kudrin via llvm-commits
> > Sent: Wednesday, September 04, 2019 9:49 PM
> > To: David Blaikie
> > Cc: llvm-commits
> > Subject: Re: [llvm] r369529 - [DWARF] Adjust return type of
> DWARFUnit::getLength().
> >
> >
> >
> > I agree with you. I tried but could not devise a sample for a positive
> scenario which would not be enormously big. Any suggestions?
> >
> >
> >
> > Best Regards,
> >
> > Igor Kudrin
> >
> > C++ Developer, Access Softek, Inc.
> >
> > ________________________________
> >
> > From: David Blaikie <dblaikie at gmail.com>
> > Sent: Thursday, September 5, 2019 07:55
> > To: Igor Kudrin
> > Cc: llvm-commits
> > Subject: Re: [llvm] r369529 - [DWARF] Adjust return type of
> DWARFUnit::getLength().
> >
> >
> >
> > CAUTION: This email originated from outside of the organization. Do not
> click links or open attachments unless you recognize the sender and know
> the content is safe.  If you suspect potential phishing or spam email,
> report it to ReportSpam at accesssoftek.com
> >
> > Ideally test cases would be good to cover this functionality and
> demonstrate the length is no longer truncated.
> >
> >
> >
> > On Wed, Aug 21, 2019 at 7:09 AM Igor Kudrin via llvm-commits <llvm-
> commits at lists.llvm.org> wrote:
> >
> > Author: ikudrin
> > Date: Wed Aug 21 07:10:57 2019
> > New Revision: 369529
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=369529&view=rev
> > Log:
> > [DWARF] Adjust return type of DWARFUnit::getLength().
> >
> > DWARFUnitHeader::getLength() returns uint64_t.
> > DWARFUnit::getLength() should do the same.
> >
> > Differential Revision: https://reviews.llvm.org/D66472
> >
> > Modified:
> >     llvm/trunk/include/llvm/DebugInfo/DWARF/DWARFUnit.h
> >     llvm/trunk/lib/DebugInfo/DWARF/DWARFCompileUnit.cpp
> >     llvm/trunk/lib/DebugInfo/DWARF/DWARFTypeUnit.cpp
> >
> > Modified: llvm/trunk/include/llvm/DebugInfo/DWARF/DWARFUnit.h
> > URL: http://llvm.org/viewvc/llvm-
> project/llvm/trunk/include/llvm/DebugInfo/DWARF/DWARFUnit.h?rev=369529&r1=
> 369528&r2=369529&view=diff
> >
> ==========================================================================
> ====
> > --- llvm/trunk/include/llvm/DebugInfo/DWARF/DWARFUnit.h (original)
> > +++ llvm/trunk/include/llvm/DebugInfo/DWARF/DWARFUnit.h Wed Aug 21
> 07:10:57 2019
> > @@ -286,7 +286,7 @@ public:
> >    uint8_t getDwarfOffsetByteSize() const {
> >      return Header.getDwarfOffsetByteSize();
> >    }
> > -  uint32_t getLength() const { return Header.getLength(); }
> > +  uint64_t getLength() const { return Header.getLength(); }
> >    uint8_t getUnitType() const { return Header.getUnitType(); }
> >    bool isTypeUnit() const { return Header.isTypeUnit(); }
> >    uint64_t getNextUnitOffset() const { return
> Header.getNextUnitOffset(); }
> >
> > Modified: llvm/trunk/lib/DebugInfo/DWARF/DWARFCompileUnit.cpp
> > URL: http://llvm.org/viewvc/llvm-
> project/llvm/trunk/lib/DebugInfo/DWARF/DWARFCompileUnit.cpp?rev=369529&r1=
> 369528&r2=369529&view=diff
> >
> ==========================================================================
> ====
> > --- llvm/trunk/lib/DebugInfo/DWARF/DWARFCompileUnit.cpp (original)
> > +++ llvm/trunk/lib/DebugInfo/DWARF/DWARFCompileUnit.cpp Wed Aug 21
> 07:10:57 2019
> > @@ -16,7 +16,7 @@ using namespace llvm;
> >
> >  void DWARFCompileUnit::dump(raw_ostream &OS, DIDumpOptions DumpOpts) {
> >    OS << format("0x%08" PRIx64, getOffset()) << ": Compile Unit:"
> > -     << " length = " << format("0x%08x", getLength())
> > +     << " length = " << format("0x%08" PRIx64, getLength())
> >       << " version = " << format("0x%04x", getVersion());
> >    if (getVersion() >= 5)
> >      OS << " unit_type = " << dwarf::UnitTypeString(getUnitType());
> >
> > Modified: llvm/trunk/lib/DebugInfo/DWARF/DWARFTypeUnit.cpp
> > URL: http://llvm.org/viewvc/llvm-
> project/llvm/trunk/lib/DebugInfo/DWARF/DWARFTypeUnit.cpp?rev=369529&r1=369
> 528&r2=369529&view=diff
> >
> ==========================================================================
> ====
> > --- llvm/trunk/lib/DebugInfo/DWARF/DWARFTypeUnit.cpp (original)
> > +++ llvm/trunk/lib/DebugInfo/DWARF/DWARFTypeUnit.cpp Wed Aug 21 07:10:57
> 2019
> > @@ -24,12 +24,12 @@ void DWARFTypeUnit::dump(raw_ostream &OS
> >    if (DumpOpts.SummarizeTypes) {
> >      OS << "name = '" << Name << "'"
> >         << " type_signature = " << format("0x%016" PRIx64,
> getTypeHash())
> > -       << " length = " << format("0x%08x", getLength()) << '\n';
> > +       << " length = " << format("0x%08" PRIx64, getLength()) << '\n';
> >      return;
> >    }
> >
> >    OS << format("0x%08" PRIx64, getOffset()) << ": Type Unit:"
> > -     << " length = " << format("0x%08x", getLength())
> > +     << " length = " << format("0x%08" PRIx64, getLength())
> >       << " version = " << format("0x%04x", getVersion());
> >    if (getVersion() >= 5)
> >      OS << " unit_type = " << dwarf::UnitTypeString(getUnitType());
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at lists.llvm.org
> > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits