<div dir="ltr">I discovered a few things during this process.<div><br></div><div>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.</div><div><br>
</div><div>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:</div>
<div><br></div><div>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.</div>
<div><br></div><div>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.</div></div><div class="gmail_extra"><br><br><div class="gmail_quote">
On Thu, Feb 27, 2014 at 9:59 AM, Todd Fiala <span dir="ltr"><<a href="mailto:tfiala@google.com" target="_blank">tfiala@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div dir="ltr"><div>This change still fails debugging Linux 32-bit ELF exes on Linux 64-bit hosts:</div><div><br></div><blockquote style="margin:0px 0px 0px 40px;border:none;padding:0px"><div><div><font size="1" face="courier new, monospace">tfiala@tfiala2:/mnt/ssd/work/svn/lgs/build$ bin/lldb ~/play/c++/sizes/main_32</font></div>
</div><div><div><font size="1" face="courier new, monospace">Current executable set to '/usr/local/google/home/tfiala/play/c++/sizes/main_32' (i386).</font></div></div><div><div><font size="1" face="courier new, monospace">(lldb) b main.cpp:5</font></div>
</div><div><div><font size="1" face="courier new, monospace">Breakpoint 1: where = main_32`main + 9 at main.cpp:5, address = 0x08048489</font></div></div><div><div><font size="1" face="courier new, monospace">(lldb) run</font></div>
</div><div><div><font size="1" face="courier new, monospace">Process 9150 launching</font></div></div><div><div><font size="1" face="courier new, monospace">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.</font></div>
</div><div><div><font size="1" face="courier new, monospace">Aborted (core dumped)</font></div></div></blockquote><div><br></div><div>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:</div>
<div><br></div><div>(1) when the *host* is a 32-bit architecture, do what you were doing above (setting the register context to the i386 version), but</div><div><br></div><div>(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.</div>
<div><br></div><div>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.</div><div class=""><div><br></div><div><br></div><div>> <span style="font-family:arial,sans-serif;font-size:13px">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.</span><span style="font-family:arial,sans-serif;font-size:13px"> </span></div>
<div><span style="font-family:arial,sans-serif;font-size:13px"><br></span></div></div><div><span style="font-family:arial,sans-serif;font-size:13px">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.</span></div>
<div><br></div><div>Back when I have some results...</div></div><div class="gmail_extra"><div><div class="h5"><br><br><div class="gmail_quote">On Thu, Feb 27, 2014 at 9:45 AM, Greg Clayton <span dir="ltr"><<a href="mailto:gclayton@apple.com" target="_blank">gclayton@apple.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br>
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?).<br>
<br>
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.<br>
<br>
Thanks for taking to time to get the 32 bit linux build working, that will be a great benefit to all LLDB users.<br>
<span><font color="#888888"><br>
Greg<br>
</font></span><div><div><br>
On Feb 27, 2014, at 5:04 AM, Matthew Gardiner <<a href="mailto:mg11@csr.com" target="_blank">mg11@csr.com</a>> wrote:<br>
<br>
> Hi Folks,<br>
><br>
> I have been struggling with trying to get a 32-bit linux build of lldb to debug<br>
> a simple program. My primary angst was the false reporting of the presence of<br>
> a watchpoint, resulting in an assertion fail. I've tracked that done now, and<br>
> the implication of my findings are that any usage of the PTRACE/PtraceWrapper<br>
> code in Linux/ProcessMonitor.cpp for 32-bit linux is buggy, if<br>
> LLDB_CONFIGURATION_BUILDANDINTEGRATION is not defined.<br>
><br>
> The crux of the problem is this prototype<br>
><br>
> extern long<br>
> PtraceWrapper(int req, lldb::pid_t pid, void *addr, void *data, size_t data_size,<br>
> const char* reqName, const char* file, int line)<br>
><br>
> using lldb::pid_t as pid which 64-bit;<br>
><br>
> when this is fed into ptrace which is prototyped as:<br>
><br>
> extern long int ptrace (enum __ptrace_request __request, ...);<br>
><br>
> so badness occurs when a 64-bit pid is pulled off the stack by an<br>
> implementation expecting 32-bits. Obviously the addr and data parameters<br>
> are corrupted...<br>
><br>
> Another problem I've encountered occurs because the RegisterValue class<br>
> returns fail value for GetAsUInt32 if the object is originally constructed<br>
> using a 64-bit integer.<br>
><br>
> Finally, the 64-bit register context was being used to debug 32-bit cores.<br>
><br>
> The following patch fixes those issues and the result is that 32-bit linux<br>
> lldb can debug 32-bit linux applications.<br>
><br>
> Index: source/Core/RegisterValue.cpp<br>
> ===================================================================<br>
> --- source/Core/RegisterValue.cpp (revision 202380)<br>
> +++ source/Core/RegisterValue.cpp (working copy)<br>
> @@ -706,6 +706,7 @@<br>
> case eTypeUInt8: return m_data.uint8;<br>
> case eTypeUInt16: return m_data.uint16;<br>
> case eTypeUInt32: return m_data.uint32;<br>
> + case eTypeUInt64: return m_data.uint32;<br>
> case eTypeFloat:<br>
> if (sizeof(float) == sizeof(uint32_t))<br>
> return m_data.uint32;<br>
> Index: source/Plugins/Process/Linux/ProcessMonitor.cpp<br>
> ===================================================================<br>
> --- source/Plugins/Process/Linux/ProcessMonitor.cpp (revision 202380)<br>
> +++ source/Plugins/Process/Linux/ProcessMonitor.cpp (working copy)<br>
> @@ -157,7 +157,7 @@<br>
> // Wrapper for ptrace to catch errors and log calls.<br>
> // Note that ptrace sets errno on error because -1 can be a valid result (i.e. for PTRACE_PEEK*)<br>
> extern long<br>
> -PtraceWrapper(int req, lldb::pid_t pid, void *addr, void *data, size_t data_size,<br>
> +PtraceWrapper(int req, pid_t pid, void *addr, void *data, size_t data_size,<br>
> const char* reqName, const char* file, int line)<br>
> {<br>
> long int result;<br>
> @@ -171,10 +171,10 @@<br>
> result = ptrace(static_cast<__ptrace_request>(req), pid, *(unsigned int *)addr, data);<br>
> else<br>
> result = ptrace(static_cast<__ptrace_request>(req), pid, addr, data);<br>
> -<br>
> +<br>
> if (log)<br>
> log->Printf("ptrace(%s, %" PRIu64 ", %p, %p, %zu)=%lX called from file %s line %d",<br>
> - reqName, pid, addr, data, data_size, result, file, line);<br>
> + reqName, static_cast<lldb::pid_t>(pid), addr, data, data_size, result, file, line);<br>
><br>
> PtraceDisplayBytes(req, data, data_size);<br>
><br>
> Index: source/Plugins/Process/POSIX/POSIXThread.cpp<br>
> ===================================================================<br>
> --- source/Plugins/Process/POSIX/POSIXThread.cpp (revision 202380)<br>
> +++ source/Plugins/Process/POSIX/POSIXThread.cpp (working copy)<br>
> @@ -190,7 +190,9 @@<br>
> reg_interface = new RegisterContextFreeBSD_x86_64(target_arch);<br>
> break;<br>
> case llvm::Triple::Linux:<br>
> - reg_interface = new RegisterContextLinux_x86_64(target_arch);<br>
> + reg_interface = ArchSpec::eCore_x86_64_x86_64 == target_arch.GetCore() ?<br>
> + static_cast<RegisterInfoInterface*>(new RegisterContextLinux_x86_64(target_arch)) :<br>
> + static_cast<RegisterInfoInterface*>(new RegisterContextLinux_i386(target_arch));<br>
> break;<br>
> default:<br>
> assert(false && "OS not supported");<br>
><br>
><br>
> Would someone be able to test that this patch is sane for 64-bit linux,<br>
> then commit it to SVN?<br>
><br>
> thanks<br>
> Matthew Gardiner<br>
><br>
><br>
> 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<br>
> More information can be found at <a href="http://www.csr.com" target="_blank">www.csr.com</a>. Keep up to date with CSR on our technical blog, <a href="http://www.csr.com/blog" target="_blank">www.csr.com/blog</a>, CSR people blog, <a href="http://www.csr.com/people" target="_blank">www.csr.com/people</a>, YouTube, <a href="http://www.youtube.com/user/CSRplc" target="_blank">www.youtube.com/user/CSRplc</a>, Facebook, <a href="http://www.facebook.com/pages/CSR/191038434253534" target="_blank">www.facebook.com/pages/CSR/191038434253534</a>, or follow us on Twitter at <a href="http://www.twitter.com/CSR_plc" target="_blank">www.twitter.com/CSR_plc</a>.<br>
> New for 2014, you can now access the wide range of products powered by aptX at <a href="http://www.aptx.com" target="_blank">www.aptx.com</a>.<br>
> _______________________________________________<br>
> lldb-commits mailing list<br>
> <a href="mailto:lldb-commits@cs.uiuc.edu" target="_blank">lldb-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits</a><br>
<br>
_______________________________________________<br>
lldb-commits mailing list<br>
<a href="mailto:lldb-commits@cs.uiuc.edu" target="_blank">lldb-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits</a><br>
</div></div></blockquote></div><br><br clear="all"><div><br></div></div></div><div class="">-- <br><div dir="ltr"><table cellspacing="0" cellpadding="0" style="color:rgb(136,136,136);font-family:'Times New Roman'">
<tbody><tr style="color:rgb(85,85,85);font-family:sans-serif;font-size:small">
<td nowrap style="border-top-style:solid;border-top-color:rgb(213,15,37);border-top-width:2px">Todd Fiala |</td><td nowrap style="border-top-style:solid;border-top-color:rgb(51,105,232);border-top-width:2px"> Software Engineer |</td>
<td nowrap style="border-top-style:solid;border-top-color:rgb(0,153,57);border-top-width:2px"> <a href="mailto:tfiala@google.com" style="color:rgb(17,85,204)" target="_blank"><span style="background-color:rgb(255,255,204);color:rgb(34,34,34);background-repeat:initial initial">tfiala@google.com</span></a> |</td>
<td nowrap style="border-top-style:solid;border-top-color:rgb(238,178,17);border-top-width:2px"><font color="#1155cc"> <a>650-943-3180</a></font></td></tr></tbody></table><br></div>
</div></div>
</blockquote></div><br><br clear="all"><div><br></div>-- <br><div dir="ltr"><table cellspacing="0" cellpadding="0" style="color:rgb(136,136,136);font-family:'Times New Roman'"><tbody><tr style="color:rgb(85,85,85);font-family:sans-serif;font-size:small">
<td nowrap style="border-top-style:solid;border-top-color:rgb(213,15,37);border-top-width:2px">Todd Fiala |</td><td nowrap style="border-top-style:solid;border-top-color:rgb(51,105,232);border-top-width:2px"> Software Engineer |</td>
<td nowrap style="border-top-style:solid;border-top-color:rgb(0,153,57);border-top-width:2px"> <a href="mailto:tfiala@google.com" style="color:rgb(17,85,204)" target="_blank"><span style="background-color:rgb(255,255,204);color:rgb(34,34,34);background-repeat:initial initial">tfiala@google.com</span></a> |</td>
<td nowrap style="border-top-style:solid;border-top-color:rgb(238,178,17);border-top-width:2px"><font color="#1155cc"> <a>650-943-3180</a></font></td></tr></tbody></table><br></div>
</div>