[Lldb-commits] [PATCH] D59235: Remove Support for DWARF64

Tue Mar 12 13:16:50 PDT 2019

JDevlieghere added a comment.

In D59235#1426169 <https://reviews.llvm.org/D59235#1426169>, @probinson wrote:

> In D59235#1425443 <https://reviews.llvm.org/D59235#1425443>, @zturner wrote:
>
> > In D59235#1425436 <https://reviews.llvm.org/D59235#1425436>, @clayborg wrote:
> >
> > > My main concern with the LLVM DWARF parser is all of the asserts in the code. If you attempt to use a DWARFDIE without first checking it for validity, it will crash on you instead of returning a good error or default value. That makes me really nervous as we shouldn't just crash the debugger. The switching over won't be too hard, just the fallout from the LLDB versions of the class that do error checking and return good error/default values and LLVM being very strict.
> >
> >
> > Sure, I'm prepared to deal all that appropriately.  I don't plan to regress LLDB's stability in the process.
> >
> > That's why for now I'm just doing very small preliminary steps to get the two interfaces to be closer to each other and simplify the problem space.  We can worry about the asserts and all of that when we actually start moving pieces of LLDB to use LLVM's classes (which isn't in this patch).
>
>
> A long term plan of moving LLVM's parser away from asserts and toward error reporting on bad input would also make the binutils that try to read DWARF more robust and useful for trying to diagnose bad object files.  I'm all for it.

Agreed, and we've been doing this for new patches for a while now. However, I very strongly prefer having asserts over "returning a default value", which only hides real bugs.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59235/new/

https://reviews.llvm.org/D59235