Skip to content

Commit 96b7c64

Browse files
JlalondJlalond
authored
[LLDB] Reapply SBSaveCore Add Memory List (#107937)
Recently in #107731 this change was revereted due to excess memory size in `TestSkinnyCore`. This was due to a bug where a range's end was being passed as size. Creating massive memory ranges. Additionally, and requiring additional review, I added more unit tests and more verbose logic to the merging of save core memory regions. @jasonmolenda as an FYI.
1 parent b7b28e7 commit 96b7c64

25 files changed

+635
-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: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -450,6 +450,12 @@ class RangeDataVector {
450450

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

453+
/// Append a range with data to the vector
454+
/// \param B The base of the memory range
455+
/// \param S The size of the memory range
456+
/// \param T The data associated with the memory range
457+
void Append(B &&b, S &&s, T &&t) { m_entries.emplace_back(Entry(b, s, t)); }
458+
453459
bool Erase(uint32_t start, uint32_t end) {
454460
if (start >= end || end > m_entries.size())
455461
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();

0 commit comments

Comments
 (0)