Skip to content

Commit d4d4e77

Browse files
JlalondJlalond
authored
[LLDB] Reappply SBSaveCore AddMemoryList (#107159)
Reapplies #106293, testing identified issue in the merging code. I used this opportunity to strip CoreFileMemoryRanges to it's own file and then add unit tests on it's behavior.
1 parent deba134 commit d4d4e77

25 files changed

+496
-67
lines changed

‎lldb/include/lldb/API/SBMemoryRegionInfo.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ class LLDB_API SBMemoryRegionInfo {
120120
private:
121121
friend class SBProcess;
122122
friend class SBMemoryRegionInfoList;
123-
123+
friend class SBSaveCoreOptions;
124124
friend class lldb_private::ScriptInterpreter;
125125

126126
lldb_private::MemoryRegionInfo &ref();

‎lldb/include/lldb/API/SBSaveCoreOptions.h

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,17 @@ class LLDB_API SBSaveCoreOptions {
8080
/// \return True if the thread was removed, false if it was not in the list.
8181
bool RemoveThread(lldb::SBThread thread);
8282

83+
/// Add a memory region to save in the core file.
84+
///
85+
/// \param region The memory region to save.
86+
/// \returns An empty SBError upon success, or an error if the region is
87+
/// invalid.
88+
/// \note Ranges that overlapped will be unioned into a single region, this
89+
/// also supercedes stack minification. Specifying full regions and a
90+
/// non-custom core style will include the specified regions and union them
91+
/// with all style specific regions.
92+
SBError AddMemoryRegionToSave(const SBMemoryRegionInfo &region);
93+
8394
/// Reset all options.
8495
void Clear();
8596

‎lldb/include/lldb/Symbol/SaveCoreOptions.h

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,15 @@
1010
#define LLDB_SOURCE_PLUGINS_OBJECTFILE_SaveCoreOPTIONS_H
1111

1212
#include "lldb/Utility/FileSpec.h"
13-
#include "lldb/lldb-forward.h"
14-
#include "lldb/lldb-types.h"
13+
#include "lldb/Utility/RangeMap.h"
1514

1615
#include <optional>
16+
#include <set>
1717
#include <string>
1818
#include <unordered_set>
1919

20+
using MemoryRanges = lldb_private::RangeVector<lldb::addr_t, lldb::addr_t>;
21+
2022
namespace lldb_private {
2123

2224
class SaveCoreOptions {
@@ -38,8 +40,12 @@ class SaveCoreOptions {
3840
Status AddThread(lldb::ThreadSP thread_sp);
3941
bool RemoveThread(lldb::ThreadSP thread_sp);
4042
bool ShouldThreadBeSaved(lldb::tid_t tid) const;
43+
bool HasSpecifiedThreads() const;
4144

4245
Status EnsureValidConfiguration(lldb::ProcessSP process_sp) const;
46+
const MemoryRanges &GetCoreFileMemoryRanges() const;
47+
48+
void AddMemoryRegionToSave(const lldb_private::MemoryRegionInfo &region);
4349

4450
void Clear();
4551

@@ -51,6 +57,7 @@ class SaveCoreOptions {
5157
std::optional<lldb::SaveCoreStyle> m_style;
5258
lldb::ProcessSP m_process_sp;
5359
std::unordered_set<lldb::tid_t> m_threads_to_save;
60+
MemoryRanges m_regions_to_save;
5461
};
5562
} // namespace lldb_private
5663

‎lldb/include/lldb/Target/CoreFileMemoryRanges.h

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
//===-- CoreFileMemoryRanges.h ----------------------------------*- C++ -*-===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
#include "lldb/Utility/RangeMap.h"
10+
#include "lldb/Utility/Status.h"
11+
12+
#include "llvm/ADT/AddressRanges.h"
13+
14+
#ifndef LLDB_TARGET_COREFILEMEMORYRANGES_H
15+
#define LLDB_TARGET_COREFILEMEMORYRANGES_H
16+
17+
namespace lldb_private {
18+
19+
struct CoreFileMemoryRange {
20+
llvm::AddressRange range; /// The address range to save into the core file.
21+
uint32_t lldb_permissions; /// A bit set of lldb::Permissions bits.
22+
23+
bool operator==(const CoreFileMemoryRange &rhs) const {
24+
return range == rhs.range && lldb_permissions == rhs.lldb_permissions;
25+
}
26+
27+
bool operator!=(const CoreFileMemoryRange &rhs) const {
28+
return !(*this == rhs);
29+
}
30+
31+
bool operator<(const CoreFileMemoryRange &rhs) const {
32+
if (range < rhs.range)
33+
return true;
34+
if (range == rhs.range)
35+
return lldb_permissions < rhs.lldb_permissions;
36+
return false;
37+
}
38+
};
39+
40+
class CoreFileMemoryRanges
41+
: public lldb_private::RangeDataVector<lldb::addr_t, lldb::addr_t,
42+
CoreFileMemoryRange> {
43+
public:
44+
/// Finalize and merge all overlapping ranges in this collection. Ranges
45+
/// will be seperated based on permissions.
46+
Status FinalizeCoreFileSaveRanges();
47+
};
48+
} // namespace lldb_private
49+
50+
#endif // LLDB_TARGET_COREFILEMEMORYRANGES_H

‎lldb/include/lldb/Target/Process.h

Lines changed: 2 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@
3535
#include "lldb/Host/ProcessLaunchInfo.h"
3636
#include "lldb/Host/ProcessRunLock.h"
3737
#include "lldb/Symbol/ObjectFile.h"
38+
#include "lldb/Symbol/SaveCoreOptions.h"
39+
#include "lldb/Target/CoreFileMemoryRanges.h"
3840
#include "lldb/Target/ExecutionContextScope.h"
3941
#include "lldb/Target/InstrumentationRuntime.h"
4042
#include "lldb/Target/Memory.h"
@@ -710,29 +712,6 @@ class Process : public std::enable_shared_from_this<Process>,
710712
/// is not supported by the plugin, error otherwise.
711713
virtual llvm::Expected<bool> SaveCore(llvm::StringRef outfile);
712714

713-
struct CoreFileMemoryRange {
714-
llvm::AddressRange range; /// The address range to save into the core file.
715-
uint32_t lldb_permissions; /// A bit set of lldb::Permissions bits.
716-
717-
bool operator==(const CoreFileMemoryRange &rhs) const {
718-
return range == rhs.range && lldb_permissions == rhs.lldb_permissions;
719-
}
720-
721-
bool operator!=(const CoreFileMemoryRange &rhs) const {
722-
return !(*this == rhs);
723-
}
724-
725-
bool operator<(const CoreFileMemoryRange &rhs) const {
726-
if (range < rhs.range)
727-
return true;
728-
if (range == rhs.range)
729-
return lldb_permissions < rhs.lldb_permissions;
730-
return false;
731-
}
732-
};
733-
734-
using CoreFileMemoryRanges = std::vector<CoreFileMemoryRange>;
735-
736715
/// Helper function for Process::SaveCore(...) that calculates the address
737716
/// ranges that should be saved. This allows all core file plug-ins to save
738717
/// consistent memory ranges given a \a core_style.

‎lldb/include/lldb/Utility/RangeMap.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -450,6 +450,8 @@ class RangeDataVector {
450450

451451
void Append(const Entry &entry) { m_entries.emplace_back(entry); }
452452

453+
void Append(B &&b, S &&s, T &&t) { m_entries.emplace_back(Entry(b, s, t)); }
454+
453455
bool Erase(uint32_t start, uint32_t end) {
454456
if (start >= end || end > m_entries.size())
455457
return false;

‎lldb/include/lldb/lldb-enumerations.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1222,6 +1222,7 @@ enum SaveCoreStyle {
12221222
eSaveCoreFull = 1,
12231223
eSaveCoreDirtyOnly = 2,
12241224
eSaveCoreStackOnly = 3,
1225+
eSaveCoreCustomOnly = 4,
12251226
};
12261227

12271228
/// Events that might happen during a trace session.

‎lldb/include/lldb/lldb-forward.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,7 @@ class StackFrameRecognizer;
207207
class StackFrameRecognizerManager;
208208
class StackID;
209209
class Status;
210+
class SaveCoreOptions;
210211
class StopInfo;
211212
class Stoppoint;
212213
class StoppointCallbackContext;

‎lldb/include/lldb/lldb-private-interfaces.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
#ifndef LLDB_LLDB_PRIVATE_INTERFACES_H
1010
#define LLDB_LLDB_PRIVATE_INTERFACES_H
1111

12-
#include "lldb/Symbol/SaveCoreOptions.h"
1312
#include "lldb/lldb-enumerations.h"
1413
#include "lldb/lldb-forward.h"
1514
#include "lldb/lldb-private-enumerations.h"

‎lldb/source/API/SBSaveCoreOptions.cpp

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
//===----------------------------------------------------------------------===//
88

99
#include "lldb/API/SBSaveCoreOptions.h"
10+
#include "lldb/API/SBMemoryRegionInfo.h"
1011
#include "lldb/Host/FileSystem.h"
1112
#include "lldb/Symbol/SaveCoreOptions.h"
1213
#include "lldb/Utility/Instrumentation.h"
@@ -89,6 +90,16 @@ bool SBSaveCoreOptions::RemoveThread(lldb::SBThread thread) {
8990
return m_opaque_up->RemoveThread(thread.GetSP());
9091
}
9192

93+
lldb::SBError
94+
SBSaveCoreOptions::AddMemoryRegionToSave(const SBMemoryRegionInfo &region) {
95+
LLDB_INSTRUMENT_VA(this, region);
96+
// Currently add memory region can't fail, so we always return a success
97+
// SBerror, but because these API's live forever, this is the most future
98+
// proof thing to do.
99+
m_opaque_up->AddMemoryRegionToSave(region.ref());
100+
return SBError();
101+
}
102+
92103
void SBSaveCoreOptions::Clear() {
93104
LLDB_INSTRUMENT_VA(this);
94105
m_opaque_up->Clear();

‎lldb/source/Commands/CommandObjectProcess.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
#include "lldb/Interpreter/OptionArgParser.h"
2626
#include "lldb/Interpreter/OptionGroupPythonClassWithDict.h"
2727
#include "lldb/Interpreter/Options.h"
28+
#include "lldb/Symbol/SaveCoreOptions.h"
2829
#include "lldb/Target/Platform.h"
2930
#include "lldb/Target/Process.h"
3031
#include "lldb/Target/StopInfo.h"

‎lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6562,13 +6562,15 @@ bool ObjectFileMachO::SaveCore(const lldb::ProcessSP &process_sp,
65626562
}
65636563

65646564
if (make_core) {
6565-
Process::CoreFileMemoryRanges core_ranges;
6565+
CoreFileMemoryRanges core_ranges;
65666566
error = process_sp->CalculateCoreFileSaveRanges(options, core_ranges);
65676567
if (error.Success()) {
65686568
const uint32_t addr_byte_size = target_arch.GetAddressByteSize();
65696569
const ByteOrder byte_order = target_arch.GetByteOrder();
65706570
std::vector<llvm::MachO::segment_command_64> segment_load_commands;
6571-
for (const auto &core_range : core_ranges) {
6571+
for (const auto &core_range_info : core_ranges) {
6572+
// TODO: Refactor RangeDataVector to have a data iterator.
6573+
const auto &core_range = core_range_info.data;
65726574
uint32_t cmd_type = LC_SEGMENT_64;
65736575
uint32_t segment_size = sizeof(llvm::MachO::segment_command_64);
65746576
if (addr_byte_size == 4) {

‎lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
#include "lldb/Core/Address.h"
1313
#include "lldb/Host/SafeMachO.h"
1414
#include "lldb/Symbol/ObjectFile.h"
15+
#include "lldb/Symbol/SaveCoreOptions.h"
1516
#include "lldb/Utility/FileSpec.h"
1617
#include "lldb/Utility/FileSpecList.h"
1718
#include "lldb/Utility/RangeMap.h"

‎lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp

Lines changed: 21 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -831,25 +831,32 @@ Status MinidumpFileBuilder::AddMemoryList() {
831831
// bytes of the core file. Thread structures in minidump files can only use
832832
// 32 bit memory descriptiors, so we emit them first to ensure the memory is
833833
// in accessible with a 32 bit offset.
834-
Process::CoreFileMemoryRanges ranges_32;
835-
Process::CoreFileMemoryRanges ranges_64;
836-
Process::CoreFileMemoryRanges all_core_memory_ranges;
834+
std::vector<CoreFileMemoryRange> ranges_32;
835+
std::vector<CoreFileMemoryRange> ranges_64;
836+
CoreFileMemoryRanges all_core_memory_ranges;
837837
error = m_process_sp->CalculateCoreFileSaveRanges(m_save_core_options,
838838
all_core_memory_ranges);
839+
840+
std::vector<CoreFileMemoryRange> all_core_memory_vec;
841+
// Extract all the data into just a vector of data. So we can mutate this in
842+
// place.
843+
for (const auto &core_range : all_core_memory_ranges)
844+
all_core_memory_vec.push_back(core_range.data);
845+
839846
if (error.Fail())
840847
return error;
841848

842849
// Start by saving all of the stacks and ensuring they fit under the 32b
843850
// limit.
844851
uint64_t total_size = GetCurrentDataEndOffset();
845-
auto iterator = all_core_memory_ranges.begin();
846-
while (iterator != all_core_memory_ranges.end()) {
852+
auto iterator = all_core_memory_vec.begin();
853+
while (iterator != all_core_memory_vec.end()) {
847854
if (m_saved_stack_ranges.count(iterator->range.start()) > 0) {
848855
// We don't save stacks twice.
849856
ranges_32.push_back(*iterator);
850857
total_size +=
851858
iterator->range.size() + sizeof(llvm::minidump::MemoryDescriptor);
852-
iterator = all_core_memory_ranges.erase(iterator);
859+
iterator = all_core_memory_vec.erase(iterator);
853860
} else {
854861
iterator++;
855862
}
@@ -869,11 +876,11 @@ Status MinidumpFileBuilder::AddMemoryList() {
869876
// Then anything overflow extends into 64b addressable space.
870877
// All core memeroy ranges will either container nothing on stacks only
871878
// or all the memory ranges including stacks
872-
if (!all_core_memory_ranges.empty())
873-
total_size += 256 + (all_core_memory_ranges.size() *
879+
if (!all_core_memory_vec.empty())
880+
total_size += 256 + (all_core_memory_vec.size() *
874881
sizeof(llvm::minidump::MemoryDescriptor_64));
875882

876-
for (const auto &core_range : all_core_memory_ranges) {
883+
for (const auto &core_range : all_core_memory_vec) {
877884
const addr_t range_size = core_range.range.size();
878885
// We don't need to check for stacks here because we already removed them
879886
// from all_core_memory_ranges.
@@ -958,15 +965,15 @@ Status MinidumpFileBuilder::DumpDirectories() const {
958965
}
959966

960967
static uint64_t
961-
GetLargestRangeSize(const Process::CoreFileMemoryRanges &ranges) {
968+
GetLargestRangeSize(const std::vector<CoreFileMemoryRange> &ranges) {
962969
uint64_t max_size = 0;
963970
for (const auto &core_range : ranges)
964971
max_size = std::max(max_size, core_range.range.size());
965972
return max_size;
966973
}
967974

968-
Status
969-
MinidumpFileBuilder::AddMemoryList_32(Process::CoreFileMemoryRanges &ranges) {
975+
Status MinidumpFileBuilder::AddMemoryList_32(
976+
std::vector<CoreFileMemoryRange> &ranges) {
970977
std::vector<MemoryDescriptor> descriptors;
971978
Status error;
972979
if (ranges.size() == 0)
@@ -1042,8 +1049,8 @@ MinidumpFileBuilder::AddMemoryList_32(Process::CoreFileMemoryRanges &ranges) {
10421049
return error;
10431050
}
10441051

1045-
Status
1046-
MinidumpFileBuilder::AddMemoryList_64(Process::CoreFileMemoryRanges &ranges) {
1052+
Status MinidumpFileBuilder::AddMemoryList_64(
1053+
std::vector<CoreFileMemoryRange> &ranges) {
10471054
Status error;
10481055
if (ranges.empty())
10491056
return error;

‎lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
#include <utility>
2424
#include <variant>
2525

26+
#include "lldb/Symbol/SaveCoreOptions.h"
2627
#include "lldb/Target/Process.h"
2728
#include "lldb/Target/Target.h"
2829
#include "lldb/Utility/DataBufferHeap.h"
@@ -120,9 +121,9 @@ class MinidumpFileBuilder {
120121
lldb_private::Status AddData(const void *data, uint64_t size);
121122
// Add MemoryList stream, containing dumps of important memory segments
122123
lldb_private::Status
123-
AddMemoryList_64(lldb_private::Process::CoreFileMemoryRanges &ranges);
124+
AddMemoryList_64(std::vector<lldb_private::CoreFileMemoryRange> &ranges);
124125
lldb_private::Status
125-
AddMemoryList_32(lldb_private::Process::CoreFileMemoryRanges &ranges);
126+
AddMemoryList_32(std::vector<lldb_private::CoreFileMemoryRange> &ranges);
126127
// Update the thread list on disk with the newly emitted stack RVAs.
127128
lldb_private::Status FixThreadStacks();
128129
lldb_private::Status FlushBufferToDisk();

‎lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
#define LLDB_SOURCE_PLUGINS_OBJECTFILE_MINIDUMP_OBJECTFILEMINIDUMP_H
2222

2323
#include "lldb/Symbol/ObjectFile.h"
24+
#include "lldb/Symbol/SaveCoreOptions.h"
2425
#include "lldb/Utility/ArchSpec.h"
2526

2627
class ObjectFileMinidump : public lldb_private::PluginInterface {

‎lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#include "lldb/Interpreter/OptionValueDictionary.h"
1818
#include "lldb/Interpreter/OptionValueProperties.h"
1919
#include "lldb/Symbol/ObjectFile.h"
20+
#include "lldb/Symbol/SaveCoreOptions.h"
2021
#include "lldb/Target/Process.h"
2122
#include "lldb/Target/SectionLoadList.h"
2223
#include "lldb/Target/Target.h"

‎lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
#include <vector>
1414

1515
#include "lldb/Symbol/ObjectFile.h"
16+
#include "lldb/Symbol/SaveCoreOptions.h"
1617
#include "llvm/Object/COFF.h"
1718

1819
class ObjectFilePECOFF : public lldb_private::ObjectFile {

0 commit comments

Comments
 (0)