[Cocci] Make rule depend on comment

[Cocci] Make rule depend on comment

[Christoph Böhmwalder]

Hi,

I have a question regarding the new "comments" feature. Suppose the following:

test.cocci
----------
@ r @
identifier fn;
comments c;
@@
fn()@c
{
}

@ script:python @
fn << r.fn;
c << r.c;
@@
print(fn + " has comment: " + c[0].after)


test.c
------
int f() /* a comment */ { }
int g() { }


spatch --sp-file test.cocci test.c
----------------------------------
f has comment: /* a comment */
g has comment:


The issue here is that I would like to trigger the python script
(i.e. satisfy the "r" rule) iff the comment is actually present.
Can I make the rule depend on the comment being there?

Thanks,

--
Christoph Böhmwalder
LINBIT | Keeping the Digital World Running
DRBD HA —  Disaster Recovery — Software defined Storage
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] Make rule depend on comment

[Markus Elfring]

> I have a question regarding the new "comments" feature.

I hope that further collateral evolution will happen around this functionality.


> @ r @

> identifier fn;

> comments c;

> @@

> fn()@c

> {

> }


* Which comments will appear between a closing parenthesis and an opening curly bracket?

* Are you also looking for function implementations which are not empty?
  (Would you like to add a SmPL ellipsis to your source code search approach?)


> The issue here is that I would like to trigger the python script

> (i.e. satisfy the "r" rule) iff the comment is actually present.

> Can I make the rule depend on the comment being there?


It seems that this expectation is not directly supported by the current software
version at the moment.
But I imagine that something can be achieved in this direction by the usage of
a supported scripting language together with multiple SmPL rules.
How do you think about to use the function “include_match”?
(See also information from “man coccilib
”.)

Regards,
Markus
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] Make rule depend on comment

[Julia Lawall]

