[Cocci] Question about the use of "...when !=" in SmPL

[Cocci] Question about the use of "...when !=" in SmPL

[ZhouYuan]

Hi,
I have met many problems when I try to use the clause of "when".
For example, I want to check whether there is an default statement in switch, I wrote the Semantic Patch as following: 
@rule at function F;position p;expression E, E1;statement S, S1;@@F at p (...){switch (E) {...when != default: S}}
@script: python at p<<rule.p;@@print p[0].file, p[0].line
and I got the result after running it:
Fatal error: exception Failure("minus: parse error:  = File "ex7.cocci", line 9, column 0,  charpos = 93    around = '...', whole content = ...when != default: S")
Also, another example can be witnessed here. I wrote another patch to check whether there is an "else" after  "if" statement:
@rule534 at function F;position p;statement S1, S2;@@F at p (...){if (...) S1...when != else S2}
But i also got the same "minus: parse error"@the line of when.
I would be so appreciated if someone could give me some suggestions about what is wrong in my Patch and how to use when correctly.
Thank you so much!
Best regards,Yuan  		 	   		  
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://systeme.lip6.fr/pipermail/cocci/attachments/20150507/d047f761/attachment.html>

[Cocci] Question about the use of "...when !=" in SmPL

[Nicholas Mc Guire]

On Thu, 07 May 2015, ZhouYuan wrote:

