[Bug tapsets/12500] New: ptrace decoder

[Bug tapsets/12500] New: ptrace decoder

[jan.kratochvil at redhat dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=12500

           Summary: ptrace decoder
           Product: systemtap
           Version: unspecified
            Status: NEW
          Severity: normal
          Priority: P2
         Component: tapsets
        AssignedTo: systemtap@sources.redhat.com
        ReportedBy: jan.kratochvil@redhat.com


Created attachment 5252
  --> http://sourceware.org/bugzilla/attachment.cgi?id=5252
ptrace decoder.

Tested on:
F14 x86_64
RHEL-6.1 s390+s390x+ppc64
ia64 is untested

There is some room for improvements but it works fine.

probe syscall.ptrace.return
// TODO: _arch_ptrace_return would need to create probe variables - how?
 - A function called from probe cannot create variables like `name'.

s390x:
// FIXME: ptrace of 32-bit debuggers is not caught by SystemTap,
//        use +4 and +8 offsets for them

On RHEL-4/RHEL-5 there may be needed more #ifndef/#define's like now in
tapset/powerpc/syscalls.stp but I did not have systemtap-1.4/HEAD on those
boxes to test it.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug tapsets/12500] ptrace decoder

[dsmith at redhat dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=12500

David Smith <dsmith at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |dsmith at redhat dot com

--- Comment #1 from David Smith <dsmith at redhat dot com> 2011-02-24 19:05:45 UTC ---
I've taken a look at the patch.  Here are some comments:

1) On x86_64 RHEL5 (2.6.18-238.1.1.el5), the following come up as undefined:
PTRACE_SINGLEBLOCK, PTRACE_GETREGSET, and PTRACE_SETREGSET.

On ia64 RHEL5 (2.6.18-194.26.1.el5), the following come up as undefined:
PTRACE_GETREGSET and PTRACE_SETREGSET.

It looks like on RHEL5, PTRACE_[GS]ETREGS is used instead of
PTRACE_[GS]ETREGSET.

2) I'm not too fond of the following style of code in
tapset/powerpc/syscalls.stp:

====
function _arch_ptrace(request,pid,addr,data)
{
    if (request == %{ PTRACE_GETVRREGS %})
        // TODO: Retrieve *data in .return
        return sprintf ("PTRACE_GETVRREGS, %d, data=%p", pid, data)
# ... stuff deleted ...
%{
#ifndef PPC_PTRACE_GETHWDBGINFO
# define PPC_PTRACE_GETHWDBGINFO 0x89
#endif
1 %}
    if (request == %{ PPC_PTRACE_GETHWDBGINFO %})
        // TODO: Retrieve *data in .return
        return sprintf ("PPC_PTRACE_GETHWDBGINFO, %d, data=%p", pid, data)
# ... stuff deleted ...
}
====

I'm not sure we envisioned using the inline embedded-C expressions that way. 
Instead I'd suggest something like the following (note that I haven't actually
tried this):

====
%{
#ifndef PPC_PTRACE_GETHWDBGINFO
# define PPC_PTRACE_GETHWDBGINFO 0x89
#endif
%}

function _arch_ptrace(request,pid,addr,data)
{
    if (request == %{ PTRACE_GETVRREGS %})
        // TODO: Retrieve *data in .return
        return sprintf ("PTRACE_GETVRREGS, %d, data=%p", pid, data)
# ... stuff deleted ...
    if (request == %{ PPC_PTRACE_GETHWDBGINFO %})
        // TODO: Retrieve *data in .return
        return sprintf ("PPC_PTRACE_GETHWDBGINFO, %d, data=%p", pid, data)
# ... stuff deleted ...
}
====

You lose a bit of locality, but gain a bit of readability (and don't have to
use a fake return value from the inline embedded-C expression).

3) I'd probably move most of the new code in the syscall.ptrace probe itself to
an auxiliary function, so that nd_syscall.ptrace could share it also.

4) Code like this:

====
probe syscall.ptrace = kernel.function("sys_ptrace").call ?
{
    name = "ptrace"
    request = $request
    pid = $pid
    addr = $addr
    data = $data

    argstr=_arch_ptrace(request,pid,addr,data)
====

is better expressed as:

probe syscall.ptrace = kernel.function("sys_ptrace").call ?
{
    name = "ptrace"
    request = $request
    pid = $pid
    addr = $addr
    data = $data

    argstr=_arch_ptrace($request,$pid,$addr,$data)
====

By using the dwarf variables directly, the optimizer has an easier time getting
rid of unused convenience variables.  (Feel free to use a convenience variable
when getting the convenience variable value is complicated.)

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.