Use the minidump exception record if present

[lemo]

Summary

If the minidump contains a saved exception record use it automatically.

(This patch is cherry picked from the larger https://reviews.llvm.org/D55142)

[zturner]

I don't think we can check in an executable file, we should try to compile it on the spot. We have 1-2 existing unit tests that check in an exe and we occasionally get reports that peoples' virus scanners flag them as trojans, even though they obviously aren't. In any case, I've been meaning to remove those tests, so I think we should set a precedent that executable binaries are never checked in.

Is there something about this executable that makes it impractical to compile on the fly using the %build substitution?

[clayborg]

I have a minidump generator if you need me to make any specific minidump files for you.

[labath]

In D56293#1345790, @zturner wrote:

Is there something about this executable that makes it impractical to compile on the fly using the %build substitution?

I think the impractical part comes when you need to use that compiled binary to generate a minidump.

That said, I am not sure if you actually need the .exe for this test. Lldb should be able to open a core file/minidump without the matching executable file. It won't be terribly useful (for example, you won't get the disassembly around the crashing instructions), but you should be able to get the stop reason and the topmost PC for instance.

[labath]

In D56293#1345790, @zturner wrote:

I don't think we can check in an executable file, we should try to compile it on the spot. We have 1-2 existing unit tests that check in an exe and we occasionally get reports that peoples' virus scanners flag them as trojans, even though they obviously aren't. In any case, I've been meaning to remove those tests, so I think we should set a precedent that executable binaries are never checked in.

While I agree that a checked-in exe shouldn't be needed in this (and most other) cases, I am not sure about the policy in general. For example, I can see a case for having a bunch of badly corrupted binaries (things like corrupted section headers, overlapping sections in the file; things that even yaml2obj will have trouble generating) and then a test that makes sure we do something reasonable (e.g., not crash) when opening them. These are exactly the kind of files that make paranoid virus scanners sound the alarm.

[lemo]

Removed sigsegv.exe from the test inputs

[labath]

Looks fine to me (but please wait for an ack from Zachary).

source/Plugins/Process/minidump/ProcessMinidump.cpp

317–318

Since you're touching this, you might as well change this to a range-based for loop.

320

LLVM tries to avoid auto overuse. For example, here I had to look up thread_context to see what type would this be. Could you put the actual type here?

[lemo]

Incorporating Pavel's suggestions (range-based loop, explicit type instead of auto)

lit/Minidump/Windows/Sigsegv/Inputs/sigsegv.cpp

+
+// nodefaultlib build: cl -Zi sigsegv.cpp /link /nodefaultlib
+
+#ifdef USE_CRT
+#include <stdio.h>
+#else
+int main();
+extern "C"
+{
+    int _fltused;
+    void mainCRTStartup() { main(); }
+    void printf(const char*, ...) {}
+}
+#endif
+
+void crash(bool crash_self)
+{
+    printf("Before...\n");
+    if(crash_self)
+    {
+        printf("Crashing in 3, 2, 1 ...\n");
+        *(volatile int*)nullptr = 0;
+    }
+    printf("After...\n");
+}
+
+int foo(int x, float y, const char* msg)
+{
+    bool flag = x > y;
+    if(flag)
+        printf("x = %d, y = %f, msg = %s\n", x, y, msg);
+    crash(flag);
+    return x << 1;
+}
+
+int main()
+{
+    foo(10, 3.14, "testing");
+}
+

lit/Minidump/Windows/Sigsegv/Inputs/sigsegv.lldbinit

+bt all
+dis

lit/Minidump/Windows/Sigsegv/sigsegv.test

