[Lldb-commits] [PATCH] D35223: Report inferior SIGSEGV/SIGILL/SIGBUS/SIGFPE as a signal instead of an exception on freebsd

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In https://reviews.llvm.org/D35223#834050, @emaste wrote:

> With this patch I observed three new failures on FreeBSD and three new unexpected passes on FreeBSD. An example of a new failure:
>
>   ======================================================================
>   FAIL: test_inferior_crashing_expr_step_and_expr_dwarf (TestInferiorCrashing.CrashingInferiorTestCase)
>      Test that lldb expressions work before and after stepping after a crash.
>   ----------------------------------------------------------------------
>   Traceback (most recent call last):
>     File "/tank/emaste/src/llvm/tools/lldb/packages/Python/lldbsuite/test/lldbtest.py", line 1732, in dwarf_test_method
>       return attrvalue(self)
>     File "/tank/emaste/src/llvm/tools/lldb/packages/Python/lldbsuite/test/decorators.py", line 110, in wrapper
>       func(*args, **kwargs)
>     File "/tank/emaste/src/llvm/tools/lldb/packages/Python/lldbsuite/test/functionalities/inferior-crashing/TestInferiorCrashing.py", line 82, in test_inferior_crashing_expr_step_and_expr
>       self.inferior_crashing_expr_step_expr()
>     File "/tank/emaste/src/llvm/tools/lldb/packages/Python/lldbsuite/test/functionalities/inferior-crashing/TestInferiorCrashing.py", line 249, in inferior_crashing_expr_step_expr
>       self.check_stop_reason()
>     File "/tank/emaste/src/llvm/tools/lldb/packages/Python/lldbsuite/test/functionalities/inferior-crashing/TestInferiorCrashing.py", line 93, in check_stop_reason
>       STOPPED_DUE_TO_EXC_BAD_ACCESS)
>   AssertionError: 0 != 1 : Process should be stopped due to bad access exception
>   Config=x86_64-/usr/bin/cc
>   ----------------------------------------------------------------------
>   Ran 7 tests in 4.320s
>
>
> From lldbsuite/test/lldbutil.py:
>
>   def is_thread_crashed(test, thread):
>       """In the test suite we dereference a null pointer to simulate a crash. The way this is
>       reported depends on the platform."""
>       if test.platformIsDarwin():
>           return thread.GetStopReason(
>           ) == lldb.eStopReasonException and "EXC_BAD_ACCESS" in thread.GetStopDescription(100)
>       elif test.getPlatform() == "linux":
>           return thread.GetStopReason() == lldb.eStopReasonSignal and thread.GetStopReasonDataAtIndex(
>               0) == thread.GetProcess().GetUnixSignals().GetSignalNumberFromName("SIGSEGV")
>       else:
>           return "invalid address" in thread.GetStopDescription(100)
>
>
> Presumably we want the second case to apply on FreeBSD as well when this patch is in, although I don't see why the existing else case shouldn't continue to work correctly.


Actually, I think you probably need to extend the `@skipIfLinux` to apply to freebsd as well. The reason is that the test is doing something which goes fundamentally against this patch -- it expects that "step" after a "crash" is a no-op, whereas the new behavior will be to let the process continue (and probably exit, unless it has a SEGV handler). That behavior may make sense for mach exceptions, but I don't think we should try to make posix signals emulate that behavior.


Repository:
  rL LLVM

https://reviews.llvm.org/D35223