[Bug testsuite/16473] New: rlimit.exp: adjust for weak symbols

[Bug testsuite/16473] New: rlimit.exp: adjust for weak symbols

[jlebon at redhat dot com]

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

            Bug ID: 16473
           Summary: rlimit.exp: adjust for weak symbols
           Product: systemtap
           Version: unspecified
            Status: NEW
          Severity: normal
          Priority: P2
         Component: testsuite
          Assignee: systemtap at sourceware dot org
          Reporter: jlebon at redhat dot com

Since support for weak symbols was added (commits 91bb908 and 0035051, also see
bug 10208), the rlimit.exp test fails as follow:

Running ../../systemtap/testsuite/systemtap.base/rlimit.exp ...
FAIL: rlimit unlimited
FAIL: rlimit AS increase (as root)
FAIL: rlimit AS realistic
FAIL: rlimit CPU increase (as root)
FAIL: rlimit CPU realistic
FAIL: rlimit NPROC increase (as root)
FAIL: rlimit NPROC realistic
FAIL: rlimit STACK increase (as root)
FAIL: rlimit STACK realistic
FAIL: rlimit FSIZE increase (as root)
FAIL: rlimit FSIZE realistic

The reason is that the rlimit.stp script is defined as:

probe kernel.function("*ar*").call { println($$vars) }

And this pattern now matches many weak symbols (especially those suffixed with
e.g. '.part.xx'). These weak symbols don't seem to have any variables
accessible, and we thus get errors when trying to resolve $$vars. This is the
kind of error we get:

[vm build]$ stap -e 'probe process("/lib*/libc.so.*").function("chmod") {
println($$vars) }' -p4
semantic error: unresolved target-symbol expression: identifier '$$vars' at
<input>:1:62
        source: probe process("/lib*/libc.so.*").function("chmod") {
println($$vars) }
                                                                             ^

Pass 2: analysis failed.  [man error::pass2]
[vm build]$ 

If this is an issue with weak symbols not giving access to $$vars when it
should, then bug 10208 should be re-opened, and this one closed. If this is an
issue with rlimit.stp being too enthusiastic, then we should change the pattern
to make it more restrictive.

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

[Bug testsuite/16473] rlimit.exp: adjust for weak symbols

[lberk at redhat dot com]

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

Lukas Berk <lberk at redhat dot com> changed:

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

--- Comment #1 from Lukas Berk <lberk at redhat dot com> ---
Running `cat /proc/kallsyms | grep "part.*"` doesn't show any weak symbols for
me.  This is more akin to PR15999.  If .part.* files are desirable then feel
free to reopen PR15999 with this added info, otherwise we might want to add
those functions to the blacklist.

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

[Bug testsuite/16473] rlimit.exp: adjust for weak symbols

[jlebon at redhat dot com]

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

--- Comment #2 from Jonathan Lebon <jlebon at redhat dot com> ---
I did some more research on the '.part.xx' functions. I believe they come from
partial function inlining, a feature introduced in gcc 4.6[1][2]. Related
interesting (but old) info re. debuginfo for those functions in [3].

