From e6aeb47f991e32c0326a0b670db997e615c1a316 Mon Sep 17 00:00:00 2001
From: Paul Molodowitch <pmolodowitch@nvidia.com>
Date: Wed, 20 Mar 2024 00:54:55 -0700
Subject: [PATCH] [arch] Arch_DebuggerIsAttachedPosix: use TracerPid in
 /proc/self/status

The old implemenation worked by trying to attach a ptrace to the current
process, and if it failed, then we assumed it was being debugged.

The problem is that we have no easy way to check if an EPERM result
from ptrace is because it's being debugged, or an actual permissions
error - and permissions checking process is somewhat complex - see the
"Ptrace access mode checking" section here:

https://man7.org/linux/man-pages/man2/ptrace.2.html

On a basic Ubuntu-22.04 test system, ptrace would return EPERM even when
the process was not being debugged, resulting in a "false positive".
The new TracerPid method was more accurate in this case.

Checking the /proc/self/status for TracerPid is also the same method
that gperftools and even gdb itself use to check if a process is being
debugged:

- https://github.com/gperftools/gperftools/blob/02adc8ceab39bbeac1f65e10bde577e1753094fa/src/heap-checker.cc#L183
- https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=gdb/nat/linux-procfs.c;h=b17e3120792e0e0790271898212b69b0577847cc;hb=HEAD#l68
- https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=gdb/testsuite/gdb.threads/siginfo-threads.c;h=22c6038206ba77fc3432da5ee30284c80641f305;hb=HEAD#l373
---
 pxr/base/arch/debugger.cpp | 74 +++++++++++++++++++++-----------------
 1 file changed, 42 insertions(+), 32 deletions(-)

diff --git a/pxr/base/arch/debugger.cpp b/pxr/base/arch/debugger.cpp
index 4039d0d57e..a4492f52af 100644
--- a/pxr/base/arch/debugger.cpp
+++ b/pxr/base/arch/debugger.cpp
@@ -45,8 +45,10 @@
 #include <cstring>
 #include <errno.h>
 #include <fcntl.h>
+#include <fstream>
 #include <unistd.h>
 #include <string>
+#include <string_view>
 #endif
 #if defined(ARCH_OS_DARWIN)
 #include <sys/sysctl.h>
@@ -344,50 +346,58 @@ Arch_DebuggerAttachExecPosix(void* data)
 
 #if defined(ARCH_OS_LINUX)
 
+// Reads /proc/self/status, finds the line starting with "field:", and
+// returns the portion following the ":".
+// Note that the returned string will generally include leading whitespace
 static
-bool
-Arch_DebuggerIsAttachedPosix()
+std::string Arch_ReadProcStatusField(const std::string_view field)
 {
-    // Check for a ptrace based debugger by trying to ptrace.
-    pid_t parent = getpid();
-    pid_t pid = nonLockingFork();
-    if (pid < 0) {
-        // fork failed.  We'll guess there's no debugger.
-        return false;
+    std::ifstream procStatusFile("/proc/self/status");
+    if (!procStatusFile) {
+        ARCH_WARNING("Unable to open /proc/self/status");
+        return std::string();
     }
-
-    // Child process.
-    if (pid == 0) {
-        // Attach to the parent with ptrace() this will fail if the
-        // parent is already being traced.
-        if (ptrace(PTRACE_ATTACH, parent, NULL, NULL) == -1) {
-            // A debugger is probably attached if the error is EPERM.
-            _exit(errno == EPERM ? 1 : 0);
+    for (std::string line; std::getline(procStatusFile, line);) {
+        // the field needs to start with the given fieldLen AND the ':' char
+        if (line.size() < field.size() + 1 ) {
+            continue;
         }
 
-        // Wait for the parent to stop as a result of the attach.
-        int status;
-        while (waitpid(parent, &status, 0) == -1 && errno == EINTR) {
-            // Do nothing
+        if (line.compare(0, field.size(), field) == 0 && line[field.size()] == ':') {
+            // We found our "field:" line
+            return line.substr(field.size() + 2);
         }
+    }
 
-        // Detach and continue the parent.
-        ptrace(PTRACE_DETACH, parent, 0, SIGCONT);
+    std::string warningStr("Unable to find given field in /proc/self/status: ");
+    warningStr += field;
+    ARCH_WARNING(warningStr.c_str());
+    return std::string();
+}
 
-        // A debugger was not attached.
-        _exit(0);
-    }
+constexpr int INVALID_INT_STR = -1;
+constexpr int OUT_OF_RANGE_INT_STR = -2;
 
-    // Parent process
-    int status;
-    while (waitpid(pid, &status, 0) == -1 && errno == EINTR) {
-        // Do nothing
+// Reads the "TracerPid:" field from /proc/self/status
+// Returns a result < 0 if there was an error.
+static
+int Arch_ReadTracerPid() {
+    try {
+        return std::stoi(Arch_ReadProcStatusField("TracerPid"));
     }
-    if (WIFEXITED(status)) {
-        return WEXITSTATUS(status) != 0;
+    catch (std::invalid_argument const &ex) {
+        return INVALID_INT_STR;
     }
-    return false;
+    catch (std::out_of_range const &ex) {
+        return OUT_OF_RANGE_INT_STR;
+    }
+}
 
+static
+bool
+Arch_DebuggerIsAttachedPosix()
+{
+    return Arch_ReadTracerPid() > 0;
 }
 
 #elif defined(ARCH_OS_DARWIN)