better stats, better err handling

Author: maurycy

Lib/test/test_external_inspection.py

@@ -3777,7 +3777,6 @@ def test_get_stats(self):
             "alias_remap_failures",
             "alias_validation_fails",
             "alias_evictions",
-            "alias_identity_mismatches",
             "alias_probe_checks",
             "alias_probe_recycles",
             "batched_read_success_rate",

Modules/_remote_debugging/_remote_debugging.h

@@ -310,7 +310,6 @@ typedef struct {
     uint64_t alias_remap_failures;           // macOS remap/protect failures
     uint64_t alias_validation_fails;         // macOS alias snapshot validation failures
     uint64_t alias_evictions;                // macOS alias-cache LRU evictions
-    uint64_t alias_identity_mismatches;      // macOS target identity mismatches
     uint64_t alias_probe_checks;             // macOS alias object identity probes
     uint64_t alias_probe_recycles;           // macOS alias recycled-page detections
 } UnwinderStats;
@@ -695,6 +694,10 @@ extern int _Py_RemoteDebug_ValidateThreadStateSnapshot(
     uintptr_t tstate_addr,
     uintptr_t current_interpreter
 );
+/* Returns 0 if bytes were served from an aliased page snapshot (callers
+ * should validate), 1 if no snapshot was involved (live syscall read or
+ * zero-length no-op; skip validation), -1 on error with an exception
+ * set. */
 extern int _Py_RemoteDebug_AliasedRead(
     RemoteUnwinderObject *unwinder,
     uintptr_t remote_addr,

Modules/_remote_debugging/alias_read.c

@@ -259,27 +259,36 @@ _Py_RemoteDebug_AliasedRead(RemoteUnwinderObject *unwinder,
 {
     AliasReadCache *cache = &unwinder->alias_cache;
     if (len == 0) {
-        return 0;
+        return 1;
     }
     if (cache->disabled) {
-        return _Py_RemoteDebug_ReadRemoteMemory(
-            &unwinder->handle, remote_addr, len, dst);
+        if (_Py_RemoteDebug_ReadRemoteMemory(
+                &unwinder->handle, remote_addr, len, dst) < 0) {
+            return -1;
+        }
+        return 1;
     }
 
     size_t page_size = (size_t)unwinder->handle.page_size;
     uintptr_t page_base = remote_addr & ~(uintptr_t)(page_size - 1);
     size_t offset = (size_t)(remote_addr - page_base);
     if (offset >= page_size || len > page_size - offset) {
-        return _Py_RemoteDebug_ReadRemoteMemory(
-            &unwinder->handle, remote_addr, len, dst);
+        if (_Py_RemoteDebug_ReadRemoteMemory(
+                &unwinder->handle, remote_addr, len, dst) < 0) {
+            return -1;
+        }
+        return 1;
     }
 
     AliasPageEntry *entry = alias_find_entry(unwinder, page_base);
     if (entry != NULL) {
         int probe = alias_maybe_probe_entry(unwinder, entry);
         if (probe < 0) {
-            return _Py_RemoteDebug_ReadRemoteMemory(
-                &unwinder->handle, remote_addr, len, dst);
+            if (_Py_RemoteDebug_ReadRemoteMemory(
+                    &unwinder->handle, remote_addr, len, dst) < 0) {
+                return -1;
+            }
+            return 1;
         }
         if (probe == 0) {
             PyErr_SetString(PyExc_RuntimeError,
@@ -296,8 +305,11 @@ _Py_RemoteDebug_AliasedRead(RemoteUnwinderObject *unwinder,
 
     STATS_INC(unwinder, alias_misses);
     if (alias_remap_page(unwinder, page_base, &entry) < 0) {
-        return _Py_RemoteDebug_ReadRemoteMemory(
-            &unwinder->handle, remote_addr, len, dst);
+        if (_Py_RemoteDebug_ReadRemoteMemory(
+                &unwinder->handle, remote_addr, len, dst) < 0) {
+            return -1;
+        }
+        return 1;
     }
     memcpy(dst, (const char *)entry->local_page_base + offset, len);
     return 0;

Modules/_remote_debugging/clinic/module.c.h

@@ -316,22 +316,22 @@ parse_frame_object_aliased(
     uintptr_t *previous_frame)
 {
     char frame[SIZEOF_INTERP_FRAME];
-    if (_Py_RemoteDebug_AliasedRead(
-            unwinder, address, SIZEOF_INTERP_FRAME,
-            frame) < 0) {
+    int rc = _Py_RemoteDebug_AliasedRead(
+        unwinder, address, SIZEOF_INTERP_FRAME, frame);
+    if (rc < 0) {
         set_exception_cause(unwinder, PyExc_RuntimeError,
                             "Failed to read interpreter frame");
         return -1;
     }
-    STATS_INC(unwinder, memory_reads);
-    STATS_ADD(unwinder, memory_bytes_read, SIZEOF_INTERP_FRAME);
 
-    if (!validate_frame_snapshot(unwinder, frame, expected_parent)) {
+    if (rc == 0 && !validate_frame_snapshot(unwinder, frame, expected_parent)) {
         STATS_INC(unwinder, alias_validation_fails);
         return parse_frame_object(unwinder, result, address,
                                   address_of_code_object, previous_frame);
     }
 
+    STATS_INC(unwinder, memory_reads);
+    STATS_ADD(unwinder, memory_bytes_read, SIZEOF_INTERP_FRAME);
     return parse_frame_buffer(unwinder, result, frame,
                               address_of_code_object, previous_frame);
 }

Modules/_remote_debugging/frames.c

@@ -1183,8 +1183,6 @@ RemoteUnwinder was created with stats=True.
         - alias_validation_fails: macOS alias snapshot validation
           failures
         - alias_evictions: macOS alias-cache LRU evictions
-        - alias_identity_mismatches: macOS target identity
-          mismatches
         - alias_probe_checks: macOS alias object identity probes
         - alias_probe_recycles: macOS alias recycled-page detections
         - frame_cache_hit_rate: Percentage of samples that hit the
@@ -1202,7 +1200,7 @@ RemoteUnwinder was created with stats=True.
 
 static PyObject *
 _remote_debugging_RemoteUnwinder_get_stats_impl(RemoteUnwinderObject *self)
-/*[clinic end generated code: output=21e36477122be2a0 input=d9bce4858ca7cbc7]*/
+/*[clinic end generated code: output=21e36477122be2a0 input=913e10ed7cabd40d]*/
 {
     if (!self->collect_stats) {
         PyErr_SetString(PyExc_RuntimeError,
@@ -1247,7 +1245,6 @@ _remote_debugging_RemoteUnwinder_get_stats_impl(RemoteUnwinderObject *self)
     ADD_STAT(alias_remap_failures);
     ADD_STAT(alias_validation_fails);
     ADD_STAT(alias_evictions);
-    ADD_STAT(alias_identity_mismatches);
     ADD_STAT(alias_probe_checks);
     ADD_STAT(alias_probe_recycles);
 

Modules/_remote_debugging/module.c

@@ -303,14 +303,15 @@ read_thread_state_and_maybe_frame(
     *frame_read = 0;
 #if defined(__APPLE__) && TARGET_OS_OSX
     if (unwinder->cache_frames) {
-        if (_Py_RemoteDebug_AliasedRead(
-                unwinder,
-                tstate_addr,
-                tstate_size,
-                tstate_buffer) < 0) {
+        int rc = _Py_RemoteDebug_AliasedRead(
+            unwinder,
+            tstate_addr,
+            tstate_size,
+            tstate_buffer);
+        if (rc < 0) {
             return -1;
         }
-        if (!_Py_RemoteDebug_ValidateThreadStateSnapshot(
+        if (rc == 0 && !_Py_RemoteDebug_ValidateThreadStateSnapshot(
                 unwinder, tstate_buffer, tstate_addr,
                 current_interpreter)) {
             STATS_INC(unwinder, alias_validation_fails);