Author: Pavel Labath
Date: 2022-08-03T15:44:19+02:00
New Revision: 69c39e2abc311aa226d54b82a2cc8fa648902c7d

URL: 
https://github.com/llvm/llvm-project/commit/69c39e2abc311aa226d54b82a2cc8fa648902c7d
DIFF: 
https://github.com/llvm/llvm-project/commit/69c39e2abc311aa226d54b82a2cc8fa648902c7d.diff

LOG: [lldb] Fix TestDeletedExecutable on linux

Currently, lldb-server was opening the executable file to determine the
process architecture (to differentiate between 32 and 64 bit
architecture flavours). This isn't a particularly trustworthy source of
information (the file could have been changed since the process was
started) and it is not always available (file could be deleted or
otherwise inaccessible).

Unfortunately, ptrace does not give us a direct API to access the
process architecture, but we can still infer it via some of its
responses -- given that the general purpose register set of 64-bit
applications is larger [citation needed] than the GPR set of 32-bit
ones, we can just ask for the application GPR set and check its size.

This is what this patch does.

Differential Revision: https://reviews.llvm.org/D130985

Added: 
    

Modified: 
    lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
    lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.cpp
    lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.h
    lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
    lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
    lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_ppc64le.cpp
    lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_s390x.cpp
    lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
    lldb/source/Plugins/Process/Utility/RegisterContextLinux_i386.cpp
    lldb/source/Plugins/Process/Utility/RegisterContextLinux_i386.h
    lldb/source/Plugins/Process/Utility/RegisterContextLinux_x86_64.cpp
    lldb/source/Plugins/Process/Utility/RegisterContextLinux_x86_64.h
    lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
    lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
    lldb/test/API/functionalities/deleted-executable/TestDeletedExecutable.py

Removed: 
    


################################################################################
diff  --git a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp 
b/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
index f7f52deb173fb..ec2aebc4ec199 100644
--- a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
@@ -246,25 +246,20 @@ NativeProcessLinux::Factory::Launch(ProcessLaunchInfo 
&launch_info,
   }
   LLDB_LOG(log, "inferior started, now in stopped state");
 
-  ProcessInstanceInfo Info;
-  if (!Host::GetProcessInfo(pid, Info)) {
-    return llvm::make_error<StringError>("Cannot get process architecture",
-                                         llvm::inconvertibleErrorCode());
-  }
-
-  // Set the architecture to the exe architecture.
-  LLDB_LOG(log, "pid = {0:x}, detected architecture {1}", pid,
-           Info.GetArchitecture().GetArchitectureName());
-
   status = SetDefaultPtraceOpts(pid);
   if (status.Fail()) {
     LLDB_LOG(log, "failed to set default ptrace options: {0}", status);
     return status.ToError();
   }
 
+  llvm::Expected<ArchSpec> arch_or =
+      NativeRegisterContextLinux::DetermineArchitecture(pid);
+  if (!arch_or)
+    return arch_or.takeError();
+
   return std::unique_ptr<NativeProcessLinux>(new NativeProcessLinux(
       pid, launch_info.GetPTY().ReleasePrimaryFileDescriptor(), 
native_delegate,
-      Info.GetArchitecture(), mainloop, {pid}));
+      *arch_or, mainloop, {pid}));
 }
 
 llvm::Expected<std::unique_ptr<NativeProcessProtocol>>
