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