[Lldb-commits] [PATCH] Fixes for 32-bit linux ptrace and RegisterContext issues
Todd Fiala
tfiala at google.com
Thu Feb 27 12:11:36 PST 2014
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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20140227/1f81b08d/attachment.html>
More information about the lldb-commits
mailing list