@@ -274,19 +269,17 @@ NativeProcessLinux::Factory::Attach(
   Log *log = GetLog(POSIXLog::Process);
   LLDB_LOG(log, "pid = {0:x}", pid);
 
-  // Retrieve the architecture for the running process.
-  ProcessInstanceInfo Info;
-  if (!Host::GetProcessInfo(pid, Info)) {
-    return llvm::make_error<StringError>("Cannot get process architecture",
-                                         llvm::inconvertibleErrorCode());
-  }
-
   auto tids_or = NativeProcessLinux::Attach(pid);
   if (!tids_or)
     return tids_or.takeError();
+  ArrayRef<::pid_t> tids = *tids_or;
+  llvm::Expected<ArchSpec> arch_or =
+      NativeRegisterContextLinux::DetermineArchitecture(tids[0]);
+  if (!arch_or)
+    return arch_or.takeError();
 
   return std::unique_ptr<NativeProcessLinux>(new NativeProcessLinux(
-      pid, -1, native_delegate, Info.GetArchitecture(), mainloop, *tids_or));
+      pid, -1, native_delegate, *arch_or, mainloop, tids));
 }
 
 NativeProcessLinux::Extension

diff  --git a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.cpp 
b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.cpp
index 920de58e07d61..c047803597ed2 100644
--- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.cpp
@@ -8,13 +8,14 @@
 
 #include "NativeRegisterContextLinux.h"
 
+#include "Plugins/Process/Linux/NativeProcessLinux.h"
+#include "Plugins/Process/POSIX/ProcessPOSIXLog.h"
+#include "lldb/Host/HostInfo.h"
 #include "lldb/Host/common/NativeProcessProtocol.h"
 #include "lldb/Host/common/NativeThreadProtocol.h"
 #include "lldb/Host/linux/Ptrace.h"
 #include "lldb/Utility/RegisterValue.h"
-
-#include "Plugins/Process/Linux/NativeProcessLinux.h"
-#include "Plugins/Process/POSIX/ProcessPOSIXLog.h"
+#include <sys/uio.h>
 
 using namespace lldb_private;
 using namespace lldb_private::process_linux;
@@ -155,3 +156,19 @@ Status NativeRegisterContextLinux::DoWriteRegisterValue(
   return NativeProcessLinux::PtraceWrapper(
       PTRACE_POKEUSER, m_thread.GetID(), reinterpret_cast<void *>(offset), 
buf);
 }
+
+llvm::Expected<ArchSpec>
+NativeRegisterContextLinux::DetermineArchitectureViaGPR(lldb::tid_t tid,
+                                                        size_t gpr64_size) {
+  std::unique_ptr<uint8_t[]> data = std::make_unique<uint8_t[]>(gpr64_size);
+  struct iovec iov;
+  iov.iov_base = data.get();
+  iov.iov_len = gpr64_size;
+  unsigned int regset = llvm::ELF::NT_PRSTATUS;
+  Status ST = NativeProcessLinux::PtraceWrapper(PTRACE_GETREGSET, tid, &regset,
+                                                &iov, sizeof(iov));
+  if (ST.Fail())
+    return ST.ToError();
+  return HostInfo::GetArchitecture(
+      iov.iov_len < gpr64_size ? HostInfo::eArchKind32 : 
HostInfo::eArchKind64);
+}

diff  --git a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.h 
b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.h
index b66f6907d8ae6..9e2326c3a784b 100644
--- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.h
+++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.h
@@ -22,14 +22,20 @@ class NativeThreadLinux;
 class NativeRegisterContextLinux
     : public virtual NativeRegisterContextRegisterInfo {
 public:
-  // This function is implemented in the NativeRegisterContextLinux_* 
subclasses
-  // to create a new instance of the host specific NativeRegisterContextLinux.
-  // The implementations can't collide as only one NativeRegisterContextLinux_*
-  // variant should be compiled into the final executable.
+  // These static methods are implemented individual
+  // NativeRegisterContextLinux_* subclasses.  The implementations can't 
collide
+  // as only one NativeRegisterContextLinux_* variant should be compiled into
+  // the final executable.
+
+  // Return a NativeRegisterContextLinux instance suitable for debugging the
+  // given thread.
   static std::unique_ptr<NativeRegisterContextLinux>
   CreateHostNativeRegisterContextLinux(const ArchSpec &target_arch,
                                        NativeThreadLinux &native_thread);
 
+  // Determine the architecture of the thread given by its ID.
+  static llvm::Expected<ArchSpec> DetermineArchitecture(lldb::tid_t tid);
+
   // Invalidates cached values in register context data structures
   virtual void InvalidateAllRegisters(){}
 
@@ -125,6 +131,11 @@ class NativeRegisterContextLinux
 
   virtual Status DoWriteRegisterValue(uint32_t offset, const char *reg_name,
                                       const RegisterValue &value);
+
+  // Determine the architecture via GPR size, as reported by
+  // PTRACE_GETREGSET(NT_PRSTATUS).
+  static llvm::Expected<ArchSpec>
+  DetermineArchitectureViaGPR(lldb::tid_t tid, size_t gpr64_size);
 };
 
 } // namespace process_linux

