[Lldb-commits] [PATCH] Fixes for 32-bit linux ptrace and RegisterContext issues

[Matthew Gardiner]

Hi Folks,

I have been struggling with trying to get a 32-bit linux build of lldb to debug
a simple program. My primary angst was the false reporting of the presence of
a watchpoint, resulting in an assertion fail. I've tracked that done now, and
the implication of my findings are that any usage of the PTRACE/PtraceWrapper
code in Linux/ProcessMonitor.cpp for 32-bit linux is buggy, if
LLDB_CONFIGURATION_BUILDANDINTEGRATION is not defined.

The crux of the problem is this prototype

extern long
PtraceWrapper(int req, lldb::pid_t pid, void *addr, void *data, size_t data_size,
               const char* reqName, const char* file, int line)

using lldb::pid_t as pid which 64-bit;

when this is fed into ptrace which is prototyped as:

extern long int ptrace (enum __ptrace_request __request, ...);

so badness occurs when a 64-bit pid is pulled off the stack by an
implementation expecting 32-bits. Obviously the addr and data parameters
are corrupted...

Another problem I've encountered occurs because the RegisterValue class
returns fail value for GetAsUInt32 if the object is originally constructed
using a 64-bit integer.

Finally, the 64-bit register context was being used to debug 32-bit cores.

The following patch fixes those issues and the result is that 32-bit linux
lldb can debug 32-bit linux applications.

Index: source/Core/RegisterValue.cpp
===================================================================
--- source/Core/RegisterValue.cpp	(revision 202380)
+++ source/Core/RegisterValue.cpp	(working copy)
@@ -706,6 +706,7 @@
          case eTypeUInt8:    return m_data.uint8;
          case eTypeUInt16:   return m_data.uint16;
          case eTypeUInt32:   return m_data.uint32;
+        case eTypeUInt64:   return m_data.uint32;
          case eTypeFloat:
              if (sizeof(float) == sizeof(uint32_t))
                  return m_data.uint32;
Index: source/Plugins/Process/Linux/ProcessMonitor.cpp
===================================================================
--- source/Plugins/Process/Linux/ProcessMonitor.cpp	(revision 202380)
+++ source/Plugins/Process/Linux/ProcessMonitor.cpp	(working copy)
@@ -157,7 +157,7 @@
  // Wrapper for ptrace to catch errors and log calls.
  // Note that ptrace sets errno on error because -1 can be a valid result (i.e. for PTRACE_PEEK*)
  extern long
-PtraceWrapper(int req, lldb::pid_t pid, void *addr, void *data, size_t data_size,
+PtraceWrapper(int req, pid_t pid, void *addr, void *data, size_t data_size,
                const char* reqName, const char* file, int line)
  {
      long int result;
@@ -171,10 +171,10 @@
          result = ptrace(static_cast<__ptrace_request>(req), pid, *(unsigned int *)addr, data);
      else
          result = ptrace(static_cast<__ptrace_request>(req), pid, addr, data);
-
+	
      if (log)
          log->Printf("ptrace(%s, %" PRIu64 ", %p, %p, %zu)=%lX called from file %s line %d",
-                    reqName, pid, addr, data, data_size, result, file, line);
+                    reqName, static_cast<lldb::pid_t>(pid), addr, data, data_size, result, file, line);

      PtraceDisplayBytes(req, data, data_size);

Index: source/Plugins/Process/POSIX/POSIXThread.cpp
===================================================================
--- source/Plugins/Process/POSIX/POSIXThread.cpp	(revision 202380)
+++ source/Plugins/Process/POSIX/POSIXThread.cpp	(working copy)
@@ -190,7 +190,9 @@
                          reg_interface = new RegisterContextFreeBSD_x86_64(target_arch);
                          break;
                      case llvm::Triple::Linux:
-                        reg_interface = new RegisterContextLinux_x86_64(target_arch);
+                        reg_interface = ArchSpec::eCore_x86_64_x86_64 == target_arch.GetCore() ?
+							static_cast<RegisterInfoInterface*>(new RegisterContextLinux_x86_64(target_arch)) :
+							static_cast<RegisterInfoInterface*>(new RegisterContextLinux_i386(target_arch));
                          break;
                      default:
                          assert(false && "OS not supported");


Would someone be able to test that this patch is sane for 64-bit linux,
then commit it to SVN?

thanks
Matthew Gardiner


Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
More information can be found at www.csr.com. Keep up to date with CSR on our technical blog, www.csr.com/blog, CSR people blog, www.csr.com/people, YouTube, www.youtube.com/user/CSRplc, Facebook, www.facebook.com/pages/CSR/191038434253534, or follow us on Twitter at www.twitter.com/CSR_plc.
New for 2014, you can now access the wide range of products powered by aptX at www.aptx.com.