labath created this revision.
labath added a reviewer: DavidSpickett.
Herald added subscribers: JDevlieghere, kbarton, nemanjai.
Herald added a project: All.
labath requested review of this revision.
Herald added a project: LLDB.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D130985

Files:
  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

Index: lldb/test/API/functionalities/deleted-executable/TestDeletedExecutable.py
===================================================================
--- lldb/test/API/functionalities/deleted-executable/TestDeletedExecutable.py
+++ lldb/test/API/functionalities/deleted-executable/TestDeletedExecutable.py
@@ -14,9 +14,6 @@
     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")
Index: lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
===================================================================
--- lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
+++ lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
@@ -81,7 +81,8 @@
   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;
 
Index: lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
===================================================================
--- lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
+++ lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
@@ -245,7 +245,7 @@
   return m_register_info_count;
 }
 
-size_t RegisterInfoPOSIX_arm64::GetGPRSize() const {
+size_t RegisterInfoPOSIX_arm64::GetGPRSizeStatic() {
   return sizeof(struct RegisterInfoPOSIX_arm64::GPR);
 }
 
Index: lldb/source/Plugins/Process/Utility/RegisterContextLinux_x86_64.h
===================================================================
--- lldb/source/Plugins/Process/Utility/RegisterContextLinux_x86_64.h
+++ lldb/source/Plugins/Process/Utility/RegisterContextLinux_x86_64.h
@@ -15,7 +15,8 @@
 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;
 
Index: lldb/source/Plugins/Process/Utility/RegisterContextLinux_x86_64.cpp
===================================================================
--- lldb/source/Plugins/Process/Utility/RegisterContextLinux_x86_64.cpp
+++ lldb/source/Plugins/Process/Utility/RegisterContextLinux_x86_64.cpp
@@ -171,7 +171,7 @@
   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 {
Index: lldb/source/Plugins/Process/Utility/RegisterContextLinux_i386.h
===================================================================
--- lldb/source/Plugins/Process/Utility/RegisterContextLinux_i386.h
+++ lldb/source/Plugins/Process/Utility/RegisterContextLinux_i386.h
@@ -15,7 +15,8 @@
 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;
 
Index: lldb/source/Plugins/Process/Utility/RegisterContextLinux_i386.cpp
===================================================================
--- lldb/source/Plugins/Process/Utility/RegisterContextLinux_i386.cpp
+++ lldb/source/Plugins/Process/Utility/RegisterContextLinux_i386.cpp
@@ -104,7 +104,7 @@
   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()) {
Index: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
===================================================================
--- lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
+++ 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 @@
       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 *
Index: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_s390x.cpp
===================================================================
--- lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_s390x.cpp
+++ lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_s390x.cpp
@@ -96,6 +96,11 @@
                                                              native_thread);
 }
 
+llvm::Expected<ArchSpec>
+NativeRegisterContextLinux::DetermineArchitecture(lldb::tid_t tid) {
+  return HostInfo::GetArchitecture();
+}
+
 // NativeRegisterContextLinux_s390x members.
 
 static RegisterInfoInterface *
Index: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_ppc64le.cpp
===================================================================
--- lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_ppc64le.cpp
+++ 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 @@
   }
 }
 
+llvm::Expected<ArchSpec>
+NativeRegisterContextLinux::DetermineArchitecture(lldb::tid_t tid) {
+  return HostInfo::GetArchitecture();
+}
+
 NativeRegisterContextLinux_ppc64le::NativeRegisterContextLinux_ppc64le(
     const ArchSpec &target_arch, NativeThreadProtocol &native_thread)
     : NativeRegisterContextRegisterInfo(
Index: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
===================================================================
--- lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
+++ 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 @@
   }
 }
 
+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)
Index: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
===================================================================
--- lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
+++ 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 @@
                                                            native_thread);
 }
 
+llvm::Expected<ArchSpec>
+NativeRegisterContextLinux::DetermineArchitecture(lldb::tid_t tid) {
+  return HostInfo::GetArchitecture();
+}
+
 #endif // defined(__arm__)
 
 NativeRegisterContextLinux_arm::NativeRegisterContextLinux_arm(
Index: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.h
===================================================================
--- lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.h
+++ lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.h
@@ -22,14 +22,20 @@
 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 @@
 
   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);
 };
 
 } // namespace process_linux
Index: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.cpp
===================================================================
--- lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.cpp
+++ 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 @@
   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) {
+  std::unique_ptr<uint8_t[]> data = std::make_unique<uint8_t[]>(gpr64);
+  struct iovec iov;
+  iov.iov_base = data.get();
+  iov.iov_len = gpr64;
+  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 ? HostInfo::eArchKind32
+                                                       : HostInfo::eArchKind64);
+}
Index: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
===================================================================
--- lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
+++ lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
@@ -246,25 +246,20 @@
   }
   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 @@
   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
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to