This can be done unambiguously because Breakpad used its own values for the
magic numbers identifying its own context structure. You’ll find this in
the ContextFlags field.
Breakpad MD_CONTEXT_ARM is 0x4000 and MD_CONTEXT_ARM64 is 0x8000.
Microsoft CONTEXT_ARM is 0x0020 and
I've reverted this to keep the bots green until the issues pointed out
by Stella and Raphael are resolved.
Based on a quick inspection, it seems that the test issue is that
`GetSystemInfo` call that has been added to MinidumpParser::Initialize
is failing. I guess that's because the
teemperor added a comment.
I don't see this mentioned here yet, so: This patch also seems to introduce a
few hundred warnings with -Wextended-offsetof (which is enabled by default on
the macOS builds):
stella.stamenova added a comment.
There are a number of minidump tests that started failing for us on both Linux
and Windows and I suspect it's due to this change. Did the unit tests pass for
you with the changes on either Linux or Windows?
Failing Tests (6):
lldb-Unit ::
Please remember to test with the cmake build when you add or remove files,
as that is the build that all of the buildbots use. I almost reverted this
since it broke every LLDB buildbot, but I noticed that it's just forgetting
to remove the files from the CMakeLists.txt so I'll fix it.
On Thu,
zturner added a comment.
Please remember to test with the cmake build when you add or remove files,
as that is the build that all of the buildbots use. I almost reverted this
since it broke every LLDB buildbot, but I noticed that it's just forgetting
to remove the files from the CMakeLists.txt
This revision was automatically updated to reflect the committed changes.
Closed by commit rL338734: Add support for ARM and ARM64 breakpad generated
minidump files (authored by gclayton, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
clayborg updated this revision to Diff 158631.
clayborg added a comment.
- run clang-format
- fix doxygen parameter names
https://reviews.llvm.org/D49750
Files:
include/lldb/Target/Target.h
lldb.xcodeproj/project.pbxproj
labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.
Looks great. I only noticed some typos when looking this over again. We can
continue the register shuffling discussion offline.
Comment at:
clayborg added inline comments.
Comment at: source/Plugins/Process/minidump/ProcessMinidump.cpp:15
// Other libraries and framework includes
+//#include "lldb/Core/Architecture.h"
#include "lldb/Core/Module.h"
lemo wrote:
> it this set for removal?
Ah yes!
clayborg updated this revision to Diff 158371.
clayborg added a comment.
Removed unnecessary Xcode project changes and removed #include that wasn't
needed.
https://reviews.llvm.org/D49750
Files:
include/lldb/Target/Target.h
lldb.xcodeproj/project.pbxproj
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
clayborg added inline comments.
Comment at: source/Plugins/Process/minidump/RegisterContextMinidump_ARM.cpp:192
+
+static size_t k_num_reg_infos = llvm::array_lengthof(g_reg_infos);
+
lemo wrote:
> constexpr?
will do
Comment at:
clayborg updated this revision to Diff 158321.
clayborg added a comment.
Herald added a subscriber: srhines.
- Fixed inline comments
- Moved platform setting into Target::SetArchitecture(...) instead of doing
this manually in the core file code.
- Added ARM64 w0-w31, d0-d31, s0-s31 and h0-h31
> On Jul 30, 2018, at 12:17 PM, Leonard Mosescu wrote:
>
> FYI, Breakpad & Crashpad will start generating the Microsoft flavor of ARM
> minidumps soon.
How do we tell the difference? I am guessing the ProcessorArchitecture will
both be set to "PROCESSOR_ARCHITECTURE_ARM". Are plug-ins
FYI, Breakpad & Crashpad will start generating the Microsoft flavor of ARM
minidumps soon.
On Wed, Jul 25, 2018 at 9:44 AM, Leonard Mosescu via Phabricator <
revi...@reviews.llvm.org> wrote:
> lemo added inline comments.
>
>
>
> Comment at:
lemo added subscribers: clayborg, labath, markmentovai, t.p.northover, zturner.
lemo added a comment.
FYI, Breakpad & Crashpad will start generating the Microsoft flavor of ARM
minidumps soon.
https://reviews.llvm.org/D49750
___
lldb-commits
lemo added inline comments.
Comment at: source/Plugins/Process/minidump/RegisterContextMinidump_ARM.cpp:51
+ reg_fpscr,
+ reg_d0, reg_d1, reg_d2, reg_d3, reg_d4, reg_d5, reg_d6, reg_d7,
+ reg_d8, reg_d9, reg_d10, reg_d11, reg_d12, reg_d13, reg_d14, reg_d15,
labath added inline comments.
Comment at: source/Plugins/Process/minidump/ProcessMinidump.cpp:179
+auto platform_sp = target.GetPlatform();
+if (platform_sp && !platform_sp->IsCompatibleArchitecture(arch, false,
nullptr)) {
+ ArchSpec platform_arch;
lemo added inline comments.
Comment at: source/Plugins/Process/minidump/MinidumpParser.cpp:216
+ if (csd_version.find("Linux") != std::string::npos)
+triple.setOS(llvm::Triple::OSType::Linux);
+ break;
clayborg wrote:
> I have run into some
clayborg added a comment.
I will make update the patch with many of the suggested inline comments.
Comment at: source/Plugins/Process/minidump/MinidumpParser.cpp:150
+ if (m_arch.IsValid())
+return m_arch;
const MinidumpSystemInfo *system_info = GetSystemInfo();
lemo added a comment.
Looks really good, a few comments inline.
This may not big a big deal, but the part about FP (and Apple vs. non-Apple) is
confusing: the FP is a pretty weak convention, and in some ABIs is not actually
"fixed" (ex. FP can be either https://reviews.llvm.org/source/openmp/
clayborg created this revision.
clayborg added reviewers: labath, zturner, markmentovai.
Herald added a reviewer: javed.absar.
Herald added subscribers: chrib, kristof.beyls.
In this patch I add support for ARM and ARM64 break pad files. There are two
flavors of ARM: Apple where FP is
23 matches
Mail list logo