diff  --git 
a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp 
b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
index 07a37514e7ede..8d4585a02a8c3 100644
--- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
@@ -14,6 +14,7 @@
 #include "Plugins/Process/Linux/Procfs.h"
 #include "Plugins/Process/POSIX/ProcessPOSIXLog.h"
 #include "Plugins/Process/Utility/RegisterInfoPOSIX_arm.h"
+#include "lldb/Host/HostInfo.h"
 #include "lldb/Utility/DataBufferHeap.h"
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/RegisterValue.h"
@@ -52,6 +53,11 @@ 
NativeRegisterContextLinux::CreateHostNativeRegisterContextLinux(
                                                            native_thread);
 }
 
+llvm::Expected<ArchSpec>
+NativeRegisterContextLinux::DetermineArchitecture(lldb::tid_t tid) {
+  return HostInfo::GetArchitecture();
+}
+
 #endif // defined(__arm__)
 
 NativeRegisterContextLinux_arm::NativeRegisterContextLinux_arm(

diff  --git 
a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp 
b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
index 6022a6a373605..61e784e28ca68 100644
--- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
@@ -11,7 +11,7 @@
 #include "NativeRegisterContextLinux_arm.h"
 #include "NativeRegisterContextLinux_arm64.h"
 
-
+#include "lldb/Host/HostInfo.h"
 #include "lldb/Host/common/NativeProcessProtocol.h"
 #include "lldb/Host/linux/Ptrace.h"
 #include "lldb/Utility/DataBufferHeap.h"
@@ -95,6 +95,12 @@ 
NativeRegisterContextLinux::CreateHostNativeRegisterContextLinux(
   }
 }
 
+llvm::Expected<ArchSpec>
+NativeRegisterContextLinux::DetermineArchitecture(lldb::tid_t tid) {
+  return DetermineArchitectureViaGPR(
+      tid, RegisterInfoPOSIX_arm64::GetGPRSizeStatic());
+}
+
 NativeRegisterContextLinux_arm64::NativeRegisterContextLinux_arm64(
     const ArchSpec &target_arch, NativeThreadProtocol &native_thread,
     std::unique_ptr<RegisterInfoPOSIX_arm64> register_info_up)

diff  --git 
a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_ppc64le.cpp 
b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_ppc64le.cpp
index d7db40eefb0b0..e96a41fe86d01 100644
--- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_ppc64le.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_ppc64le.cpp
@@ -13,6 +13,7 @@
 
 #include "NativeRegisterContextLinux_ppc64le.h"
 
+#include "lldb/Host/HostInfo.h"
 #include "lldb/Host/common/NativeProcessProtocol.h"
 #include "lldb/Utility/DataBufferHeap.h"
 #include "lldb/Utility/Log.h"
@@ -123,6 +124,11 @@ 
NativeRegisterContextLinux::CreateHostNativeRegisterContextLinux(
   }
 }
 
