[Lldb-commits] [PATCH] D49750: Add support for ARM and ARM64 breakpad generated minidump files.

Leonard Mosescu via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Tue Jul 31 11:29:25 PDT 2018

lemo added a comment.

Thanks Greg, looks good to me (a couple of inline comments left at your discretion)

Comment at: source/Plugins/Process/minidump/ProcessMinidump.cpp:15
 // Other libraries and framework includes
+//#include "lldb/Core/Architecture.h"
 #include "lldb/Core/Module.h"
it this set for removal?

Comment at: source/Plugins/Process/minidump/RegisterContextMinidump_ARM.cpp:195
+// ARM general purpose registers.
+const uint32_t g_gpr_regnums[] = {
+  reg_r0,  reg_r1,  reg_r2,  reg_r3,  reg_r4, reg_r5, reg_r6, reg_r7,
clayborg wrote:
> lemo wrote:
> > use std::array for these kind of static arrays? (debug bounds checks, easy access to the static size, ...)
> Tried it but it introduces a global constructor. We try to avoid those.
We shouldn't have a dynamic initializer: that's strange, if that's the case we have a compiler bug on our hands. A quick experiment indicates that even with -O0 recent clang/llvm do the right thing: https://godbolt.org/g/NMUFLP

Is the problem only with arrays of RegisterInfo structs? If that's the case the cause is RegisterInfo itself and std::array should not make a difference (ie. we'd see the dynamic initializer even with plain C arrays)


More information about the lldb-commits mailing list