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

[David Blaikie via llvm-commits]

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.

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=369528&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