There was another stray plain int I'd missed in the code that reads
registers, truncating the PC.
The attached patch makes lldb compile (modulo some link ordering dependency
problems that
I don't have enough cmake-fu to fix properly) and work on Linux (and as a
bonus prevents a
segfault when attempting to attach to a running process, though that
functionality appears
to be unimplemented as yet). I also added some extra logging for ptrace
that helped me catch
some of the problems.
On Thu, Oct 27, 2011 at 12:54 PM, Greg Clayton <[email protected]> wrote:
>
> The darwin ptrace call looks from <sys/ptrace.h> looks like:
>
> int ptrace(int _request, pid_t _pid, caddr_t _addr, int _data);
>
> I never try to use "int" "long" as types when programming in LLDB and we
> would try to exclusively use "uint*_t" and "int*_t" where the type is
> explicit. The only exception to the rule is if you are wrapping an API
> (like say "ptrace") where it returns a specific type in the header file
> ("int" in our header file). If the return types differ from system to
> system, we should templatize the code the uses it.
>
> So overall we should try to use the explicitly sized integer typedefs from
> stdint.h (uint8_t, uint16_t, uint32_t, etc) to avoid any such issues. It
> sounds like there are some issues in the Linux plug-in. The function is:
>
> void
> LinuxThread::BreakNotify(const ProcessMessage &message)
> {
> bool status;
> LogSP log (ProcessLinuxLog::GetLogIfAllCategoriesSet
> (LINUX_LOG_THREAD));
>
> assert(GetRegisterContextLinux());
> status = GetRegisterContextLinux()->UpdateAfterBreakpoint();
> assert(status && "Breakpoint update failed!");
>
> // With our register state restored, resolve the breakpoint object
> // corresponding to our current PC.
> assert(GetRegisterContext());
> lldb::addr_t pc = GetRegisterContext()->GetPC();
> if (log)
> log->Printf ("LinuxThread::%s () PC=0x%8.8llx", __FUNCTION__, pc);
> lldb::BreakpointSiteSP
> bp_site(GetProcess().GetBreakpointSiteList().FindByAddress(pc));
> assert(bp_site);
> lldb::break_id_t bp_id = bp_site->GetID();
> assert(bp_site && bp_site->ValidForThisThread(this));
>
>
> m_breakpoint = bp_site;
> m_stop_info = StopInfo::CreateStopReasonWithBreakpointSiteID(*this,
> bp_id);
> }
>
>
>
> Liiks like the PC is read from the register context and there doesn't seem
> to be a breakpoint site. Enable the logging before you run:
>
> (lldb) log enable plugin.process.linux thread
> (lldb) run
>
> This should cause the PC to be logged. Then you should check that a
> software breakpoint was indeed set at this location. You might also want to
> verify that no one removed the breakpoint site after stopping?
>
>
>
Index: source/Plugins/Process/Linux/ProcessLinux.cpp
===================================================================
--- source/Plugins/Process/Linux/ProcessLinux.cpp (revision 142727)
+++ source/Plugins/Process/Linux/ProcessLinux.cpp (working copy)
@@ -87,10 +87,6 @@
m_in_limbo(false),
m_exit_now(false)
{
- // FIXME: Putting this code in the ctor and saving the byte order in a
- // member variable is a hack to avoid const qual issues in GetByteOrder.
- ObjectFile *obj_file = GetTarget().GetExecutableModule()->GetObjectFile();
- m_byte_order = obj_file->GetByteOrder();
}
ProcessLinux::~ProcessLinux()
@@ -485,9 +481,8 @@
ByteOrder
ProcessLinux::GetByteOrder() const
{
- // FIXME: We should be able to extract this value directly. See comment in
- // ProcessLinux().
- return m_byte_order;
+ ObjectFile *obj_file = const_cast<ProcessLinux *>(this)->GetTarget().GetExecutableModule()->GetObjectFile();
+ return obj_file->GetByteOrder();
}
size_t
Index: source/Plugins/Process/Linux/ProcessMonitor.cpp
===================================================================
--- source/Plugins/Process/Linux/ProcessMonitor.cpp (revision 142727)
+++ source/Plugins/Process/Linux/ProcessMonitor.cpp (working copy)
@@ -16,6 +16,7 @@
#include <sys/socket.h>
#include <sys/types.h>
#include <sys/wait.h>
+#include <sys/user.h>
// C++ Includes
// Other libraries and framework includes
@@ -33,7 +34,10 @@
#include "ProcessLinuxLog.h"
#include "ProcessMonitor.h"
+//#define DEBUG_PTRACE
+#define DEBUG_PTRACE_MAXBYTES 20
+
using namespace lldb_private;
// FIXME: this code is host-dependent with respect to types and
@@ -46,21 +50,138 @@
// avoid the additional indirection and checks.
#ifndef LLDB_CONFIGURATION_BUILDANDINTEGRATION
+#ifdef DEBUG_PTRACE
+void display_bytes(char * buf, void * addr, int count)
+{
+ unsigned char * ptr = (unsigned char *)addr;
+ int max = (count > DEBUG_PTRACE_MAXBYTES) ? DEBUG_PTRACE_MAXBYTES : count;
+ for(int loopc=0; loopc<max; loopc++)
+ {
+ buf += sprintf(buf, "[%x]", (unsigned int)(*ptr));
+ ptr++;
+ }
+}
+#endif
+
// Wrapper for ptrace to catch errors and log calls.
extern long
PtraceWrapper(__ptrace_request req, pid_t pid, void *addr, void *data,
const char* reqName, const char* file, int line)
{
- int result;
-
+ long int result;
+ char buf[(DEBUG_PTRACE_MAXBYTES+1)*5];
+
LogSP log (ProcessLinuxLog::GetLogIfAllCategoriesSet (LINUX_LOG_PTRACE));
if (log)
log->Printf("ptrace(%s, %u, %p, %p) called from file %s line %d",
reqName, pid, addr, data, file, line);
-
+
+#ifdef DEBUG_PTRACE
+ if (log)
+ {
+ switch(req)
+ {
+ case PTRACE_POKETEXT:
+ {
+ display_bytes(buf, &data, 8);
+ log->Printf("PTRACE_POKETEXT %s", buf);
+ break;
+ }
+ case PTRACE_POKEDATA:
+ {
+ display_bytes(buf, &data, 8);
+ log->Printf("PTRACE_POKEDATA %s", buf);
+ break;
+ }
+ case PTRACE_POKEUSER:
+ {
+ display_bytes(buf, &data, 8);
+ log->Printf("PTRACE_POKEUSER %s", buf);
+ break;
+ }
+ case PTRACE_SETREGS:
+ {
+ display_bytes(buf, data, sizeof(user_regs_struct));
+ log->Printf("PTRACE_SETREGS %s", buf);
+ break;
+ }
+ case PTRACE_SETFPREGS:
+ {
+ display_bytes(buf, data, sizeof(user_fpregs_struct));
+ log->Printf("PTRACE_SETFPREGS %s", buf);
+ break;
+ }
+ case PTRACE_SETSIGINFO:
+ {
+ display_bytes(buf, data, sizeof(siginfo_t));
+ log->Printf("PTRACE_SETSIGINFO %s", buf);
+ break;
+ }
+ default:
+ {
+ }
+ }
+ }
+#endif
+
errno = 0;
+
result = ptrace(req, pid, addr, data);
-
+
+#ifdef DEBUG_PTRACE
+ if (log)
+ {
+ switch(req)
+ {
+ case PTRACE_PEEKTEXT:
+ {
+ display_bytes(buf, (void *)(&result), sizeof(result));
+ log->Printf("PTRACE_PEEKTEXT %s", buf);
+ break;
+ }
+ case PTRACE_PEEKDATA:
+ {
+ display_bytes(buf, (void *)(&result), sizeof(result));
+ log->Printf("PTRACE_PEEKDATA %s", buf);
+ break;
+ }
+ case PTRACE_PEEKUSER:
+ {
+ display_bytes(buf, (void *)(&result), sizeof(result));
+ log->Printf("PTRACE_PEEKUSER %s", buf);
+ break;
+ }
+ case PTRACE_GETREGS:
+ {
+ display_bytes(buf, data, sizeof(user_regs_struct));
+ log->Printf("PTRACE_GETREGS %s", buf);
+ break;
+ }
+ case PTRACE_GETFPREGS:
+ {
+ display_bytes(buf, data, sizeof(user_fpregs_struct));
+ log->Printf("PTRACE_GETFPREGS %s", buf);
+ break;
+ }
+ case PTRACE_GETSIGINFO:
+ {
+ display_bytes(buf, data, sizeof(siginfo_t));
+ log->Printf("PTRACE_GETSIGINFO %s", buf);
+ break;
+ }
+ case PTRACE_GETEVENTMSG:
+ {
+ display_bytes(buf, data, sizeof(unsigned long));
+ log->Printf("PTRACE_GETEVENTMSG %s", buf);
+ break;
+ }
+ default:
+ {
+ }
+ }
+ }
+#endif
+
if (log && (result == -1 || errno != 0))
{
const char* str;
@@ -352,7 +473,7 @@
// Set errno to zero so that we can detect a failed peek.
errno = 0;
- uint32_t data = PTRACE(PTRACE_PEEKUSER, pid, (void*)m_offset, NULL);
+ lldb::addr_t data = PTRACE(PTRACE_PEEKUSER, pid, (void*)m_offset, NULL);
if (data == -1UL && errno)
m_result = false;
else
Index: source/Plugins/Process/Utility/RegisterContextDarwin_i386.cpp
===================================================================
--- source/Plugins/Process/Utility/RegisterContextDarwin_i386.cpp (revision 142727)
+++ source/Plugins/Process/Utility/RegisterContextDarwin_i386.cpp (working copy)
@@ -9,6 +9,8 @@
// C Includes
+#include <stddef.h>
+
// C++ Includes
// Other libraries and framework includes
#include "lldb/Core/DataBufferHeap.h"
@@ -18,6 +20,7 @@
#include "lldb/Core/Scalar.h"
#include "lldb/Host/Endian.h"
+
// Project includes
#include "RegisterContextDarwin_i386.h"
Index: source/Plugins/Process/Utility/RegisterContextDarwin_x86_64.cpp
===================================================================
--- source/Plugins/Process/Utility/RegisterContextDarwin_x86_64.cpp (revision 142727)
+++ source/Plugins/Process/Utility/RegisterContextDarwin_x86_64.cpp (working copy)
@@ -10,6 +10,7 @@
// C Includes
#include <stdarg.h>
+#include <stddef.h>
// C++ Includes
// Other libraries and framework includes
Index: source/Core/Stream.cpp
===================================================================
--- source/Core/Stream.cpp (revision 142727)
+++ source/Core/Stream.cpp (working copy)
@@ -282,7 +282,7 @@
Stream&
Stream::operator<< (void *p)
{
- Printf ("0x%.*tx", (int)sizeof(void*) * 2, (ptrdiff_t)p);
+ Printf ("0x%.*tx", (int)sizeof(void*) * 2, (std::ptrdiff_t)p);
return *this;
}
_______________________________________________
lldb-dev mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev
