mirror of
https://github.com/intel/llvm.git
synced 2026-01-13 19:08:21 +08:00
### Context Over a year ago, I landed support for 64b Memory ranges in Minidump (#95312). In this patch we added the Memory64 list stream, which is effectively a Linked List on disk. The layout is a sixteen byte header and then however many Memory descriptors. ### The Bug This is a classic off-by one error, where I added 8 bytes instead of 16 for the header. This caused the first region to start 8 bytes before the correct RVA, thus shifting all memory reads by 8 bytes. We are correctly writing all the regions to disk correctly, with no physical corruption but the RVA is defined wrong, meaning we were incorrectly reading memory  ### Why wasn't this caught? One problem we've had is forcing Minidump to actually use the 64b mode, it would be a massive waste of resources to have a test that actually wrote >4.2gb of IO to validate the 64b regions, and so almost all validation has been manual. As a weakness of manual testing, this issue is psuedo non-deterministic, as what regions end up in 64b or 32b is handled greedily and iterated in the order it's laid out in /proc/pid/maps. We often validated 64b was written correctly by hexdumping the Minidump itself, which was not corrupted (other than the BaseRVA)  ### Why is this showing up now? During internal usage, we had a bug report that the Minidump wasn't displaying values. I was unable to repro the issue, but during my investigation I saw the variables were in the 64b regions which resulted in me identifying the bug. ### How do we prevent future regressions? To prevent regressions, and honestly to save my sanity for figuring out where 8 bytes magically came from, I've added a new API to SBSaveCoreOptions. ```SBSaveCoreOptions::GetMemoryRegionsToSave()``` The ability to get the memory regions that we intend to include in the Coredump. I added this so we can compare what we intended to include versus what was actually included. Traditionally we've always had issues comparing regions because Minidump includes `/proc/pid/maps` and it can be difficult to know what memoryregion read failure was a genuine error or just a page that wasn't meant to be included. We are also leveraging this API to choose the memory regions to be generated, as well as for testing what regions should be bytewise 1:1. After much debate with @clayborg, I've moved all non-stack memory to the Memory64 List. This list doesn't incur us any meaningful overhead and Greg originally suggested doing this in the original 64b PR. This also means we're exercising the 64b path every single time we save a Minidump, preventing regressions on this feature from slipping through testing in the future. Snippet produced by [minidump.py](https://github.com/clayborg/scripts) ``` MINIDUMP_MEMORY_LIST: NumberOfMemoryRanges = 0x00000002 MemoryRanges[0] = [0x00007f61085ff9f0 - 0x00007f6108601000) @ 0x0003f655 MemoryRanges[1] = [0x00007ffe47e50910 - 0x00007ffe47e52000) @ 0x00040c65 MINIDUMP_MEMORY64_LIST: NumberOfMemoryRanges = 0x000000000000002e BaseRva = 0x0000000000042669 MemoryRanges[0] = [0x00005584162d8000 - 0x00005584162d9000) MemoryRanges[1] = [0x00005584162d9000 - 0x00005584162db000) MemoryRanges[2] = [0x00005584162db000 - 0x00005584162dd000) MemoryRanges[3] = [0x00005584162dd000 - 0x00005584162ff000) MemoryRanges[4] = [0x00007f6100000000 - 0x00007f6100021000) MemoryRanges[5] = [0x00007f6108800000 - 0x00007f6108828000) MemoryRanges[6] = [0x00007f6108828000 - 0x00007f610899d000) MemoryRanges[7] = [0x00007f610899d000 - 0x00007f61089f9000) MemoryRanges[8] = [0x00007f61089f9000 - 0x00007f6108a08000) MemoryRanges[9] = [0x00007f6108bf5000 - 0x00007f6108bf7000) ``` ### Misc As a part of this fix I had to look at LLDB logs a lot, you'll notice I added `0x` to many of the PRIx64 `LLDB_LOGF`. This is so the user (or I) can directly copy paste the address in the logs instead of adding the hex prefix themselves. Added some SBSaveCore tests for the new GetMemoryAPI, and Docstrings. CC: @DavidSpickett, @da-viper @labath because we've been working together on save-core plugins, review it optional and I didn't tag you but figured you'd want to know
215 lines
6.0 KiB
C++
215 lines
6.0 KiB
C++
//===-- SaveCoreOptions.cpp -------------------------------------*- C++ -*-===//
|
|
//
|
|
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
|
|
// See https://llvm.org/LICENSE.txt for license information.
|
|
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
|
|
//
|
|
//===----------------------------------------------------------------------===//
|
|
|
|
#include "lldb/Symbol/SaveCoreOptions.h"
|
|
#include "lldb/Core/PluginManager.h"
|
|
#include "lldb/Target/Process.h"
|
|
#include "lldb/Target/Thread.h"
|
|
|
|
using namespace lldb;
|
|
using namespace lldb_private;
|
|
|
|
Status SaveCoreOptions::SetPluginName(const char *name) {
|
|
Status error;
|
|
if (!name || !name[0]) {
|
|
m_plugin_name = std::nullopt;
|
|
return error;
|
|
}
|
|
|
|
std::vector<llvm::StringRef> plugin_names =
|
|
PluginManager::GetSaveCorePluginNames();
|
|
if (!llvm::is_contained(plugin_names, name)) {
|
|
StreamString stream;
|
|
stream.Printf("plugin name '%s' is not a valid ObjectFile plugin name.",
|
|
name);
|
|
|
|
if (!plugin_names.empty()) {
|
|
stream.PutCString(" Valid names are: ");
|
|
std::string plugin_names_str = llvm::join(plugin_names, ", ");
|
|
stream.PutCString(plugin_names_str);
|
|
stream.PutChar('.');
|
|
}
|
|
return Status(stream.GetString().str());
|
|
}
|
|
|
|
m_plugin_name = name;
|
|
return error;
|
|
}
|
|
|
|
void SaveCoreOptions::SetStyle(lldb::SaveCoreStyle style) { m_style = style; }
|
|
|
|
void SaveCoreOptions::SetOutputFile(FileSpec file) { m_file = file; }
|
|
|
|
std::optional<std::string> SaveCoreOptions::GetPluginName() const {
|
|
return m_plugin_name;
|
|
}
|
|
|
|
lldb::SaveCoreStyle SaveCoreOptions::GetStyle() const {
|
|
return m_style.value_or(lldb::eSaveCoreUnspecified);
|
|
}
|
|
|
|
const std::optional<lldb_private::FileSpec>
|
|
SaveCoreOptions::GetOutputFile() const {
|
|
return m_file;
|
|
}
|
|
|
|
Status SaveCoreOptions::SetProcess(lldb::ProcessSP process_sp) {
|
|
Status error;
|
|
if (!process_sp) {
|
|
ClearProcessSpecificData();
|
|
m_process_sp.reset();
|
|
return error;
|
|
}
|
|
|
|
if (!process_sp->IsValid()) {
|
|
error = Status::FromErrorString("Cannot assign an invalid process.");
|
|
return error;
|
|
}
|
|
|
|
// Don't clear any process specific data if the process is the same.
|
|
if (m_process_sp == process_sp)
|
|
return error;
|
|
|
|
ClearProcessSpecificData();
|
|
m_process_sp = process_sp;
|
|
return error;
|
|
}
|
|
|
|
Status SaveCoreOptions::AddThread(lldb::ThreadSP thread_sp) {
|
|
Status error;
|
|
if (!thread_sp) {
|
|
error = Status::FromErrorString("invalid thread");
|
|
return error;
|
|
}
|
|
|
|
if (m_process_sp) {
|
|
if (m_process_sp != thread_sp->GetProcess()) {
|
|
error = Status::FromErrorString(
|
|
"Cannot add a thread from a different process.");
|
|
return error;
|
|
}
|
|
} else {
|
|
m_process_sp = thread_sp->GetProcess();
|
|
}
|
|
|
|
m_threads_to_save.insert(thread_sp->GetID());
|
|
return error;
|
|
}
|
|
|
|
bool SaveCoreOptions::RemoveThread(lldb::ThreadSP thread_sp) {
|
|
return thread_sp && m_threads_to_save.erase(thread_sp->GetID()) > 0;
|
|
}
|
|
|
|
bool SaveCoreOptions::ShouldThreadBeSaved(lldb::tid_t tid) const {
|
|
// If the user specified no threads to save, then we save all threads.
|
|
if (m_threads_to_save.empty())
|
|
return true;
|
|
return m_threads_to_save.count(tid) > 0;
|
|
}
|
|
|
|
bool SaveCoreOptions::HasSpecifiedThreads() const {
|
|
return !m_threads_to_save.empty();
|
|
}
|
|
|
|
void SaveCoreOptions::AddMemoryRegionToSave(
|
|
const lldb_private::MemoryRegionInfo ®ion) {
|
|
m_regions_to_save.Insert(region.GetRange(), /*combine=*/true);
|
|
}
|
|
|
|
const MemoryRanges &SaveCoreOptions::GetCoreFileMemoryRanges() const {
|
|
return m_regions_to_save;
|
|
}
|
|
Status SaveCoreOptions::EnsureValidConfiguration() const {
|
|
Status error;
|
|
std::string error_str;
|
|
if (!m_threads_to_save.empty() && GetStyle() == lldb::eSaveCoreFull)
|
|
error_str += "Cannot save a full core with a subset of threads\n";
|
|
|
|
if (!m_process_sp)
|
|
error_str += "Need to assign a valid process\n";
|
|
|
|
if (!error_str.empty())
|
|
error = Status(error_str);
|
|
|
|
return error;
|
|
}
|
|
|
|
lldb_private::ThreadCollection::collection
|
|
SaveCoreOptions::GetThreadsToSave() const {
|
|
lldb_private::ThreadCollection::collection thread_collection;
|
|
// In cases where no process is set, such as when no threads are specified.
|
|
if (!m_process_sp)
|
|
return thread_collection;
|
|
|
|
ThreadList &thread_list = m_process_sp->GetThreadList();
|
|
for (const auto &tid : m_threads_to_save)
|
|
thread_collection.push_back(thread_list.FindThreadByID(tid));
|
|
|
|
return thread_collection;
|
|
}
|
|
|
|
llvm::Expected<lldb_private::CoreFileMemoryRanges>
|
|
SaveCoreOptions::GetMemoryRegionsToSave() {
|
|
Status error;
|
|
if (!m_process_sp)
|
|
return Status::FromErrorString("Requires a process to be set.").takeError();
|
|
|
|
error = EnsureValidConfiguration();
|
|
if (error.Fail())
|
|
return error.takeError();
|
|
|
|
CoreFileMemoryRanges ranges;
|
|
error = m_process_sp->CalculateCoreFileSaveRanges(*this, ranges);
|
|
if (error.Fail())
|
|
return error.takeError();
|
|
|
|
return ranges;
|
|
}
|
|
|
|
llvm::Expected<uint64_t> SaveCoreOptions::GetCurrentSizeInBytes() {
|
|
Status error;
|
|
if (!m_process_sp)
|
|
return Status::FromErrorString("Requires a process to be set.").takeError();
|
|
|
|
error = EnsureValidConfiguration();
|
|
if (error.Fail())
|
|
return error.takeError();
|
|
|
|
CoreFileMemoryRanges ranges;
|
|
error = m_process_sp->CalculateCoreFileSaveRanges(*this, ranges);
|
|
if (error.Fail())
|
|
return error.takeError();
|
|
|
|
llvm::Expected<lldb_private::CoreFileMemoryRanges> core_file_ranges_maybe =
|
|
GetMemoryRegionsToSave();
|
|
if (!core_file_ranges_maybe)
|
|
return core_file_ranges_maybe.takeError();
|
|
const lldb_private::CoreFileMemoryRanges &core_file_ranges =
|
|
*core_file_ranges_maybe;
|
|
uint64_t total_in_bytes = 0;
|
|
for (const auto &core_range : core_file_ranges)
|
|
total_in_bytes += core_range.data.range.size();
|
|
|
|
return total_in_bytes;
|
|
}
|
|
|
|
void SaveCoreOptions::ClearProcessSpecificData() {
|
|
// Deliberately not following the formatter style here to indicate that
|
|
// this method will be expanded in the future.
|
|
m_threads_to_save.clear();
|
|
}
|
|
|
|
void SaveCoreOptions::Clear() {
|
|
m_file = std::nullopt;
|
|
m_plugin_name = std::nullopt;
|
|
m_style = std::nullopt;
|
|
m_threads_to_save.clear();
|
|
m_process_sp.reset();
|
|
m_regions_to_save.Clear();
|
|
}
|