zturner added inline comments.
================
Comment at:
packages/Python/lldbsuite/test/functionalities/postmortem/minidump/TestMiniDump.py:40
@@ +39,3 @@
+ thread = self.process.GetThreadAtIndex(0)
+ # The crash is in main, so there should be one frame on the stack.
+ self.assertEqual(thread.GetNumFrames(), 1)
----------------
I remember us being able to get call stacks higher than main. But now that I
think about it I guess that's only true for live debugging since you have a
physical copy of the module loaded in your process, and you can read it's EAT.
In any case, this assumption probably won't be true anymore once we understand
PDBs and symbol servers. Although at that point hopefully we'll have many more
tests covering different scenarios.
================
Comment at:
packages/Python/lldbsuite/test/functionalities/postmortem/minidump/TestMiniDump.py:45
@@ +44,3 @@
+ pc = frame.GetPC()
+ eip = frame.FindRegister("eip")
+ self.assertTrue(eip.IsValid())
----------------
Does this work if you change `eip` to `pc`? If so that would allow this test
to work in the presence of 64 bit.
================
Comment at: source/Plugins/Process/Windows/MiniDump/ProcessWinMiniDump.cpp:193
@@ +192,3 @@
+ const auto &mini_dump_thread = thread_list_ptr->Threads[i];
+ std::shared_ptr<ThreadWinMiniDump> thread_sp(new
ThreadWinMiniDump(*this, mini_dump_thread.ThreadId));
+ if (mini_dump_thread.ThreadContext.DataSize >= sizeof(CONTEXT))
----------------
Can you change this to
auto thread_sp = llvm::make_shared<ThreadWinMiniDump>(*this,
mini_dump_thread.ThreadId);
================
Comment at: source/Plugins/Process/Windows/MiniDump/ThreadWinMiniDump.h:42-44
@@ -44,4 +41,5 @@
protected:
- std::string m_thread_name;
lldb::RegisterContextSP m_reg_context_sp;
+ class Data;
+ std::unique_ptr<Data> m_data; // for WinAPI-specific data
----------------
Why does this class need a separate `CONTEXT` than the one that is already
stored in `m_reg_context_sp`? It seems like now we're storing the `CONTEXT`
twice.
================
Comment at:
source/Plugins/Process/Windows/MiniDump/x64/RegisterContextWindowsMiniDump_x64.h:14
@@ +13,3 @@
+#include "lldb/lldb-forward.h"
+#include "../../Common/x64/RegisterContextWindows_x64.h"
+
----------------
Instead of using relative paths, I think you can write this as
#include "Plugins/Process/Windows/Common/x64/RegisterContextWindows_x64.h"
http://reviews.llvm.org/D14591
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
