Bug 11504 - Wrong location for inferred type conflict in error message
Summary: Wrong location for inferred type conflict in error message
Status: RESOLVED FIXED
Alias: None
Product: systemtap
Classification: Unclassified
Component: translator (show other bugs)
Version: unspecified
: P2 normal
Target Milestone: ---
Assignee: Unassigned
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-04-16 10:52 UTC by Mark Wielaard
Modified: 2011-07-20 21:30 UTC (History)
0 users

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


Attachments
Patch for 11504 (1.71 KB, text/plain)
2010-05-14 19:16 UTC, Charley
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Wielaard 2010-04-16 10:52:08 UTC
The following script will give a wrong location for the actual derived type
conflict:

probe nfs.aop.readpages {
	printf("size=%d\n", size);
}

probe nfs.aop.readpages.return {
	printf("size=%s\n", size);
}

$ stap -p2 t.stp
semantic error: probe_2472 with type mismatch (long vs. string): identifier
'size' at /usr/share/systemtap/tapset/nfs.stp:930:13
        source:             size = $return
                            ^
semantic error: probe_2472 type first inferred here (string): identifier 'size'
at t.stp:2:22
        source: 	printf("size=%d\n", size);
                	                    ^
Pass 2: analysis failed.  Try again with another '--vp 01' option.

The problem isn't on line 2, where size is used as a long, but on line 6 where
size is used as string.
Comment 1 Charley 2010-04-16 16:43:55 UTC
fwiw, it looks like typeresolutioninfo::mismatched in elaborate.cxx searches
resolved_toks for the first matching definition. The error should have been
thrown when parsing the second instance of size in the given script, but it
seems to have gotten thrown off because 'size' is not a locally declared global.
Instead of comparing (string) size to (global double) size it does the reverse.
I'll look for a way to unreverse it without breaking anything else.

Checking types might be a safe way to resolve this problem, but resolved_toks
don't have an exp_type to compare to.

-C
Comment 2 Charley 2010-04-16 19:36:43 UTC
I was mistaken, size is not a global in nfs.stp. 'size' is a local parameter
taken from the probe point nfs.aop.readpages.{,return}. 

I could fix this by giving tokens an exp_type attribute, or (hand-waving) some
sort of context-filtering to determine if two tokens are visible to each other.
Not all tokens can have a sensible exp_type.

On a side note, declaring size as global in nfs.stp gets rid of this problem.
Caveat: type mismatch with size as a global creates a longer error:

semantic error: probe_1978 with type mismatch (long vs. string): identifier
'size' at /usr/local/share/systemtap/tapset/nfs.stp:875:13
        source:             size = $return
                            ^
semantic error: probe_1978 type first inferred here (string): identifier 'size'
at temp/11504.stp:2:22
        source: 	printf("size=%s\n", size);
                	                    ^
semantic error: probe_1980 with type mismatch (string vs. long): identifier
'nr_to_write' at /usr/local/share/systemtap/tapset/nfs.stp:1004:16
        source:         size = nr_to_write
                               ^
semantic error: probe_1980 type first inferred here (long): identifier
'nr_to_write' at :996:9
        source:         nr_to_write = $wbc->nr_to_write
Comment 3 Frank Ch. Eigler 2010-04-30 18:00:19 UTC
The typeresolution_info::mismatch routine, basing matches on the
token->content fields, seems too naive, and leads to junky stuff like:

stap -p2 -e 'probe begin { log(2 + "yo") }'
semantic error: probe_1971 uses invalid operator: operator '+' at <input>:1:21
        source: probe begin { log(2 + "yo") }
                                    ^
semantic error: probe_1971 with type mismatch (string vs. long): string 'yo' at
:1:23
        source: probe begin { log(2 + "yo") }
                                      ^
semantic error: probe_1971 with type mismatch (long vs. string): operator '+' at
:1:21
        source: probe begin { log(2 + "yo") }
                                    ^
semantic error: probe_1971 type first inferred here (string): operator '+' at :1:21
        source: probe begin { log(2 + "yo") }
                                    ^
semantic error: probe_1971 with type mismatch (long vs. string): identifier
'log' at :1:15
        source: probe begin { log(2 + "yo") }


The core issue is that it is not tokens that get types, but expression objects.
Comment 4 Charley 2010-05-04 19:19:16 UTC
(In reply to comment #3)
> The typeresolution_info::mismatch routine, basing matches on the
> token->content fields, seems too naive, and leads to junky stuff like:
> The core issue is that it is not tokens that get types, but expression objects.

Should I add an exp_type to tokens? It wouldn't be used very often.
Alternatively, we could keep an array of resolved expression objects and use
that to check for mismatches?
Comment 5 Frank Ch. Eigler 2010-05-10 14:33:16 UTC
(In reply to comment #4)
> (In reply to comment #3)
> > The typeresolution_info::mismatch routine, basing matches on the
> > token->content fields, seems too naive, and leads to junky stuff like:
> > The core issue is that it is not tokens that get types, but expression objects.
> 
> Should I add an exp_type to tokens? It wouldn't be used very often.

No, that would not make sense.

> Alternatively, we could keep an array of resolved expression objects and use
> that to check for mismatches?

That would make much more sense.  Try passing/storing expression* and/or
symbol* instead of token*'s to the various resolved/invalid/mismatch()
family of functions.
Comment 6 Charley 2010-05-14 19:16:01 UTC
Created attachment 4793 [details]
Patch for 11504

Attempt #2. Very minor hack to keep an exp_type* vector that's updated whenever
the tokens vector is updated. This appears to fix the problem, though I don't
know if it is efficient enough.

Attempt #1 (not attached) was passing expression to
typeresolution_info::resolved, which failed because I couldn't get an
expression* from the Referrer/Referent templates in
elaborate.cxx:resolve_2types. 

I will try getting the exp_type from symbol* as well, if this patch is not
acceptable.
Comment 7 Mark Wielaard 2011-07-20 21:30:46 UTC
Current stap gets this right:
$ stap -p2 t.stp
semantic error: type mismatch (string vs. long): identifier 'size' at t.stp:6:22
        source: 	printf("size=%s\n", size);
                	                    ^
semantic error: type was first inferred here (long): identifier 'size' at :2:22
        source: 	printf("size=%d\n", size);
                	                    ^
Pass 2: analysis failed.  Try again with another '--vp 01' option.