Avoid repeated cache fills for unreadable memory maps

news/guard-page-cache.bugfix

@@ -0,0 +1,2 @@
+Avoid repeated whole-map reads when a readable memory map contains an inaccessible guard page.
+Reads of accessible addresses in the same map now bypass the cache after the first failed cache fill.

src/pystack/_pystack.pyi

@@ -84,6 +84,12 @@ def get_process_threads_for_core(
 def get_bss_info(binary: Union[str, pathlib.Path]) -> Optional[Dict[str, Any]]: ...
 def _check_interpreter_shutdown(manager: ProcessManager) -> None: ...
 def is_eval_frame(symbol: str, python_version: Tuple[int, int]) -> bool: ...
+def _copy_memory_for_testing(
+    pid: int,
+    map_start: int,
+    map_end: int,
+    reads: List[Tuple[int, int]],
+) -> List[bytes]: ...
 def _normalize_threads_for_testing(
     thread_descs: List[Dict[str, Any]],
     native_mode: NativeReportingMode,

src/pystack/_pystack/bindings.cpp

@@ -1046,6 +1046,28 @@ NB_MODULE(_pystack, m)
           "python_version"_a,
           "Return True if the symbol is a CPython eval frame function");
 
+    m.def(
+            "_copy_memory_for_testing",
+            [](pid_t pid,
+               pystack::remote_addr_t map_start,
+               pystack::remote_addr_t map_end,
+               const std::vector<std::pair<pystack::remote_addr_t, size_t>>& reads) {
+                std::vector<pystack::VirtualMap> maps;
+                maps.emplace_back(map_start, map_end, map_end - map_start, "rw-p", 0, "00:00", 0, "");
+                pystack::ProcessMemoryManager manager(pid, maps);
+                nb::list result;
+                for (const auto& [address, size] : reads) {
+                    std::vector<char> buffer(size);
+                    manager.copyMemoryFromProcess(address, size, buffer.data());
+                    result.append(nb::bytes(buffer.data(), buffer.size()));
+                }
+                return result;
+            },
+            "pid"_a,
+            "map_start"_a,
+            "map_end"_a,
+            "reads"_a);
+
     m.def(
             "_normalize_threads_for_testing",
             [](nb::list thread_descs,

src/pystack/_pystack/mem.cpp

@@ -256,7 +256,9 @@ ProcessMemoryManager::copyMemoryFromProcess(remote_addr_t addr, size_t len, void
         return vmap.containsAddr(addr) && vmap.containsAddr(addr + len - 1);
     });
 
-    if (vmap == d_vmaps.end() || !d_lru_cache.can_fit(vmap->Size())) {
+    if (vmap == d_vmaps.end() || !d_lru_cache.can_fit(vmap->Size())
+        || d_uncacheable_vmaps.contains(vmap->Start()))
+    {
         return readChunk(addr, len, reinterpret_cast<char*>(dst));
     }
 
@@ -272,7 +274,8 @@ ProcessMemoryManager::copyMemoryFromProcess(remote_addr_t addr, size_t len, void
             d_lru_cache.put(key, std::move(buf));
         } catch (const InvalidRemoteAddress&) {
             // The full vmap read failed (e.g. guard pages in JIT mappings).
-            // Fall back to reading just the requested bytes directly.
+            // Future reads from this vmap should avoid the same failing cache fill.
+            d_uncacheable_vmaps.insert(key);
             return readChunk(addr, len, reinterpret_cast<char*>(dst));
         }
     }

src/pystack/_pystack/mem.h

@@ -10,6 +10,7 @@
 #include <string>
 #include <sys/mman.h>
 #include <sys/stat.h>
+#include <unordered_set>
 #include <vector>
 
 #include "elf_common.h"
@@ -174,6 +175,7 @@ class ProcessMemoryManager : public AbstractRemoteMemoryManager
     pid_t d_pid;
     std::vector<VirtualMap> d_vmaps;
     mutable LRUCache d_lru_cache;
+    mutable std::unordered_set<uintptr_t> d_uncacheable_vmaps;
     mutable file_unique_ptr d_memfile;
 
     // Methods

tests/unit/test_memory_manager.py

@@ -0,0 +1,37 @@
+import ctypes
+import mmap
+import os
+
+import pytest
+
+from pystack._pystack import _copy_memory_for_testing
+
+
+def test_reads_accessible_memory_from_map_with_guard_page(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    monkeypatch.delenv("_PYSTACK_NO_PROCESS_VM_READV", raising=False)
+    page_size = mmap.PAGESIZE
+    mapping = mmap.mmap(-1, page_size * 3)
+    address = ctypes.addressof(ctypes.c_char.from_buffer(mapping))
+    expected = b"pthread data"
+    requested_address = address + page_size
+    mapping[page_size : page_size + len(expected)] = expected
+
+    libc = ctypes.CDLL(None, use_errno=True)
+    mprotect = libc.mprotect
+    mprotect.argtypes = [ctypes.c_void_p, ctypes.c_size_t, ctypes.c_int]
+    mprotect.restype = ctypes.c_int
+    assert mprotect(address, page_size, 0) == 0
+    try:
+        result = _copy_memory_for_testing(
+            os.getpid(),
+            address,
+            address + len(mapping),
+            [(requested_address, len(expected)), (requested_address, len(expected))],
+        )
+    finally:
+        assert mprotect(address, page_size, mmap.PROT_READ | mmap.PROT_WRITE) == 0
+        mapping.close()
+
+    assert result == [expected, expected]