<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Sep 8, 2017, at 11:45 PM, Zachary Turner <<a href="mailto:zturner@google.com" class="">zturner@google.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div class=""><br class=""><div class="gmail_quote"><div dir="auto" class="">On Fri, Sep 8, 2017 at 8:19 PM Jason Molenda <<a href="mailto:jmolenda@apple.com" class="">jmolenda@apple.com</a>> wrote:<br class=""></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Also keep in mind that debug sessions have a tendency to be long lived. I may be working through a problem for half an hour -- or this may be the one rare instance where a bug reproduces -- and crashing because some bogus piece of debug info that I don't care about is invalid is not acceptable. I've gotten those bug reports, where someone has spent all day getting to just the point where they have the problem in front of them, only to have the debugger crash, and they are rightfully enraged at the debugger for this.<br class="">
<br class="">
An interactive tool like the debugger cannot use a model where asserting is acceptable. Even if lldb is out-of-process, you're losing all of the work you'd done until then, and complex bugs can involve extraordinarily long running debug sessions. The fact that llvm/clang can assert out from under us is to the detriment of lldb's quality.<br class="">
<br class="">
J</blockquote><div dir="auto" class="">Fwiw i think we all agree that it shouldn't crash. What we disagree on is the best way of making it crash less. It's easy to see why someone would argue that adding an assert increases the incidence of crashes. I mean on the surface it seems ridiculous to argue otherwise. </div><div dir="auto" class=""><br class=""></div><div dir="auto" class="">But otherwise is precisely what we all continue to argue.</div><div dir="auto" class=""><br class=""></div><div dir="auto" class="">Your position is that by going out of your way to avoid crashes, it will crash less and be more stable. Pretty intuitive position if I'm being honest. But I don't agree. </div><div dir="auto" class=""><br class=""></div><div dir="auto" class="">I think you are trading short term metrics for long term technical debt</div><div dir="auto" class="">You fix one crash by adding a null check, without figuring out why null ever got in there in the first place. </div><div dir="auto" class=""><br class=""></div><div dir="auto" class="">You get fewer bug reports because instead of crashing, which would hopefully trigger some automatic crash uploader in Xcode that automatically files a rdar, you just never find out about the bug in the first place.</div><div dir="auto" class=""><br class=""></div><div dir="auto" class="">You introduce even more bugs, because when you go to edit a function, its complexity and branching is through the roof, and you overlook something some corner case.</div><div dir="auto" class=""><br class=""></div><div dir="auto" class="">You have less test coverage because you introduce another untested branch in the code, reducing the already abysmal code coverage even further.</div><div dir="auto" class=""><br class=""></div><div dir="auto" class="">The way to reduce crashing is to *increase* the code coverage of your test suite. That is the solution. And you don't do that by adding null checks everywhere, you do it by removing them and asserting.</div><div dir="auto" class=""><br class=""></div><div dir="auto" class="">Yes, it means Xcode might crash. But you know the bug exists! How many bugs are out there right now that nobody even knows about because they are hidden by a null pointer check and instead of the user trying to figure out what's going and filing a bug report, they just give up and use gdb instead. That's one more person you're never going to get back.</div><div dir="auto" class=""><br class=""></div><div dir="auto" class="">Crashing Xcode is annoying, but it's fixable. But when your technical debt is going up and to the right, and your test coverage is going down and to the right, that's an existential threat to the project .</div><div dir="auto" class=""><br class=""></div><div dir="auto" class="">Btw, i still don't understand why asserts cause anything to crash, given that they're supposed to be turned off in a release build</div></div></div>
</div></blockquote></div><br class=""><div class=""> I was objecting to the use of llvm_report_fatal_error. That crashes no matter what. And I was suggesting replacing that with an lldb_assert. So I’m not sure we are so much in disagreement about that.</div><div class=""><br class=""></div><div class="">I was also objecting to the approach where instead of figuring out why the errors from one call - DoResmume - were not getting handled properly - we add ANOTHER earlier check - CanResume - and then we have to deal with what happens when there’s a code path that doesn’t get caught by the added check. That is just adding weakness, and a chance that some obscure programmer error gets us confused.</div><div class=""><br class=""></div><div class="">I also am skeptical about the "just assert if things aren’t quite right" approach, because it removes the responsibility for thinking about how you would get to that state and how you would get out of it safely. But more crucially, it exaggerates the importance of subsections of lldb’s functionality. For instance, to the JIT, being asked to handle a relocation that it doesn’t support is a fatal error, so it seems okay by the fail soon lights to just fatal error. But to lldb no expression is important enough to quit if it doesn’t work, so we would have really very much preferred if the JIT had been forced to back out from the error. Ditto with llvm & clang and handling types. Even if the types from one CU are horked up beyond belief, there’s lots of useful debugging you can still do.</div><div class=""><br class=""></div><div class="">In areas like this we generally put in lldb asserts and then ALSO try to handle the error as best we can. If you trigger the assert running the test suite, great, you have a way to figure out how not to do that. Finding the bug in the field is getting somebody else to pay for work we should be doing. And either the bug was going to cause a crash anyway, in which case we can work with the reporter - generally getting a repo case or getting some logging etc. - to figure out what went on. Or it doesn’t crash, we get some one thing wrong, or maybe can’t run an expression, but the user gets to continue their debugging session. We work on lldb for the satisfaction of its users, not us. So if keeping going whenever possible makes our job harder, but makes lldb more useful to the folks using it, then that seems the me an acceptable tradeoff.</div><div class=""><br class=""></div><div class="">It also seems like you are suggesting some conflict between adding more testing and trying to handle errors rather than exiting when they arise. There’s really no linkage between the two so far as I can see.</div><div class=""><br class=""></div><div class="">Jim</div><div class=""><br class=""></div><div class=""><br class=""></div><div class=""><br class=""></div></body></html>