+llvm::Expected<ArchSpec>
+NativeRegisterContextLinux::DetermineArchitecture(lldb::tid_t tid) {
+  return HostInfo::GetArchitecture();
+}
+
 NativeRegisterContextLinux_ppc64le::NativeRegisterContextLinux_ppc64le(
     const ArchSpec &target_arch, NativeThreadProtocol &native_thread)
     : NativeRegisterContextRegisterInfo(

diff  --git 
a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_s390x.cpp 
b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_s390x.cpp
index 121f405e546ed..f940261d4714b 100644
--- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_s390x.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_s390x.cpp
@@ -96,6 +96,11 @@ 
NativeRegisterContextLinux::CreateHostNativeRegisterContextLinux(
                                                              native_thread);
 }
 
+llvm::Expected<ArchSpec>
+NativeRegisterContextLinux::DetermineArchitecture(lldb::tid_t tid) {
+  return HostInfo::GetArchitecture();
+}
+
 // NativeRegisterContextLinux_s390x members.
 
 static RegisterInfoInterface *

diff  --git 
a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp 
b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
index d8b8f50b6de7c..e23ec49ee6184 100644
--- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
@@ -9,7 +9,6 @@
 #if defined(__i386__) || defined(__x86_64__)
 
 #include "NativeRegisterContextLinux_x86_64.h"
-
 #include "Plugins/Process/Linux/NativeThreadLinux.h"
 #include "Plugins/Process/Utility/RegisterContextLinux_i386.h"
 #include "Plugins/Process/Utility/RegisterContextLinux_x86_64.h"
@@ -255,6 +254,12 @@ 
NativeRegisterContextLinux::CreateHostNativeRegisterContextLinux(
       new NativeRegisterContextLinux_x86_64(target_arch, native_thread));
 }
 
+llvm::Expected<ArchSpec>
+NativeRegisterContextLinux::DetermineArchitecture(lldb::tid_t tid) {
+  return DetermineArchitectureViaGPR(
+      tid, RegisterContextLinux_x86_64::GetGPRSizeStatic());
+}
+
 // NativeRegisterContextLinux_x86_64 members.
 
 static RegisterInfoInterface *

diff  --git a/lldb/source/Plugins/Process/Utility/RegisterContextLinux_i386.cpp 
b/lldb/source/Plugins/Process/Utility/RegisterContextLinux_i386.cpp
index 39bec20649a4c..bb3bb01bdf02c 100644
--- a/lldb/source/Plugins/Process/Utility/RegisterContextLinux_i386.cpp
+++ b/lldb/source/Plugins/Process/Utility/RegisterContextLinux_i386.cpp
@@ -104,7 +104,7 @@ RegisterContextLinux_i386::RegisterContextLinux_i386(
   d_register_infos.push_back(orig_ax);
 }
 
