[Lldb-commits] [PATCH] D18646: Fix DWO breakage in r264909

[Enrico Granata via lldb-commits]

> On Mar 31, 2016, at 11:15 AM, Zachary Turner via lldb-commits <lldb-commits at lists.llvm.org> wrote:
> 
> I think it depends on how bad the thing that happens is.  If you are out of memory, sure you can assert.  If a syscall fails that is supposed to never fail, you can assert. 

Admittedly, if you’re out of memory there’s not much forward progress you’re going to be able to make. But, in general, I’d argue that a library shouldn’t assert. The process that is using LLDB might actually know how to deal with an out-of-memory, and it’s not our job as a library to say otherwise.

lldbassert() is what you should be using - it will assert() in a debug build, which is what you want - but in a release build, it will print out some spiel about what happened, where it happened, and asking to file a bug against lldb - and then continue. Admittedly, you might crash somewhere down the road anyway, but that crash should be considered a bug.

> But if one piece of debug info appears corrupt, I don't think it's worth bringing down the whole process, which could be an entire IDE.  (FWIW yes I agree that having LLDB out of process would be an even better solution to this, but that's quite a large undertaking).  You could be debugging other processes using other debug info which is not corrupt, but you terminate all those sessions because one piece of unrelated debug info in an unrelated process is bad.  Doesn't seem right to me.
> 

+100

> debug info is program input, and you would never assert on program input, you have to be able to handle unreasonable values gracefully.
> 

+200

> On Thu, Mar 31, 2016 at 11:06 AM Jim Ingham via lldb-commits <lldb-commits at lists.llvm.org <mailto:lldb-commits at lists.llvm.org>> wrote:
> jingham added a subscriber: jingham.
> jingham added a comment.
> 
> I don't agree that asserts are good in released code unless you have no way of backing out of the situation you find yourself in.  After all, you are saying to some unlucky user out there that they can't use the debugger on their app and in general there's nothing they can do about it.  Greg's suggestion is for this low-level API to say "I couldn't find this DIE" and then if that's something higher layers can work around - by saying "Yeah I couldn't find that type" then you've allowed the user to continue their debug session instead of stopping them cold.
> 
> Not asserting prematurely is particularly important for handling debug information; since we don't control the compiler we need to handle as much junk information as gracefully as possible.
> 
> Also, asserts, especially for debug information, don't tend to be very helpful in the field.  You get a crash trace which really doesn't tell you the important stuff - what debug file was this, what DIE was bad, etc...  And given the nature of life, this error is going to occur for a user who can't give you their project to repro the bug and can't reduce it to a smaller test case.  Logs are pretty much all you have to go on.  So an un-annotated assert like this is not a good idea.
> 
> So orthogonal to the assert issue, if you find something not copacetic in the debug information, you should log out as much local information as you can regardless of what you are going to do with the error.
> 
> Jim
> 
> 
> Repository:
>   rL LLVM
> 
> http://reviews.llvm.org/D18646 <http://reviews.llvm.org/D18646>
> 
> 
> 
> _______________________________________________
> lldb-commits mailing list
> lldb-commits at lists.llvm.org <mailto:lldb-commits at lists.llvm.org>
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits <http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits>
> _______________________________________________
> lldb-commits mailing list
> lldb-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


Thanks,
- Enrico
📩 egranata@.com ☎️ 27683

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20160331/3d82d91f/attachment.html>