Bug 11346 - @defined() with '--skip-badvars' doesn't work correctly
Summary: @defined() with '--skip-badvars' doesn't work correctly
Status: RESOLVED FIXED
Alias: None
Product: systemtap
Classification: Unclassified
Component: translator (show other bugs)
Version: unspecified
: P2 normal
Target Milestone: ---
Assignee: Josh Stone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-03-03 20:57 UTC by David Smith
Modified: 2010-03-18 22:56 UTC (History)
0 users

See Also:
Host:
Target:
Build:
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description David Smith 2010-03-03 20:57:42 UTC
On kernel 2.6.31.12-174.2.22.fc12.x86_64, assume the following script:

====
probe kernel.function("SyS_getxattr").call !,                                   
      kernel.function("sys_getxattr").call                                      
{                                                                               
        path = user_string(@defined($foo) ? $foo : $pathname)                   
        printf("getxattr: %s\n", path)                                          
}                                                                               
====

When run, you'll get this:

====
#  stap -v -p2 ../defined1.stp
Pass 1: parsed user script and 65 library script(s) using
48504virt/19368res/1656shr kb, in 140usr/40sys/180real ms.
# functions
_dwarf_tvar_get_pathname_2:long ()
%{
#define fetch_register k_fetch_register
#define store_register k_store_register
{
  {
    uint64_t addr;
    { // DWARF expression: 0x55
    { uint64_t value = fetch_register (5); addr = value; }
    }
    THIS->__retvalue = addr;
  }
  goto out;
if (0) goto deref_fault;
deref_fault:
  goto out;
}
/* pure *//* unprivileged */
#undef fetch_register

#undef store_register
%}
user_string:string (addr:long)
return user_string2(addr, "<unknown>")
user_string2:string (addr:long, err_msg:string)
%{ /* pure */ /* unprivileged */
  assert_is_myproc();
  if (_stp_strncpy_from_user (THIS->__retvalue, 
        (const char __user*) (uintptr_t) THIS->addr,
        MAXSTRINGLEN) < 0)
    strlcpy (THIS->__retvalue, THIS->err_msg, MAXSTRINGLEN);
%}
# probes
kernel.function("sys_getxattr@fs/xattr.c:348").call /* pc=_stext+0x11e167 */ /*
<- kernel.function("SyS_getxattr").call!,kernel.function("sys_getxattr").call */
  # locals
  path:string
{
(path) = (user_string(_dwarf_tvar_get_pathname_2()))
printf("getxattr: %s\\n", path)
}
Pass 2: analyzed script: 1 probe(s), 3 function(s), 0 embed(s), 0 global(s)
using 185792virt/116112res/74748shr kb, in 600usr/620sys/1227real ms.
====

Notice how the line:
    path = user_string(@defined($foo) ? $foo : $pathname)
got replaced by:
    (path) = (user_string(_dwarf_tvar_get_pathname_2()))

This looks reasonable.

Now run the same stap command again, but add '--skip-badvars':

====
# stap --skip-badvars -v -p2 ../defined1.stp
Pass 1: parsed user script and 65 library script(s) using
48504virt/19368res/1656shr kb, in 130usr/40sys/171real ms.
WARNING: Bad $context variable being substituted with literal 0: identifier
'$foo' at ../defined1.stp:5:37
 source:         path = user_string(@defined($foo) ? $foo : $pathname)
                                             ^
# functions
user_string:string (addr:long)
return user_string2(addr, "<unknown>")
user_string2:string (addr:long, err_msg:string)
%{ /* pure */ /* unprivileged */
  assert_is_myproc();
  if (_stp_strncpy_from_user (THIS->__retvalue, 
        (const char __user*) (uintptr_t) THIS->addr,
        MAXSTRINGLEN) < 0)
    strlcpy (THIS->__retvalue, THIS->err_msg, MAXSTRINGLEN);
%}
# probes
kernel.function("sys_getxattr@fs/xattr.c:348").call /* pc=_stext+0x11e167 */ /*
<- kernel.function("SyS_getxattr").call!,kernel.function("sys_getxattr").call */
  # locals
  path:string
{
(path) = (user_string(0))
printf("getxattr: %s\\n", path)
}
Pass 2: analyzed script: 1 probe(s), 2 function(s), 0 embed(s), 0 global(s)
using 185792virt/116108res/74748shr kb, in 500usr/570sys/1074real ms.
====

Notice the new warning we get:

  WARNING: Bad $context variable being substituted with literal 0: identifier
'$foo' at ../defined1.stp:5:37
   source:         path = user_string(@defined($foo) ? $foo : $pathname)

Notice how the line:
    path = user_string(@defined($foo) ? $foo : $pathname)
got replaced by:
    (path) = (user_string(0))

This replacement is wrong.
Comment 1 Josh Stone 2010-03-03 22:31:36 UTC
(In reply to comment #0)
> Notice how the line:
>     path = user_string(@defined($foo) ? $foo : $pathname)
> got replaced by:
>     (path) = (user_string(0))
> 
> This replacement is wrong.

Well, it's "correct" in that $foo is "defined" to 0 when it doesn't exist -- but
I agree this fails to DWIM.  I think we can just bypass the badvar replacement
while inside a @defined traversal.
Comment 2 Frank Ch. Eigler 2010-03-03 22:45:08 UTC
> Well, it's "correct" in that $foo is "defined" to 0 when it doesn't exist -- but
> I agree this fails to DWIM.  I think we can just bypass the badvar replacement
> while inside a @defined traversal.

tapsets.cxx:2611 ish intends to set the policy w.r.t. conflicting --skip-badvars
vs @defined().  We might just flip it around.
Comment 3 Josh Stone 2010-03-05 02:18:13 UTC
commit b26c8b31
Comment 4 David Smith 2010-03-18 20:22:32 UTC
I'm still seeing this with with HEAD systemtap:

# stap -p2 --skip-badvars -e 'probe kernel.function("sys_getxattr")
{printf("%s\n", user_string(@defined($pathname) ? $pathname : $path)) }'
WARNING: Bad $context variable being substituted with literal 0: identifier
'$path' at <input>:1:101
 source: probe kernel.function("sys_getxattr") {printf("%s\n",
user_string(@defined($pathname) ? $pathname : $path)) }
                                                                               
                             ^
# functions
_dwarf_tvar_get_pathname_1:long ()
user_string:string (addr:long)
user_string2:string (addr:long, err_msg:string)
# probes
kernel.function("sys_getxattr@fs/xattr.c:377") /* pc=_stext+0x1394c2 */ /* <-
kernel.function("sys_getxattr") */
Comment 5 Josh Stone 2010-03-18 22:56:14 UTC
(In reply to comment #4)
> I'm still seeing this with with HEAD systemtap:
> 
> # stap -p2 --skip-badvars -e 'probe kernel.function("sys_getxattr")
> {printf("%s\n", user_string(@defined($pathname) ? $pathname : $path)) }'
> WARNING: Bad $context variable being substituted with literal 0: identifier
> '$path' at <input>:1:101

The generated code was actually doing the right thing, but you're right, we
don't even want to see the WARNING at all.  I pushed a new fix with a testcase
too, so hopefully now it's ok.

commit 9fab2262961c9cd1ab3efea5d362b8a6a1c0c7c3