[Lldb-commits] [PATCH] D68096: ProcessMinidump: inject SIGSTOP on Linux if no thread has a signal

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Thanks for cleaning this up, and for sending the minidump. Looking at the dump, I am pretty sure that the problem is the lack of proper exception stream support in yaml2obj. The stream contains a reference to a thread context, and since yaml2obj does not understand this, it copies the offsets verbatim, and so they end up pointing to random garbage after the yaml round trip.

It doesn't look like it should be too hard to add yaml support for the exceptions stream -- it should only be a matter of adapting the patterns already used for other stream to work for this particular case. Could you give a go at that? Alternatively, you can wait a bit until I finish with the MemoryInfoList stream. After that, I can squeeze some time to do the Exception stream too.



================
Comment at: lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp:219
+
+  if (arch.GetTriple().isOSLinux())
+    SetUnixSignals(UnixSignals::Create(GetArchitecture()));
----------------
I think you can create this unconditionally.


================
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");
+    }
----------------
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?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68096