Hopefully fix vmmap recursion issues (#1585)

* Hopefully fix vmmap recursion issues

* fixes

* fixes

* Add test for issue 1565

* add missing test file

* fix makefile (pthread)

* fix corefile vmmap case

* Fix comments

pwndbg/gdblib/vmmap.py

@@ -6,7 +6,6 @@ The reason that we need robustness is that not every operating
 system has /proc/$$/maps, which backs 'info proc mapping'.
 """
 import bisect
-import os
 from typing import Any
 from typing import List
 from typing import Optional
@@ -88,13 +87,25 @@ def get() -> Tuple[pwndbg.lib.memory.Page, ...]:
     # Note: debugging a coredump does still show proc.alive == True
     if not pwndbg.gdblib.proc.alive:
         return tuple()
-    pages = []
-    pages.extend(proc_pid_maps())
 
-    if (
-        not pages
-        and pwndbg.gdblib.qemu.is_qemu_kernel()
-        and pwndbg.gdblib.arch.current in ("i386", "x86-64", "aarch64", "riscv:rv64")
+    if is_corefile():
+        return tuple(coredump_maps())
+
+    proc_maps = proc_pid_maps()
+
+    # The `proc_maps` is usually a tuple of Page objects but it can also be:
+    #   None    - when /proc/$pid/maps does not exist/is not available
+    #   tuple() - when the process has no maps yet which happens only during its very early init
+    #             (usually when we attach to a process)
+    if proc_maps is not None:
+        return proc_maps
+
+    pages = []
+    if pwndbg.gdblib.qemu.is_qemu_kernel() and pwndbg.gdblib.arch.current in (
+        "i386",
+        "x86-64",
+        "aarch64",
+        "riscv:rv64",
     ):
 
         # If kernel_vmmap_via_pt is not set to the default value of "deprecated",
@@ -112,12 +123,7 @@ def get() -> Tuple[pwndbg.lib.memory.Page, ...]:
         elif kernel_vmmap == "monitor":
             pages.extend(kernel_vmmap_via_monitor_info_mem())
 
-    if not pages and is_corefile():
-        pages.extend(coredump_maps())
-
-    # TODO/FIXME: Do we still need it after coredump_maps()?
-    # Add tests for other cases and see if this is needed e.g. for QEMU user
-    # if not, remove the code below & cleanup other parts of Pwndbg codebase
+    # TODO/FIXME: Add tests for  QEMU-user targets when this is needed
     if not pages:
         # If debuggee is launched from a symlink the debuggee memory maps will be
         # labeled with symlink path while in normal scenario the /proc/pid/maps
@@ -338,13 +344,14 @@ def proc_pid_maps():
     Parse the contents of /proc/$PID/maps on the server.
 
     Returns:
-        A list of pwndbg.lib.memory.Page objects.
+        A tuple of pwndbg.lib.memory.Page objects or None if
+        /proc/$pid/maps doesn't exist or when we debug a qemu-user target
     """
 
     # If we debug remotely a qemu-user or qemu-system target,
     # there is no point of hitting things further
     if pwndbg.gdblib.qemu.is_qemu():
-        return tuple()
+        return None
 
     # Example /proc/$pid/maps
     # 7f95266fa000-7f95268b5000 r-xp 00000000 08:01 418404                     /lib/x86_64-linux-gnu/libc-2.19.so
@@ -381,6 +388,10 @@ def proc_pid_maps():
         except (OSError, gdb.error):
             continue
     else:
+        return None
+
+    # Process hasn't been fully created yet; it is in Z (zombie) state
+    if data == "":
         return tuple()
 
     pages = []
@@ -725,10 +736,3 @@ def check_aslr():
     # access to procfs.
     output = gdb.execute("show disable-randomization", to_string=True)
     return ("is off." in output), "show disable-randomization"
-
-
-@pwndbg.gdblib.events.cont
-def mark_pc_as_executable() -> None:
-    mapping = find(pwndbg.gdblib.regs.pc)
-    if mapping and not mapping.execute:
-        mapping.flags |= os.X_OK

tests/gdb-tests/tests/binaries/issue_1565.c

@@ -0,0 +1,17 @@
+#include <pthread.h>
+#include <stdio.h>
+#include <unistd.h>
+#include <stdlib.h>
+
+void *thread_function(void *arg) {
+  free(malloc(0x20));
+  sleep(100);
+  return NULL;
+}
+
+int main(void) {
+  pthread_t thread;
+  pthread_create(&thread, NULL, thread_function, NULL);
+  pthread_exit(NULL);
+  return 0;
+}

tests/gdb-tests/tests/binaries/makefile

@@ -107,6 +107,10 @@ tls.i386.out: tls.i386.c
 	-target i386-linux-gnu \
 	-o tls.i386.out tls.i386.c
 
+issue_1565.out: issue_1565.c
+	@echo "[+] Building issue_1565.out"
+	${CC} -g -O0 -o issue_1565.out issue_1565.c -pthread -lpthread
+
 clean :
 	@echo "[+] Cleaning stuff"
 	@rm -f $(COMPILED) $(LINKED) $(COMPILED_ASM) $(LINKED_ASM) $(COMPILED_GO)

tests/gdb-tests/tests/test_command_vmmap.py

@@ -7,6 +7,7 @@ import pwndbg
 import tests
 
 CRASH_SIMPLE_BINARY = tests.binaries.get("crash_simple.out.hardcoded")
+BINARY_ISSUE_1565 = tests.binaries.get("issue_1565.out")
 
 
 def get_proc_maps():
@@ -156,3 +157,21 @@ def test_command_vmmap_on_coredump_on_crash_simple_binary(start_binary, unload_f
     vmmaps = [i.split() for i in vmmaps[2:]]
 
     assert_maps()
+
+
+def test_vmmap_issue_1565(start_binary):
+    """
+    https://github.com/pwndbg/pwndbg/issues/1565
+
+    In tests this bug is reported as:
+    >       gdb.execute("context")
+    E       gdb.error: Error occurred in Python: maximum recursion depth exceeded in comparison
+
+    In a normal GDB session this is reported as:
+        Exception occurred: context: maximum recursion depth exceeded while calling a Python object (<class 'RecursionError'>)
+    """
+    gdb.execute(f"file {BINARY_ISSUE_1565}")
+    gdb.execute("break thread_function")
+    gdb.execute("run")
+    gdb.execute("next")
+    gdb.execute("context")