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

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil added inline comments.


================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.cpp:263
   uint64_t length = data.GetU32(&offset);
-  bool isDwarf64 = (length == 0xffffffff);
-  if (isDwarf64)
----------------
zturner wrote:
> jankratochvil wrote:
> > Here should be an error that DWARF64 is not supported.
> How do we report that error?  This function doesn't return an error code, and neither does the function that calls it (I didn't look higher than that), and we don't build with exceptions.  We can assert, but we try to avoid asserts (although admittedly, we're going to crash anyway).  I can try to do more work to propagate errors up the stack, but at this point this whole file (and even most of the folder that the file is in) is in theory going to be going away soon as we converge on LLVM's DWARF parser, so it may make sense to just deal with the problem at that level.
Just that there should be something, at least a FIXME comment.
There is `dwarf2Data->GetObjectFile()->GetModule()->ReportError()` but it will currently lock up since implementation of multithreaded DWARF reading. But there are AFAIK already many such errors which do not happen in realworld but they would lock up if they did. I agree returning an error value is the proper solution, I have no idea when it is the right time to implement that.



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

https://reviews.llvm.org/D59235