[Bug tapsets/2861] New: user_string fault handling

[Bug tapsets/2861] New: user_string fault handling

[hunt at redhat dot com]

Fallout from bug 2637.  Now that we have more page faults, we must handle them
gracefully.

Currently when user_string() fails, it sets CONTEXT->last_error, which causes
the script to exit, unless you set MAXERRORS to something high enough. Using
MAXERRORS this way is not appropriate and you end up just always setting it to
something high so your scripts don't terminate, even if they should because they
are dividing by zero or overflowing arrays. Userspace data not always being
available is an expected limitation of our implementation, not an unexpected error.

I think a better solution is to generate a warning and return "<unknown>".
The warning might even be optional. 

This has been discussed before with no consensus.  Leaving things broken is not
an option. Please document your objections and post alternatives.

-- 
           Summary: user_string fault handling
           Product: systemtap
           Version: unspecified
            Status: NEW
          Severity: normal
          Priority: P1
         Component: tapsets
        AssignedTo: systemtap at sources dot redhat dot com
        ReportedBy: hunt at redhat dot com


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

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

[Bug tapsets/2861] user_string fault handling

[fche at redhat dot com]

------- Additional Comments From fche at redhat dot com  2006-06-28 18:56 -------
For the short term, I would like to see a separate user_string() variant that
silently tolerates errors.  This variant could return a fixed string in this
case, or (even better) the string given by an additional parameter:

   user_string (addr) ==> noisily fails as presently
   user_string2 (addr,msg) ==> quietly returns msg on access error

The syscalls tapset would presumably switch to the second form.

One might modify the translator to permit overloading by function arity, thus
not requiring such awesome creativity with function naming.


-- 


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

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

[Bug tapsets/2861] user_string fault handling

[varap at us dot ibm dot com]

------- Additional Comments From varap at us dot ibm dot com  2006-06-28 20:43 -------
FYI: Adding kprobes maintainers so they are aware of the proposal that are
discussed in this context and if it has any affect on their work.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |anil dot s dot keshavamurthy
                   |                            |at intel dot com, prasanna
                   |                            |at in dot ibm dot com,
                   |                            |ananth at in dot ibm dot
                   |                            |com, jkenisto at us dot ibm
                   |                            |dot com


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

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

[Bug tapsets/2861] user_string fault handling

[hien at us dot ibm dot com]

------- Additional Comments From hien at us dot ibm dot com  2006-06-28 20:52 -------
>    user_string2 (addr,msg) ==> quietly returns msg on access error

This should be good for syscalls tapset.

-- 


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

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

[Bug tapsets/2861] user_string fault handling

[hunt at redhat dot com]

------- Additional Comments From hunt at redhat dot com  2006-06-28 21:59 -------
Subject: Re:  user_string fault handling

On Wed, 2006-06-28 at 18:56 +0000, fche at redhat dot com wrote:
> ------- Additional Comments From fche at redhat dot com  2006-06-28 18:56 -------
> For the short term, I would like to see a separate user_string() variant that
> silently tolerates errors.  This variant could return a fixed string in this
> case, or (even better) the string given by an additional parameter:
> 
>    user_string (addr) ==> noisily fails as presently
>    user_string2 (addr,msg) ==> quietly returns msg on access error
> 
> The syscalls tapset would presumably switch to the second form.

Is there really a need for a caller-specified error message that would
justify the overhead of the extra strcpy? 

I'd like to avoid changing all the current code. How about

user_string(addr) ==> returns "<unknown>" on error
user_string(addr, 0) ==> same as above but prints warning
user_string(addr, 1) ==> prints error message and sets lasterr

> One might modify the translator to permit overloading by function arity, thus
> not requiring such awesome creativity with function naming.

A good idea independent regardless of how we solve this problem. Is
there a PR for it?




-- 


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

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

[Bug tapsets/2861] user_string fault handling

[fche at redhat dot com]

------- Additional Comments From fche at redhat dot com  2006-06-29 01:08 -------
> Is there really a need for a caller-specified error message that would
> justify the overhead of the extra strcpy? 