+// RUN: cd %p/Inputs
+// RUN: env LLDB_USE_NATIVE_PDB_READER=1 \
+// RUN:   %lldb -c sigsegv.dmp -s sigsegv.lldbinit | FileCheck %s
+
+CHECK: * thread #1, stop reason = Exception 0xc0000005 encountered at address 0x7ff7a13110d9
+CHECK:   * frame #0: 0x00007ff7a13110d9 sigsegv.exe
+
+CHECK: ->  0x7ff7a13110d9: movl   $0x0, 0x0
+CHECK:     0x7ff7a13110e4: leaq   0x1f45(%rip), %rcx
+CHECK:     0x7ff7a13110eb: callq  0x7ff7a1311019
+CHECK:     0x7ff7a13110f0: addq   $0x28, %rsp
+CHECK:     0x7ff7a13110f4: retq
+CHECK:     0x7ff7a13110f5: int3

source/Plugins/Process/minidump/MinidumpParser.h

public:
   llvm::ArrayRef<uint8_t> GetStream(MinidumpStreamType stream_type);
 
   llvm::Optional<std::string> GetMinidumpString(uint32_t rva);
 
   UUID GetModuleUUID(const MinidumpModule* module);
 
   llvm::ArrayRef<MinidumpThread> GetThreads();
 
+  llvm::ArrayRef<uint8_t>
+  GetThreadContext(const MinidumpLocationDescriptor &location);
+
   llvm::ArrayRef<uint8_t> GetThreadContext(const MinidumpThread &td);
 
   llvm::ArrayRef<uint8_t> GetThreadContextWow64(const MinidumpThread &td);
 
   const MinidumpSystemInfo *GetSystemInfo();
 
   ArchSpec GetArchitecture();
 

source/Plugins/Process/minidump/MinidumpParser.cpp

llvm::ArrayRef<MinidumpThread> MinidumpParser::GetThreads() {
 
   if (data.size() == 0)
     return llvm::None;
 
   return MinidumpThread::ParseThreadList(data);
 }
 
 llvm::ArrayRef<uint8_t>
