[Lldb-commits] [PATCH] D67776: Use UnixSignals::ShouldSuppress to filter out signals a process expects

[Jim Ingham via Phabricator via lldb-commits]

jingham requested changes to this revision.
jingham added a comment.
This revision now requires changes to proceed.

This is all about the decision in "batch" mode for what constitutes a stop by the program that should cause batch mode to return the control to the user.

I don't think anybody would want to stop for SIGSTOP and SIGINT if the program under the debugger can recover.

However, __builtin_trap on most UNIX systems results in a SIGTRAP.  It's not at all uncommon to use __builtin_trap to signal a fatal condition in a library (for instance Swift uses this on array overflow and for a number of other error conditions.)  It has the benefit over "abort" that it doesn't push a bunch of stack frames onto the stack before dying, which makes crash analysis easier.  This use means we certainly want batch mode to stop for SIGTRAP.

If the SIGTRAP is for a breakpoint, lldb will convert the incoming signal event into a breakpoint-hit event, which doesn't cause the batch mode to stop.

So maybe we don't actually need to have "should suppress" be true for SIGTRAP.  We should be able to distinguish between traps that were really in the code of the binary and traps lldb inserted, and make the decision based on that.  That was just copied from gdb, which kept the breakpoint SIGTRAP's raw.

I can't tell how real breakpoint SIGTRAPs work through lldb because on MacOS because don't come in as SIGTRAP's (they come in as Mach exceptions).  It would be worth seeing how this works for Linux hosts.

But in any case, a SIGTRAP that doesn't have an underlying breakpoint should definitely cause batch mode to stop.  Under what circumstances did you need to suppress it?



================
Comment at: source/Interpreter/CommandInterpreter.cpp:2164
+    StopReason reason = stop_info->GetStopReason();
+    if (eStopReasonException == reason || eStopReasonInstrumentation == reason)
+      return true;
----------------
tatyana-krasnukha wrote:
> JDevlieghere wrote:
> > I'm curious why you swapped the two operands. Is there a benefit in `eStopReasonException == reason` compared to `reason == eStopReasonException`? 
> This technique prevents unintended assignment instead of comparison since left-hand value is constant. 
We don't use that convention anywhere else in lldb.  Introducing it piecemeal just makes the code harder to read, and I personally find this convention makes code generally harder to parse in my head.  


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D67776