One question: is source/Plugins/Process/Linux/ProcessMonitor.cpp only compiled 
natively on systems that can use it? If so, the the "pid_t" argument will 
probably work. Otherwise, you might want to change it to use a explicitly sized 
int type (int32_t?).

I worry that using RegisterValue::GetAsUInt32() on a 64 bit register value and 
truncating the data with you change: "case eTypeUInt64:   return 
m_data.uint32;" will cause problems. Can you just switch clients over to use 
RegisterValue::GetAsUInt64() for those that need to? That would seem like a 
better fix.

Thanks for taking to time to get the 32 bit linux build working, that will be a 
great benefit to all LLDB users.

Greg

On Feb 27, 2014, at 5:04 AM, Matthew Gardiner <[email protected]> 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
> [email protected]
> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits

_______________________________________________
lldb-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits

Reply via email to