Yes, as a style matter, in order to avoid setting arbitrary english strings as
values that may need to be further processed by script.
 
> I'd like to avoid changing all the current code.  How about
> 
> user_string(addr) ==> returns "<unknown>" on error

This works okay only if the returned string is simply going to be passed
straight to an english-speaking user, and not e.g. tested by the script
for its own error-detection purposes.  Plus it has no way of telling apart
legal occurrences of the fixed string "<unknown>" from the error indication.
In the absence of multiple return values and exceptions, letting the caller
specify their favorite soft-error value seems to be he next best thing.

> user_string(addr, 0) ==> same as above but prints warning

The ", 0" setting is not helpful (nor is the "0" too informative).  We
don't print run-time warnings at the present, and for good reason: they
are only noise.

> user_string(addr, 1) ==> prints error message and sets lasterr

I don't find the ", 1" is too informative.


Nevertheless, I am not strongly attached to these options, so if others
wish to voice support for hunt's suggestion, or offer new ones, please do.


-- 


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

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

Re: [Bug tapsets/2861] user_string fault handling

[James Dickens]

On 29 Jun 2006 01:08:46 -0000, fche at redhat dot com
<sourceware-bugzilla@sourceware.org> wrote:
>
> ------- Additional Comments From fche at redhat dot com  2006-06-29 01:08 -------
> > Is there really a need for a caller-specified error message that would
> > justify the overhead of the extra strcpy?
>
> Yes, as a style matter, in order to avoid setting arbitrary english strings as
> values that may need to be further processed by script.
>
> > I'd like to avoid changing all the current code.  How about
> >
> > user_string(addr) ==> returns "<unknown>" on error

shouldn't it return NULL or possibly -1 and then the script then
request the exact error code as common with C programming?

James Dickens
uadmin.blogspot.com



>
> This works okay only if the returned string is simply going to be passed
> straight to an english-speaking user, and not e.g. tested by the script
> for its own error-detection purposes.  Plus it has no way of telling apart
> legal occurrences of the fixed string "<unknown>" from the error indication.
> In the absence of multiple return values and exceptions, letting the caller
> specify their favorite soft-error value seems to be he next best thing.
>
> > user_string(addr, 0) ==> same as above but prints warning
>
> The ", 0" setting is not helpful (nor is the "0" too informative).  We
> don't print run-time warnings at the present, and for good reason: they
> are only noise.
>
> > user_string(addr, 1) ==> prints error message and sets lasterr
>
> I don't find the ", 1" is too informative.
>
>
> Nevertheless, I am not strongly attached to these options, so if others
> wish to voice support for hunt's suggestion, or offer new ones, please do.
>
>
> --
>
>
> http://sourceware.org/bugzilla/show_bug.cgi?id=2861
>
> ------- You are receiving this mail because: -------
> You are the assignee for the bug, or are watching the assignee.
>

Re: [Bug tapsets/2861] user_string fault handling

[Martin Hunt]

On Thu, 2006-06-29 at 01:08 +0000, fche at redhat dot com wrote:

> 
> > user_string(addr, 0) ==> same as above but prints warning
> 
> The ", 0" setting is not helpful (nor is the "0" too informative).  We
> don't print run-time warnings at the present, and for good reason: they
> are only noise.

Unless, of course, you are interested in that "noise".  As a developer I
often want to see it.

> > user_string(addr, 1) ==> prints error message and sets lasterr
> 
> I don't find the ", 1" is too informative.

How is it less informative than user_string2() as you proposed?  I was
thinking it would be better to have parameters that modify the functions
behavior than lots of functions that do almost the same thing.

OK, here is what I am going to do for now. 

user_string(addr:long) - returns "<unknown>" on error, otherwise no
warnings, errors, etc. This is simplest and cleanest and will satisfy
99.99% of uses. 

user_string2(addr:long, err_msg:string) - Those who want to check for
errors and are afraid that files named "<unknown>" might confuse their
script can use this and give it something less likely to be a valid
filename or data.  This could be renamed to user_string() too if/when we
change the translator.

