[Lldb-commits] [lldb] [lldb][debugserver][NFC] Share the expedited-memory type and emitters (PR #222911)
Yao Qi via lldb-commits
lldb-commits at lists.llvm.org
Fri Sep 11 03:33:03 PDT 2026
https://github.com/qiyao created https://github.com/llvm/llvm-project/pull/222911
Expedited memory had two representations:
- The frame pointer backchain used `StackMemory`, a fixed
`uint8_t[2 * sizeof(nub_addr_t)]` plus a length, held in a `std::map` keyed
by frame address.
- Frame 0's window used `ExpeditedMemory`, which owns a `std::vector<uint8_t>`.
Three call sites then open-coded the same emit loop, twice into a JSON array
and once into a stop reply.
Keep `ExpeditedMemory` for both and emit through `AppendExpeditedMemoryToJSON`
and `AppendExpeditedMemoryToStopReply`. `ReadFrameZeroStackMemory`'s local
`read_range` lambda becomes `AppendExpeditedMemory`. `ReadStackMemory` keeps
its own read, because it needs the bytes to continue the walk.
`ReadStackMemory` gains an explicit `visited` set. It previously relied on its
`std::map` to stop a corrupt frame pointer chain that loops back on itself, by
testing whether the key was already present. A `std::vector` has no such test,
so the guard becomes explicit. This keeps the existing behavior rather than
changing it.
Chunk count and chunk sizes are unchanged. Backchain chunks now go out in
frame-walk order rather than ascending frame address; the bytes for a given
address are the same either way, so the cache the client builds is unchanged.
This prepares for expediting the another memory read range, which have
arbitrary address and length. `StackMemory`'s fixed buffer cannot hold a range
of a few hundred bytes, and appending and emitting one costs three lines with
these functions in place rather than a fourth copy of the emit loop.
Assited-by: claude
>From d9edc5d4de373dcc36a290a78b87bdeff2f85433 Mon Sep 17 00:00:00 2001
From: Yao Qi <yao_qi at apple.com>
Date: Sat, 29 Aug 2026 16:51:45 +0100
Subject: [PATCH] [lldb][debugserver][NFC] Share the expedited-memory type and
emitters
Expedited memory had two representations:
- The frame pointer backchain used `StackMemory`, a fixed
`uint8_t[2 * sizeof(nub_addr_t)]` plus a length, held in a `std::map` keyed
by frame address.
- Frame 0's window used `ExpeditedMemory`, which owns a `std::vector<uint8_t>`.
Three call sites then open-coded the same emit loop, twice into a JSON array
and once into a stop reply.
Keep `ExpeditedMemory` for both and emit through `AppendExpeditedMemoryToJSON`
and `AppendExpeditedMemoryToStopReply`. `ReadFrameZeroStackMemory`'s local
`read_range` lambda becomes `AppendExpeditedMemory`. `ReadStackMemory` keeps
its own read, because it needs the bytes to continue the walk.
`ReadStackMemory` gains an explicit `visited` set. It previously relied on its
`std::map` to stop a corrupt frame pointer chain that loops back on itself, by
testing whether the key was already present. A `std::vector` has no such test,
so the guard becomes explicit. This keeps the existing behavior rather than
changing it.
Chunk count and chunk sizes are unchanged. Backchain chunks now go out in
frame-walk order rather than ascending frame address; the bytes for a given
address are the same either way, so the cache the client builds is unchanged.
This prepares for expediting the another memory read range, which have
arbitrary address and length. `StackMemory`'s fixed buffer cannot hold a range
of a few hundred bytes, and appending and emitting one costs three lines with
these functions in place rather than a fourth copy of the emit loop.
Assited-by: claude
---
lldb/tools/debugserver/source/RNBRemote.cpp | 130 ++++++++++----------
1 file changed, 62 insertions(+), 68 deletions(-)
diff --git a/lldb/tools/debugserver/source/RNBRemote.cpp b/lldb/tools/debugserver/source/RNBRemote.cpp
index 075ef8670ac98..a32b03fe894c2 100644
--- a/lldb/tools/debugserver/source/RNBRemote.cpp
+++ b/lldb/tools/debugserver/source/RNBRemote.cpp
@@ -26,6 +26,7 @@
#include <os/security_config.h>
#endif
#include <pwd.h>
+#include <set>
#include <string>
#include <sys/stat.h>
#include <sys/sysctl.h>
@@ -2687,14 +2688,49 @@ void RNBRemote::DispatchQueueOffsets::GetThreadQueueInfo(
}
}
-struct StackMemory {
- uint8_t bytes[2 * sizeof(nub_addr_t)];
- nub_size_t length;
+// A single contiguous chunk of expedited memory.
+struct ExpeditedMemory {
+ nub_addr_t addr;
+ std::vector<uint8_t> bytes;
};
-typedef std::map<nub_addr_t, StackMemory> StackMemoryMap;
+
+// Read one range and append it as an expedited chunk. A chunk is appended
+// only when the whole range was read, so its bytes always cover its address.
+static void AppendExpeditedMemory(nub_process_t pid, nub_addr_t addr,
+ nub_size_t size,
+ std::vector<ExpeditedMemory> &chunks) {
+ if (size == 0)
+ return;
+ std::vector<uint8_t> buf(size);
+ if (DNBProcessMemoryRead(pid, addr, size, buf.data()) != size)
+ return;
+ chunks.push_back({addr, std::move(buf)});
+}
+
+static void
+AppendExpeditedMemoryToJSON(const std::vector<ExpeditedMemory> &chunks,
+ JSONGenerator::ArraySP &memory_array_sp) {
+ for (const ExpeditedMemory &chunk : chunks) {
+ JSONGenerator::DictionarySP chunk_sp(new JSONGenerator::Dictionary());
+ chunk_sp->AddIntegerItem("address", chunk.addr);
+ chunk_sp->AddBytesAsHexASCIIString("bytes", chunk.bytes.data(),
+ chunk.bytes.size());
+ memory_array_sp->AddItem(chunk_sp);
+ }
+}
+
+static void
+AppendExpeditedMemoryToStopReply(const std::vector<ExpeditedMemory> &chunks,
+ std::ostringstream &ostrm) {
+ for (const ExpeditedMemory &chunk : chunks) {
+ ostrm << "memory:" << HEXBASE << chunk.addr << '=';
+ append_hex_value(ostrm, chunk.bytes.data(), chunk.bytes.size(), false);
+ ostrm << ';';
+ }
+}
static void ReadStackMemory(nub_process_t pid, nub_thread_t tid,
- StackMemoryMap &stack_mmap,
+ std::vector<ExpeditedMemory> &chunks,
uint32_t backtrace_limit = 256) {
std::unique_ptr<DNBRegisterValue> reg_value =
std::make_unique<DNBRegisterValue>();
@@ -2706,6 +2742,7 @@ static void ReadStackMemory(nub_process_t pid, nub_thread_t tid,
fp = reg_value->value.uint32;
else
fp = reg_value->value.uint64;
+ std::set<uint64_t> visited;
while (fp != 0) {
// Make sure we never recurse more than 256 times so we don't recurse too
// far or
@@ -2714,21 +2751,19 @@ static void ReadStackMemory(nub_process_t pid, nub_thread_t tid,
break;
const nub_size_t read_size = reg_value->info.size * 2;
- StackMemory stack_memory;
- stack_memory.length = read_size;
- if (DNBProcessMemoryRead(pid, fp, read_size, stack_memory.bytes) !=
- read_size)
+ std::vector<uint8_t> buf(read_size);
+ if (DNBProcessMemoryRead(pid, fp, read_size, buf.data()) != read_size)
break;
// Make sure we don't try to put the same stack memory in more than once
- if (stack_mmap.find(fp) != stack_mmap.end())
+ if (!visited.insert(fp).second)
break;
- // Put the entry into the cache
- stack_mmap[fp] = stack_memory;
+ const nub_addr_t frame_addr = fp;
// Dereference the frame pointer to get to the previous frame pointer
if (reg_value->info.size == 4)
- fp = ((uint32_t *)stack_memory.bytes)[0];
+ fp = ((uint32_t *)buf.data())[0];
else
- fp = ((uint64_t *)stack_memory.bytes)[0];
+ fp = ((uint64_t *)buf.data())[0];
+ chunks.push_back({frame_addr, std::move(buf)});
}
}
}
@@ -2745,12 +2780,6 @@ static const nub_size_t k_expedite_stack_arg_size = 160;
static_assert(k_expedite_stack_arg_size <= k_expedite_stack_window,
"above-fp arg window must fit within the total stack budget");
-// A single contiguous chunk of expedited memory.
-struct ExpeditedMemory {
- nub_addr_t addr;
- std::vector<uint8_t> bytes;
-};
-
// Heuristic to decide whether frame 0's $fp looks like a valid frame pointer.
static bool FrameZeroFPLooksValid(nub_process_t pid, nub_thread_t tid,
uint64_t sp, uint64_t fp,
@@ -2789,31 +2818,23 @@ static std::vector<ExpeditedMemory> ReadFrameZeroStackMemory(nub_process_t pid,
uint64_t sp = (ptr_size == 4) ? sp_value.value.uint32 : sp_value.value.uint64;
uint64_t fp = (ptr_size == 4) ? fp_value.value.uint32 : fp_value.value.uint64;
- auto read_range = [&](uint64_t start, uint64_t length) {
- if (length == 0)
- return;
- std::vector<uint8_t> buf(length);
- if (DNBProcessMemoryRead(pid, start, length, buf.data()) != length)
- return;
- chunks.push_back({start, std::move(buf)});
- };
-
if (FrameZeroFPLooksValid(pid, tid, sp, fp, ptr_size)) {
// above-fp: stack-passed params, skipping the already-expedited frame
// record.
- read_range(fp + 2 * ptr_size, k_expedite_stack_arg_size);
+ AppendExpeditedMemory(pid, fp + 2 * ptr_size, k_expedite_stack_arg_size,
+ chunks);
// below-fp: locals + spilled register args, clamped at $sp so a small frame
// reads only [sp, fp).
uint64_t below = std::min<uint64_t>(fp - sp, k_expedite_stack_window -
k_expedite_stack_arg_size);
- read_range(fp - below, below);
+ AppendExpeditedMemory(pid, fp - below, below, chunks);
return chunks;
}
// Frameless / cannot validate $fp: expedite a single window anchored at $sp.
if (sp != 0)
- read_range(sp, k_expedite_stack_window);
+ AppendExpeditedMemory(pid, sp, k_expedite_stack_window, chunks);
return chunks;
}
@@ -3063,18 +3084,10 @@ rnb_err_t RNBRemote::SendStopReplyPacketForThread(nub_thread_t tid) {
}
// Add expedited stack memory so stack backtracing doesn't need to read
- // anything from the
- // frame pointer chain.
- StackMemoryMap stack_mmap;
- ReadStackMemory(pid, tid, stack_mmap, 2);
- if (!stack_mmap.empty()) {
- for (const auto &stack_memory : stack_mmap) {
- ostrm << "memory:" << HEXBASE << stack_memory.first << '=';
- append_hex_value(ostrm, stack_memory.second.bytes,
- stack_memory.second.length, false);
- ostrm << ';';
- }
- }
+ // anything from the frame pointer chain.
+ std::vector<ExpeditedMemory> stack_chunks;
+ ReadStackMemory(pid, tid, stack_chunks, 2);
+ AppendExpeditedMemoryToStopReply(stack_chunks, ostrm);
std::vector<uint64_t> added_binaries;
JSONGenerator::ObjectSP detailed_binary_infos;
@@ -5948,38 +5961,19 @@ RNBRemote::GetJSONThreadsInfo(bool threads_with_valid_stop_info_only) {
}
// Add expedited stack memory so stack backtracing doesn't need to read
- // anything from the
- // frame pointer chain.
- StackMemoryMap stack_mmap;
- ReadStackMemory(pid, tid, stack_mmap);
+ // anything from the frame pointer chain.
+ std::vector<ExpeditedMemory> stack_chunks;
+ ReadStackMemory(pid, tid, stack_chunks);
JSONGenerator::ArraySP memory_array_sp(new JSONGenerator::Array());
-
- if (!stack_mmap.empty()) {
- for (const auto &stack_memory : stack_mmap) {
- JSONGenerator::DictionarySP stack_memory_sp(
- new JSONGenerator::Dictionary());
- stack_memory_sp->AddIntegerItem("address", stack_memory.first);
- stack_memory_sp->AddBytesAsHexASCIIString(
- "bytes", stack_memory.second.bytes, stack_memory.second.length);
- memory_array_sp->AddItem(stack_memory_sp);
- }
- }
+ AppendExpeditedMemoryToJSON(stack_chunks, memory_array_sp);
// Also expedite the innermost frame's stack memory of the thread that
// stopped.
if (tid == DNBProcessGetCurrentThread(pid)) {
std::vector<ExpeditedMemory> frame_zero_chunks =
ReadFrameZeroStackMemory(pid, tid);
-
- for (const auto &chunk : frame_zero_chunks) {
- JSONGenerator::DictionarySP frame_zero_sp(
- new JSONGenerator::Dictionary());
- frame_zero_sp->AddIntegerItem("address", chunk.addr);
- frame_zero_sp->AddBytesAsHexASCIIString("bytes", chunk.bytes.data(),
- chunk.bytes.size());
- memory_array_sp->AddItem(frame_zero_sp);
- }
+ AppendExpeditedMemoryToJSON(frame_zero_chunks, memory_array_sp);
}
if (!memory_array_sp->empty())
More information about the lldb-commits
mailing list