https://github.com/qiyao updated https://github.com/llvm/llvm-project/pull/212706
>From b5e1a0fcfa3935cbad087603035e6db6180b5dc1 Mon Sep 17 00:00:00 2001 From: Yao Qi <[email protected]> Date: Thu, 25 Jun 2026 16:57:15 +0100 Subject: [PATCH 1/2] [lldb][debugserver] Expedite the stopped frame's stack memory in jThreadsInfo `ReadStackMemory` only expedites the frame-pointer backchain, which is enough to unwind the stack but not to read a frame's locals: a local's bytes live between `$sp` and `$fp`, addresses that are never sent up. As a result, examining the stopped frame's locals at a public stop generated one memory-read packets. Add `ReadFrameZeroStackMemory`, which expedites the innermost frame's stack memory. Rather than a single `[$sp, $fp)` block (which would have to give up entirely on large frames), it covers where variables actually live using two anchored windows: `[$fp - k_expedite_stack_window, $fp)` for locals/spills and `[$sp, $sp + k_expedite_stack_window)` for params. k_expedite_stack_window = 512 is sized to cover the common case (variables sit within a few hundred bytes of their anchor) while bounding the per-frame cost to 2*k_expedite_stack_window bytes regardless of frame size. For a small frame (`$fp - $sp <= 2*k_expedite_stack_window`) the two windows would overlap, so a single contiguous `[$sp, $fp)` chunk is emitted instead, giving full coverage with no gap. The `$fp` side always stops at `$fp` exclusive so it does not duplicate the saved `{fp, lr}` pair already covered by the backchain expedite. Each window is emitted as its own chunk because lldb's L1 cache only serves a read fully contained in one expedited chunk. A debugger reads a frame's locals only for the selected frame of the thread that stopped, so this frame-0 window is emitted for the current thread only rather than every thread, avoiding stop-reply bloat proportional to thread count. `GetJSONThreadsInfo` now builds the `"memory"` array from both sources and emits it whenever either produced an entry. Add `JSONGenerator::Array::empty` to support that check. --- .../TestExpeditedStackMemory.py | 72 ++++++++------ lldb/tools/debugserver/source/JSONGenerator.h | 2 + lldb/tools/debugserver/source/RNBRemote.cpp | 95 ++++++++++++++++++- 3 files changed, 138 insertions(+), 31 deletions(-) diff --git a/lldb/test/API/macosx/expedited-stack-memory/TestExpeditedStackMemory.py b/lldb/test/API/macosx/expedited-stack-memory/TestExpeditedStackMemory.py index e8c8076699af7..e76750c13aa25 100644 --- a/lldb/test/API/macosx/expedited-stack-memory/TestExpeditedStackMemory.py +++ b/lldb/test/API/macosx/expedited-stack-memory/TestExpeditedStackMemory.py @@ -3,19 +3,21 @@ public stop. On Darwin, debugserver expedites the frame-pointer backchain (up to 256 frames, -for every thread) in the jThreadsInfo response at a public stop, and lldb seeds -those bytes into its memory cache. Consequences exercised here: +for every thread) and the stopped frame's stack memory in the jThreadsInfo +response at a public stop, and lldb seeds those bytes into its memory cache. +Consequences exercised here: * A backtrace (GetNumFrames() / GetFrameAtIndex() for every frame) is satisfied entirely from the expedited/cached backchain and sends no packets. With the cache disabled it must read the backchain frame by frame, which confirms the test is really exercising the unwinder's memory reads. - * Examining frame local variables the way an IDE does is NOT covered by the - expedite: the values live at addresses that were never sent up, so reading - them produces memory-read packets. This is checked two ways, mirroring - an IDE: examining only the selected frame's locals (frame 0, what a - variables view does on a stop) and examining every frame's locals (the + * Examining the stopped frame's locals is covered by the stack expedite, so + it sends no stack memory-read packets; heap buffers behind pointers are not + on the stack and are still read from the stub. Examining deeper frames' + locals does produce stack memory-read packets. This is checked two ways, + mirroring an IDE: examining only the selected frame's locals (frame 0, what + a variables view does on a stop) and examining every frame's locals (the "view all frames" case). """ @@ -50,24 +52,31 @@ def test_memory_reads_when_examining_frame0_locals(self): """Model an IDE stop: walk the whole stack (a backtrace / debug navigator) but examine the locals of only the selected frame 0. Frame 0 (func_e in main.c) carries scalar, aggregate, and - pointer-to-heap locals, so examining it alone reads both stack and - heap memory.""" - self.check_memory_reads_when_examining_locals(examine_all_frames=False) + pointer-to-heap locals. Its stack is expedited, so examining it + reads heap memory but no stack memory.""" + self.check_memory_reads_when_examining_locals( + examine_all_frames=False, expect_stack_reads=False + ) @skipUnlessDarwin def test_memory_reads_when_examining_all_frames_locals(self): """Model "view all frames": walk the whole stack and examine every - frame's locals. This reads the same variety of memory across several - frames.""" - self.check_memory_reads_when_examining_locals(examine_all_frames=True) + frame's locals. Only the stopped frame's stack is expedited, so the + deeper frames' locals still read stack memory.""" + self.check_memory_reads_when_examining_locals( + examine_all_frames=True, expect_stack_reads=True + ) - def check_memory_reads_when_examining_locals(self, examine_all_frames): - """Examining frame locals reads value memory that is not expedited. - Classify those reads into stack vs heap and check the counts. + def check_memory_reads_when_examining_locals( + self, examine_all_frames, expect_stack_reads + ): + """Examining frame locals reads value memory; classify those reads into + stack vs heap and check the counts. - The frame-pointer backchain is expedited, but the locals' *values* are - not, so both stack-resident locals and heap buffers behind pointers are - read from the stub today. + The frame-pointer backchain and the stopped frame's stack are expedited, + so frame 0's stack-resident locals are served from the cache while heap + buffers behind pointers (and deeper frames' locals) are read from the + stub. We have two regions and ask the process which one each read falls in: * the stack region: whichever region the stack pointer points into. @@ -131,15 +140,22 @@ def per_frame(idx, frame): ) ) - # Examining locals reads both stack and heap memory. - self.assertGreater( - len(stack_reads), - 0, - "expected stack memory reads while examining stack-resident " - "locals.\n" + breakdown, - ) - # Heap reads come from disclosing the pointer-to-heap local; a stack - # expedite would NOT remove these. + if expect_stack_reads: + # Deeper frames' stacks are not expedited. + self.assertGreater( + len(stack_reads), + 0, + "expected stack memory reads while examining deeper frames' " + "stack-resident locals.\n" + breakdown, + ) + else: + # The stopped frame's stack is expedited. + self.assertEqual( + len(stack_reads), + 0, + "expected NO stack memory reads for frame 0 (its stack is " + "expedited in jThreadsInfo).\n" + breakdown, + ) self.assertGreater( len(heap_reads), 0, diff --git a/lldb/tools/debugserver/source/JSONGenerator.h b/lldb/tools/debugserver/source/JSONGenerator.h index b545a6e29a767..114de9dd992af 100644 --- a/lldb/tools/debugserver/source/JSONGenerator.h +++ b/lldb/tools/debugserver/source/JSONGenerator.h @@ -127,6 +127,8 @@ class JSONGenerator { void AddItem(ObjectSP item) { m_items.push_back(item); } + bool empty() const { return m_items.empty(); } + void AddIntegerItem(uint64_t value) { AddItem(ObjectSP(new Integer(value))); } diff --git a/lldb/tools/debugserver/source/RNBRemote.cpp b/lldb/tools/debugserver/source/RNBRemote.cpp index 91d611f9ecbc5..a3ca5a73ddfc2 100644 --- a/lldb/tools/debugserver/source/RNBRemote.cpp +++ b/lldb/tools/debugserver/source/RNBRemote.cpp @@ -2733,6 +2733,76 @@ static void ReadStackMemory(nub_process_t pid, nub_thread_t tid, } } +// The size of each per-side stack window we expedite. 512 is sized to cover +// the common case (locals and params sit within a few hundred bytes) while +// bounding the per-frame cost to 1K bytes regardless of frame size. +static const nub_size_t k_expedite_stack_window = 512; + +// A single contiguous chunk of expedited memory. +struct ExpeditedMemory { + nub_addr_t addr; + std::vector<uint8_t> bytes; +}; + +// Read the innermost frame's stack memory so that examining its local variables +// at a public stop is served from the expedited cache instead of generating one +// memory-read packet. +// +// We produce either: +// +// - one chunk [$sp, $fp) for a small frame ($fp - $sp <= +// 2*k_expedite_stack_window), which covers the whole frame with no gap, or +// +// - two chunks [$sp, $sp + k_expedite_stack_window) and [$fp - +// k_expedite_stack_window, $fp) for a large frame, covering the params near +// $sp and the locals near $fp while leaving the (rarely-interesting) middle +// spill area out so the cost stays bounded. +static void ReadFrameZeroStackMemory(nub_process_t pid, nub_thread_t tid, + std::vector<ExpeditedMemory> &chunks) { + chunks.clear(); + std::unique_ptr<DNBRegisterValue> sp_value = + std::make_unique<DNBRegisterValue>(); + std::unique_ptr<DNBRegisterValue> fp_value = + std::make_unique<DNBRegisterValue>(); + if (!DNBThreadGetRegisterValueByID(pid, tid, REGISTER_SET_GENERIC, + GENERIC_REGNUM_SP, sp_value.get()) || + !DNBThreadGetRegisterValueByID(pid, tid, REGISTER_SET_GENERIC, + GENERIC_REGNUM_FP, fp_value.get())) + return; + + const nub_size_t ptr_size = sp_value->info.size; + 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; + + // The stack grows down, so a normal frame has sp < fp. Bail on a leaf/empty + // frame (sp == fp) or anything that doesn't look like a frame. + if (sp == 0 || fp <= sp) + return; + + auto read_range = [&](uint64_t start, uint64_t length) { + std::vector<uint8_t> buf(length); + if (DNBProcessMemoryRead(pid, start, length, buf.data()) != length) + return; + chunks.push_back({start, std::move(buf)}); + }; + + const uint64_t frame_size = fp - sp; + const nub_size_t window = k_expedite_stack_window; + + if (frame_size <= 2 * window) { + // Small frame: cover the whole frame as a single contiguous chunk [sp, fp). + read_range(sp, frame_size); + return; + } + + // Large frame: cover the params near $sp and the locals near $fp with two + // bounded windows, leaving the middle spill area out to keep the cost capped. + read_range(sp, window); // [sp, sp + WINDOW) + read_range(fp - window, window); // [fp - WINDOW, fp) +} + rnb_err_t RNBRemote::SendStopReplyPacketForThread(nub_thread_t tid) { const nub_process_t pid = m_ctx.ProcessID(); if (pid == INVALID_NUB_PROCESS) @@ -5866,9 +5936,10 @@ RNBRemote::GetJSONThreadsInfo(bool threads_with_valid_stop_info_only) { // frame pointer chain. StackMemoryMap stack_mmap; ReadStackMemory(pid, tid, stack_mmap); - if (!stack_mmap.empty()) { - JSONGenerator::ArraySP memory_array_sp(new JSONGenerator::Array()); + 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()); @@ -5877,9 +5948,27 @@ RNBRemote::GetJSONThreadsInfo(bool threads_with_valid_stop_info_only) { "bytes", stack_memory.second.bytes, stack_memory.second.length); memory_array_sp->AddItem(stack_memory_sp); } - thread_dict_sp->AddItem("memory", 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, frame_zero_chunks); + + 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); + } + } + + if (!memory_array_sp->empty()) + thread_dict_sp->AddItem("memory", memory_array_sp); + std::vector<uint64_t> added_binaries; JSONGenerator::ObjectSP detailed_binary_infos; >From 38bde0caa503fc4fe970ae58368e9ba47cd75e16 Mon Sep 17 00:00:00 2001 From: Yao Qi <[email protected]> Date: Thu, 30 Jul 2026 17:48:38 +0100 Subject: [PATCH 2/2] fixup! [lldb][debugserver] Expedite the stopped frame's stack memory in jThreadsInfo --- lldb/tools/debugserver/source/RNBRemote.cpp | 35 ++++++++++----------- 1 file changed, 16 insertions(+), 19 deletions(-) diff --git a/lldb/tools/debugserver/source/RNBRemote.cpp b/lldb/tools/debugserver/source/RNBRemote.cpp index a3ca5a73ddfc2..a92a3a3eab14a 100644 --- a/lldb/tools/debugserver/source/RNBRemote.cpp +++ b/lldb/tools/debugserver/source/RNBRemote.cpp @@ -2757,29 +2757,25 @@ struct ExpeditedMemory { // k_expedite_stack_window, $fp) for a large frame, covering the params near // $sp and the locals near $fp while leaving the (rarely-interesting) middle // spill area out so the cost stays bounded. -static void ReadFrameZeroStackMemory(nub_process_t pid, nub_thread_t tid, - std::vector<ExpeditedMemory> &chunks) { - chunks.clear(); - std::unique_ptr<DNBRegisterValue> sp_value = - std::make_unique<DNBRegisterValue>(); - std::unique_ptr<DNBRegisterValue> fp_value = - std::make_unique<DNBRegisterValue>(); +static std::vector<ExpeditedMemory> ReadFrameZeroStackMemory(nub_process_t pid, + nub_thread_t tid) { + std::vector<ExpeditedMemory> chunks; + DNBRegisterValue sp_value; + DNBRegisterValue fp_value; if (!DNBThreadGetRegisterValueByID(pid, tid, REGISTER_SET_GENERIC, - GENERIC_REGNUM_SP, sp_value.get()) || + GENERIC_REGNUM_SP, &sp_value) || !DNBThreadGetRegisterValueByID(pid, tid, REGISTER_SET_GENERIC, - GENERIC_REGNUM_FP, fp_value.get())) - return; + GENERIC_REGNUM_FP, &fp_value)) + return chunks; - const nub_size_t ptr_size = sp_value->info.size; - 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; + const nub_size_t ptr_size = sp_value.info.size; + 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; // The stack grows down, so a normal frame has sp < fp. Bail on a leaf/empty // frame (sp == fp) or anything that doesn't look like a frame. if (sp == 0 || fp <= sp) - return; + return chunks; auto read_range = [&](uint64_t start, uint64_t length) { std::vector<uint8_t> buf(length); @@ -2794,13 +2790,14 @@ static void ReadFrameZeroStackMemory(nub_process_t pid, nub_thread_t tid, if (frame_size <= 2 * window) { // Small frame: cover the whole frame as a single contiguous chunk [sp, fp). read_range(sp, frame_size); - return; + return chunks; } // Large frame: cover the params near $sp and the locals near $fp with two // bounded windows, leaving the middle spill area out to keep the cost capped. read_range(sp, window); // [sp, sp + WINDOW) read_range(fp - window, window); // [fp - WINDOW, fp) + return chunks; } rnb_err_t RNBRemote::SendStopReplyPacketForThread(nub_thread_t tid) { @@ -5953,8 +5950,8 @@ RNBRemote::GetJSONThreadsInfo(bool threads_with_valid_stop_info_only) { // 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, frame_zero_chunks); + std::vector<ExpeditedMemory> frame_zero_chunks = + ReadFrameZeroStackMemory(pid, tid); for (const auto &chunk : frame_zero_chunks) { JSONGenerator::DictionarySP frame_zero_sp( _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