These functions are artificial since they are added by the compiler, and do not
appear in the source code. Additionally, as far as I can tell, SystemTap cannot
read any debuginfo variables from them. (Not sure if this is by design, or if
we're not picking up something we should).

If they're still considered useful for users to probe, then we can keep them
and just change the rlimit testcase to not pick those up. If not, then as Lukas
mentioned, we can blacklist them.

[1] http://gcc.gnu.org/gcc-4.6/changes.html
[2] http://gcc.gnu.org/ml/gcc-patches/2010-06/msg02143.html
[3] http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44712

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

[Bug testsuite/16473] rlimit.exp: adjust for weak symbols

[jlebon at redhat dot com]

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

--- Comment #3 from Jonathan Lebon <jlebon at redhat dot com> ---
This may be a duplicate of bug 12609.

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

[Bug testsuite/16473] rlimit.exp: adjust for weak symbols

[jlebon at redhat dot com]

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

--- Comment #4 from Jonathan Lebon <jlebon at redhat dot com> ---
Created attachment 7455
  --> https://sourceware.org/bugzilla/attachment.cgi?id=7455&action=edit
rlimit.patch

Looking more closely into this, I think I have a better idea of what's going
on. During partial inlining, non-inlined bits are left in e.g. foo.part.0().
This function has no DWARF info associated with it (GCC PR44712 discusses
implementing this, but it doesn't seem like there's anything in yet). It does
however show up in the symtab.

Since assess_dbinfo_reqt() only sees a function_str, it deems that the symtab
should be investigated as well. So when we tell stap to list all the functions
matching e.g. "*ar*", we pick up both DWARF functions as well as symtab
functions matching this.

This causes things that require DWARF such as $$vars to not get resolved, which
then causes an error thrown later on during type resolution.

So the error is correct insomuch as stap did try to do its best in finding
matching functions. It could be argued that a better error message should be
output (e.g. "need DWARF information for this").

As for rlimit.exp, all we need is a simple patch that will cause
assess_dbinfo_reqt() to require DWARF info (see attached patch).

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

[Bug testsuite/16473] rlimit.exp: adjust for weak symbols

[jlebon at redhat dot com]

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

--- Comment #5 from Jonathan Lebon <jlebon at redhat dot com> ---
(In reply to Jonathan Lebon from comment #4)
> Created attachment 7455 [details]
> rlimit.patch
> 
> Looking more closely into this, I think I have a better idea of what's going
> on. During partial inlining, non-inlined bits are left in e.g. foo.part.0().
> This function has no DWARF info associated with it (GCC PR44712 discusses
> implementing this, but it doesn't seem like there's anything in yet). It
> does however show up in the symtab.
> 
> Since assess_dbinfo_reqt() only sees a function_str, it deems that the
> symtab should be investigated as well. So when we tell stap to list all the
> functions matching e.g. "*ar*", we pick up both DWARF functions as well as
> symtab functions matching this.
> 
> This causes things that require DWARF such as $$vars to not get resolved,
> which then causes an error thrown later on during type resolution.
> 
> So the error is correct insomuch as stap did try to do its best in finding
> matching functions. It could be argued that a better error message should be
> output (e.g. "need DWARF information for this").
> 
> As for rlimit.exp, all we need is a simple patch that will cause
> assess_dbinfo_reqt() to require DWARF info (see attached patch).

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

[Bug testsuite/16473] rlimit.exp: adjust for weak symbols

[jlebon at redhat dot com]

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

--- Comment #6 from Jonathan Lebon <jlebon at redhat dot com> ---
(Sorry for the double-post)

(In reply to Jonathan Lebon from comment #4)
> So the error is correct insomuch as stap did try to do its best in finding
> matching functions. It could be argued that a better error message should be
> output (e.g. "need DWARF information for this").

The alternative is to have SystemTap silently exclude these functions by first
doing a pass on the probe body to determine if dwarf info is required. This
could be seen as misleading since the actual set of functions no longer depends
on the probe point syntax, but on whether the body requires dwarf or not.

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

[Bug testsuite/16473] rlimit.exp: adjust for weak symbols

[jistone at redhat dot com]

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

Josh Stone <jistone at redhat dot com> changed:

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

--- Comment #7 from Josh Stone <jistone at redhat dot com> ---
IMO it's fine to let $$vars/$$parms/etc. be an empty string for such functions.

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

[Bug testsuite/16473] rlimit.exp: adjust for weak symbols

[jlebon at redhat dot com]

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

Jonathan Lebon <jlebon at redhat dot com> changed:

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

--- Comment #8 from Jonathan Lebon <jlebon at redhat dot com> ---
(In reply to Josh Stone from comment #7)
> IMO it's fine to let $$vars/$$parms/etc. be an empty string for such
> functions.

This is done in commit a769688. The rlimit.exp testcase no longer fails.

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