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

[Todd Fiala]

I'll try this out locally on 64-bit and 32-bit hosts.

> +        case eTypeUInt64:   return m_data.uint32;
On the surface this line looks a little suspect but I need to see it in
context.




On Thu, Feb 27, 2014 at 5:04 AM, Matthew Gardiner <mg11 at csr.com> wrote:

> 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.
> _______________________________________________
> lldb-commits mailing list
> lldb-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
>



-- 
Todd Fiala | Software Engineer | tfiala at google.com | 650-943-3180
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20140227/05bf9640/attachment.html>