Bug 10877

(In reply to comment #0)
> Currently we store just one tok* per probe_point, instead of one
> per probe point element (the stuff around the dots).  This leads
> to hard-to-interpret error messages
> 
> % stap -p2 -e 'probe kernel.junkction("sdf") {}'
> semantic error: probe point mismatch at position 1 (alternatives:
> function(number) function(string) mark(string) statement(number)
> statement(string) trace(string)): identifier 'kernel' at <input>:1:7 while
> resolving probe point kernel.junkction("sdf")
>         source: probe kernel.junkction("sdf") {}
>                       ^
> Pass 2: analysis failed.  Try again with another '--vp 01' option.
> 
> Instead, we should store a tok* per probe point element, so that an
> error can point the caret at just the right spot, and we can drop
> the "at position N" from the error description body.


To confirm, you are suggesting the use of:

>         source: probe kernel.junkction("sdf") {}
>                              ^

Yes?

Created attachment 4366 [details]
In parse_probe, each block now stores the current token value

Appears to work on basic tests:
[toto:systemtap]$ stap -e 'probe kernel.blah {printf("blah")}' 

semantic error: probe point mismatch at position 1 (alternatives:
function(number) function(string) mark(string) statement(number)
statement(string) trace(string)): operator '{' at <input>:1:19 while resolving
probe point kernel.blah
	source: probe kernel.blah {printf("blah")}
				  ^
Pass 2: analysis failed.  Try again with another '--vp 01' option.

Created attachment 4373 [details]
Possible patch for 10877

Seems too simplistic, but I'm not sure what features it currently messes with.
I've tested the working scripts I have, as well as checked the following for
proper error messages:

stap -e 'probe kernel.asdfg {printf("HI")}'
stap -e 'probe kernel.asdfg (printf("HI")}'
stap -e 'probe kernel.asdfg {printf{"HI")}'
stap -e 'probe kernel.asdfg {printf("HI")} probe kernel.blah {printf("HI")}'

In all cases the arrow points to an acceptable error (in the first three cases,
only one error is identified per probe point, but it is one of the valid errors
(the 'right-most' one).

Example output for the latter:
semantic error: probe point mismatch at position 1 (alternatives:
function(number) function(string) mark(string) statement(number)
statement(string) trace(string)): identifier 'asdfg' at <input>:1:14 while
resolving probe point kernel.asdfg
	source: probe kernel.asdfg {printf("HI")} probe kernel.blah
{printf("HELLO")}
			     ^
semantic error: probe point mismatch at position 1 (alternatives:
function(number) function(string) mark(string) statement(number)
statement(string) trace(string)): identifier 'blah' at :1:48 while resolving
probe point kernel.blah
	source: probe kernel.asdfg {printf("HI")} probe kernel.blah
{printf("HELLO")}
							       ^

Created attachment 4374 [details]
Large patch for 10877

Add token* to components, remove from probe_point.

In many cases, p->tok was replaced with p->components.front()->tok, which seems
to approximate what was being done before (with p->tok storing the first token,
and since it seems components are being added via push_back). Error reporting
uses component[pos]. Only removed a single unnecessary call to p->tok, rest
were modified.

Example output:
[toto:systemtap]$ stap -e 'probe kernel.asdfg {printf("HI")} probe nogood.blah
{printf("HELLO")}'
semantic error: probe point mismatch at position 1 (alternatives:
function(number) function(string) mark(string) statement(number)
statement(string) trace(string)): identifier 'asdfg' at <input>:1:14 while
resolving probe point kernel.asdfg
	source: probe kernel.asdfg {printf("HI")} probe nogood.blah
{printf("HELLO")}
			     ^
semantic error: probe point mismatch at position 0 (alternatives: __scheduler
_linuxmib _signal _sunrpc _syscall _vfs begin begin(number) end end(number)
error error(number) generic ioblock ioscheduler ipmib irq_handler kernel kprobe
kprocess linuxmib module(string) nd_syscall netdev never nfs nfsd perfmon
process process(number) process(string) procfs procfs(string) scheduler scsi
signal socket softirq stap staprun sunrpc syscall tcp tcpmib timer tty udp vfs
vm workqueue): identifier 'nogood' at :1:41 while resolving probe point
nogood.blah
	source: probe kernel.asdfg {printf("HI")} probe nogood.blah
{printf("HELLO")}
							^

Created attachment 4380 [details]
Patch for 10877, add const tok* to all components

In the first batch, the arrow will point at the start of the last token in the
truncated probe point, rather than the end. It may be preferable to just have
it point to the beginning of the probe point...? 

Here's a result of a diff on make check before/after changes:

42c42
< systemtap version: version 1.0/0.142 commit release-1.0-197-gdd905cf +
changes
---
> systemtap version: version 1.0/0.142 commit release-1.0-197-gdd905cf
45c45
< Test Run By chwang on Tue Nov 10 10:19:27 2009
---
> Test Run By chwang on Mon Nov  9 17:25:18 2009
59c59
< Snapshot: version 1.0/0.142 commit release-1.0-197-gdd905cf + changes
---
> Snapshot: version 1.0/0.142 commit release-1.0-197-gdd905cf
263c263
< Started a systemtap server as PID==29984
---
> Started a systemtap server as PID==32154
270c270
< Stopping the systemtap server with PID==29984
---
> Stopping the systemtap server with PID==32154

committed