Skip to content

Commit e79e656

Browse files
JlalondJlalond
committed
Move the CoreFileMemoryRanges to it's own class, and add some unit tests. Fix the big where we only update the range on data, not the entry list
1 parent 63a343d commit e79e656

File tree

10 files changed

+224
-71
lines changed

10 files changed

+224
-71
lines changed

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

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
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+
41+
class CoreFileMemoryRanges : public lldb_private::RangeDataVector<lldb::addr_t, lldb::addr_t, CoreFileMemoryRange> {
42+
public:
43+
/// Finalize and merge all overlapping ranges in this collection. Ranges
44+
/// will be seperated based on permissions.
45+
Status FinalizeCoreFileSaveRanges();
46+
};
47+
} // namespace lldb_private
48+
49+
#endif // LLDB_TARGET_COREFILEMEMORYRANGES_H

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

Lines changed: 1 addition & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
#include "lldb/Host/ProcessRunLock.h"
3737
#include "lldb/Symbol/ObjectFile.h"
3838
#include "lldb/Symbol/SaveCoreOptions.h"
39+
#include "lldb/Target/CoreFileMemoryRanges.h"
3940
#include "lldb/Target/ExecutionContextScope.h"
4041
#include "lldb/Target/InstrumentationRuntime.h"
4142
#include "lldb/Target/Memory.h"
@@ -711,31 +712,6 @@ class Process : public std::enable_shared_from_this<Process>,
711712
/// is not supported by the plugin, error otherwise.
712713
virtual llvm::Expected<bool> SaveCore(llvm::StringRef outfile);
713714

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

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6562,7 +6562,7 @@ 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();

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

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -828,13 +828,13 @@ Status MinidumpFileBuilder::AddMemoryList() {
828828
// bytes of the core file. Thread structures in minidump files can only use
829829
// 32 bit memory descriptiors, so we emit them first to ensure the memory is
830830
// in accessible with a 32 bit offset.
831-
std::vector<Process::CoreFileMemoryRange> ranges_32;
832-
std::vector<Process::CoreFileMemoryRange> ranges_64;
833-
Process::CoreFileMemoryRanges all_core_memory_ranges;
831+
std::vector<CoreFileMemoryRange> ranges_32;
832+
std::vector<CoreFileMemoryRange> ranges_64;
833+
CoreFileMemoryRanges all_core_memory_ranges;
834834
error = m_process_sp->CalculateCoreFileSaveRanges(m_save_core_options,
835835
all_core_memory_ranges);
836836

837-
std::vector<Process::CoreFileMemoryRange> all_core_memory_vec;
837+
std::vector<CoreFileMemoryRange> all_core_memory_vec;
838838
// Extract all the data into just a vector of data. So we can mutate this in
839839
// place.
840840
for (const auto &core_range : all_core_memory_ranges)
@@ -962,15 +962,15 @@ Status MinidumpFileBuilder::DumpDirectories() const {
962962
}
963963

964964
static uint64_t
965-
GetLargestRangeSize(const std::vector<Process::CoreFileMemoryRange> &ranges) {
965+
GetLargestRangeSize(const std::vector<CoreFileMemoryRange> &ranges) {
966966
uint64_t max_size = 0;
967967
for (const auto &core_range : ranges)
968968
max_size = std::max(max_size, core_range.range.size());
969969
return max_size;
970970
}
971971

972972
Status MinidumpFileBuilder::AddMemoryList_32(
973-
std::vector<Process::CoreFileMemoryRange> &ranges) {
973+
std::vector<CoreFileMemoryRange> &ranges) {
974974
std::vector<MemoryDescriptor> descriptors;
975975
Status error;
976976
if (ranges.size() == 0)
@@ -1047,7 +1047,7 @@ Status MinidumpFileBuilder::AddMemoryList_32(
10471047
}
10481048

10491049
Status MinidumpFileBuilder::AddMemoryList_64(
1050-
std::vector<Process::CoreFileMemoryRange> &ranges) {
1050+
std::vector<CoreFileMemoryRange> &ranges) {
10511051
Status error;
10521052
if (ranges.empty())
10531053
return error;

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -121,9 +121,9 @@ class MinidumpFileBuilder {
121121
lldb_private::Status AddData(const void *data, uint64_t size);
122122
// Add MemoryList stream, containing dumps of important memory segments
123123
lldb_private::Status AddMemoryList_64(
124-
std::vector<lldb_private::Process::CoreFileMemoryRange> &ranges);
124+
std::vector<lldb_private::CoreFileMemoryRange> &ranges);
125125
lldb_private::Status AddMemoryList_32(
126-
std::vector<lldb_private::Process::CoreFileMemoryRange> &ranges);
126+
std::vector<lldb_private::CoreFileMemoryRange> &ranges);
127127
// Update the thread list on disk with the newly emitted stack RVAs.
128128
lldb_private::Status FixThreadStacks();
129129
lldb_private::Status FlushBufferToDisk();

‎lldb/source/Target/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ add_lldb_library(lldbTarget
1111
ABI.cpp
1212
AssertFrameRecognizer.cpp
1313
DynamicRegisterInfo.cpp
14+
CoreFileMemoryRanges.cpp
1415
ExecutionContext.cpp
1516
InstrumentationRuntime.cpp
1617
InstrumentationRuntimeStopInfo.cpp

‎lldb/source/Target/CoreFileMemoryRanges.cpp

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
//===-- CoreFileMemoryRanges.cpp --------------------------------*- 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/Target/CoreFileMemoryRanges.h"
10+
11+
using namespace lldb;
12+
using namespace lldb_private;
13+
14+
Status CoreFileMemoryRanges::FinalizeCoreFileSaveRanges() {
15+
Status error;
16+
std::vector<size_t> indexes_to_remove;
17+
this->Sort();
18+
for (size_t i = this->GetSize() - 1; i > 0; i--) {
19+
auto region = this->GetMutableEntryAtIndex(i);
20+
auto next_region = this->GetMutableEntryAtIndex(i - 1);
21+
if (next_region->GetRangeEnd() >= region->GetRangeBase() &&
22+
region->GetRangeBase() <= next_region->GetRangeEnd() &&
23+
region->data.lldb_permissions == next_region->data.lldb_permissions) {
24+
const addr_t base =
25+
std::min(region->GetRangeBase(), next_region->GetRangeBase());
26+
const addr_t byte_size =
27+
std::max(region->GetRangeEnd(), next_region->GetRangeEnd()) - base;
28+
29+
next_region->SetRangeBase(base);
30+
next_region->SetByteSize(byte_size);
31+
32+
// Because this is a range data vector, the entry has a base as well
33+
// as the data contained in the entry. So we have to update both.
34+
// And llvm::AddressRange isn't mutable so we have to create a new one.
35+
llvm::AddressRange range (base, base + byte_size);
36+
const CoreFileMemoryRange core_range = {range, next_region->data.lldb_permissions};
37+
next_region->data = core_range;
38+
if (!this->Erase(i, i + 1)) {
39+
error = Status::FromErrorString(
40+
"Core file memory ranges mutated outside of "
41+
"CalculateCoreFileSaveRanges");
42+
return error;
43+
}
44+
}
45+
}
46+
47+
return error;
48+
}

‎lldb/source/Target/Process.cpp

Lines changed: 9 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -6498,7 +6498,7 @@ Status Process::WriteMemoryTags(lldb::addr_t addr, size_t len,
64986498
}
64996499

65006500
// Create a CoreFileMemoryRange from a MemoryRegionInfo
6501-
static Process::CoreFileMemoryRange
6501+
static CoreFileMemoryRange
65026502
CreateCoreFileMemoryRange(const MemoryRegionInfo &region) {
65036503
const addr_t addr = region.GetRange().GetRangeBase();
65046504
llvm::AddressRange range(addr, addr + region.GetRange().GetByteSize());
@@ -6509,7 +6509,7 @@ CreateCoreFileMemoryRange(const MemoryRegionInfo &region) {
65096509
// were added. Return false if the dirty page information is not valid or in
65106510
// the region.
65116511
static bool AddDirtyPages(const MemoryRegionInfo &region,
6512-
Process::CoreFileMemoryRanges &ranges) {
6512+
CoreFileMemoryRanges &ranges) {
65136513
const auto &dirty_page_list = region.GetDirtyPageList();
65146514
if (!dirty_page_list)
65156515
return false;
@@ -6548,7 +6548,7 @@ static bool AddDirtyPages(const MemoryRegionInfo &region,
65486548
// will be added to \a ranges, else the entire range will be added to \a
65496549
// ranges.
65506550
static void AddRegion(const MemoryRegionInfo &region, bool try_dirty_pages,
6551-
Process::CoreFileMemoryRanges &ranges) {
6551+
CoreFileMemoryRanges &ranges) {
65526552
// Don't add empty ranges.
65536553
if (region.GetRange().GetByteSize() == 0)
65546554
return;
@@ -6565,7 +6565,7 @@ static void AddRegion(const MemoryRegionInfo &region, bool try_dirty_pages,
65656565

65666566
static void SaveOffRegionsWithStackPointers(
65676567
Process &process, const SaveCoreOptions &core_options,
6568-
const MemoryRegionInfos &regions, Process::CoreFileMemoryRanges &ranges,
6568+
const MemoryRegionInfos &regions, CoreFileMemoryRanges &ranges,
65696569
std::set<addr_t> &stack_ends) {
65706570
const bool try_dirty_pages = true;
65716571

@@ -6606,7 +6606,7 @@ static void SaveOffRegionsWithStackPointers(
66066606
// for a full core file style.
66076607
static void GetCoreFileSaveRangesFull(Process &process,
66086608
const MemoryRegionInfos &regions,
6609-
Process::CoreFileMemoryRanges &ranges,
6609+
CoreFileMemoryRanges &ranges,
66106610
std::set<addr_t> &stack_ends) {
66116611

66126612
// Don't add only dirty pages, add full regions.
@@ -6622,7 +6622,7 @@ static void GetCoreFileSaveRangesFull(Process &process,
66226622
// page information fall back to saving out all ranges with write permissions.
66236623
static void GetCoreFileSaveRangesDirtyOnly(
66246624
Process &process, const MemoryRegionInfos &regions,
6625-
Process::CoreFileMemoryRanges &ranges, std::set<addr_t> &stack_ends) {
6625+
CoreFileMemoryRanges &ranges, std::set<addr_t> &stack_ends) {
66266626

66276627
// Iterate over the regions and find all dirty pages.
66286628
bool have_dirty_page_info = false;
@@ -6653,7 +6653,7 @@ static void GetCoreFileSaveRangesDirtyOnly(
66536653
// stack region.
66546654
static void GetCoreFileSaveRangesStackOnly(
66556655
Process &process, const MemoryRegionInfos &regions,
6656-
Process::CoreFileMemoryRanges &ranges, std::set<addr_t> &stack_ends) {
6656+
CoreFileMemoryRanges &ranges, std::set<addr_t> &stack_ends) {
66576657
const bool try_dirty_pages = true;
66586658
// Some platforms support annotating the region information that tell us that
66596659
// it comes from a thread stack. So look for those regions first.
@@ -6668,7 +6668,7 @@ static void GetCoreFileSaveRangesStackOnly(
66686668

66696669
static void GetUserSpecifiedCoreFileSaveRanges(
66706670
Process &process, const MemoryRegionInfos &regions,
6671-
const SaveCoreOptions &options, Process::CoreFileMemoryRanges &ranges) {
6671+
const SaveCoreOptions &options, CoreFileMemoryRanges &ranges) {
66726672
const auto &option_ranges = options.GetCoreFileMemoryRanges();
66736673
if (option_ranges.IsEmpty())
66746674
return;
@@ -6682,33 +6682,6 @@ static void GetUserSpecifiedCoreFileSaveRanges(
66826682
}
66836683
}
66846684

6685-
static Status
6686-
FinalizeCoreFileSaveRanges(Process::CoreFileMemoryRanges &ranges) {
6687-
Status error;
6688-
ranges.Sort();
6689-
for (size_t i = ranges.GetSize() - 1; i > 0; i--) {
6690-
auto region = ranges.GetMutableEntryAtIndex(i);
6691-
auto next_region = ranges.GetMutableEntryAtIndex(i - 1);
6692-
if (next_region->GetRangeEnd() >= region->GetRangeBase() &&
6693-
region->GetRangeBase() <= next_region->GetRangeEnd() &&
6694-
region->data.lldb_permissions == next_region->data.lldb_permissions) {
6695-
const addr_t base =
6696-
std::min(region->GetRangeBase(), next_region->GetRangeBase());
6697-
const addr_t byte_size =
6698-
std::max(region->GetRangeEnd(), next_region->GetRangeEnd()) - base;
6699-
next_region->SetRangeBase(base);
6700-
next_region->SetByteSize(byte_size);
6701-
if (!ranges.Erase(i, i + 1)) {
6702-
error = Status::FromErrorString(
6703-
"Core file memory ranges mutated outside of "
6704-
"CalculateCoreFileSaveRanges");
6705-
return error;
6706-
}
6707-
}
6708-
}
6709-
return error;
6710-
}
6711-
67126685
Status Process::CalculateCoreFileSaveRanges(const SaveCoreOptions &options,
67136686
CoreFileMemoryRanges &ranges) {
67146687
lldb_private::MemoryRegionInfos regions;
@@ -6758,7 +6731,7 @@ Status Process::CalculateCoreFileSaveRanges(const SaveCoreOptions &options,
67586731
return Status::FromErrorString(
67596732
"no valid address ranges found for core style");
67606733

6761-
return FinalizeCoreFileSaveRanges(ranges);
6734+
return ranges.FinalizeCoreFileSaveRanges();
67626735
}
67636736

67646737
std::vector<ThreadSP>

‎lldb/unittests/Process/Utility/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ add_lldb_unittest(ProcessUtilityTests
1818
LinuxProcMapsTest.cpp
1919
MemoryTagManagerAArch64MTETest.cpp
2020
RegisterContextTest.cpp
21+
CoreFileMemoryRangesTest.cpp
2122
${PLATFORM_SOURCES}
2223

2324
LINK_LIBS

0 commit comments

Comments
 (0)