[Lldb-commits] [PATCH] D68096: ProcessMinidump: Suppress reporting stop for signal '0'

[Pavel Labath via Phabricator via lldb-commits]

labath marked an inline comment as done.
labath added inline comments.


================
Comment at: lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp:258-265
+    uint32_t signo = m_active_exception->exception_record.exception_code;
+
+    if (signo == 0) {
+      // Artifically inject a SIGSTOP so that we'll find a stop reason
+      // when we process the stop event.
+      signo = GetUnixSignals()->GetSignalNumberFromName("SIGSTOP");
+    }
----------------
JosephTremoulet wrote:
> labath wrote:
> > After some investigation, I re-learned about the DumpRequested constant, which is already checked for in this code (and it is tested (TestMiniDumpNew.py:test_snapshow_minidump) and works/does not hang). It seems to me like it would make sense to treat this case the same way as DumpRequested, and just don't return any stop info (instead of returning a stop info with signal 0 or SIGSTOP). IOW, you would just put `if (signo==0) return;` here. Eventually we could apply the same change to process elf core as well..
> > 
> > WDYT?
> SGTM, and seems to work for my use case.  I wasn't sure though whether to leave the `if (signo==0) return` under `if (arch.GetTripe().getOS() == llvm::Triple::Linux)`.  Originally I'd put logic here just because I was copying logic from Process**Elf**Core.  It looks like the Apple case here would maybe similarly have an issue with exception code zero (?), but the `else` case (Windows?) doesn't reference the exception_code at all so I'd hesitate to apply this change there... I've left it under the check for Linux, but happy to move it if that seems wrong.
Yeah, the exception code seems to be used completely differently on other OSs, so making this linux specific seems like the right choice.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68096