>    Hi,
>    I have met many problems when I try to use the clause of "when".
>    For example, I want to check whether there is an default statement in
>    switch, I wrote the Semantic Patch as following:
> 
>      @rule@
>      function F;
>      position p;
>      expression E, E1;
>      statement S, S1;
>      @@
>      F at p (...){
>      switch (E) {
>      ...when != default: S
>      }
>      }
>      @script: python@
>      p<<rule.p;
>      @@
>      print p[0].file, p[0].line
> 
>    and I got the result after running it:
> 
>      Fatal error: exception Failure("minus: parse error:
>       = File "ex7.cocci", line 9, column 0,  charpos = 93
>          around = '...', whole content = ...when != default: S
>      ")
> 
>    Also, another example can be witnessed here. I wrote another patch to
>    check whether there is an "else" after  "if" statement:
>
probably there is a simpler way - the "solution" I used was
2 rules - one that scans for case statements in the switch 
and one that scans for default statements in a switch body
and then condition the python on hascase && !hasdefault

virtual report                                                                  
virtual org

@hascase@
position p;
@@
switch at p (...)
{
 case...:...
}

@hasdefault@
@@
switch (...)
{
 default:...
}

@script: python depends on hascase && !hasdefault@
p<<hascase.p;
@@
print "%s %s" % (p[0].file, p[0].line)

there likely is a simpler solution than this though.

thx!
hofrat

[Cocci] Source code analysis around "switch"

[SF Markus Elfring]

> @hascase@
> position p;
> @@
> switch at p (...)
> {
>  case...:...
> }
> 
> @hasdefault@
> @@
> switch (...)
> {
>  default:...
> }
> 
> @script: python depends on hascase && !hasdefault@
> p<<hascase.p;
> @@
> print "%s %s" % (p[0].file, p[0].line)
> 
> there likely is a simpler solution than this though.

I find such a SmPL approach incomplete.
Is the shown condition expression fragile and questionable?

Do you need to work more with position variables there
so that only really appropriate source code places
will be combined and then checked?

Regards,
Markus

[Cocci] Source code analysis around "switch"

[Nicholas Mc Guire]

On Sat, 09 May 2015, SF Markus Elfring wrote:

> > @hascase@
> > position p;
> > @@
> > switch at p (...)
> > {
> >  case...:...
> > }
> > 
> > @hasdefault@
> > @@
> > switch (...)
> > {
> >  default:...
> > }
> > 
> > @script: python depends on hascase && !hasdefault@
> > p<<hascase.p;
> > @@
> > print "%s %s" % (p[0].file, p[0].line)
> > 
> > there likely is a simpler solution than this though.
> 
> I find such a SmPL approach incomplete.

What is the incompleteness - or what case do you think is unhandled ?

> Is the shown condition expression fragile and questionable?
> 
> Do you need to work more with position variables there
> so that only really appropriate source code places
> will be combined and then checked?
>
why ? hascase has a position variable in it and only if
that rule applied is the python script executed - and if there
was a default then hasdefault matched and so the python script
will not be executed when hasdefault matched => only switch
statements with no default are reported. Atleast it sems to work
for me - but as noted there well may be a better solution and
maybe its even incomplete - but I do not see that its a missing
positional variable here.

thx!
hofrat 

[Cocci] Question about the use of "...when !=" in SmPL

[Julia Lawall]

On Sat, 9 May 2015, Nicholas Mc Guire wrote:

> On Thu, 07 May 2015, ZhouYuan wrote:
> 
> >    Hi,
> >    I have met many problems when I try to use the clause of "when".
> >    For example, I want to check whether there is an default statement in
> >    switch, I wrote the Semantic Patch as following:
> > 
> >      @rule@
> >      function F;
> >      position p;
> >      expression E, E1;
> >      statement S, S1;
> >      @@
> >      F at p (...){
> >      switch (E) {
> >      ...when != default: S
> >      }
> >      }
> >      @script: python@
> >      p<<rule.p;
> >      @@
> >      print p[0].file, p[0].line
> > 
> >    and I got the result after running it:
> > 
> >      Fatal error: exception Failure("minus: parse error:
> >       = File "ex7.cocci", line 9, column 0,  charpos = 93
> >          around = '...', whole content = ...when != default: S
> >      ")
> > 
> >    Also, another example can be witnessed here. I wrote another patch to
> >    check whether there is an "else" after  "if" statement:
> >
> probably there is a simpler way - the "solution" I used was
> 2 rules - one that scans for case statements in the switch 
> and one that scans for default statements in a switch body
> and then condition the python on hascase && !hasdefault
> 
> virtual report                                                                  
> virtual org
> 
> @hascase@
> position p;
> @@
> switch at p (...)
> {
>  case...:...
> }
> 
> @hasdefault@
> @@
> switch (...)
> {
>  default:...
> }
> 
> @script: python depends on hascase && !hasdefault@
> p<<hascase.p;
> @@
> print "%s %s" % (p[0].file, p[0].line)
> 
> there likely is a simpler solution than this though.

No given the limitations on switch, I think that this is the only 
solution.  When I tried something like this, it gave me all switched, 
though.  Not sure why.

julia

[Cocci] Source code analysis around "switch"

[SF Markus Elfring]

> hascase has a position variable in it and only if
> that rule applied is the python script executed - and if there
> was a default then hasdefault matched and so the python script
> will not be executed when hasdefault matched

Do the source code positions matter also for your second SmPL rule?


> => only switch statements with no default are reported.

Do the concrete statements fit really together?


> Atleast it sems to work for me

I guess that there are further reviews needed.


> - but as noted there well may be a better solution
> and maybe its even incomplete

That is usual.


> - but I do not see that its a missing positional variable here.

I suggest to reconsider this opinion.
Do you need to establish safer relationships for the desired checks?

Regards,
Markus

[Cocci] Question about the use of "...when !=" in SmPL

[Nicholas Mc Guire]

On Sat, 09 May 2015, Julia Lawall wrote:

> 
> 
> On Sat, 9 May 2015, Nicholas Mc Guire wrote:
> 
> > On Thu, 07 May 2015, ZhouYuan wrote:
> > 
> > >    Hi,
> > >    I have met many problems when I try to use the clause of "when".
> > >    For example, I want to check whether there is an default statement in
> > >    switch, I wrote the Semantic Patch as following:
> > > 
> > >      @rule@
> > >      function F;
> > >      position p;
> > >      expression E, E1;
> > >      statement S, S1;
> > >      @@
> > >      F at p (...){
> > >      switch (E) {
> > >      ...when != default: S
> > >      }
> > >      }
> > >      @script: python@
> > >      p<<rule.p;
> > >      @@
> > >      print p[0].file, p[0].line
> > > 
> > >    and I got the result after running it:
> > > 
> > >      Fatal error: exception Failure("minus: parse error:
> > >       = File "ex7.cocci", line 9, column 0,  charpos = 93
> > >          around = '...', whole content = ...when != default: S
> > >      ")
> > > 
> > >    Also, another example can be witnessed here. I wrote another patch to
> > >    check whether there is an "else" after  "if" statement:
> > >
> > probably there is a simpler way - the "solution" I used was
> > 2 rules - one that scans for case statements in the switch 
> > and one that scans for default statements in a switch body
> > and then condition the python on hascase && !hasdefault
> > 
> > virtual report                                                                  
> > virtual org
> > 
> > @hascase@
> > position p;
> > @@
> > switch at p (...)
> > {
> >  case...:...
> > }
> > 
> > @hasdefault@
> > @@
> > switch (...)
> > {
> >  default:...
> > }
> > 
> > @script: python depends on hascase && !hasdefault@
> > p<<hascase.p;
> > @@
> > print "%s %s" % (p[0].file, p[0].line)
> > 
> > there likely is a simpler solution than this though.
> 
> No given the limitations on switch, I think that this is the only 
> solution.  When I tried something like this, it gave me all switched, 
> though.  Not sure why.

I still have spatch version 1.0.0-rc21 on this box - will check
if its behaving any different with a 1.0.

>
just ran this one on the entire kernel and it did find
some cases that were incorrectly handled like

drivers/tty/serial/ip22zilog.c 820 (linux-next 4.1-rc2)
        case CS8:
        default:
which was reported as having no default - but this is ugly code
anyway so its ok to report it.

checked a few others - and except for the above case (which does
pop up a few times) it looks ok (totally 2240 were reported in the 
entire kernel so only checked 25 or so)

thx!
hofrat

[Cocci] Question about the use of "...when !=" in SmPL

[Nicholas Mc Guire]

On Thu, 07 May 2015, ZhouYuan wrote:

your second question regardin if/else was not yet addressed - so here are
some notes on trying to solve this - again this might not be the right 
approach - only got it to work by removing the if structure isomorphisms !

<snip>
> 
>    Also, another example can be witnessed here. I wrote another patch to
>    check whether there is an "else" after  "if" statement:
> 
>      @rule534@
>      function F;
>      position p;
>      statement S1, S2;
>      @@
>      F at p (...){
>      if (...) S1
>      ...when != else S2
>      }
> 
>    But i also got the same "minus: parse error" at the line of when.
>    I would be so appreciated if someone could give me some suggestions about
>    what is wrong in my Patch and how to use when correctly.
>    Thank you so much!

The problem here I think is the default.iso that does not allow
to distinguish the if and else as you like - if we take a simple
general if match like:

<snip>
@ifelse@
function f;
statement S1, S2;
position p;
@@
f (...){
<...
if@p (...) S1 else S2
...>
}
<snip>

and run 
hofrat at debian:/tmp$ spatch --parse-cocci if.cocci 
we get:

ifelse:f(...) {
  <...
  
    
    (
    
    (
    if at ifelse:p (...) ifelse:S1 else ifelse:S2
    |
    if at ifelse:p (...) ifelse:S1
    )
    |
    
    (
    if at ifelse:p (...) ifelse:S2 else ifelse:S1
    |
    if@ifelse:p (...) ifelse:S2
    )
    )
  ...>
}

so the standard.iso "if structure isomorphisms" are not allowing you
to differenciate the if and else here. if you copy that file into
"my.iso" and remove those if structure isomorphisms then you can
differenciate if with and without else.

<snip>
@ifonly@
function f;
statement S1;
position p;
@@
f (...){
<...
*if@p (...) S1
...>
}

@script: python depends on ifonly@
p<<ifonly.p;
@@
print "%s %s" % (p[0].file, p[0].line)
<snip>

The test-case is:

#include <stdio.h>                                                              
<snip>
int bar(void)
{
        int x=2;
        if (x) {
                printf("2\n");
        } else {
                printf("!2\n");
        }
        return 0;
}

int foo(void)
{
        int x=2;
        if (x) {
                printf("2\n");
        }
}
<snip>

run this now with the changed iso file

hofrat at debian:/tmp$ spatch --sp-file if.cocci --iso-file my.iso if.c
init_defs_builtins: /usr/local/share/coccinelle/standard.h
HANDLING: if.c
if.c 17
diff = 
--- if.c
+++ /tmp/cocci-output-12260-c3738e-if.c
@@ -14,8 +14,5 @@ int bar(void)
 int foo(void)
 {
        int x=2;
-       if (x) {
-               printf("2\n");
-       }
 }


thx!
hofrat 

[Cocci] Question about the use of "...when !=" in SmPL

[Julia Lawall]

> @ifonly@
> function f;
> statement S1;
> position p;
> @@
> f (...){
> <...
> *if at p (...) S1
> ...>
> }

This is OK, but unnecessarily complicated.  A position variable has a field
current_element, which is the name ofthe containing function.  So from just
the match if at p (...) S1 one can get everything that is needed.

julia

[Cocci] Question about the use of "...when !=" in SmPL

[Nicholas Mc Guire]

On Sat, 09 May 2015, Nicholas Mc Guire wrote:

> On Thu, 07 May 2015, ZhouYuan wrote:
> 
> your second question regardin if/else was not yet addressed - so here are
> some notes on trying to solve this - again this might not be the right 
> approach - only got it to work by removing the if structure isomorphisms !
> 
> <snip>
> > 
> >    Also, another example can be witnessed here. I wrote another patch to
> >    check whether there is an "else" after  "if" statement:
> > 
> >      @rule534@
> >      function F;
> >      position p;
> >      statement S1, S2;
> >      @@
> >      F at p (...){
> >      if (...) S1
> >      ...when != else S2
> >      }
> > 
> >    But i also got the same "minus: parse error" at the line of when.
> >    I would be so appreciated if someone could give me some suggestions about
> >    what is wrong in my Patch and how to use when correctly.
> >    Thank you so much!
>
forget my first attempt - obviously confused my self sufficiently to find
a very complicated solution to a simple problem - no need to change the
iso file either....

<snip>
virtual report                                                                  
virtual org

@ifonly@
function f;
statement S1;
position p;
@@
f (...){
<...
*if@p (...) S1
...>
}

@script: python depends on ifonly@
p<<ifonly.p;
@@
print "%s %s if without else" % (p[0].file, p[0].line)
<snip>

note that if one really would need to disable some specific isomorphism
then one can do that with "disable" e.g. from the "if structure isomorphisms"
section in standard.iso you could selectively disable neg_if and ne_if like
so:

@ifonly disable ne_if,neg_if@
(see http://www.emn.fr/z-info/coccinelle/docs/main_grammar002.html#disable-iso)

but for this case this is not needed anyway.

So the only problem with your soluion was that you were trying to explicitly
describe the absence while all you would need is to describe what you want
to see. 

Just ran this on the kernel and the first few pages of output looked like 
its working.

thx!
hofrat 

[Cocci] Data provided by position variables

[SF Markus Elfring]

> A position variable has a field current_element,
> which is the name ofthe containing function.

Would you like to extend descriptions in the SmPL manuals
for the corresponding data structures?

Regards,
Markus

[Cocci] Improving small SmPL examples

[SF Markus Elfring]

> forget my first attempt - obviously confused my self sufficiently
> to find a very complicated solution to a simple problem
> - no need to change the iso file either....
> 
> <snip>
> virtual report                                                                  
> virtual org

How do you think about to avoid also references for unneeded variables?

Regards,
Markus

[Cocci] Data provided by position variables

[Julia Lawall]

On Sun, 10 May 2015, SF Markus Elfring wrote:

> > A position variable has a field current_element,
> > which is the name ofthe containing function.
>
> Would you like to extend descriptions in the SmPL manuals
> for the corresponding data structures?

Why don't you propose a patch if you find the documentation insufficient.

Personally, I type man Coccilib and see what is available.

julia

[Cocci] Data provided by position variables

[SF Markus Elfring]

> Personally, I type man Coccilib and see what is available.

How do you think about to improve the documentation with additional links
in the other SmPL manuals for Coccinelle's data structure library?

Regards,
Markus

[Cocci] Question about the use of "...when !=" in SmPL

[Julia Lawall]

Here is my solution for the switch without detection problem.  It is
probably pretty much the same as what Nicholas already proposed:

@ok exists@
position p;
expression E;
@@
switch at p (E) {
default: ...
}

@bad exists@
position p != ok.p;
expression E,e;
@@
switch at p (E) {
case e: ...
}

@script: python@
p<<bad.p;
@@
print p[0].file, p[0].line

-------------------------------------------------------------------------

As Nicholas noted, this currently gives false positives when the default
is hidden under another case.  A patch that fixes this problem is below.

julia

diff --git a/parsing_c/control_flow_c_build.ml b/parsing_c/control_flow_c_build.ml
index c0e8f52..2d353cd 100644
--- a/parsing_c/control_flow_c_build.ml
+++ b/parsing_c/control_flow_c_build.ml
@@ -486,6 +486,14 @@ let rec aux_statement : (nodei option * xinfo) -> statement -> nodei option =

       !g +> add_arc_opt (starti, newswitchi);

+      (* allows multiple case labels to stack up *)
+      let rec contains_default s =
+	match Ast_c.unwrap_st s with
+	  Labeled (Ast_c.Default _) -> true
+	| Labeled (Ast_c.Case(e,s)) -> contains_default s
+	| Labeled (Ast_c.CaseRange(e1,e2,s)) -> contains_default s
+	| _ -> false in
+
        (* call compound case. Need special info to pass to compound case
         * because we need to build a context_info that need some of the
         * information build inside the compound case: the nodei of {
@@ -512,11 +520,7 @@ let rec aux_statement : (nodei option * xinfo) -> statement -> nodei option =
                 * between start to end.
                 * todo? except if the case[range] coverthe whole spectrum
                 *)
-               if not (statxs +> List.exists (fun x ->
-                 match Ast_c.unwrap_st x with
-                 | Labeled (Ast_c.Default _) -> true
-                 | _ -> false
-               ))
+               if not (statxs +> List.exists contains_default)
                then begin
                  (* when there is no default, then a valid path is
                   * from the switchheader to the end. In between we

[Cocci] Fine-tuning for switch statements

[SF Markus Elfring]

> @bad exists@
> position p != ok.p;
> expression E,e;
> @@
> switch at p (E) {
> case e: ...
> }

How do you think about to restrict the label "e"
to a constant expression by the means of the semantic
patch language?
http://en.cppreference.com/w/c/language/switch

Does it matter to make the affected SmPL script
a bit more precise?

Regards,
Markus

[Cocci] Fine-tuning for switch statements

[Julia Lawall]

On Sun, 10 May 2015, SF Markus Elfring wrote:

> > @bad exists@
> > position p != ok.p;
> > expression E,e;
> > @@
> > switch at p (E) {
> > case e: ...
> > }
>
> How do you think about to restrict the label "e"
> to a constant expression by the means of the semantic
> patch language?
> http://en.cppreference.com/w/c/language/switch
>
> Does it matter to make the affected SmPL script
> a bit more precise?

I see no benefit from doing that.  The case is only there to satisfy the
SmPL parser, which for some reason doesn't allow just ...  There is no
intent to restrict what is matched in any way.

julia

[Cocci] Fine-tuning for switch statements

[SF Markus Elfring]

> The case is only there to satisfy the SmPL parser,
> which for some reason doesn't allow just ...

Would you like to change the handling of the SmPL ellipsis
for such an use case?

Regards,
Markus