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

[Zachary Turner via lldb-commits]

+1. I’ve seen some of the changes in llvm that have changed asserts to
return a default value. IMHO these should be changed to return Expected<T>
instead
On Tue, Mar 12, 2019 at 1:16 PM Jonas Devlieghere via Phabricator <
reviews at reviews.llvm.org> wrote:

> 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
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20190312/c3e7ab07/attachment.html>