From ccd4aa668aa760e754e5fdec550f89ee62c4ff22 Mon Sep 17 00:00:00 2001
From: Juan Sebastian Hoyos Ayala <juhoyosa@microsoft.com>
Date: Wed, 31 Mar 2021 03:17:32 -0700
Subject: [PATCH 1/2] [release/5.0] Fix usage of process_vm_readv in
 createdump; fallback to pread64

---
 .../src/debug/createdump/crashinfo.cpp        |  2 --
 src/coreclr/src/debug/createdump/crashinfo.h  |  5 ++-
 .../src/debug/createdump/crashinfounix.cpp    | 34 +++++++++++++------
 3 files changed, 25 insertions(+), 16 deletions(-)

diff --git a/src/coreclr/src/debug/createdump/crashinfo.cpp b/src/coreclr/src/debug/createdump/crashinfo.cpp
index 612fafb7c34e6..42c47a99e0a2d 100644
--- a/src/coreclr/src/debug/createdump/crashinfo.cpp
+++ b/src/coreclr/src/debug/createdump/crashinfo.cpp
@@ -16,10 +16,8 @@ CrashInfo::CrashInfo(pid_t pid) :
     m_task = 0;
 #else
     m_auxvValues.fill(0);
-#ifndef HAVE_PROCESS_VM_READV
     m_fd = -1;
 #endif
-#endif
 }
 
 CrashInfo::~CrashInfo()
diff --git a/src/coreclr/src/debug/createdump/crashinfo.h b/src/coreclr/src/debug/createdump/crashinfo.h
index 9bdf63be49e78..46b0825c0a7e3 100644
--- a/src/coreclr/src/debug/createdump/crashinfo.h
+++ b/src/coreclr/src/debug/createdump/crashinfo.h
@@ -45,9 +45,8 @@ class CrashInfo : public ICLRDataEnumMemoryRegionsCallback,
 #ifdef __APPLE__
     vm_map_t m_task;                                // the mach task for the process
 #else
-#ifndef HAVE_PROCESS_VM_READV
+    bool m_canUseProcVmReadSyscall;
     int m_fd;                                       // /proc/<pid>/mem handle
-#endif
 #endif
     std::string m_coreclrPath;                      // the path of the coreclr module or empty if none
 #ifdef __APPLE__
@@ -112,7 +111,7 @@ class CrashInfo : public ICLRDataEnumMemoryRegionsCallback,
     void VisitModule(uint64_t baseAddress, std::string& moduleName);
     void VisitProgramHeader(uint64_t loadbias, uint64_t baseAddress, ElfW(Phdr)* phdr);
     bool EnumerateModuleMappings();
-#endif 
+#endif
     bool EnumerateMemoryRegionsWithDAC(MINIDUMP_TYPE minidumpType);
     bool EnumerateManagedModules(IXCLRDataProcess* pClrDataProcess);
     bool UnwindAllThreads(IXCLRDataProcess* pClrDataProcess);
diff --git a/src/coreclr/src/debug/createdump/crashinfounix.cpp b/src/coreclr/src/debug/createdump/crashinfounix.cpp
index 4c72cc78739fb..bca21b094fdc5 100644
--- a/src/coreclr/src/debug/createdump/crashinfounix.cpp
+++ b/src/coreclr/src/debug/createdump/crashinfounix.cpp
@@ -8,7 +8,6 @@ bool GetStatus(pid_t pid, pid_t* ppid, pid_t* tgid, std::string* name);
 bool
 CrashInfo::Initialize()
 {
-#ifndef HAVE_PROCESS_VM_READV
     char memPath[128];
     _snprintf_s(memPath, sizeof(memPath), sizeof(memPath), "/proc/%lu/mem", m_pid);
 
@@ -18,12 +17,13 @@ CrashInfo::Initialize()
         fprintf(stderr, "open(%s) FAILED %d (%s)\n", memPath, errno, strerror(errno));
         return false;
     }