-MinidumpParser::GetThreadContext(const MinidumpThread &td) {
-  if (td.thread_context.rva + td.thread_context.data_size > GetData().size())
+MinidumpParser::GetThreadContext(const MinidumpLocationDescriptor &location) {
+  if (location.rva + location.data_size > GetData().size())
     return {};
+  return GetData().slice(location.rva, location.data_size);
+}
 
-  return GetData().slice(td.thread_context.rva, td.thread_context.data_size);
+llvm::ArrayRef<uint8_t>
+MinidumpParser::GetThreadContext(const MinidumpThread &td) {
+  return GetThreadContext(td.thread_context);
 }
 
 llvm::ArrayRef<uint8_t>
 MinidumpParser::GetThreadContextWow64(const MinidumpThread &td) {
   // On Windows, a 32-bit process can run on a 64-bit machine under WOW64. If
   // the minidump was captured with a 64-bit debugger, then the CONTEXT we just
   // grabbed from the mini_dump_thread is the one for the 64-bit "native"
   // process rather than the 32-bit "guest" process we care about.  In this

source/Plugins/Process/minidump/ProcessMinidump.cpp

 #include "lldb/Utility/LLDBAssert.h"
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/State.h"
 
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Threading.h"
 
 #include "Plugins/Process/Utility/StopInfoMachException.h"
+
 // C includes
 // C++ includes
 
 using namespace lldb;
 using namespace lldb_private;
 using namespace minidump;
 
 //------------------------------------------------------------------
lldb::SectionSP section_sp(new Section(
         0,                      // Flags for this section.
         1));                    // Number of host bytes per target byte
     section_sp->SetPermissions(ePermissionsExecutable | ePermissionsReadable);
     GetSectionList()->AddSection(section_sp);
     target.GetSectionLoadList().SetSectionLoadAddress(
         section_sp, module->base_of_image);
   }
 
-  ObjectFile *GetObjectFile() override { return nullptr; }
 
   SectionList *GetSectionList() override {
     return Module::GetUnifiedSectionList();
   }
 };
 
 ConstString ProcessMinidump::GetPluginNameStatic() {
   static ConstString g_name("minidump");
Status ProcessMinidump::GetMemoryRegions(
   region_list = m_minidump_parser.GetMemoryRegions();
   return Status();
 }
 
 void ProcessMinidump::Clear() { Process::m_thread_list.Clear(); }
 
 bool ProcessMinidump::UpdateThreadList(ThreadList &old_thread_list,
                                        ThreadList &new_thread_list) {
-  uint32_t num_threads = 0;
-  if (m_thread_list.size() > 0)
-    num_threads = m_thread_list.size();
+  for (const MinidumpThread& thread : m_thread_list) {
+    MinidumpLocationDescriptor context_location = thread.thread_context;
+
+    // If the minidump contains an exception context, use it
+    if (m_active_exception != nullptr &&
+        m_active_exception->thread_id == thread.thread_id) {
+      context_location = m_active_exception->thread_context;
+    }
 
-  for (lldb::tid_t tid = 0; tid < num_threads; ++tid) {
     llvm::ArrayRef<uint8_t> context;
     if (!m_is_wow64)
-      context = m_minidump_parser.GetThreadContext(m_thread_list[tid]);
+      context = m_minidump_parser.GetThreadContext(context_location);
     else
-      context = m_minidump_parser.GetThreadContextWow64(m_thread_list[tid]);
+      context = m_minidump_parser.GetThreadContextWow64(thread);
 
-    lldb::ThreadSP thread_sp(
-        new ThreadMinidump(*this, m_thread_list[tid], context));
+    lldb::ThreadSP thread_sp(new ThreadMinidump(*this, thread, context));
     new_thread_list.AddThread(thread_sp);
   }
   return new_thread_list.GetSize(false) > 0;
 }
 
 void ProcessMinidump::ReadModuleList() {
   std::vector<const MinidumpModule *> filtered_modules =
       m_minidump_parser.GetFilteredModuleList();
: CommandObjectParsed(interpreter, "process plugin dump",
     APPEND_OPT(m_dump_linux_auxv);
     APPEND_OPT(m_dump_linux_maps);
     APPEND_OPT(m_dump_linux_proc_stat);
     APPEND_OPT(m_dump_linux_proc_uptime);
     APPEND_OPT(m_dump_linux_proc_fd);
     APPEND_OPT(m_dump_linux_all);
     m_option_group.Finalize();
   }
-  
   ~CommandObjectProcessMinidumpDump() {}
-  
   Options *GetOptions() override { return &m_option_group; }
 
   bool DoExecute(Args &command, CommandReturnObject &result) override {
     const size_t argc = command.GetArgumentCount();
     if (argc > 0) {
       result.AppendErrorWithFormat("'%s' take no arguments, only options",
                                    m_cmd_name.c_str());
       result.SetStatus(eReturnStatusFailed);
       return false;
     }
     SetDefaultOptionsIfNoneAreSet();
-    
     ProcessMinidump *process = static_cast<ProcessMinidump *>(
         m_interpreter.GetExecutionContext().GetProcessPtr());
     result.SetStatus(eReturnStatusSuccessFinishResult);
     Stream &s = result.GetOutputStream();
     MinidumpParser &minidump = process->m_minidump_parser;
     if (DumpDirectory()) {
       s.Printf("RVA        SIZE       TYPE       MinidumpStreamType\n");
       s.Printf("---------- ---------- ---------- --------------------------\n");

 public:
   CommandObjectMultiwordProcessMinidump(CommandInterpreter &interpreter)
     : CommandObjectMultiword(interpreter, "process plugin",
           "Commands for operating on a ProcessMinidump process.",
           "process plugin <subcommand> [<subcommand-options>]") {
     LoadSubCommand("dump",
         CommandObjectSP(new CommandObjectProcessMinidumpDump(interpreter)));
   }
-  
   ~CommandObjectMultiwordProcessMinidump() {}
 };
 
 CommandObject *ProcessMinidump::GetPluginCommandObject() {
   if (!m_command_sp)
     m_command_sp.reset(new CommandObjectMultiwordProcessMinidump(
         GetTarget().GetDebugger().GetCommandInterpreter()));
   return m_command_sp.get();
 }