[Bug tapsets/12748] New: need syscall-number database in tapset

[Bug tapsets/12748] New: need syscall-number database in tapset

[fche at redhat dot com]

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

           Summary: need syscall-number database in tapset
           Product: systemtap
           Version: unspecified
            Status: NEW
          Severity: normal
          Priority: P2
         Component: tapsets
        AssignedTo: systemtap@sourceware.org
        ReportedBy: fche@redhat.com


For use by process().syscall and kernel.trace("sys_{enter,exit}"),
scripts need to know the syscall numbering conventions for the
current process.  A tapset should give it to them, perhaps in
the form:

global __syscall_32_num2name[400]
global __syscall_32_name2num[400]
global __syscall_64_num2name[400]
global __syscall_64_name2num[400]

probe begin { 
  .... initialize based on %( arch ... %)
}

function syscall_num(name) {
  return (current_is32bit() ? __syscall_32[name] : __syscall_64[name])
}
function syscall_name(num) { 
  ....
}

-- 
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/12748] need syscall-number database in tapset

[mcermak at redhat dot com]

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

Martin Cermak <mcermak at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
                 CC|                            |mcermak at redhat dot com

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

[Bug tapsets/12748] need syscall-number database in tapset

[mcermak at redhat dot com]

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

--- Comment #1 from Martin Cermak <mcermak at redhat dot com> ---
Created attachment 9343
  --> https://sourceware.org/bugzilla/attachment.cgi?id=9343&action=edit
syscall table tapset generator script

Attached script generates tapset file providing syscall_name() and
syscall_num() functions by parsing the strace sources.  Currently only
x86_64/i386 syscalls are supported, but extension is trivial.

However, it turns out that stap syscall names do not match their strace
counterparts exactly.  For example, all of 'stat64', 'lstat64', and 'fstat64',
known to strace, are being interpreted as 'stat' by systemtap.

Another problem is that e.g. when running the accept.c stap syscall testcase,
_stp_syscall_nr() returns value 102 for various syscalls like 'accept',
'socket', and 'bind'.  But 102 maps to 'socketcall' syscall as known to strace.
Not sure how to reasonably deal with this yet.

So, in general, it's easy to create a tapset carrying the per arch syscall
tables.  But values returned by _stp_syscall_nr() transformed to syscall names
using such tables, very often do not match systemtap syscall names as provided
by the 'name' context variable of probe syscall.

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

[Bug tapsets/12748] need syscall-number database in tapset

[mcermak at redhat dot com]

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

--- Comment #2 from Martin Cermak <mcermak at redhat dot com> ---
Created attachment 9344
  --> https://sourceware.org/bugzilla/attachment.cgi?id=9344&action=edit
Attached patch shows the result of the above generator script, plus it has a
testsuite bit included.

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

[Bug tapsets/12748] need syscall-number database in tapset

[dsmith at redhat dot com]

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

David Smith <dsmith at redhat dot com> changed:

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

