[Lldb-commits] [PATCH] D25905: Minidump plugin: Adding ProcessMinidump, ThreadMinidump and register the plugin in SystemInitializerFull

[Adrian McCarthy via lldb-commits]

amccarth accepted this revision.
amccarth added a reviewer: amccarth.
amccarth added a comment.
This revision is now accepted and ready to land.

I like that this keeps the WoW64 detection and support.  That's a very Windows-specific thing, and I was concerned that doing generic minidump parsing could lose this.

This looks really good.  On Monday or Tuesday, I'll try patching it into a Windows build to see if it's complete enough to remove the Windows-specific implementation.



================
Comment at: packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/makefile.txt:16
+# Then the binaries dynamically link to that lib.
+# The other optimisation is not using the standart library (hense the _start
+# instead of main). We only link dynamically to some standart libraries.
----------------
s/hense/hence/


================
Comment at: packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/makefile.txt:17
+# The other optimisation is not using the standart library (hense the _start
+# instead of main). We only link dynamically to some standart libraries.
+# This way we have a tiny binary (~8K) that has debug symbols and uses breakpad
----------------
x/standart/standard/


================
Comment at: source/Plugins/Process/minidump/ProcessMinidump.cpp:273
+    // 64 bit windows
+    if (llvm::StringRef(name.getValue()).endswith_lower("wow64.dll")) {
+      m_is_wow64 = true;
----------------
This is probably fine, but it's a slightly less precise check than what we're doing in ProcessWinMiniDump.cpp.  The endswith check would  accept "bowwow64.dll".  It's probably not important, since detecting WoW64 is by looking at module names is a hack anyway.


https://reviews.llvm.org/D25905