[Lldb-commits] [PATCH] Fixes for 32-bit linux ptrace and RegisterContext issues
Todd Fiala
tfiala at google.com
Thu Feb 27 12:53:47 PST 2014
I just checked in r202428, which resolves this issue. We now have 32-bit
Linux EXEs debugging on both Linux 32-bit hosts and 64-bit hosts.
On Thu, Feb 27, 2014 at 12:11 PM, Todd Fiala <tfiala at google.com> wrote:
> I discovered a few things during this process.
>
> I did implement my (1) and (2) above locally, and that is getting Linux
> x86 (32-bit) exes debugging on 64-bit and 32-bit hosts.
>
> I dug more into the GetAsUInt32() issue. The fix in the original patch is
> definitely not something we want to do. However, I found at least part of
> the root cause of it wanting to read 64-bits when it should have been 32
> bits:
>
> The i386 Register Context returns register infos per a templated include
> file that is embedding the wrong size for the FPU registers because the
> FPR_SIZE macro is defined in terms of the 64-bit FPU registers (and thus is
> implying the register size for a read should be 64 bit where it really
> shouldn't in some cases). That info is then getting embedded into the
> register_infos incorrectly for the i386 register set.
>
> I'm attempting to rectify that part now. It may not be the whole story,
> but it should move us forward with Linux x86 debugging on an x86 host.
>
>
> On Thu, Feb 27, 2014 at 9:59 AM, Todd Fiala <tfiala at google.com> wrote:
>
>> This change still fails debugging Linux 32-bit ELF exes on Linux 64-bit
>> hosts:
>>
>> tfiala at tfiala2:/mnt/ssd/work/svn/lgs/build$ bin/lldb
>> ~/play/c++/sizes/main_32
>> Current executable set to
>> '/usr/local/google/home/tfiala/play/c++/sizes/main_32' (i386).
>> (lldb) b main.cpp:5
>> Breakpoint 1: where = main_32`main + 9 at main.cpp:5, address = 0x08048489
>> (lldb) run
>> Process 9150 launching
>> lldb:
>> /mnt/ssd/work/svn/lgs/llvm/tools/lldb/source/Plugins/Process/POSIX/RegisterContextPOSIXProcessMonitor_x86.cpp:511:
>> virtual bool
>> RegisterContextPOSIXProcessMonitor_x86_64::IsWatchpointHit(uint32_t):
>> Assertion `false && "Could not initialize watchpoint registers"' failed.
>> Aborted (core dumped)
>>
>>
>> I have a few ideas, though. I'll give them a try. Assuming this works
>> when I try it on a 32-bit Ubuntu host, I will try the following:
>>
>> (1) when the *host* is a 32-bit architecture, do what you were doing
>> above (setting the register context to the i386 version), but
>>
>> (2) when the *host* is a 64-bit architecture, continue to use the x86_64
>> context, which knows how to work for 32-bit in a 64-bit host.
>>
>> This should at least get you running and not break the existing
>> functional case of 32-bit Linux exe debugging on a 64-bit host.
>>
>>
>> > 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.
>>
>> I'm a bit concerned about that myself. I may spend a bit with the 32-bit
>> side just to see if there's a less intrusive way to handle the core issue.
>>
>> Back when I have some results...
>>
>>
>> On Thu, Feb 27, 2014 at 9:45 AM, Greg Clayton <gclayton at apple.com> wrote:
>>
>>>
>>> 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 <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
>>>
>>> _______________________________________________
>>> 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
>>
>
>
>
> --
> Todd Fiala | Software Engineer | tfiala at google.com | 650-943-3180
>
--
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/41670b0c/attachment.html>
More information about the lldb-commits
mailing list