[-- Attachment #1: Type: text/plain, Size: 1312 bytes --]



On Fri, 2 Aug 2019, Christoph Böhmwalder wrote:

> Hi,
>
> I have a question regarding the new "comments" feature. Suppose the following:
>
> test.cocci
> ----------
> @ r @
> identifier fn;
> comments c;
> @@
> fn()@c
> {
> }
>
> @ script:python @
> fn << r.fn;
> c << r.c;
> @@
> print(fn + " has comment: " + c[0].after)
>
>
> test.c
> ------
> int f() /* a comment */ { }
> int g() { }
>
>
> spatch --sp-file test.cocci test.c
> ----------------------------------
> f has comment: /* a comment */
> g has comment:
>
>
> The issue here is that I would like to trigger the python script
> (i.e. satisfy the "r" rule) iff the comment is actually present.
> Can I make the rule depend on the comment being there?

A choice could have been to let c only match if there is a comment in at
least one place (before, within, or after).  I don't know if that would be
desirable.

But in your case, would it suffice just to put an if in the python code?
Or is it essential that the first rule actually not match?

julia

>
> Thanks,
>
> --
> Christoph Böhmwalder
> LINBIT | Keeping the Digital World Running
> DRBD HA —  Disaster Recovery — Software defined Storage
> _______________________________________________
> Cocci mailing list
> Cocci@systeme.lip6.fr
> https://systeme.lip6.fr/mailman/listinfo/cocci
>

[-- Attachment #2: Type: text/plain, Size: 136 bytes --]

_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] Make rule depend on comment

[Christoph Böhmwalder]

Thanks for your reply!

> But in your case, would it suffice just to put an if in the python code?
> Or is it essential that the first rule actually not match?
> 
> julia
I'm pretty sure the best way to go about this is to have the script rule
actually not match.
Allow me to outline my real world use-case for this:

Basically, we're building a kernel compatibility layer based on coccinelle for
an out-of-tree module.
This particular semantic patch circles around the k[un]map_atomic() functions.
Specifically, somewhere in the kernel's history, the way to use these functions
went from this:

    void *addr = kmap_atomic(page, KM_USER0);

to this:

    void *addr = kmap_atomic(page);

As we're looking to eventually upstream our module, we obviously cannot have the
old KM_* constants lying around in our code, so we somehow need to restore the
semantic information that was lost by removing this parameter.
The way we decided to go about this is to add a "tag" to each function containing
a k[un]map_atomic call; imagine the following:

    int foo()
    /* kmap compat: KM_USER0 */
    {
        void *addr = kmap_atomic(page);
        // ...
        kunmap_atomic(addr);
    }

It would now be coccinelles job to make sure that each k[un]map_atomic call is
augmented by this legacy parameter (KM_USER0 in this case). Here's the semantic
patch I came up with to accomplish this:


    @ find_kmap_tagged_function @
    comments tag;
    identifier fn;
    @@
    fn(...)@tag {
    ...
    }

    @ script:python parse_kmap_tag @
    tag << find_kmap_tagged_function.tag;
    km;
    @@
    import re
    match = re.search(r'^\/\*\skmap compat: (.*)\s\*\/$', tag[0].after)
    if match:
        coccinelle.km = match.group(1)

    @@
    expression page, addr;
    identifier find_kmap_tagged_function.fn;
    identifier parse_kmap_tag.km;
    @@
    fn(...) {
    <...
    (
    -kmap_atomic(page)
    +kmap_atomic(page, km)
    |
    -kunmap_atomic(addr)
    +kunmap_atomic(addr, km)
    )
    ...>
    }


The first rule finds the comment between the argument list and the opening curly
brace, the second rule parses out the KM_* constant using a regex, and the third
rule replaces all calls to k[un]map_atomic -- inserting the correct parameter.

This works just fine, except for the case where no "tag" was found. Then spatch
produces output like:


     int foo()
     {
            int page = 1234;
    -       void *addr = kmap_atomic(page);
    +       void *addr = kmap_atomic(page,
    +                                initial value: consider using coccinelle.varname);
            // ...
    -       kunmap_atomic(addr);
    +       kunmap_atomic(addr, initial value: consider using coccinelle.varname);


Which seems to me like it would confuse someone who doesn't know exactly what
patching is done behind the scenes.

Ideally, I would like for coccinelle to not touch the function at all if it doesn't
have a "tag". I'm not sure if this is objectively the best solution, but it seems
the most logical to me.

I'd be glad to hear any input you might have regarding this.

Again, thanks for all the help!

--
Christoph Böhmwalder
LINBIT | Keeping the Digital World Running
DRBD HA —  Disaster Recovery — Software defined Storage
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] Make rule depend on comment

[Julia Lawall]

Le 05.08.2019 10:36, Christoph Böhmwalder a écrit :
> Thanks for your reply!
> 
>> But in your case, would it suffice just to put an if in the python 
>> code?
>> Or is it essential that the first rule actually not match?
>> 
>> julia
> I'm pretty sure the best way to go about this is to have the script 
> rule
> actually not match.

OK, this can be done by associating script code with the metavariable 
and having the script code check that the set of comments is non-empty.  
I'll try to add this within the next day.

julia

> Allow me to outline my real world use-case for this:
> 
> Basically, we're building a kernel compatibility layer based on 
> coccinelle for
> an out-of-tree module.
> This particular semantic patch circles around the k[un]map_atomic() 
> functions.
> Specifically, somewhere in the kernel's history, the way to use these 
> functions
> went from this:
> 
>     void *addr = kmap_atomic(page, KM_USER0);
> 
> to this:
> 
>     void *addr = kmap_atomic(page);
> 
> As we're looking to eventually upstream our module, we obviously cannot 
> have the
> old KM_* constants lying around in our code, so we somehow need to 
> restore the
> semantic information that was lost by removing this parameter.
> The way we decided to go about this is to add a "tag" to each function
> containing
> a k[un]map_atomic call; imagine the following:
> 
>     int foo()
>     /* kmap compat: KM_USER0 */
>     {
>         void *addr = kmap_atomic(page);
>         // ...
>         kunmap_atomic(addr);
>     }
> 
> It would now be coccinelles job to make sure that each k[un]map_atomic 
> call is
> augmented by this legacy parameter (KM_USER0 in this case). Here's the 
> semantic
> patch I came up with to accomplish this:
> 
> 
>     @ find_kmap_tagged_function @
>     comments tag;
>     identifier fn;
>     @@
>     fn(...)@tag {
>     ...
>     }
> 
>     @ script:python parse_kmap_tag @
>     tag << find_kmap_tagged_function.tag;
>     km;
>     @@
>     import re
>     match = re.search(r'^\/\*\skmap compat: (.*)\s\*\/$', tag[0].after)
>     if match:
>         coccinelle.km = match.group(1)
> 
>     @@
>     expression page, addr;
>     identifier find_kmap_tagged_function.fn;
>     identifier parse_kmap_tag.km;
>     @@
>     fn(...) {
>     <...
>     (
>     -kmap_atomic(page)
>     +kmap_atomic(page, km)
>     |
>     -kunmap_atomic(addr)
>     +kunmap_atomic(addr, km)
>     )
>     ...>
>     }
> 
> 
> The first rule finds the comment between the argument list and the 
> opening curly
> brace, the second rule parses out the KM_* constant using a regex, and 
> the third
> rule replaces all calls to k[un]map_atomic -- inserting the correct 
> parameter.
> 
> This works just fine, except for the case where no "tag" was found. 
> Then spatch
> produces output like:
> 
> 
>      int foo()
>      {
>             int page = 1234;
>     -       void *addr = kmap_atomic(page);
>     +       void *addr = kmap_atomic(page,
>     +                                initial value: consider using
> coccinelle.varname);
>             // ...
>     -       kunmap_atomic(addr);
>     +       kunmap_atomic(addr, initial value: consider using
> coccinelle.varname);
> 
> 
> Which seems to me like it would confuse someone who doesn't know 
> exactly what
> patching is done behind the scenes.
> 
> Ideally, I would like for coccinelle to not touch the function at all
> if it doesn't
> have a "tag". I'm not sure if this is objectively the best solution,
> but it seems
> the most logical to me.
> 
> I'd be glad to hear any input you might have regarding this.
> 
> Again, thanks for all the help!
> 
> --
> Christoph Böhmwalder
> LINBIT | Keeping the Digital World Running
> DRBD HA —  Disaster Recovery — Software defined Storage
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] Make rule depend on comment

[Markus Elfring]

>     if match:
>         coccinelle.km = match.group(1)

I suggest to add a bit of code like the following.

    else:
         cocci.include_match(False)


How do you think about to use an other SmPL rule variant?

@addition@
expression context;
identifier find_kmap_tagged_function.fn, parse_kmap_tag.km;
@@
 fn(...)
 {
 <+...
(kmap_atomic
|kunmap_atomic
)(context
+, km
 )
 ...+>
 }


Would the use of an additional position variable influence the run time characteristics
in positive ways for this data processing approach?


> This works just fine, except for the case where no "tag" was found.

Would you like to extend corresponding case distinctions any further?


> Ideally, I would like for coccinelle to not touch the function at all if it doesn't
> have a "tag". I'm not sure if this is objectively the best solution, but it seems
> the most logical to me.

Did you check any development possibilities around constraints for metavariables?

Regards,
Markus
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] Make rule depend on comment