-#endif
     // Get the process info
     if (!GetStatus(m_pid, &m_ppid, &m_tgid, &m_name))
     {
         return false;
     }
+
+    m_canUseProcVmReadSyscall = true;
     return true;
 }
 
@@ -39,13 +39,11 @@ CrashInfo::CleanupAndResumeProcess()
             waitpid(thread->Tid(), &waitStatus, __WALL);
         }
     }
-#ifndef HAVE_PROCESS_VM_READV
     if (m_fd != -1)
     {
         close(m_fd);
         m_fd = -1;
     }
-#endif
 }
 
 //
@@ -253,7 +251,7 @@ CrashInfo::GetDSOInfo()
     int phnum = m_auxvValues[AT_PHNUM];
     assert(m_auxvValues[AT_PHENT] == sizeof(Phdr));
     assert(phnum != PN_XNUM);
-    return EnumerateElfInfo(phdrAddr, phnum); 
+    return EnumerateElfInfo(phdrAddr, phnum);
 }
 
 //
@@ -334,17 +332,31 @@ CrashInfo::ReadProcessMemory(void* address, void* buffer, size_t size, size_t* r
 {
     assert(buffer != nullptr);
     assert(read != nullptr);
+    *read = 0;
 
 #ifdef HAVE_PROCESS_VM_READV
-    iovec local{ buffer, size };
-    iovec remote{ address, size };
-    *read = process_vm_readv(m_pid, &local, 1, &remote, 1, 0);
-#else
-    assert(m_fd != -1);
-    *read = pread64(m_fd, buffer, size, (off64_t)address);
+    if (m_canUseProcVmReadSyscall)
+    {
+        iovec local{ buffer, size };
+        iovec remote{ address, size };
+        *read = process_vm_readv(m_pid, &local, 1, &remote, 1, 0);
+    }
+
+    if (!m_canUseProcVmReadSyscall || (*read == (size_t)-1 && errno == EPERM))
 #endif
+    {
+        // If we've failed, avoid going through expensive syscalls
+        // After all, the use of process_vm_readv is largely as a
+        // performance optimization.
+        m_canUseProcVmReadSyscall = false;
+        assert(m_fd != -1);
+        *read = pread64(m_fd, buffer, size, (off64_t)address);
+    }
+
     if (*read == (size_t)-1)
     {
+        int readErrno = errno;
+        TRACE("ReadProcessMemory FAILED, addr: %" PRIA PRIx ", size: %zu, ERRNO %d: %s\n", address, size, readErrno, strerror(readErrno));
         return false;
     }
     return true;

From 98b8cb55eb6e19fd2018b9651ea2f000f0cbacc4 Mon Sep 17 00:00:00 2001
From: Juan Hoyos <juan.hoyos@microsoft.com>
Date: Wed, 31 Mar 2021 10:37:12 -0700
Subject: [PATCH 2/2] No-op logging

---
 src/coreclr/src/debug/createdump/crashinfounix.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/coreclr/src/debug/createdump/crashinfounix.cpp b/src/coreclr/src/debug/createdump/crashinfounix.cpp
index bca21b094fdc5..39941e37f53b2 100644
--- a/src/coreclr/src/debug/createdump/crashinfounix.cpp
+++ b/src/coreclr/src/debug/createdump/crashinfounix.cpp
@@ -356,7 +356,7 @@ CrashInfo::ReadProcessMemory(void* address, void* buffer, size_t size, size_t* r
     if (*read == (size_t)-1)
     {
         int readErrno = errno;
-        TRACE("ReadProcessMemory FAILED, addr: %" PRIA PRIx ", size: %zu, ERRNO %d: %s\n", address, size, readErrno, strerror(readErrno));
+        TRACE_VERBOSE("ReadProcessMemory FAILED, addr: %" PRIA PRIx ", size: %zu, ERRNO %d: %s\n", address, size, readErrno, strerror(readErrno));
         return false;
     }
     return true;