--- Comment #3 from David Smith <dsmith at redhat dot com> ---
(In reply to Martin Cermak from comment #1)
> Created attachment 9343 [details]
> syscall table tapset generator script
> 
> Attached script generates tapset file providing syscall_name() and
> syscall_num() functions by parsing the strace sources.  Currently only
> x86_64/i386 syscalls are supported, but extension is trivial.
> 
> However, it turns out that stap syscall names do not match their strace
> counterparts exactly.  For example, all of 'stat64', 'lstat64', and
> 'fstat64', known to strace, are being interpreted as 'stat' by systemtap.

That's by design. The user always calls "stat", "lstat", or "fstat" -
underneath glibc will call either stat/stat64 based on whether a couple of
defines are defined. If the following are defined, glibc will call the 64-bit
versions:

#define _LARGEFILE64_SOURCE
#define _FILE_OFFSET_BITS 64

(BTW, some platforms also have newfstat/newfstatat/newlstat/newstat.)

systemtap tries to report things from the user's point of view, so if we get a
stat/stat64/newstat syscall, it reports all of them just as "stat".

> Another problem is that e.g. when running the accept.c stap syscall
> testcase, _stp_syscall_nr() returns value 102 for various syscalls like
> 'accept', 'socket', and 'bind'.  But 102 maps to 'socketcall' syscall as
> known to strace. Not sure how to reasonably deal with this yet.

I'm not sure you can without looking at the args. What is going on there is
that on some platforms
connect/getpeername/getsockname/getsockopt/listen/recv/recvfrom/recvmsg/send/sendmsg/sendto/setsockopt/shutdown/socket/socketpair
can either be individual syscalls or some/all of them can use socketcall as a
common kernel entry point.

Once again systemtap tries to report what the user actually called, not what
kernel entry point got used.

> So, in general, it's easy to create a tapset carrying the per arch syscall
> tables.  But values returned by _stp_syscall_nr() transformed to syscall
> names using such tables, very often do not match systemtap syscall names as
> provided by the 'name' context variable of probe syscall.

I think we're just going to have to live with them not matching sometimes.

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

[Bug tapsets/12748] need syscall-number database in tapset

[dsmith at redhat dot com]

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

--- Comment #4 from David Smith <dsmith at redhat dot com> ---
(In reply to Martin Cermak from comment #2)
> Created attachment 9344 [details]
> Attached patch shows the result of the above generator script, plus it has a
> testsuite bit included.

This bit here:

====
@define return_sanitized(indexable, key)
%(
    present = 0
    foreach (_key in @indexable)
        if (_key == @key)
            present = 1
    if (present == 0)
        return -1
    else
        return @indexable[@key]
%)
====

can be replaced by the following, so that once we find the key we stop:

====
@define return_sanitized(indexable, key)
%(
    present = 0
    foreach (_key in @indexable) {
        if (_key == @key) {
            present = 1
            break
        }
    }
    if (present == 0)
        return -1
    else
        return @indexable[@key]
%)
====

However, there is an even easer way. stap has a built-in operator for array
index inclusion - "in":

====
@define return_sanitized(indexable, key)
%(
    if (@key in @indexable)
        return @indexable[@key]
    else
        return -1
%)
====

I imagine that the syscall tables themselves will end up in arch-specific
directories. I can't imagine wanting to look up an s390x syscall number on an
x86_64 machine.

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

[Bug tapsets/12748] need syscall-number database in tapset

[dsmith at redhat dot com]

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

--- Comment #5 from David Smith <dsmith at redhat dot com> ---
(In reply to Martin Cermak from comment #1)
> Created attachment 9343 [details]
> syscall table tapset generator script

This script will probably need to become more generic. Right now it assumes an
arch has both a 64-bit and 32-bit syscall table. 32-bit only arches (i686, arm)
will only have 1 32-bit syscall table. Some 64-bit arches don't have a
corresponding 32-bit syscall tables (aarch64, ia64). s390x has a 31-bit syscall
table, but we can probably treat it like a 32-bit syscall table.

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

[Bug tapsets/12748] need syscall-number database in tapset

[mcermak at redhat dot com]

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

--- Comment #6 from Martin Cermak <mcermak at redhat dot com> ---
Created attachment 9345
  --> https://sourceware.org/bugzilla/attachment.cgi?id=9345&action=edit
proposed patch

Attaching updated patch hopefully addressing all of the above.  It contains not
only updated generator script scripts/dump-syscalls.sh, but also
tapset/linux/syscall_table.stp with some generic stuff, and finally 3 arch
specific syscall tables and a testsuite bit.

So far it tests fine with the accept.c testcase, which isn't much, but should
be good enough for a proof of concept.  To address the mapping between
syscalls, function syscall_map(name1, name2) has been added.  Currently it is
being called from testsuite/systemtap.syscall/sys.stp.

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

[Bug tapsets/12748] need syscall-number database in tapset

[dsmith at redhat dot com]

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

--- Comment #7 from David Smith <dsmith at redhat dot com> ---
Comment on attachment 9345
  --> https://sourceware.org/bugzilla/attachment.cgi?id=9345
proposed patch

Right now the script only handles x86_64, ppc64, and s390x. I'd like to see it
handle an arch that has only one ABI (like i686, arm, or aarch64). That will
flush out some issues I'd bet.

I'll bikeshed the following:

====
function syscall_name(num) {
%( CONFIG_COMPAT == "y" %?
    if (@__compat_task)
        return __syscall_32_num2name[num]
%)
    if (%{ BITS_PER_LONG %} == 64)
        return __syscall_64_num2name[num]
    else
        return __syscall_32_num2name[num]
}
====

It would be a bit better as:

====
function syscall_name(num) {
%( CONFIG_COMPAT == "y" %?
   if (@__compat_task)
        return __syscall_32_num2name[num]
%)
%( CONFIG_64BIT == "y" %?
   return __syscall_64_num2name[num]
%:
   return __syscall_32_num2name[num]
%)
}
====

The new version converts that 64-bit runtime check to a compile-time check. The
same thing can be done to the syscall_num() function.

How do you see syscall_map() being used? Is it just for use by the testsuite?

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

[Bug tapsets/12748] need syscall-number database in tapset

[mcermak at redhat dot com]

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

--- Comment #8 from Martin Cermak <mcermak at redhat dot com> ---
Created attachment 9353
  --> https://sourceware.org/bugzilla/attachment.cgi?id=9353&action=edit
proposed patch

This attached patch seems to work fine on rhel-6 and rhel-7 all supported
arches, plus fedora rawhide (x86_64 and armhfp).

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

[Bug tapsets/12748] need syscall-number database in tapset

[dsmith at redhat dot com]

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

--- Comment #9 from David Smith <dsmith at redhat dot com> ---
Comment on attachment 9353
  --> https://sourceware.org/bugzilla/attachment.cgi?id=9353
proposed patch

On single ABI arches, to we really need the other global __syscall_X_num2name
and __syscall_X_name2num arrays? If so, can we set the size to 0 instead of
400?

Those unused arrays will take up a good chunk of space in the module.

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

[Bug tapsets/12748] need syscall-number database in tapset

[mcermak at redhat dot com]

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

--- Comment #10 from Martin Cermak <mcermak at redhat dot com> ---
Created attachment 9356
  --> https://sourceware.org/bugzilla/attachment.cgi?id=9356&action=edit
proposed patch

Good point! Attached updated patch only allocates arrays of needed length,
saving the memory.  It works fine except for i686, where I'm hitting PR20297.

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

[Bug tapsets/12748] need syscall-number database in tapset

[mcermak at redhat dot com]

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

--- Comment #11 from Martin Cermak <mcermak at redhat dot com> ---
Created attachment 9361
  --> https://sourceware.org/bugzilla/attachment.cgi?id=9361&action=edit
proposed patch

One more update fixing the i686 issue.  This patch was tested also on Fedora
i686 and got updated accordingly.  I've been trying to add support for ia64
too, but although stap builds with devtoolset-2-gcc-4.8.2-15.1.el5 recompiled
for ia64 (-w), the resulting systemtap modules do not reasonably kbuild on
rhel5.  I skipped this.  Attached patch was tested on all the platforms
available to me.

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

[Bug tapsets/12748] need syscall-number database in tapset

[mcermak at redhat dot com]

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

Martin Cermak <mcermak at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #12 from Martin Cermak <mcermak at redhat dot com> ---
Fixed in commit 7fe1b7bb8e665c9edd38eaf34a499618ee763293

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