This is the mail archive of the systemtap@sourceware.org mailing list for the systemtap project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Issues with current systemtap and example scripts


>> [wcohen@paketa systemtap]$ stap -e 'probe tp_syscall.open.return{
>> printf("tp %x %x %d %d\n", __tracepoint_arg_regs, $regs,
>> _stp_syscall_nr(), returnval()); exit() }'
>> ERROR: returnval() not defined in this context
>> [...]
>
> This is improved in error.stp.

I spoke too fast; that change is reverted because it hides the error
rather than solving the problem, which is:

- The syscalls tapset does not have a standard -variable- in .return
  probes to pass back the numeric return value.  It has 'retstr' only,
  which is a pretty-printed form.  This was an oversight.

- Because of that oversight, scripts left and right started using the
  returnval() function in errno.stp, which was meant for internal use,
  is documented as imperfect, and normally used from dwarfless probes.

- With 4.17+, the syscalls.* aliases have tp rather than k/u-probes
  alternatives.  Thus no registers, thus a runtime error when
  returnval()/returnstr() are used.  This is what wcohen's tests
  noticed.

So, what to do.

1) Correct the old oversight: add and document a "retval" variable to be
supplied by all the syscall.*.return probe aliases.  Adjust sample
scripts to taste.  Should be a mechanical change to the (gulp) hundreds
of tapset/linux/sysc_* files.

     ...
     retstr = returnstr(1) 
  +  retval = returnval()
     ...
or
     ...
     retstr = return_str(1,$ret)
  +  retval = $ret
     ...
etc.


2) To carry over limping scripts that already broke under 4.17 and used
returnval()/returnstr() in these contexts, maybe also add to all the
sysc_* files:

     ...
     retstr = return_str(1,$ret)
  +  retval = $ret
  +  set_returnval($ret)
     ...
etc., to hide the incoming value in a new reserved field in the CONTEXT
structure.  The returnval()/returnstr() functions would be modified to
look in there.

One problem with this solution is that the set_returnval() function
would be hard to optimize away in case the end-user script doesn't call
returnval().  (This is one of the reasons we prefer to pass such values
via variables - they are optimized away.)  I suggest deprecating this
part of the behaviour in the near future, shifting users toward
retval/retstr.


3) To hide all this boilerplate ret* stuff, suggest adding macros
  @SYSCALL_RETVALSTR($return)     /* for dw aliases */
                    (returnval()) /* for nd aliases */
                    ($ret)        /* for tp aliases */

to a common .stpm file, and using them throughout sysc_* to set
all the ret* values.


- FChE


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]