Further Linux adventures

I now have lldb compiling and attempting to execute a program. There seems to be an assumption in the code that sizeof(int) is 8 on a 64 bit platform. This is not the case with Linux/gcc 4.6, leading to some rather nasty bugs (the return value of ptrace() was being assigned to an int, truncating data from peeks, for instance!). I’ve fixed the ptrace() issue but another one is happening somewhere around breakpoints. I added some logging which displays the problem -

Process 4017 launched: ‘/home/jo/reader/calliope’ (x86_64)
(lldb) Looking for address 406780, list size is 1
Entry 0 is 406780 sizeof 8
Looking for address 270f9300, list size is 2
Entry 0 is 406780 sizeof 8
Entry 1 is 7ffb270f9300 sizeof 8
lldb: /home/jo/lldb/llvm/tools/lldb/source/Plugins/Process/Linux/LinuxThread.cpp:249: void LinuxThread::BreakNotify(const ProcessMessage&): Assertion `bp_site’ failed.

Clearly the second breakpoint address is being truncated down to 32 bits somewhere in the code. Before I go spelunking in the Linux subdirectories, is sizeof(int)==8 something that the development team is assuming based on MacOS X? If so, after all, it would be more worth my time to make gcc comply with the OS X definition.

Hmm, you seem to be further away than me. lldb reports the debugged executable as crashing in _start with invalid instruction (on a program that just returns 0 in main). This is after fixing the ptrace PEEKUSER issue, on a somewhat similar setup: gcc 4.5 on x86_64.

Thanks,
Dragos

No it isn't. "int" is 4 bytes on MacOSX. We have 4 byte ints, 8 byte longs, and 8 byte long longs.

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?

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.

linux.diff (7.19 KB)

If the LinuxPlugin is always debugging a native local process, the GetByteOrder() should look like:

ByteOrder
ProcessLinux::GetByteOrder() const
{
lldb::endian::InlHostByteOrder();
}

I don’t think, someone correct me if I am wrong, that the ProcessLinux plug-in can be used for remote debugging? If so, the above fix will work. Else, checking with the object file is ok as long as you always have the correct file selected prior to running.

For the ptrace logging instead of using sprintf, I would suggest using the StringStream:

#include “lldb/Core/StreamString.h”

StringStream strm;

Then your display_bytes() function would take a “lldb_private::Stream &” instead of a “char *” and your code would look like:

void
display_bytes (lldb_private::StreamString &strm, void *bytes, uint32_t count)
{
uint8_t *ptr = (uint8_t *)bytes;
const uint32_t loop_count = std::min<uint32_t>(DEBUG_PTRACE_MAXBYTES, count);
for(uint32_t i=0; i<loop_count; i++)
{
strm.Printf ("[%x]", *ptr);
ptr++;
}
}

Also, don’t worry about #ifdef’ing out the ptrace logging, there is already a “ptrace” logging channel that you are using, If you only want to see the logging when “verbose” is enabled which can be done via:

LogSP log (ProcessLinuxLog::GetLogIfAllCategoriesSet (LINUX_LOG_PTRACE));

LogSP verbose_log (ProcessLinuxLog::GetLogIfAllCategoriesSet (LINUX_LOG_PTRACE | LINUX_LOG_VERBOSE));

Then just check for the “verbose_log” when you want to dump the bits. So instead of:

#ifdef DEBUG_PTRACE
if (log)
{

You would have:

if (verbose_log)
{

So I would switch over to using the StreamString and don’t use the #ifdef DEBUG_PTRACE. One goal in LLDB is to always be able to log the things that are going wrong, and the ptrace calls and the data fall into those categories.

Greg Clayton

Hi,

Here’s a patch with the suggested improvements by Greg. Thanks Joel for fixing lldb on linux!

Thanks,
Dragos

linux_ptrace_fixes_jdillon.patch (4.59 KB)

Can someone please apply this patch (from Dragos's email)?

Thanks!
-Dawn

% svn commit
Sending Linux/ProcessLinux.cpp
Sending Linux/ProcessLinux.h
Sending Linux/ProcessMonitor.cpp
Transmitting file data ...
Committed revision 143772.

I also applied a fix to ProcessLinux for the new way that the Process::DoLaunch() pure virtual function is implemented.

I wasn't able to verify that this compiled, but the fix should be really close and hopefully will just work.

Greg Clayton