Add support for arm64 registers in minidump core file saving. #72315
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This patch adds support for saving minidumps with the arm64 architecture. It also will cause unsupported architectures to emit an error where before this patch it would emit a minidump with partial information. This new code is tested by the arm64 windows buildbot that was failing: https://lab.llvm.org/buildbot/#/builders/219/builds/6868 This is needed following this PR: llvm#71772
|
@llvm/pr-subscribers-lldb Author: Greg Clayton (clayborg) ChangesThis patch adds support for saving minidumps with the arm64 architecture. It also will cause unsupported architectures to emit an error where before this patch it would emit a minidump with partial information. This new code is tested by the arm64 windows buildbot that was failing: https://lab.llvm.org/buildbot/#/builders/219/builds/6868 This is needed following this PR: #71772 Full diff: https://github.com/llvm/llvm-project/pull/72315.diff 4 Files Affected:
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
index e8e0d09b5324d0f..8699479cadb36ad 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
@@ -9,6 +9,7 @@
#include "MinidumpFileBuilder.h"
#include "Plugins/Process/minidump/RegisterContextMinidump_x86_64.h"
+#include "Plugins/Process/minidump/RegisterContextMinidump_ARM64.h"
#include "lldb/Core/Module.h"
#include "lldb/Core/ModuleList.h"
@@ -293,7 +294,7 @@ Status MinidumpFileBuilder::AddModuleList(Target &target) {
}
uint16_t read_register_u16_raw(RegisterContext *reg_ctx,
- const std::string ®_name) {
+ llvm::StringRef reg_name) {
const RegisterInfo *reg_info = reg_ctx->GetRegisterInfoByName(reg_name);
if (!reg_info)
return 0;
@@ -305,7 +306,7 @@ uint16_t read_register_u16_raw(RegisterContext *reg_ctx,
}
uint32_t read_register_u32_raw(RegisterContext *reg_ctx,
- const std::string ®_name) {
+ llvm::StringRef reg_name) {
const RegisterInfo *reg_info = reg_ctx->GetRegisterInfoByName(reg_name);
if (!reg_info)
return 0;
@@ -317,7 +318,7 @@ uint32_t read_register_u32_raw(RegisterContext *reg_ctx,
}
uint64_t read_register_u64_raw(RegisterContext *reg_ctx,
- const std::string ®_name) {
+ llvm::StringRef reg_name) {
const RegisterInfo *reg_info = reg_ctx->GetRegisterInfoByName(reg_name);
if (!reg_info)
return 0;
@@ -329,25 +330,44 @@ uint64_t read_register_u64_raw(RegisterContext *reg_ctx,
}
llvm::support::ulittle16_t read_register_u16(RegisterContext *reg_ctx,
- const std::string ®_name) {
+ llvm::StringRef reg_name) {
return static_cast<llvm::support::ulittle16_t>(
read_register_u16_raw(reg_ctx, reg_name));
}
llvm::support::ulittle32_t read_register_u32(RegisterContext *reg_ctx,
- const std::string ®_name) {
+ llvm::StringRef reg_name) {
return static_cast<llvm::support::ulittle32_t>(
read_register_u32_raw(reg_ctx, reg_name));
}
llvm::support::ulittle64_t read_register_u64(RegisterContext *reg_ctx,
- const std::string ®_name) {
+ llvm::StringRef reg_name) {
return static_cast<llvm::support::ulittle64_t>(
read_register_u64_raw(reg_ctx, reg_name));
}
+void read_register_u128(RegisterContext *reg_ctx, llvm::StringRef reg_name,
+ uint8_t *dst) {
+ const RegisterInfo *reg_info = reg_ctx->GetRegisterInfoByName(reg_name);
+ if (reg_info) {
+ lldb_private::RegisterValue reg_value;
+ if (reg_ctx->ReadRegister(reg_info, reg_value)) {
+ Status error;
+ uint32_t bytes_copied =
+ reg_value.GetAsMemoryData(*reg_info, dst, 16,
+ lldb::ByteOrder::eByteOrderLittle,
+ error);
+ if (bytes_copied == 16)
+ return;
+ }
+ }
+ // If anything goes wrong, then zero out the register value.
+ memset(dst, 0, 16);
+}
+
lldb_private::minidump::MinidumpContext_x86_64
-GetThreadContext_64(RegisterContext *reg_ctx) {
+GetThreadContext_x86_64(RegisterContext *reg_ctx) {
lldb_private::minidump::MinidumpContext_x86_64 thread_context = {};
thread_context.p1_home = {};
thread_context.context_flags = static_cast<uint32_t>(
@@ -381,6 +401,73 @@ GetThreadContext_64(RegisterContext *reg_ctx) {
return thread_context;
}
+minidump::RegisterContextMinidump_ARM64::Context
+GetThreadContext_ARM64(RegisterContext *reg_ctx) {
+ minidump::RegisterContextMinidump_ARM64::Context thread_context = {};
+ thread_context.context_flags = static_cast<uint32_t>(
+ minidump::RegisterContextMinidump_ARM64::Flags::ARM64_Flag |
+ minidump::RegisterContextMinidump_ARM64::Flags::Integer |
+ minidump::RegisterContextMinidump_ARM64::Flags::FloatingPoint);
+ char reg_name[16];
+ for (uint32_t i=0; i<31; ++i) {
+ snprintf(reg_name, sizeof(reg_name), "x%u", i);
+ thread_context.x[i] = read_register_u64(reg_ctx, reg_name);
+ }
+ // Work around a bug in debugserver where "sp" on arm64 doesn't have the alt
+ // name set to "x31"
+ thread_context.x[31] = read_register_u64(reg_ctx, "sp");
+ thread_context.pc = read_register_u64(reg_ctx, "pc");
+ thread_context.cpsr = read_register_u32(reg_ctx, "cpsr");
+ thread_context.fpsr = read_register_u32(reg_ctx, "fpsr");
+ thread_context.fpcr = read_register_u32(reg_ctx, "fpcr");
+ for (uint32_t i=0; i<32; ++i) {
+ snprintf(reg_name, sizeof(reg_name), "v%u", i);
+ read_register_u128(reg_ctx, reg_name, &thread_context.v[i*16]);
+ }
+ return thread_context;
+}
+
+class ArchThreadContexts {
+ llvm::Triple::ArchType m_arch;
+ union {
+ lldb_private::minidump::MinidumpContext_x86_64 x86_64;
+ lldb_private::minidump::RegisterContextMinidump_ARM64::Context arm64;
+ };
+public:
+ ArchThreadContexts(llvm::Triple::ArchType arch) : m_arch(arch) {}
+
+ bool prepareRegisterContext(RegisterContext *reg_ctx) {
+ switch (m_arch) {
+ case llvm::Triple::ArchType::x86_64:
+ x86_64 = GetThreadContext_x86_64(reg_ctx);
+ return true;
+ case llvm::Triple::ArchType::aarch64:
+ arm64 = GetThreadContext_ARM64(reg_ctx);
+ return true;
+ default:
+ break;
+ }
+ return false;
+ }
+
+ const void *data() const {
+ return &x86_64;
+ }
+
+ size_t size() const {
+ switch (m_arch) {
+ case llvm::Triple::ArchType::x86_64:
+ return sizeof(x86_64);
+ case llvm::Triple::ArchType::aarch64:
+ return sizeof(arm64);
+ default:
+ break;
+ }
+ return 0;
+ }
+
+};
+
// Function returns start and size of the memory region that contains
// memory location pointed to by the current stack pointer.
llvm::Expected<std::pair<addr_t, addr_t>>
@@ -434,11 +521,19 @@ Status MinidumpFileBuilder::AddThreadList(const lldb::ProcessSP &process_sp) {
return error;
}
RegisterContext *reg_ctx = reg_ctx_sp.get();
- auto thread_context = GetThreadContext_64(reg_ctx);
- uint64_t rsp = read_register_u64_raw(reg_ctx, "rsp");
- auto expected_address_range = findStackHelper(process_sp, rsp);
+ Target &target = process_sp->GetTarget();
+ const ArchSpec &arch = target.GetArchitecture();
+ ArchThreadContexts thread_context(arch.GetMachine());
+ if (!thread_context.prepareRegisterContext(reg_ctx)) {
+ error.SetErrorStringWithFormat("architecture %s not supported.",
+ arch.GetTriple().getArchName().str().c_str());
+ return error;
+ }
+ uint64_t sp = reg_ctx->GetSP();
+ auto expected_address_range = findStackHelper(process_sp, sp);
if (!expected_address_range) {
+ consumeError(expected_address_range.takeError());
error.SetErrorString("Unable to get the stack address.");
return error;
}
@@ -468,13 +563,14 @@ Status MinidumpFileBuilder::AddThreadList(const lldb::ProcessSP &process_sp) {
LocationDescriptor thread_context_memory_locator;
thread_context_memory_locator.DataSize =
- static_cast<llvm::support::ulittle32_t>(sizeof(thread_context));
+ static_cast<llvm::support::ulittle32_t>(thread_context.size());
thread_context_memory_locator.RVA = static_cast<llvm::support::ulittle32_t>(
size_before + thread_stream_size + helper_data.GetByteSize());
+ // Cache thie thread context memory so we can reuse for exceptions.
+ m_tid_to_reg_ctx[thread_sp->GetID()] = thread_context_memory_locator;
+
+ helper_data.AppendData(thread_context.data(), thread_context.size());
- helper_data.AppendData(
- &thread_context,
- sizeof(lldb_private::minidump::MinidumpContext_x86_64));
llvm::minidump::Thread t;
t.ThreadId = static_cast<llvm::support::ulittle32_t>(thread_sp->GetID());
@@ -492,68 +588,55 @@ Status MinidumpFileBuilder::AddThreadList(const lldb::ProcessSP &process_sp) {
return Status();
}
-Status MinidumpFileBuilder::AddException(const lldb::ProcessSP &process_sp) {
- Status error;
+void MinidumpFileBuilder::AddExceptions(const lldb::ProcessSP &process_sp) {
lldb_private::ThreadList thread_list = process_sp->GetThreadList();
const uint32_t num_threads = thread_list.GetSize();
- uint32_t stop_reason_thread_idx = 0;
- for (stop_reason_thread_idx = 0; stop_reason_thread_idx < num_threads;
- ++stop_reason_thread_idx) {
- ThreadSP thread_sp(thread_list.GetThreadAtIndex(stop_reason_thread_idx));
+ for (uint32_t thread_idx = 0; thread_idx < num_threads; ++thread_idx) {
+ ThreadSP thread_sp(thread_list.GetThreadAtIndex(thread_idx));
StopInfoSP stop_info_sp = thread_sp->GetStopInfo();
-
- if (stop_info_sp && stop_info_sp->IsValid())
- break;
- }
-
- if (stop_reason_thread_idx == num_threads) {
- error.SetErrorString("No stop reason thread found.");
- return error;
+ bool add_exception = false;
+ if (stop_info_sp) {
+ switch (stop_info_sp->GetStopReason()) {
+ case eStopReasonSignal:
+ case eStopReasonException:
+ add_exception = true;
+ break;
+ default:
+ break;
+ }
+ }
+ if (add_exception) {
+ constexpr size_t minidump_exception_size =
+ sizeof(llvm::minidump::ExceptionStream);
+ AddDirectory(StreamType::Exception, minidump_exception_size);
+ StopInfoSP stop_info_sp = thread_sp->GetStopInfo();
+ RegisterContextSP reg_ctx_sp(thread_sp->GetRegisterContext());
+ Exception exp_record = {};
+ exp_record.ExceptionCode =
+ static_cast<llvm::support::ulittle32_t>(stop_info_sp->GetValue());
+ exp_record.ExceptionFlags = static_cast<llvm::support::ulittle32_t>(0);
+ exp_record.ExceptionRecord = static_cast<llvm::support::ulittle64_t>(0);
+ exp_record.ExceptionAddress = reg_ctx_sp->GetPC();
+ exp_record.NumberParameters = static_cast<llvm::support::ulittle32_t>(0);
+ exp_record.UnusedAlignment = static_cast<llvm::support::ulittle32_t>(0);
+ // exp_record.ExceptionInformation;
+
+ ExceptionStream exp_stream;
+ exp_stream.ThreadId =
+ static_cast<llvm::support::ulittle32_t>(thread_sp->GetID());
+ exp_stream.UnusedAlignment = static_cast<llvm::support::ulittle32_t>(0);
+ exp_stream.ExceptionRecord = exp_record;
+ auto Iter = m_tid_to_reg_ctx.find(thread_sp->GetID());
+ if (Iter != m_tid_to_reg_ctx.end()) {
+ exp_stream.ThreadContext = Iter->second;
+ } else {
+ exp_stream.ThreadContext.DataSize = 0;
+ exp_stream.ThreadContext.RVA = 0;
+ }
+ m_data.AppendData(&exp_stream, minidump_exception_size);
+ }
}
-
- constexpr size_t minidump_exception_size =
- sizeof(llvm::minidump::ExceptionStream);
- AddDirectory(StreamType::Exception, minidump_exception_size);
- size_t size_before = GetCurrentDataEndOffset();
-
- ThreadSP thread_sp(thread_list.GetThreadAtIndex(stop_reason_thread_idx));
- RegisterContextSP reg_ctx_sp(thread_sp->GetRegisterContext());
- RegisterContext *reg_ctx = reg_ctx_sp.get();
- auto thread_context = GetThreadContext_64(reg_ctx);
- StopInfoSP stop_info_sp = thread_sp->GetStopInfo();
-
- DataBufferHeap helper_data;
-
- LocationDescriptor thread_context_memory_locator;
- thread_context_memory_locator.DataSize =
- static_cast<llvm::support::ulittle32_t>(sizeof(thread_context));
- thread_context_memory_locator.RVA = static_cast<llvm::support::ulittle32_t>(
- size_before + minidump_exception_size + helper_data.GetByteSize());
-
- helper_data.AppendData(
- &thread_context, sizeof(lldb_private::minidump::MinidumpContext_x86_64));
-
- Exception exp_record = {};
- exp_record.ExceptionCode =
- static_cast<llvm::support::ulittle32_t>(stop_info_sp->GetValue());
- exp_record.ExceptionFlags = static_cast<llvm::support::ulittle32_t>(0);
- exp_record.ExceptionRecord = static_cast<llvm::support::ulittle64_t>(0);
- exp_record.ExceptionAddress = read_register_u64(reg_ctx, "rip");
- exp_record.NumberParameters = static_cast<llvm::support::ulittle32_t>(0);
- exp_record.UnusedAlignment = static_cast<llvm::support::ulittle32_t>(0);
- // exp_record.ExceptionInformation;
-
- ExceptionStream exp_stream;
- exp_stream.ThreadId =
- static_cast<llvm::support::ulittle32_t>(thread_sp->GetID());
- exp_stream.UnusedAlignment = static_cast<llvm::support::ulittle32_t>(0);
- exp_stream.ExceptionRecord = exp_record;
- exp_stream.ThreadContext = thread_context_memory_locator;
-
- m_data.AppendData(&exp_stream, minidump_exception_size);
- m_data.AppendData(helper_data.GetBytes(), helper_data.GetByteSize());
- return error;
}
lldb_private::Status
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
index cae355799fa7247..b2e984191983ff0 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
@@ -17,6 +17,7 @@
#define LLDB_SOURCE_PLUGINS_OBJECTFILE_MINIDUMP_MINIDUMPFILEBUILDER_H
#include <cstddef>
+#include <map>
#include "lldb/Target/Target.h"
#include "lldb/Utility/DataBufferHeap.h"
@@ -59,10 +60,8 @@ class MinidumpFileBuilder {
// at the moment of core saving. Contains information about thread
// contexts.
lldb_private::Status AddThreadList(const lldb::ProcessSP &process_sp);
- // Add Exception stream, this contains information about the exception
- // that stopped the process. In case no thread made exception it return
- // failed status.
- lldb_private::Status AddException(const lldb::ProcessSP &process_sp);
+ // Add Exception streams for any threads that stopped with exceptions.
+ void AddExceptions(const lldb::ProcessSP &process_sp);
// Add MemoryList stream, containing dumps of important memory segments
lldb_private::Status AddMemoryList(const lldb::ProcessSP &process_sp,
lldb::SaveCoreStyle core_style);
@@ -88,6 +87,11 @@ class MinidumpFileBuilder {
// Main data buffer consisting of data without the minidump header and
// directories
lldb_private::DataBufferHeap m_data;
+
+ // More that one place can mention the register thread context locations,
+ // so when we emit the thread contents, remember where it is so we don't have
+ // to duplicate it in the exception data.
+ std::map<lldb::tid_t, llvm::minidump::LocationDescriptor> m_tid_to_reg_ctx;
};
#endif // LLDB_SOURCE_PLUGINS_OBJECTFILE_MINIDUMP_MINIDUMPFILEBUILDER_H
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
index f5294b2f08c66e1..fe609c7f3d2001e 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
@@ -78,19 +78,16 @@ bool ObjectFileMinidump::SaveCore(const lldb::ProcessSP &process_sp,
builder.AddMiscInfo(process_sp);
- if (target.GetArchitecture().GetMachine() == llvm::Triple::ArchType::x86_64) {
- error = builder.AddThreadList(process_sp);
- if (error.Fail())
- return false;
-
- error = builder.AddException(process_sp);
- if (error.Fail())
- return false;
-
- error = builder.AddMemoryList(process_sp, core_style);
- if (error.Fail())
- return false;
- }
+ error = builder.AddThreadList(process_sp);
+ if (error.Fail())
+ return false;
+
+ // Add any exceptions but only if there are any in any threads.
+ builder.AddExceptions(process_sp);
+
+ error = builder.AddMemoryList(process_sp, core_style);
+ if (error.Fail())
+ return false;
if (target.GetArchitecture().GetTriple().getOS() ==
llvm::Triple::OSType::Linux) {
diff --git a/lldb/source/Plugins/Process/minidump/RegisterContextMinidump_ARM64.h b/lldb/source/Plugins/Process/minidump/RegisterContextMinidump_ARM64.h
index 8ae751095c04fdb..f50c0ff49cec11c 100644
--- a/lldb/source/Plugins/Process/minidump/RegisterContextMinidump_ARM64.h
+++ b/lldb/source/Plugins/Process/minidump/RegisterContextMinidump_ARM64.h
@@ -67,13 +67,13 @@ class RegisterContextMinidump_ARM64 : public lldb_private::RegisterContext {
uint8_t v[32 * 16]; // 32 128-bit floating point registers
};
-protected:
enum class Flags : uint32_t {
ARM64_Flag = 0x80000000,
Integer = ARM64_Flag | 0x00000002,
FloatingPoint = ARM64_Flag | 0x00000004,
LLVM_MARK_AS_BITMASK_ENUM(/* LargestValue = */ FloatingPoint)
};
+protected:
Context m_regs;
};
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
|
LGTM. minidump tests are now OK on linaro-armv8-windows-msvc machine. |
Feel free to accept this patch then and I can get it merged! |
antmox
approved these changes
Nov 15, 2023
|
@antmox Hopefully this clears up the issue on the build bot! Thanks for testing this for me. |
|
Thanks @clayborg, the buildbot is now green again : https://lab.llvm.org/buildbot/#/builders/219/builds/6947 |
zahiraam
pushed a commit
to zahiraam/llvm-project
that referenced
this pull request
Nov 20, 2023
…2315) This patch adds support for saving minidumps with the arm64 architecture. It also will cause unsupported architectures to emit an error where before this patch it would emit a minidump with partial information. This new code is tested by the arm64 windows buildbot that was failing: https://lab.llvm.org/buildbot/#/builders/219/builds/6868 This is needed following this PR: llvm#71772
Guzhu-AMD
pushed a commit
to GPUOpen-Drivers/llvm-project
that referenced
this pull request
Nov 23, 2023
Local branch amd-gfx 70925fa Merged main:e35b60628093 into amd-gfx:b59526a8f0aa Remote branch main beb702c Add support for arm64 registers in minidump core file saving. (llvm#72315)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch adds support for saving minidumps with the arm64 architecture. It also will cause unsupported architectures to emit an error where before this patch it would emit a minidump with partial information. This new code is tested by the arm64 windows buildbot that was failing:
https://lab.llvm.org/buildbot/#/builders/219/builds/6868
This is needed following this PR: #71772