[Bug tapsets/20333] New: merge syscall and nd_syscall tapsets

[Bug tapsets/20333] New: merge syscall and nd_syscall tapsets

[dsmith at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=20333

            Bug ID: 20333
           Summary: merge syscall and nd_syscall tapsets
           Product: systemtap
           Version: unspecified
            Status: NEW
          Severity: enhancement
          Priority: P2
         Component: tapsets
          Assignee: systemtap at sourceware dot org
          Reporter: dsmith at redhat dot com
  Target Milestone: ---

It might be possible to merge the syscall and nd_syscall tapsets. Internally,
it might look something like this:

====
probe syscall.open = kernel.function("sys_open").call !,                        
                     kprobe.function("sys_open") ?                              
{                                                                               
        name = "open"                                                           
        if (@defined($filename) && @defined($flags) && @defined($mode))         
        {                                                                       
                filename = user_string_quoted(__uint32($filename))              
                flags = __int32($flags)                                         
                mode = __uint32($mode)                                          
        }                                                                       
        else                                                                    
        {                                                                       
                asmlinkage()
                filename = user_string_quoted(pointer_arg(1))
                flags = int_arg(2)
                mode = uint_arg(3)
        }
        if (flags & @const("O_CREAT"))
                argstr = sprintf("%s, %s, %#o", filename,
                                 _sys_open_flag_str(flags), mode)
        else
                argstr = sprintf("%s, %s", filename,
                                 _sys_open_flag_str(flags))
}

probe nd_syscall.open = syscall.open
====

Note that the above code really isn't sufficient (or tested). The above code
doesn't handle return probes, compat functions, and syscall nesting.

There are some issues with this idea:

- If you have debuginfo installed, there really isn't a way to specify you want
the non-debuginfo version of the probe. (Although why you'd really want to do
that I'm not so sure). The above code prefers the debuginfo version.

- Perhaps the biggest problem with this idea is how users actually use syscall
probes. I've seen user code (and I believe some of our examples) use a syscall
probe, but then also use the 'raw' $FOO variables, which certainly wouldn't
work correctly if no debuginfo was installed.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug tapsets/20333] merge syscall and nd_syscall tapsets

[dsmith at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=20333

--- Comment #1 from David Smith <dsmith at redhat dot com> ---
Another issue to think about here. If the upstream kernel changes the interface
to a syscall, we won't notice, since we'll fall into using the non-debuginfo
version of the probe.

If the interface change was something simple like renaming a parameter, that
could be considered a "feature". If it was a more substantial change, that
would more likely be considered a "bug".

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug tapsets/20333] merge syscall and nd_syscall tapsets

[jistone at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=20333

Josh Stone <jistone at redhat dot com> changed:

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

--- Comment #2 from Josh Stone <jistone at redhat dot com> ---
(In reply to David Smith from comment #0)
> - If you have debuginfo installed, there really isn't a way to specify you
> want the non-debuginfo version of the probe. (Although why you'd really want
> to do that I'm not so sure). The above code prefers the debuginfo version.

I doubt end-users should ever need to force non-debuginfo, but we'll want to do
this for testing at least.  Setting an invalid SYSTEMTAP_DEBUGINFO_PATH should
suffice.

> - Perhaps the biggest problem with this idea is how users actually use
> syscall probes. I've seen user code (and I believe some of our examples) use
> a syscall probe, but then also use the 'raw' $FOO variables, which certainly
> wouldn't work correctly if no debuginfo was installed.

If it weren't for this point, I would have suggested we just ditch the
debuginfo flavor altogether.  But at least having the two share the same probe
body will make the doubled-maintenance more obvious, and make it easier to stay
consistent.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug tapsets/20333] merge syscall and nd_syscall tapsets

[mcermak at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=20333

Martin Cermak <mcermak at redhat dot com> changed:

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

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug tapsets/20333] merge syscall and nd_syscall tapsets

[mcermak at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=20333

Martin Cermak <mcermak at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|systemtap at sourceware dot org    |mcermak at redhat dot com

-- 
You are receiving this mail because:
You are the assignee for the bug.