[Bug translator/19510] New: the "private" keyword support has made -p1 output less useful

[Bug translator/19510] New: the "private" keyword support has made -p1 output less useful

[dsmith at redhat dot com]

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

            Bug ID: 19510
           Summary: the "private" keyword support has made -p1 output less
                    useful
           Product: systemtap
           Version: unspecified
            Status: NEW
          Severity: normal
          Priority: P2
         Component: translator
          Assignee: systemtap at sourceware dot org
          Reporter: dsmith at redhat dot com
  Target Milestone: ---

As mentioned in bug #19136 comment #2, commit 38bf68a introduced the "private"
keyword. This works, but had the side effect of changing parser output. With
commit 38bf68a, you'll see this:

====
# stap -p1 ../src/testsuite/buildok/array_size.stp | head -n 11
# parse tree dump
# file ../src/testsuite/buildok/array_size.stp
global __global_a[1]
global __global_b[100000]
global __global_c
probe begin{
(a[42, "foobar"]) = ("Hello World!");
(b["foo", "bar", "baz", 42]) = (314159265);
(c[42]) = (161803399);
printf("%s %d %d\\n", a[42, "foobar"], b["foo", "bar", "baz", 42], c[42]);
}
====

As you can see, the globals 'a', 'b', and 'c' have been mangled to add the
'__global_' prefix. However, their uses in the begin probe haven't been mangled
yet. This makes the parser output less useful and less readable for the user.

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

[Bug translator/19510] the "private" keyword support has made -p1 output less useful

[dsmith at redhat dot com]

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

--- Comment #1 from David Smith <dsmith at redhat dot com> ---
Commit 9fef07f improves the situation and adds a testcase (in
systemtap.pass1-4/parse-semok.exp). The testcase tries to make sure that if
'stap -p2' works on a script in testsuite/buildok, that the 'stap -p1' output
passed to 'stap -p2' also works.

With the code changes in that commit, the testcase failures drop from 38
failures in the original code to 2 failures in the new code.

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

[Bug translator/19510] the "private" keyword support has made -p1 output less useful

[dsmith at redhat dot com]

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

--- Comment #2 from David Smith <dsmith at redhat dot com> ---
The 2 failues in the testcase are:

====
Running ../../src/testsuite/systemtap.pass1-4/parse-semok.exp ...
FAIL: buildok/delete.stp parse-semok - parsed output is not semok
FAIL: buildok/one.stp parse-semok - parsed output is not semok
====

1) delete.stp: Here's the smallest delete problem:

====
# cat minidelete.stp 
global a, b
probe begin {
        a = 1
        delete a
        b = "b"
        delete b
}
#
# stap -p2 minidelete.stp
# globals
a:long
b:string
# probes
begin /* <- begin */
#
# stap -p1 minidelete.stp > minidelete.p1
# cat minidelete.p1
# parse tree dump
# file minidelete.stp
global a
global b
probe begin{
(a) = (1)
delete a
(b) = ("b")
delete b
}

# stap -p2 minidelete.p1
semantic error: unresolved function (similar: __global_HZ, __global_pn,
__global_pp, __global_cpu, __global_gid): identifier 'a' at minidelete.p1:7:8
        source: delete a
                       ^

Pass 2: analysis failed.  [man error::pass2]
====

So, for some reason the addition of parenthesis around 'a' causes stap to not
be able to find 'a'. I'm not sure that it matters, but in the error message
stap seems to think that 'a' is a function, not a global variable.

2) one.stp: Here's the smallest 'one' problem described:

====
# cat minione.stp
#! stap -p4

global foo
probe begin
{
        foo["hello"] = 25
        foo["hello"]++
        foo["hello"] = 0;
        ++foo["hello"]
}
# stap -p2 minione.stp
# globals
foo:long [string]
# probes
begin /* <- begin */
end /* <- end */
  # locals
  :string
  :long
# stap -p1 minione.stp > minione.p1
# cat minione.p1
# parse tree dump
# file minione.stp
global foo
probe begin{
(foo["hello"]) = (25)
(foo["hello"])++
(foo["hello"]) = (0)
++(foo["hello"])
}
# stap -p2 minione.p1
WARNING: Eliding side-effect-free expression : operator '(' at minione.p1:8:3
 source: ++(foo["hello"])
           ^
# globals
foo:long [string]
# probes
begin /* <- begin */
end /* <- end */
  # locals
  :string
  :long
====

So, somehow the addition of parentheses aournd '++(foo["hello")' causes the
warning.

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

[Bug translator/19510] the "private" keyword support has made -p1 output less useful

[jistone at redhat dot com]

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

Josh Stone <jistone at redhat dot com> changed:

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

--- Comment #3 from Josh Stone <jistone at redhat dot com> ---
Both of these are due to not using semicolons.  Whitespace is not significant,
including newlines, so write it on one line to get an idea what the parser
sees.

> delete a b = "b"
> delete a (b) = ("b")

The latter looks like it's calling a(b).

> foo["hello"] = 0 ; ++ foo["hello"]
> (foo["hello"]) = (0) ++ (foo["hello"])

Note the former required a semicolon to parse as desired!
The latter is parsed like (0)++, then side-effect-free foo["hello"].


We probably should output semicolons after all statements to disambiguate
things, similar to how we wrap all expressions in parentheses.

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

[Bug translator/19510] the "private" keyword support has made -p1 output less useful

[dsmith at redhat dot com]

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

David Smith <dsmith at redhat dot com> changed:

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

--- Comment #4 from David Smith <dsmith at redhat dot com> ---
(In reply to Josh Stone from comment #3)
> Both of these are due to not using semicolons.  Whitespace is not
> significant, including newlines, so write it on one line to get an idea what
> the parser sees.
> 
> > delete a b = "b"
> > delete a (b) = ("b")
> 
> The latter looks like it's calling a(b).
> 
> > foo["hello"] = 0 ; ++ foo["hello"]
> > (foo["hello"]) = (0) ++ (foo["hello"])
> 
> Note the former required a semicolon to parse as desired!
> The latter is parsed like (0)++, then side-effect-free foo["hello"].
> 
> 
> We probably should output semicolons after all statements to disambiguate
> things, similar to how we wrap all expressions in parentheses.

Yep, it looks like that was the problem.

I solved this by cherry-picking commit 3070a37 from the dsmith/interactive
branch (where I had already fixed this problem). This commit adds a semi-colon
to the end of statements in '-p1' mode. On the master branch, this is commit
d89c429. I also added commit 62ac4ea, which fixes a few places missed by commit
9fef07f when adding the unmangled name global variable output support.

The new parse-semok.exp test now passes completely on f23
(4.3.3-300.fc23.x86_64).

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