user_string_warn(addr:long) - For debugging/developers. If copies are
failing often, maybe we want some more details. Not likely to be used
much, but I sometimes use something like it.

[Bug tapsets/2861] user_string fault handling

[jamesd dot wi at gmail dot com]

------- Additional Comments From jamesd dot wi at gmail dot com  2006-06-29 03:16 -------
Subject: Re:  user_string fault handling

On 29 Jun 2006 01:08:46 -0000, fche at redhat dot com
<sourceware-bugzilla@sourceware.org> wrote:
>
> ------- Additional Comments From fche at redhat dot com  2006-06-29 01:08 -------
> > Is there really a need for a caller-specified error message that would
> > justify the overhead of the extra strcpy?
>
> Yes, as a style matter, in order to avoid setting arbitrary english strings as
> values that may need to be further processed by script.
>
> > I'd like to avoid changing all the current code.  How about
> >
> > user_string(addr) ==> returns "<unknown>" on error

shouldn't it return NULL or possibly -1 and then the script then
request the exact error code as common with C programming?

James Dickens
uadmin.blogspot.com



>
> This works okay only if the returned string is simply going to be passed
> straight to an english-speaking user, and not e.g. tested by the script
> for its own error-detection purposes.  Plus it has no way of telling apart
> legal occurrences of the fixed string "<unknown>" from the error indication.
> In the absence of multiple return values and exceptions, letting the caller
> specify their favorite soft-error value seems to be he next best thing.
>
> > user_string(addr, 0) ==> same as above but prints warning
>
> The ", 0" setting is not helpful (nor is the "0" too informative).  We
> don't print run-time warnings at the present, and for good reason: they
> are only noise.
>
> > user_string(addr, 1) ==> prints error message and sets lasterr
>
> I don't find the ", 1" is too informative.
>
>
> Nevertheless, I am not strongly attached to these options, so if others
> wish to voice support for hunt's suggestion, or offer new ones, please do.
>
>
> --
>
>
> http://sourceware.org/bugzilla/show_bug.cgi?id=2861
>
> ------- You are receiving this mail because: -------
> You are the assignee for the bug, or are watching the assignee.
>


-- 


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

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

[Bug tapsets/2861] user_string fault handling

[hunt at redhat dot com]

------- Additional Comments From hunt at redhat dot com  2006-06-30 19:13 -------
Subject: Re:  user_string fault handling

On Thu, 2006-06-29 at 01:08 +0000, fche at redhat dot com wrote:

> 
> > user_string(addr, 0) ==> same as above but prints warning
> 
> The ", 0" setting is not helpful (nor is the "0" too informative).  We
> don't print run-time warnings at the present, and for good reason: they
> are only noise.

Unless, of course, you are interested in that "noise".  As a developer I
often want to see it.

> > user_string(addr, 1) ==> prints error message and sets lasterr
> 
> I don't find the ", 1" is too informative.

How is it less informative than user_string2() as you proposed?  I was
thinking it would be better to have parameters that modify the functions
behavior than lots of functions that do almost the same thing.

OK, here is what I am going to do for now. 

user_string(addr:long) - returns "<unknown>" on error, otherwise no
warnings, errors, etc. This is simplest and cleanest and will satisfy
99.99% of uses. 

user_string2(addr:long, err_msg:string) - Those who want to check for
errors and are afraid that files named "<unknown>" might confuse their
script can use this and give it something less likely to be a valid
filename or data.  This could be renamed to user_string() too if/when we
change the translator.

user_string_warn(addr:long) - For debugging/developers. If copies are
failing often, maybe we want some more details. Not likely to be used
much, but I sometimes use something like it.





-- 


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

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

[Bug tapsets/2861] user_string fault handling

[hunt at redhat dot com]

------- Additional Comments From hunt at redhat dot com  2006-07-11 21:17 -------
FYI, I created a special function for the syscall tapset.

user_string_quoted(path) returns
NULL        for path = 0
<unknown>   on error
"/home/me/file name"  (the string in quotes) if successful.

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


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

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