labath added a comment.
Just a bit more cleanup and this will be fine. Sorry about the back and forth,
I was in a hurry yesterday.
Zachary, Adrian: any thoughts on your side?
================
Comment at:
packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/linux-x86_64_not_crashed.cpp:7
+int
+bar(int x)
+{
----------------
labath wrote:
> Please format these consistently (llvm-style). clang-format will not work by
> default as we are still ignoring the test folder.
Still not correct (4 char indent, inconsistent braces). It might be easiest to
just temporarily remove `packages/Python/lldbsuite/.clang-format`. We'll need
to figure out a better solution for formatting new test code in the long run
though.
================
Comment at:
packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/makefile.txt:1
+CC=g++
+FLAGS=-g --std=c++11
----------------
Please add a short blurb explaining how does this build work and why did we
choose such a complicated way of generating the minidumps.
================
Comment at: source/Plugins/Process/minidump/ProcessMinidump.cpp:67
+ // skip if the Minidump file is Windows generated, because we are still
+ // work-in-progress
+ if (!minidump_parser ||
----------------
Zach, Adrian: IIUC, the new plugin should generally have feature parity with
the windows-only plugin. (Dimitar: could you say exactly what bits are
missing?). You should be able to test out this plugin on windows minidumps by
removing the windows check below.
After this goes in, we'll be looking to remove the windows-only plugin.
================
Comment at: source/Plugins/Process/minidump/ProcessMinidump.cpp:89
+
+ // calling this here, and not in DoLoadCore, because we need to be sure this
+ // gets called first, because it initialises m_is_wow64 which is used in
----------------
Sorry about the back-and-forth, but I think we should put this back to
LoadCore(). I did not realize that ReadModuleList was a private function and
not a public interface of the class. :(
In this case we can assume that anyone will call LoadCore before expecting any
other values to be valid, and we shouldn't be doing any parsing (especially not
if it calls target->AddModule and such) as that is not the pattern anywhere
else.
================
Comment at: source/Plugins/Process/minidump/ProcessMinidump.cpp:218
+ arch_spec.SetTriple(triple);
+ return arch_spec;
+}
----------------
return ArchSpec(triple);
https://reviews.llvm.org/D25905
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