-size_t RegisterContextLinux_i386::GetGPRSize() const { return sizeof(GPR); }
+size_t RegisterContextLinux_i386::GetGPRSizeStatic() { return sizeof(GPR); }
 
 const RegisterInfo *RegisterContextLinux_i386::GetRegisterInfo() const {
   switch (m_target_arch.GetMachine()) {

diff  --git a/lldb/source/Plugins/Process/Utility/RegisterContextLinux_i386.h 
b/lldb/source/Plugins/Process/Utility/RegisterContextLinux_i386.h
index ef731a5a79948..e0f8114fa58f0 100644
--- a/lldb/source/Plugins/Process/Utility/RegisterContextLinux_i386.h
+++ b/lldb/source/Plugins/Process/Utility/RegisterContextLinux_i386.h
@@ -15,7 +15,8 @@ class RegisterContextLinux_i386 : public 
lldb_private::RegisterInfoInterface {
 public:
   RegisterContextLinux_i386(const lldb_private::ArchSpec &target_arch);
 
-  size_t GetGPRSize() const override;
+  static size_t GetGPRSizeStatic();
+  size_t GetGPRSize() const override { return GetGPRSizeStatic(); }
 
   const lldb_private::RegisterInfo *GetRegisterInfo() const override;
 

diff  --git 
a/lldb/source/Plugins/Process/Utility/RegisterContextLinux_x86_64.cpp 
b/lldb/source/Plugins/Process/Utility/RegisterContextLinux_x86_64.cpp
index 20b8d74f8c0db..85c57ca698d43 100644
--- a/lldb/source/Plugins/Process/Utility/RegisterContextLinux_x86_64.cpp
+++ b/lldb/source/Plugins/Process/Utility/RegisterContextLinux_x86_64.cpp
@@ -171,7 +171,7 @@ RegisterContextLinux_x86_64::RegisterContextLinux_x86_64(
   d_register_infos.push_back(orig_ax);
 }
 
-size_t RegisterContextLinux_x86_64::GetGPRSize() const { return sizeof(GPR); }
+size_t RegisterContextLinux_x86_64::GetGPRSizeStatic() { return sizeof(GPR); }
 
 const std::vector<lldb_private::RegisterInfo> *
 RegisterContextLinux_x86_64::GetDynamicRegisterInfoP() const {

diff  --git a/lldb/source/Plugins/Process/Utility/RegisterContextLinux_x86_64.h 
b/lldb/source/Plugins/Process/Utility/RegisterContextLinux_x86_64.h
index ea21b913d5c51..a09deebed23af 100644
--- a/lldb/source/Plugins/Process/Utility/RegisterContextLinux_x86_64.h
+++ b/lldb/source/Plugins/Process/Utility/RegisterContextLinux_x86_64.h
@@ -15,7 +15,8 @@ class RegisterContextLinux_x86_64 : public 
lldb_private::RegisterInfoInterface {
 public:
   RegisterContextLinux_x86_64(const lldb_private::ArchSpec &target_arch);
 
-  size_t GetGPRSize() const override;
+  static size_t GetGPRSizeStatic();
+  size_t GetGPRSize() const override { return GetGPRSizeStatic(); }
 
   const lldb_private::RegisterInfo *GetRegisterInfo() const override;
 

diff  --git a/lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp 
b/lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
index 4db7abe603d4c..569e3a0785fe0 100644
--- a/lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
+++ b/lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
@@ -245,7 +245,7 @@ uint32_t RegisterInfoPOSIX_arm64::GetRegisterCount() const {
   return m_register_info_count;
 }
 
-size_t RegisterInfoPOSIX_arm64::GetGPRSize() const {
+size_t RegisterInfoPOSIX_arm64::GetGPRSizeStatic() {
   return sizeof(struct RegisterInfoPOSIX_arm64::GPR);
 }
 

diff  --git a/lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h 
b/lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
index 96cab49d5ac85..3e992acb1f002 100644
--- a/lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
+++ b/lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
@@ -81,7 +81,8 @@ class RegisterInfoPOSIX_arm64
   RegisterInfoPOSIX_arm64(const lldb_private::ArchSpec &target_arch,
                           lldb_private::Flags opt_regsets);
 
-  size_t GetGPRSize() const override;
+  static size_t GetGPRSizeStatic();
+  size_t GetGPRSize() const override { return GetGPRSizeStatic(); }
 
   size_t GetFPRSize() const override;
 

diff  --git 
a/lldb/test/API/functionalities/deleted-executable/TestDeletedExecutable.py 
b/lldb/test/API/functionalities/deleted-executable/TestDeletedExecutable.py
index 248ea143d9d6e..fd082b710a1dc 100644
--- a/lldb/test/API/functionalities/deleted-executable/TestDeletedExecutable.py
+++ b/lldb/test/API/functionalities/deleted-executable/TestDeletedExecutable.py
@@ -14,9 +14,6 @@ class TestDeletedExecutable(TestBase):
     NO_DEBUG_INFO_TESTCASE = True
 
     @skipIfWindows # cannot delete a running executable
-    @expectedFailureAll(oslist=["linux"],
-        triple=no_match('aarch64-.*-android'))
-        # determining the architecture of the process fails
     def test(self):
         self.build()
         exe = self.getBuildArtifact("a.out")


        
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to