[Julia Lawall]

If you pull Coccinelle from github, you will be able to write the
following.  The code on line 2 probably has to be an ocaml script at the
moment, because I didn't do anything to support python.  You can use the
code that is shown in the example directly.  It checks that the list of
comments between the ) and the { is not empty.  The second scriptcan use
ocaml or python as you like.

@ find_kmap_tagged_function @
comments tag : script:ocaml () { let (c1b,c1m,c1a) = List.hd tag in not
(c1a = []) };
identifier fn;
@@
fn(...)@tag {
...
}

@ script:ocaml parse_kmap_tag @
fn << find_kmap_tagged_function.fn;
@@
Printf.printf "have comment in %s\n" fn

--------

julia
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] Make rule depend on comment

[Markus Elfring]

> The code on line 2 probably has to be an ocaml script at the moment,
> because I didn't do anything to support python.

How are the chances to extend the software further (including the corresponding documentation)?


> @ find_kmap_tagged_function @
> comments tag : script:ocaml () { let (c1b,c1m,c1a) = List.hd tag in not
> (c1a = []) };

Would you like to use also regular expressions for such scripted SmPL constraints?

Regards,
Markus
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] Make rule depend on comment

[Christoph Böhmwalder]

On 07.08.19 23:40, Julia Lawall wrote:
> If you pull Coccinelle from github, you will be able to write the
> following.  The code on line 2 probably has to be an ocaml script at the
> moment, because I didn't do anything to support python.  You can use the
> code that is shown in the example directly.  It checks that the list of
> comments between the ) and the { is not empty.  The second scriptcan use
> ocaml or python as you like.
> 
> @ find_kmap_tagged_function @
> comments tag : script:ocaml () { let (c1b,c1m,c1a) = List.hd tag in not
> (c1a = []) };
> identifier fn;
> @@
> fn(...)@tag {
> ...
> }
> 
> @ script:ocaml parse_kmap_tag @
> fn << find_kmap_tagged_function.fn;
> @@
> Printf.printf "have comment in %s\n" fn
> 
> --------
> 
> julia
> 

Great, thanks a lot for implementing this so quickly!

--
Christoph Böhmwalder
LINBIT | Keeping the Digital World Running
DRBD HA —  Disaster Recovery — Software defined Storage
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] Make rule depend on comment

[Markus Elfring]

> If you pull Coccinelle from github, you will be able to write the following.

I am curious how the support will evolve further for scripted SmPL constraints.


> The second scriptcan use ocaml or python as you like.

How do you think about to clarify the following SmPL rule variant?

@addition@
km;
comments tag : script:ocaml ()
 {
 let (_, _, ca) = List.hd tag in
 match ca with
 [] -> false
 | _ -> let xa = String.concat "" ca in
        if Str.string_match (Str.regexp "kmap compat: \(KM_USER[01]\)") xa 2
        then km := make_ident (Str.matched_group 1 xa); true
        else false
 };
expression context;
identifier fn;
@@
 fn(...)@tag
 {
 <+...
(kmap_atomic
|kunmap_atomic
)(context
+, km
 )
 ...+>
 }


int foo()
/* kmap compat: KM_USER0 */
{
void *addr = kmap_atomic(page);
// ...
kunmap_atomic(addr);
}


Can such a data processing approach ever work in the shown direction?

Regards,
Markus
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci