[Lldb-commits] [PATCH] D68096: Add Linux signal support to ProcessMinidump

Fri Sep 27 08:47:12 PDT 2019

JosephTremoulet marked an inline comment as done.
JosephTremoulet added a comment.

> Fortunately, for the functionality you're testing, I don't think you really need the executable file, so you can just ignore the elf bit and test with a plain `lldb -c foo.dmp` (obviously, you won't get the backtrace that way, but you don't really need that here.

Ah, yeah, good point.

> here it looks like you actually want to test the case where the exception stream is not present (?)

If I'm reading things correctly, the exception stream is there and we get a non-null exception record, which has a tid, but its code and flags are both zero.
This makes me realize that the title I've put here is very misleading, I'll fix that when I update the patch.

> If you can send me the minidump you have generated I can try to play around with it to see if I can make a reasonable yaml out of it.

Thanks!  Will send.

================
Comment at: lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp:219-237
+
+  if (arch.GetTriple().isOSLinux()) {
+
+    SetUnixSignals(UnixSignals::Create(GetArchitecture()));
+
+    if (!m_thread_list.empty() &&
+        (!m_active_exception ||
----------------
labath wrote:
> Would it be possible to move this code (except maybe the SetUnixSignals bit) into the `RefreshStateAfterStop` function? Would be less confusing and it would avoid the need for extra member variables...
Yeah, happy to.  I wasn't sure what the implications of doing so would be, but I guess for a dump file there will only ever be one stop?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68096