Re: [Cocci] Coccinelle: Length/Size of char array?
On Mon, 2 Aug 2021, Joe Perches wrote:
> On Mon, 2021-08-02 at 19:35 +0200, Julia Lawall wrote:
> >
> > On Mon, 2 Aug 2021, Joe Perches wrote:
> >
> > > Is it possible to determine the length of a matched char array and use
> > > the length in a test?
> > >
> > > For instance, add something like a test to show only the instances
> > > where a src buffer overruns a dest buffer.
> > >
> > > void foo(void)
> > > {
> > > char foo[5];
> > >
> > > strcpy(foo, "fits");
> > > }
> > >
> > > it would be useful to see only the instances where the dest
> > > buffer would be overrun like:
> > >
> > > void foo(void)
> > > {
> > > char foo[5];
> > >
> > > strcpy(foo, "doesn't fit");
> > > }
> > >
> > > ---
> > >
> > > This would find all instances of a constant src array into non-pointer
> > > dst:
> > >
> > > @@
> > > char [] dest;
> > > constant char [] src;
> > > @@
> > >
> > > * strcpy(dest, src)
> > >
> > > ---
> > >
> > > Is there a mexhanism like:
> > >
> > > @@
> > > char [] dest;
> > > constant char [] src;
> > > @@
> > >
> > > when (some cocci grammar testing length(dest) < length(src))
> > > * strcpy(dest, src)
> >
> > You can match the size and the string, and then use python or ocaml code
> > to do the needed comparisons.
>
> Pardon the question, but how do you determine the size?
In the case of a local variable, you can do:
@r@
constant int n;
identifier i;
constant char [] c;
position p1,p2;
@@
char i@p1[n];
... when exists
strcpy@p2(i,c);
@script:ocaml@
p1 << r.p1;
p2 << r.p2;
n << r.n;
c << r.c;
@@
if string_of_int n < String.length c
then ...
A similar script can be written in python.
If the array is allocated somewhere else, it would be more complicated.
julia
>
> > Does it occur often enough that the string
> > is explicit in the call to make it worth it?
>
> The idea is just to find defects/buffer overruns.
>
>
>
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Coccinelle: Length/Size of char array?
On Mon, 2 Aug 2021, Joe Perches wrote:
> Is it possible to determine the length of a matched char array and use
> the length in a test?
>
> For instance, add something like a test to show only the instances
> where a src buffer overruns a dest buffer.
>
> void foo(void)
> {
> char foo[5];
>
> strcpy(foo, "fits");
> }
>
> it would be useful to see only the instances where the dest
> buffer would be overrun like:
>
> void foo(void)
> {
> char foo[5];
>
> strcpy(foo, "doesn't fit");
> }
>
> ---
>
> This would find all instances of a constant src array into non-pointer dst:
>
> @@
> char [] dest;
> constant char [] src;
> @@
>
> * strcpy(dest, src)
>
> ---
>
> Is there a mexhanism like:
>
> @@
> char [] dest;
> constant char [] src;
> @@
>
> when (some cocci grammar testing length(dest) < length(src))
> * strcpy(dest, src)
You can match the size and the string, and then use python or ocaml code
to do the needed comparisons. Does it occur often enough that the string
is explicit in the call to make it worth it?
julia
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH] scripts: coccinelle: allow list_entry_is_head() to use pos
On Fri, 30 Jul 2021, Daniel Thompson wrote:
> Currently use_after_iter.cocci generates false positives for code of the
> following form:
> ~~~
> list_for_each_entry(d, >irq_list, node) {
> if (irq == d->irq)
> break;
> }
>
> if (list_entry_is_head(d, >irq_list, node))
> return IRQ_NONE;
> ~~~
> [This specific example comes from drivers/power/supply/cpcap-battery.c]
>
> Most list macros use list_entry_is_head() as loop exit condition meaning it
> is not unsafe to reuse pos (a.k.a. d) in the code above.
>
> Let's avoid reporting these cases.
>
> Signed-off-by: Daniel Thompson
> ---
>
> Notes:
> I'm pretty much a complete beginner w.r.t. SmPL. This is written
> entirely by finding previous fixes and emulating them!
>
> However I did test it by running the checker across the current kernel
> tree. The changes reduced the error count by four... which was small
> enough for me to eyeball each one and check they match the pattern I
> was targetting.
This looks fine. Thanks for the proposal.
julia
>
> scripts/coccinelle/iterators/use_after_iter.cocci | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/scripts/coccinelle/iterators/use_after_iter.cocci
> b/scripts/coccinelle/iterators/use_after_iter.cocci
> index 9be48b520879..676edd562eef 100644
> --- a/scripts/coccinelle/iterators/use_after_iter.cocci
> +++ b/scripts/coccinelle/iterators/use_after_iter.cocci
> @@ -123,6 +123,8 @@ hlist_for_each_entry_safe(c,...) S
> |
> list_remove_head(x,c,...)
> |
> +list_entry_is_head(c,...)
> +|
> sizeof(<+...c...+>)
> |
> >member
>
> base-commit: 2734d6c1b1a089fb593ef6a23d4b70903526fe0c
> --
> 2.30.2
>
>
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] how to make substitutions at the end of the function, vs. the end of each block ?
Here is another attempt:
@initialize:ocaml@
@@
let check p =
let p = List.hd p in
p.line_end = p.current_element_line_end
@ detect_func @
identifier CLI_FN, AVM, AIN, ACMD;
fresh identifier LAIN = "line_" ## AIN;
expression ERR, exp;
statement S1;
typedef clib_error_t, vlib_main_t, unformat_input_t, vlib_cli_command_t;
@@
static clib_error_t *CLI_FN (vlib_main_t * AVM, unformat_input_t *
AIN, vlib_cli_command_t * ACMD)
{
+ clib_error_t *e = 0;
+ unformat_input_t *LAIN;
...
+ if (!unformat_user (AIN, unformat_line_input, LAIN)) {
+return 0;
+ }
+
- while (unformat_check_input (AIN) != UNFORMAT_END_OF_INPUT)
+ while (unformat_check_input (LAIN) != UNFORMAT_END_OF_INPUT)
S1
<...
- return ERR;
+ e = ERR;
+ goto done;
...>
}
// the following rule should rematch the function matched in the previous rule
@@
identifier detect_func.CLI_FN, detect_func.AVM, detect_func.AIN,
detect_func.LAIN, detect_func.ACMD;
position p : script:ocaml() { check p }; // check that the matched position is
at the end of the function
@@
static clib_error_t *CLI_FN (vlib_main_t * AVM, unformat_input_t *
AIN, vlib_cli_command_t * ACMD)
{
... when exists
+done:
+ unformat_free(LAIN);
+ return e;
}@p
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] how to make substitutions at the end of the function, vs. the end of each block ?
> yourtch@ayourtch-lnx:~/cocci$ diff -c patch-old.cocci patch-new.cocci > *** patch-old.cocci 2021-07-22 22:41:19.516957878 +0200 > --- patch-new.cocci 2021-07-22 22:41:52.625184341 +0200 > *** > *** 3,8 > --- 3,9 > fresh identifier LAIN = "line_" ## AIN; > > statement S1; > + expression exp; > > typedef clib_error_t, vlib_main_t, unformat_input_t, vlib_cli_command_t; > @@ > *** > *** 20,30 > - while (unformat_check_input (AIN) != UNFORMAT_END_OF_INPUT) > + while (unformat_check_input (LAIN) != UNFORMAT_END_OF_INPUT) > S1 > ! <... > - return ERR; > + e = ERR; > + goto done; > ! ...> > +done: > + unformat_free(LAIN); > + return e; > --- 21,32 > - while (unformat_check_input (AIN) != UNFORMAT_END_OF_INPUT) > + while (unformat_check_input (LAIN) != UNFORMAT_END_OF_INPUT) > S1 > ! <... when != true exp > ! when exists > - return ERR; > + e = ERR; > + goto done; > ! ...> > +done: > + unformat_free(LAIN); > + return e; > ayourtch@ayourtch-lnx:~/cocci$ > > And the result was the same... My trick doesn't work because the return of interest is under a switch, where there is no test expression that has the value true or false. I will see if something else can be done. julia ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] how to make substitutions at the end of the function, vs. the end of each block ?
> I missed the fact that the "return ERR" were actually not replaced on > this example at all, This is because there are no occurrences of return ERR; in your code. Probably you expected ERR to be a metavariable, but it's not. Actually, fully capitalizing your metavariables is not a good idea. Coccinelle expects that fully capitalized things are constants, as in #define ERR -1, and so when they appear at random places, it doesn't comment about that. If it had been in lowercase and used as the argument of a return (or as a function argument, right-hand side of an assignment, etc), Coccinelle would have printed a warning wondering if you expected it to be a metavariable. If ERR is declared as an expression metavariable, then lots of returns are replaced. julia ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] how to make substitutions at the end of the function, vs. the end of each block ?
On Thu, 22 Jul 2021, Andrew Yourtchenko wrote:
> Hi all,
>
> I work on the VPP project (http://fd.io/ - open source software
> dataplane), and tried to use coccinelle to make a relatively
> non-trivial change
> as in the mail https://lists.fd.io/g/vpp-dev/message/17532 - it seemed
> to be a very good candidate - boring enough to be painful to do by
> hand, complex enough to make sed inadequate for it.
Thanks for trying Coccinelle :)
>
> I came up with this semantic patch:
>
>
> @ detect_func @
> identifier CLI_FN, AVM, AIN, ACMD;
> fresh identifier LAIN = "line_" ## AIN;
>
> statement S1;
>
> typedef clib_error_t, vlib_main_t, unformat_input_t, vlib_cli_command_t;
> @@
>
> static clib_error_t *CLI_FN (vlib_main_t * AVM, unformat_input_t *
> AIN, vlib_cli_command_t * ACMD)
> {
> + clib_error_t *e = 0;
> + unformat_input_t *LAIN;
> ...
> + if (!unformat_user (AIN, unformat_line_input, LAIN)) {
> +return 0;
> + }
> +
> - while (unformat_check_input (AIN) != UNFORMAT_END_OF_INPUT)
> + while (unformat_check_input (LAIN) != UNFORMAT_END_OF_INPUT)
> S1
> <...
> - return ERR;
> + e = ERR;
> + goto done;
> ...>
> +done:
> + unformat_free(LAIN);
> + return e;
> }
The problem has to do with the fact that Coccinelle is actually oriented
around control-flow graphs. So it doesn't know which end of a control-flow
path is actually the end of the function.
You can try adjusting the line <... above as follows:
<... when != true exp
when exists
exp should be declared as an expression metavariable. The when != true
thing means that the path cannot cross a true branch across a test of an
expression that matches exp (ie any expression). The when exists means
that the paths through this region of code are considered individually.
I'm not certain that this will work in every case. It will be necessary
to check the results carefully.
Another possible hack is to first replace every return under and if,
while, etc by something else, and then rewrite all of the returns in a
third rule afterwards. This is pretty ugly, but may be more reliable.
julia
>
> I attempt to run it on this test file:
>
> ubuntu@vpp-dev:~$ cat ~/test.c
> static clib_error_t *
> syn_filter_enable_disable_command_fn (vlib_main_t * vm,
> unformat_input_t * input,
> vlib_cli_command_t * cmd)
> {
> vnet_main_t *vnm = vnet_get_main ();
> u32 sw_if_index = ~0;
> int enable_disable = 1;
> int rv;
>
> while (unformat_check_input (input) != UNFORMAT_END_OF_INPUT)
> {
> if (unformat (input, "disable"))
> enable_disable = 0;
> else if (unformat (input, "%U", unformat_vnet_sw_interface,
> vnm, _if_index))
> ;
> else
> break;
> }
>
> if (sw_if_index == ~0)
> return clib_error_return (0, "Please specify an interface...");
>
> rv = syn_filter_enable_disable (sw_if_index, enable_disable);
>
> switch (rv)
> {
> case 0:
> break;
>
> case VNET_API_ERROR_INVALID_SW_IF_INDEX:
> return clib_error_return
> (0, "Invalid interface, only works on physical ports");
> break;
>
> case VNET_API_ERROR_UNIMPLEMENTED:
> return clib_error_return (0,
> "Device driver doesn't support redirection");
> break;
>
> case VNET_API_ERROR_INVALID_VALUE:
> return clib_error_return (0, "feature arc not found");
>
> case VNET_API_ERROR_INVALID_VALUE_2:
> return clib_error_return (0, "feature node not found");
>
> default:
> return clib_error_return (0, "syn_filter_enable_disable returned %d",
> rv);
> }
> return 0;
> }
> ubuntu@vpp-dev:~$
>
>
> However, when I run it, the "done: " label, etc. gets inserted twice:
>
> ubuntu@vpp-dev:~$ spatch --sp-file /tmp/rules.sp
> --allow-inconsistent-paths ~/test.c
> init_defs_builtins: /usr/bin/../lib/coccinelle/standard.h
> HANDLING: /home/ubuntu/test.c
> diff =
> --- /home/ubuntu/test.c
> +++ /tmp/cocci-output-56896-8f35c5-test.c
> @@ -3,12 +3,18 @@ syn_filter_enable_disable_command_fn (vl
> unformat_input_t * input,
> vlib_cli_command_t * cmd)
> {
> + clib_error_t *e = 0;
> + unformat_input_t *line_input;
>vnet_main_t *vnm = vnet_get_main ();
>u32 sw_if_index = ~0;
>int enable_disable = 1;
>int rv;
>
> - while (unformat_check_input (input) != UNFORMAT_END_OF_INPUT)
> + if (!unformat_user(input, unformat_line_input, line_input)) {
> + return 0;
> + }
> +
> +while (unformat_check_input(line_input) != UNFORMAT_END_OF_INPUT)
> {
>if (unformat (input, "disable"))
> enable_disable = 0;
> @@ -48,6 +54,12 @@ syn_filter_enable_disable_command_fn (vl
> default:
>return clib_error_return (0, "syn_filter_enable_disable returned %d",
> rv);
> -}
> +done:
Re: [Cocci] cocci script to convert linux-kernel allocs with BITS_TO_LONGS to bitmap_alloc
On Tue, 13 Jul 2021, Joe Perches wrote: > On Tue, 2021-07-13 at 23:33 +0200, Julia Lawall wrote: > > > > On Fri, 9 Jul 2021, Joe Perches wrote: > > > > > Here is a cocci script to convert various types of bitmap allocations > > > > > that use BITS_TO_LONGS to the more typical bitmap_alloc functions. > > > > I see that there is also a bitmap_free. Maybe the rule should be > > introducing that as well? > > Yes, but as far as I know, it's difficult for coccinelle to convert > the kfree() calls of any previous bitmap_alloc to bitmap_free as > most frequently the kfree() call is in a separate function. Often the code says a->b = foo(); and then the a->b in another function is the same one that was the result of foo(). One could check that this is the only assignment to a->b in the file for more confidence. I'll add it to the rule and see how it goes. julia > > Please do it if you know how, you're probably the best in the world > at coccinelle. I don't know how... > > cheers, Joe > > ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] cocci script to convert linux-kernel allocs with BITS_TO_LONGS to bitmap_alloc
On Sat, 10 Jul 2021, Joe Perches wrote: > On Sat, 2021-07-10 at 21:50 +0200, Julia Lawall wrote: > > On Fri, 9 Jul 2021, Joe Perches wrote: > > > > > Here is a cocci script to convert various types of bitmap allocations > > > that use BITS_TO_LONGS to the more typical bitmap_alloc functions. I see that there is also a bitmap_free. Maybe the rule should be introducing that as well? julia > > > > > > Perhaps something like it could be added to scripts/coccinelle. > > > The diff produced by the script is also below. > > > > > > $ cat bitmap_allocs.cocci > > > // typical uses of bitmap allocations > [] > > > @@ > > > expression val; > > > expression e1; > > > expression e2; > > > @@ > > > > > > - val = kcalloc(BITS_TO_LONGS(e1), sizeof(*val), e2) > > > + val = bitmap_zalloc(e1, e2) > > > > Is there something that guarantees that val has a type that has a size that > > is the same as a long? > > no, but afaict, all do. > > > ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] cocci script to convert linux-kernel allocs with BITS_TO_LONGS to bitmap_alloc
On Sat, 10 Jul 2021, Joe Perches wrote:
> On Sat, 2021-07-10 at 21:50 +0200, Julia Lawall wrote:
> > On Fri, 9 Jul 2021, Joe Perches wrote:
> >
> > > Here is a cocci script to convert various types of bitmap allocations
> > > that use BITS_TO_LONGS to the more typical bitmap_alloc functions.
> > >
> > > Perhaps something like it could be added to scripts/coccinelle.
> > > The diff produced by the script is also below.
> > >
> > > $ cat bitmap_allocs.cocci
> > > // typical uses of bitmap allocations
> []
> > > @@
> > > expression val;
> > > expression e1;
> > > expression e2;
> > > @@
> > >
> > > - val = kcalloc(BITS_TO_LONGS(e1), sizeof(*val), e2)
> > > + val = bitmap_zalloc(e1, e2)
> >
> > Is there something that guarantees that val has a type that has a size that
> > is the same as a long?
>
> no, but afaict, all do.
It might be nicer for the val metavariable to be declared as
{long,unsigned long} val;, although that might lose some results if the
type is something else that has the same size. I can check what is the
impact of adding that constraint.
julia
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] cocci script to convert linux-kernel allocs with BITS_TO_LONGS to bitmap_alloc
On Fri, 9 Jul 2021, Joe Perches wrote: > Here is a cocci script to convert various types of bitmap allocations > that use BITS_TO_LONGS to the more typical bitmap_alloc functions. > > Perhaps something like it could be added to scripts/coccinelle. > The diff produced by the script is also below. > > $ cat bitmap_allocs.cocci > // typical uses of bitmap allocations > > @@ > expression val; > expression e1; > expression e2; > @@ > > - val = kcalloc(BITS_TO_LONGS(e1), sizeof(\(long\|unsigned long\)), e2) > + val = bitmap_zalloc(e1, e2) > > @@ > expression val; > expression e1; > expression e2; > @@ > > - val = kcalloc(BITS_TO_LONGS(e1), sizeof(*val), e2) > + val = bitmap_zalloc(e1, e2) Is there something that guarantees that val has a type that has a size that is the same as a long? julia ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH 0/2] Add "use-patchdiff" option
On Fri, 2 Jul 2021, Sumera Priyadarsini wrote: > On Wed, May 26, 2021 at 10:13 PM Markus Elfring wrote: > > > > > This patchset adds a feature to enable Coccinelle > > > to only check all those files in a directory which were > > > modified. It parses all the files obtained from the > > > output of "git diff" and checks them against the specified > > > cocci script. > > > > > > An example for passing the "use-patchdiff" option is: > > > > How do you think about to use the parameter name “use-files-from-diff”? > > I would prefer something shorter, like "use-filesdiff" but I am okay > with either name as > long as the maintainers are okay with it. :) > > Julia, what do you think? I will send a v2 with any of the above name options > (and/or any other changes that are suggested.) I think that shorter names are better. The current name could be ok, but a - between patch and diff might be more natural. julia___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH] Coccinelle: Update and rename api/alloc/pci_free_consistent.cocci
On Wed, 16 Jun 2021, Christophe JAILLET wrote:
> 'pci_alloc_consistent()' is about to be removed from the kernel.
> It is now more useful to check for dma_alloc_coherent/dma_free_coherent.
>
> So change the script accordingly and rename it.
There also seem to be a lot of false positives, where the value is used in
unexpected ways, such as:
for (i = 0; i < nr_pages; ++i) {
cpu_addr = dma_alloc_coherent(dma_dev, PAGE_SIZE, _addr,
CIO_DMA_GFP);
if (!cpu_addr)
return gp_dma;
gen_pool_add_virt(gp_dma, (unsigned long) cpu_addr,
dma_addr, PAGE_SIZE, -1);
}
Maybe the rule should be dropped?
julia
>
> Signed-off-by: Christophe JAILLET
> ---
> Not sure that the script works.
> There are 718 'dma_alloc_coherent' calls in 5.13-rc6. It is surprising
> to have no match at all, not even a single false positive.
> ---
> ..._consistent.cocci => dma_free_coherent.cocci} | 16
> 1 file changed, 8 insertions(+), 8 deletions(-)
> rename scripts/coccinelle/free/{pci_free_consistent.cocci =>
> dma_free_coherent.cocci} (52%)
>
> diff --git a/scripts/coccinelle/free/pci_free_consistent.cocci
> b/scripts/coccinelle/free/dma_free_coherent.cocci
> similarity index 52%
> rename from scripts/coccinelle/free/pci_free_consistent.cocci
> rename to scripts/coccinelle/free/dma_free_coherent.cocci
> index d51e92556b42..75f159e7b6d7 100644
> --- a/scripts/coccinelle/free/pci_free_consistent.cocci
> +++ b/scripts/coccinelle/free/dma_free_coherent.cocci
> @@ -1,10 +1,10 @@
> // SPDX-License-Identifier: GPL-2.0-only
> -/// Find missing pci_free_consistent for every pci_alloc_consistent.
> +/// Find missing dma_free_coherent for every dma_alloc_coherent.
> ///
> // Confidence: Moderate
> // Copyright: (C) 2013 Petr Strnad.
> // URL: http://coccinelle.lip6.fr/
> -// Keywords: pci_free_consistent, pci_alloc_consistent
> +// Keywords: dma_free_coherent, dma_alloc_coherent
> // Options: --no-includes --include-headers
>
> virtual report
> @@ -17,12 +17,12 @@ position p1,p2;
> type T;
> @@
>
> -id = pci_alloc_consistent@p1(x,y,)
> +id = dma_alloc_coherent@p1(x,y,)
> ... when != e = id
> if (id == NULL || ...) { ... return ...; }
> -... when != pci_free_consistent(x,y,id,z)
> -when != if (id) { ... pci_free_consistent(x,y,id,z) ... }
> -when != if (y) { ... pci_free_consistent(x,y,id,z) ... }
> +... when != dma_free_coherent(x,y,id,z)
> +when != if (id) { ... dma_free_coherent(x,y,id,z) ... }
> +when != if (y) { ... dma_free_coherent(x,y,id,z) ... }
> when != e = (T)id
> when exists
> (
> @@ -40,7 +40,7 @@ p1 << search.p1;
> p2 << search.p2;
> @@
>
> -msg = "ERROR: missing pci_free_consistent; pci_alloc_consistent on line %s
> and return without freeing on line %s" % (p1[0].line,p2[0].line)
> +msg = "ERROR: missing dma_free_coherent; dma_alloc_coherent on line %s and
> return without freeing on line %s" % (p1[0].line,p2[0].line)
> coccilib.report.print_report(p2[0],msg)
>
> @script:python depends on org@
> @@ -48,6 +48,6 @@ p1 << search.p1;
> p2 << search.p2;
> @@
>
> -msg = "ERROR: missing pci_free_consistent; pci_alloc_consistent on line %s
> and return without freeing on line %s" % (p1[0].line,p2[0].line)
> +msg = "ERROR: missing dma_free_coherent; dma_alloc_coherent on line %s and
> return without freeing on line %s" % (p1[0].line,p2[0].line)
> cocci.print_main(msg,p1)
> cocci.print_secs("",p2)
> --
> 2.30.2
>
>
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH] Coccinelle: Update and rename api/alloc/pci_free_consistent.cocci
On Wed, 16 Jun 2021, Christophe JAILLET wrote:
> 'pci_alloc_consistent()' is about to be removed from the kernel.
> It is now more useful to check for dma_alloc_coherent/dma_free_coherent.
dma_alloc_coherent has four arguments, and in the script there are only
three. Is the number of arguments to dma_alloc_coherent going to change?
julia
>
> So change the script accordingly and rename it.
>
> Signed-off-by: Christophe JAILLET
> ---
> Not sure that the script works.
> There are 718 'dma_alloc_coherent' calls in 5.13-rc6. It is surprising
> to have no match at all, not even a single false positive.
> ---
> ..._consistent.cocci => dma_free_coherent.cocci} | 16
> 1 file changed, 8 insertions(+), 8 deletions(-)
> rename scripts/coccinelle/free/{pci_free_consistent.cocci =>
> dma_free_coherent.cocci} (52%)
>
> diff --git a/scripts/coccinelle/free/pci_free_consistent.cocci
> b/scripts/coccinelle/free/dma_free_coherent.cocci
> similarity index 52%
> rename from scripts/coccinelle/free/pci_free_consistent.cocci
> rename to scripts/coccinelle/free/dma_free_coherent.cocci
> index d51e92556b42..75f159e7b6d7 100644
> --- a/scripts/coccinelle/free/pci_free_consistent.cocci
> +++ b/scripts/coccinelle/free/dma_free_coherent.cocci
> @@ -1,10 +1,10 @@
> // SPDX-License-Identifier: GPL-2.0-only
> -/// Find missing pci_free_consistent for every pci_alloc_consistent.
> +/// Find missing dma_free_coherent for every dma_alloc_coherent.
> ///
> // Confidence: Moderate
> // Copyright: (C) 2013 Petr Strnad.
> // URL: http://coccinelle.lip6.fr/
> -// Keywords: pci_free_consistent, pci_alloc_consistent
> +// Keywords: dma_free_coherent, dma_alloc_coherent
> // Options: --no-includes --include-headers
>
> virtual report
> @@ -17,12 +17,12 @@ position p1,p2;
> type T;
> @@
>
> -id = pci_alloc_consistent@p1(x,y,)
> +id = dma_alloc_coherent@p1(x,y,)
> ... when != e = id
> if (id == NULL || ...) { ... return ...; }
> -... when != pci_free_consistent(x,y,id,z)
> -when != if (id) { ... pci_free_consistent(x,y,id,z) ... }
> -when != if (y) { ... pci_free_consistent(x,y,id,z) ... }
> +... when != dma_free_coherent(x,y,id,z)
> +when != if (id) { ... dma_free_coherent(x,y,id,z) ... }
> +when != if (y) { ... dma_free_coherent(x,y,id,z) ... }
> when != e = (T)id
> when exists
> (
> @@ -40,7 +40,7 @@ p1 << search.p1;
> p2 << search.p2;
> @@
>
> -msg = "ERROR: missing pci_free_consistent; pci_alloc_consistent on line %s
> and return without freeing on line %s" % (p1[0].line,p2[0].line)
> +msg = "ERROR: missing dma_free_coherent; dma_alloc_coherent on line %s and
> return without freeing on line %s" % (p1[0].line,p2[0].line)
> coccilib.report.print_report(p2[0],msg)
>
> @script:python depends on org@
> @@ -48,6 +48,6 @@ p1 << search.p1;
> p2 << search.p2;
> @@
>
> -msg = "ERROR: missing pci_free_consistent; pci_alloc_consistent on line %s
> and return without freeing on line %s" % (p1[0].line,p2[0].line)
> +msg = "ERROR: missing dma_free_coherent; dma_alloc_coherent on line %s and
> return without freeing on line %s" % (p1[0].line,p2[0].line)
> cocci.print_main(msg,p1)
> cocci.print_secs("",p2)
> --
> 2.30.2
>
>
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Adding a newline after a variable definition
On Wed, 23 Jun 2021, Fuad Tabba wrote:
> Hi,
>
> I have a semantic patch that inserts a new variable definition into a
> function. I would like it if that variable definition is the only one
> in the function, then it should add a new line to separate the
> definition from following statements (Linux code formatting style).
>
> I thought that doing this in two steps might be easier, i.e., add the
> definition, then check and add a newline if a statement follows:
>
> @@
> identifier x;
> identifier func;
> statement S;
> @@
> func(...)
> {
> struct kvm_cpu_context *x = ...;
> + newline;
> S
> ...
> }
>
> The above works as expected, and it adds "newline;" after the
> definition of x. The thing is, is it possible to add an actual new
> line, as opposed to a non-whitespace string? I tried just using a +
> but that didn't work.
I think that the problem is not that the change is not being made, but
that spatch doesn't think it's worth showing you the change, since the
only change is in the whitespace. Try adding the argument --force-diff
Note that you can cause this argument to always be used with your semantic
patch by putting
#spatch --force-diff
at the top of the file.
julia
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] struct type renaming in the absence of certain function calls on members
On Sat, 5 Jun 2021, Jason A. Donenfeld wrote: > That makes sense. Thank you so much for your help. Committed here (and > credited you as co-author): > https://git.zx2c4.com/wireguard-freebsd/commit/?id=b13579613ffcbe56c28df79972a74025007a85b7 Cool. Thanks :) julia ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] struct type renaming in the absence of certain function calls on members
On Sat, 5 Jun 2021, Jason A. Donenfeld wrote: > Thanks! That seems to be it indeed. Interestingly, including the > actual path to the real headers with -I didn't fix the issue. Most of the time, Coccinelle doesn't use macro definitions. The -I could have been useful for figuring out some types, but probably wasn't needed in this case. > I tried > adding other options passed as -D to the compiler in a macro file, in > case that was influencing the parsing, to no avail. So in the end I > indeed went with just defining those to int. Seems to work well! Last > minor cosmetic issue is that it turns `struct a > [space][space][space][space][space][space][space] x` into `struct b > [space] x`, which is easy enough to fix up by hand. That's beyond the degree of cleverness we try t oprovide. but in this particular case, it would probably not have been needed if the rule for changing the type name had only changed the name, and not the whole member declaration. julia > (The BSDs do weird > and terrible things with whitespace sometimes.) > > Jason > ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] struct type renaming in the absence of certain function calls on members
Try making a file def.h: #define CK_LIST_HEAD(x,y) int #define CK_LIST_ENTRY(x) int and then add to your command line --macro-file def.h There seem to be a few other such macros that are needed. Unrelatedly, you can also add --very-quiet to the command line, to have a less verbose output. julia ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] struct type renaming in the absence of certain function calls on members
On Sat, 5 Jun 2021, Jason A. Donenfeld wrote:
> Hi Julia,
>
> Holy mackerel, that's amazing. Thank you so much! Very fancy embedding
> the ocaml in there, and just using a hash table. Neat idea.
>
> I'm running it (well, a modified version, pasted below) on a codebase
> and finding that it fails to replace the "struct rwlock x" with "struct
> mtx x" in most cases, except one, which is strange. I wonder if it's the
> parser choking on macros it doesn't understand in the code? Or on
> something else?
It seems to be choking on the macros...
julia
>
> Code is here, if you're curious:
>
> $ git clone https://git.zx2c4.com/wireguard-freebsd
> $ cd wireguard-freebsd/src
> $ spatch.opt --sp-file doit.cocci -j 4 --recursive-includes
> --include-headers-for-types --include-headers --in-place .
>
> I'll keep playing with it to see what's happening...
>
> Jason
>
> == doit.cocci ==
>
> virtual after_start
>
> @initialize:ocaml@
> @@
>
> let has_write_table = Hashtbl.create 101
> let has_read_table = Hashtbl.create 101
>
> let ok i m =
> let entry = (i,m) in
> Hashtbl.mem has_write_table entry && not(Hashtbl.mem has_read_table entry)
>
> @hasw depends on !after_start@
> identifier i,m;
> struct i x;
> @@
>
> (
> rw_wlock()
> |
> rw_wunlock()
> )
>
> @script:ocaml@
> i << hasw.i;
> m << hasw.m;
> @@
> Hashtbl.replace has_write_table (i,m) ()
>
> @hasr depends on !after_start@
> identifier i,m;
> struct i x;
> @@
>
> (
> rw_rlock()
> |
> rw_runlock()
> )
>
> @script:ocaml@
> i << hasr.i;
> m << hasr.m;
> @@
> Hashtbl.replace has_read_table (i,m) ()
>
> @finalize:ocaml depends on !after_start@
> wt << merge.has_write_table;
> rt << merge.has_read_table;
> @@
>
> let redo ts dst =
> List.iter (Hashtbl.iter (fun k _ -> Hashtbl.add dst k ())) ts in
> redo wt has_write_table;
> redo rt has_read_table;
>
> let it = new iteration() in
> it#add_virtual_rule After_start;
> it#register()
>
> (* --- *)
>
> @ty2 depends on after_start@
> identifier i;
> identifier m : script:ocaml(i) { ok i m };
> @@
>
> struct i {
> ...
> - struct rwlock m;
> + struct mtx m;
> ...
> }
>
> @depends on after_start disable fld_to_ptr@
> identifier m;
> identifier i : script:ocaml(m) { ok i m };
> struct i x;
> @@
>
> - rw_wlock
> + mtx_lock
> ()
>
> @depends on after_start disable fld_to_ptr@
> identifier m;
> identifier i : script:ocaml(m) { ok i m };
> struct i x;
> @@
>
> - rw_wunlock
> + mtx_unlock
> ()
>
> @depends on after_start disable fld_to_ptr@
> identifier m;
> expression e;
> identifier i : script:ocaml(m) { ok i m };
> struct i x;
> @@
>
> - rw_init(, e);
> + mtx_init(, e, NULL, MTX_DEF);
>
> @depends on after_start disable fld_to_ptr@
> identifier m;
> identifier i : script:ocaml(m) { ok i m };
> struct i x;
> @@
>
> - rw_destroy
> + mtx_destroy
> ()
>
> @depends on after_start disable fld_to_ptr, ptr_to_array@
> identifier m;
> identifier i : script:ocaml(m) { ok i m };
> struct i *x;
> @@
>
> - rw_wlock
> + mtx_lock
> (>m)
>
> @depends on after_start disable fld_to_ptr, ptr_to_array@
> identifier m;
> identifier i : script:ocaml(m) { ok i m };
> struct i *x;
> @@
>
> - rw_wunlock
> + mtx_unlock
> (>m)
>
> @depends on after_start disable fld_to_ptr, ptr_to_array@
> identifier m;
> expression e;
> identifier i : script:ocaml(m) { ok i m };
> struct i *x;
> @@
>
> - rw_init(>m, e);
> + mtx_init(>m, e, NULL, MTX_DEF);
>
> @depends on after_start disable fld_to_ptr, ptr_to_array@
> identifier m;
> identifier i : script:ocaml(m) { ok i m };
> struct i *x;
> @@
>
> - rw_destroy
> + mtx_destroy
> (>m)
>___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] struct type renaming in the absence of certain function calls on members
On Sat, 5 Jun 2021, Jason A. Donenfeld wrote:
> Hi Julia,
>
> Holy mackerel, that's amazing. Thank you so much! Very fancy embedding
> the ocaml in there, and just using a hash table. Neat idea.
>
> I'm running it (well, a modified version, pasted below) on a codebase
> and finding that it fails to replace the "struct rwlock x" with "struct
> mtx x" in most cases, except one, which is strange. I wonder if it's the
> parser choking on macros it doesn't understand in the code? Or on
> something else?
>
> Code is here, if you're curious:
>
> $ git clone https://git.zx2c4.com/wireguard-freebsd
> $ cd wireguard-freebsd/src
> $ spatch.opt --sp-file doit.cocci -j 4 --recursive-includes
> --include-headers-for-types --include-headers --in-place .
>
> I'll keep playing with it to see what's happening...
Thanks for the link to the code. Maybe you need a -I option to tell it
where the header files are? To check for parsing problems, you can say
spatch --parse-c wireguard-freebsd. You may need to remove any limit on
the stacksize.
julia
>
> Jason
>
> == doit.cocci ==
>
> virtual after_start
>
> @initialize:ocaml@
> @@
>
> let has_write_table = Hashtbl.create 101
> let has_read_table = Hashtbl.create 101
>
> let ok i m =
> let entry = (i,m) in
> Hashtbl.mem has_write_table entry && not(Hashtbl.mem has_read_table entry)
>
> @hasw depends on !after_start@
> identifier i,m;
> struct i x;
> @@
>
> (
> rw_wlock()
> |
> rw_wunlock()
> )
>
> @script:ocaml@
> i << hasw.i;
> m << hasw.m;
> @@
> Hashtbl.replace has_write_table (i,m) ()
>
> @hasr depends on !after_start@
> identifier i,m;
> struct i x;
> @@
>
> (
> rw_rlock()
> |
> rw_runlock()
> )
>
> @script:ocaml@
> i << hasr.i;
> m << hasr.m;
> @@
> Hashtbl.replace has_read_table (i,m) ()
>
> @finalize:ocaml depends on !after_start@
> wt << merge.has_write_table;
> rt << merge.has_read_table;
> @@
>
> let redo ts dst =
> List.iter (Hashtbl.iter (fun k _ -> Hashtbl.add dst k ())) ts in
> redo wt has_write_table;
> redo rt has_read_table;
>
> let it = new iteration() in
> it#add_virtual_rule After_start;
> it#register()
>
> (* --- *)
>
> @ty2 depends on after_start@
> identifier i;
> identifier m : script:ocaml(i) { ok i m };
> @@
>
> struct i {
> ...
> - struct rwlock m;
> + struct mtx m;
> ...
> }
>
> @depends on after_start disable fld_to_ptr@
> identifier m;
> identifier i : script:ocaml(m) { ok i m };
> struct i x;
> @@
>
> - rw_wlock
> + mtx_lock
> ()
>
> @depends on after_start disable fld_to_ptr@
> identifier m;
> identifier i : script:ocaml(m) { ok i m };
> struct i x;
> @@
>
> - rw_wunlock
> + mtx_unlock
> ()
>
> @depends on after_start disable fld_to_ptr@
> identifier m;
> expression e;
> identifier i : script:ocaml(m) { ok i m };
> struct i x;
> @@
>
> - rw_init(, e);
> + mtx_init(, e, NULL, MTX_DEF);
>
> @depends on after_start disable fld_to_ptr@
> identifier m;
> identifier i : script:ocaml(m) { ok i m };
> struct i x;
> @@
>
> - rw_destroy
> + mtx_destroy
> ()
>
> @depends on after_start disable fld_to_ptr, ptr_to_array@
> identifier m;
> identifier i : script:ocaml(m) { ok i m };
> struct i *x;
> @@
>
> - rw_wlock
> + mtx_lock
> (>m)
>
> @depends on after_start disable fld_to_ptr, ptr_to_array@
> identifier m;
> identifier i : script:ocaml(m) { ok i m };
> struct i *x;
> @@
>
> - rw_wunlock
> + mtx_unlock
> (>m)
>
> @depends on after_start disable fld_to_ptr, ptr_to_array@
> identifier m;
> expression e;
> identifier i : script:ocaml(m) { ok i m };
> struct i *x;
> @@
>
> - rw_init(>m, e);
> + mtx_init(>m, e, NULL, MTX_DEF);
>
> @depends on after_start disable fld_to_ptr, ptr_to_array@
> identifier m;
> identifier i : script:ocaml(m) { ok i m };
> struct i *x;
> @@
>
> - rw_destroy
> + mtx_destroy
> (>m)
>___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] struct type renaming in the absence of certain function calls on members
On Sat, 5 Jun 2021, Jason A. Donenfeld wrote:
> Hi,
>
> I'm trying to write a spatch rule that has some inversion logic for a
> given struct member, but I'm struggling to express this in SmPL. I'm
> also a bit of a coccinelle novice, however. Specifically, I'm trying
> to express:
>
> For all structs, for each member `M` of type `struct rwlock`, if there
> are calls to `rwlock_wlock()` and `rwlock_wunlock()`, but there
> are no calls to `rwlock_rlock()` or `rwlock_runlock()`, then
> change `M`'s type from `struct rwlock` to `struct lock` and change
> calls to `rwlock_wlock()` and `rwlock_wunlock()` to
> `lock_lock()` and `lock_unlock()`, respectively.
>
> In other words, "if I'm not using the reader part of an rwlock, make
> it into a normal boring lock."
>
> I realize that tracking whether, in a call to f(>M),
> something is actually of the type in which rwlock was replaced is kind
> of hard (I think?). But I'd be willing to compromise on just assuming
> that something->M is always correct, because M always has
> unique-enough names. Or maybe that compromise isn't needed due to some
> sort of amazing coccinelle type inference, but anyway, I'm willing to
> compromise.
>
> The trickiest part seems to be, however, only doing this in the case
> where there aren't calls to `rwlock_rlock()` and
> `rwlock_runlock()`. I tried using a virtual rule and a depends
> clause, but those dependencies don't seem to work over a given
> instance of an identifier.
>
> I could probably hack my way toward that with a ridiculous sed
> expression, or do it procedurally after parsing the AST. But it seems
> like this would be a good case for where Coccinelle makes things
> easier, so I thought I'd commit to getting it done with spatch. Any
> pointers would be appreciated.
A possible semantic patch is below. Note that the first line with the
#spatch gives some implicit command line options. You may want to change
the number of cores. Note that the semantic patch will directly modify
your code, so don't run it on anything you dont' want to destroy...
There are two parts. The first part finds relevant structures and locking
functions, and stores which operations are used on which structures in
some hash tables. That part is run over the entire code base.
After the completion of the first part, the second part looks for
structure definitions and calls that can be changed.
Hopefully the code based doesn't have multiple definitions of the same
structure with different properties. If that is a concern, it could be
possible to collect the names of the relevant structure definitions in the
first phase as well, and complain about or ignore structure names that are
defined twice.
julia
#spatch -j 4 --recursive-includes --include-headers-for-types --include-headers
--in-place
virtual after_start
@initialize:ocaml@
@@
let has_write_table = Hashtbl.create 101
let has_read_table = Hashtbl.create 101
let ok i m =
let entry = (i,m) in
Hashtbl.mem has_write_table entry && not(Hashtbl.mem has_read_table entry)
@hasw depends on !after_start@
identifier i,m;
struct i x;
@@
(
rwlock_wlock()
|
rwlock_wunlock()
)
@script:ocaml@
i << hasw.i;
m << hasw.m;
@@
Hashtbl.replace has_write_table (i,m) ()
@hasr depends on !after_start@
identifier i,m;
struct i x;
@@
(
rwlock_rlock()
|
rwlock_runlock()
)
@script:ocaml@
i << hasr.i;
m << hasr.m;
@@
Hashtbl.replace has_read_table (i,m) ()
@finalize:ocaml depends on !after_start@
wt << merge.has_write_table;
rt << merge.has_read_table;
@@
let redo ts dst =
List.iter (Hashtbl.iter (fun k _ -> Hashtbl.add dst k ())) ts in
redo wt has_write_table;
redo rt has_read_table;
let it = new iteration() in
it#add_virtual_rule After_start;
it#register()
(* --- *)
@ty2 depends on after_start@
identifier i;
identifier m : script:ocaml(i) { ok i m };
@@
struct i {
...
- struct rwlock m;
+ struct lock m;
...
}
@depends on after_start disable fld_to_ptr@
identifier m;
identifier i : script:ocaml(m) { ok i m };
struct i x;
@@
- rwlock_wlock
+ lock_lock
()
@depends on after_start disable fld_to_ptr@
identifier m;
identifier i : script:ocaml(m) { ok i m };
struct i x;
@@
- rwlock_wunlock
+ lock_unlock
()
@depends on after_start disable fld_to_ptr, ptr_to_array@
identifier m;
identifier i : script:ocaml(m) { ok i m };
struct i *x;
@@
- rwlock_wlock
+ lock_lock
(>m)
@depends on after_start disable fld_to_ptr, ptr_to_array@
identifier m;
identifier i : script:ocaml(m) { ok i m };
struct i *x;
@@
- rwlock_wunlock
+ lock_unlock
(>m)
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] No coccicheck target
On Fri, 4 Jun 2021, Jonas Schrottenbaum wrote: > Hello, > i'm currently trying to check out coccinelle and already could apply a simple > semantic patch to a small C program with the spatch command. > > Please excuse me, but i'm quite a beginner in this field. > > I read on this page, about coccicheck and the different modes. > It says, that i have to run: > > make coccicheck MODE=report > or > make coccicheck MODE=patch > > in order to switch the mode. All of this is only for the Linux kernel. Coccicheck is a target in the Linux kernel make file. > > But where do i have to run this? Is this just for using coccinelle on the > linux kernel? Is it not relevant, if i want to use coccinelle for general C > programs? No, it is not relevant for general C programs. > I thought it is a make target for coccinelle itself, but there is no target > called coccicheck defined in the coccinelle makefile, as far as i can tell. > So if i run it in the same dir as the make install for coccinelle, all i get > is: > > make: *** no rule to make target coccicheck. stop. > > So, am i doing something wrong, or is coccicheck only relevant for linux > kernel development? > It says: "A Coccinelle-specific target is defined in the top level Makefile." > which Makefile? The linux top level Makefile? you should run spatch directly for other software. Here is a document that describes common usages and the complete set of command line options (it is a little bit out of date, but everything there should still be usable). https://coccinelle.gitlabpages.inria.fr/website/docs/options.pdf julia___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Adding a line to a function after all variable declarations
On Thu, 27 May 2021, Fuad Tabba wrote:
> On Thu, May 27, 2021 at 12:44 PM Julia Lawall wrote:
> >
> > > @@
> > > expression a, b;
> > > identifier vcpu;
> > > fresh identifier vcpu_ctxt = vcpu ## "_ctxt";
> > > iterator name kvm_for_each_vcpu;
> > > identifier fc;
> > > @@
> > > kvm_for_each_vcpu(a, vcpu, b)
> > > {
> > > /* hop over any declarations */
> > > + vcpu_ctxt = >arch.ctxt;
> > > <+...
> > > fc(..., vcpu, ...)
> > > ...+>
> > > }
> > >
> > > So I'm trying to add the line "vcpu_ctxt = >arch.ctxt" after any
> > > declarations, only if there's a function in that block that's using
> > > vcpu. This works, but the vcpu_ctxt assignment is of course always
> > > added first, before any declarations.
> > >
> > > Doing the following partially works, but it of course misses the case
> > > where the function call with vcpu comes immediately after the
> > > vcpu_ctxt assignment. Removing S2 altogether gives me a parse error.
> > >
> > > @@
> > > expression a, b;
> > > identifier vcpu;
> > > fresh identifier vcpu_ctxt = vcpu ## "_ctxt";
> > > iterator name kvm_for_each_vcpu;
> > > identifier fc;
> > > statement S1, S2;
> > > @@
> > > kvm_for_each_vcpu(a, vcpu, b)
> > > {
> > > ... when != S1
> > > + vcpu_ctxt = >arch.ctxt;
> > > S2;
> > > <+...
> > > fc(..., vcpu, ...)
> > > ...+>
> > > }
> >
> > @@
> > identifier f;
> > statement S1,S2;
> > @@
> >
> > f(...) {
> > ... when != S1
> > (
> > + new_code
> > S2
> > ... when any
> > &
> > <+...
> > fc(..., vcpu, ...)
> > ...+>
> > )
> > }
>
> I get a parse error when I try that (both copying it verbatim, or
> adapting it to my code):
>
> minus: parse error:
> File "test.cocci", line 18, column 0, charpos = 209
> around = '<+...',
> whole content = <+...
OK, perhaps that is a corner that is not supported. It might work bettwe
with
... when any
when exists
fc(..., vcpu, ...)
... when any
in place of the <+... ...+>
But since you have a satisfactory solution, that is fine too.
julia
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Adding a line to a function after all variable declarations
> @@
> expression a, b;
> identifier vcpu;
> fresh identifier vcpu_ctxt = vcpu ## "_ctxt";
> iterator name kvm_for_each_vcpu;
> identifier fc;
> @@
> kvm_for_each_vcpu(a, vcpu, b)
> {
> /* hop over any declarations */
> + vcpu_ctxt = >arch.ctxt;
> <+...
> fc(..., vcpu, ...)
> ...+>
> }
>
> So I'm trying to add the line "vcpu_ctxt = >arch.ctxt" after any
> declarations, only if there's a function in that block that's using
> vcpu. This works, but the vcpu_ctxt assignment is of course always
> added first, before any declarations.
>
> Doing the following partially works, but it of course misses the case
> where the function call with vcpu comes immediately after the
> vcpu_ctxt assignment. Removing S2 altogether gives me a parse error.
>
> @@
> expression a, b;
> identifier vcpu;
> fresh identifier vcpu_ctxt = vcpu ## "_ctxt";
> iterator name kvm_for_each_vcpu;
> identifier fc;
> statement S1, S2;
> @@
> kvm_for_each_vcpu(a, vcpu, b)
> {
> ... when != S1
> + vcpu_ctxt = >arch.ctxt;
> S2;
> <+...
> fc(..., vcpu, ...)
> ...+>
> }
@@
identifier f;
statement S1,S2;
@@
f(...) {
... when != S1
(
+ new_code
S2
... when any
&
<+...
fc(..., vcpu, ...)
...+>
)
}
julia
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Adding a line to a function after all variable declarations
On Thu, 27 May 2021, Julia Lawall wrote:
>
>
> On Thu, 27 May 2021, Fuad Tabba wrote:
>
> > Hi,
> >
> > I'm trying to write a semantic patch that would add a new line to a
> > function, but would like that line to come after all local variables
> > (if any) have been declared. For example (distilled from what I am
> > actually trying to accomplish):
> >
> > @@
> > identifier f, s;
> > @@
> > f(..., struct mystruct *s, ...) {
> > /* after any variable declarations*/
> > + x = s->x;
> > ...
> > }
> >
> > I would like it to patch as follows:
> >
> > int function(struct mystruct *s) {
> > + x = s->x;
> > return 0;
> > }
> >
> > as well as
> >
> > int function(struct mystruct *s)
> > {
> > int y;
> > + x = s->x;
> > return 0;
> > }
> >
> > or
> >
> > int function(struct mystruct *s)
> > {
> > int y, z = 10;
> > float f;
> > + x = s->x;
> > return 0;
> > }
> >
> > Any suggestions on how I could do that?
>
> @@
> @@
>
> f(...) {
> ... when != S
> + new_code
> S
> ... when any
> }
Sorry, that was not quite right. The first S should be S1 and the second
S should be S2, ie they should be different. S1 and S2 should be
statement metavariables and f should be an identifier metavariable.
julia
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Adding a line to a function after all variable declarations
On Thu, 27 May 2021, Fuad Tabba wrote:
> Hi,
>
> I'm trying to write a semantic patch that would add a new line to a
> function, but would like that line to come after all local variables
> (if any) have been declared. For example (distilled from what I am
> actually trying to accomplish):
>
> @@
> identifier f, s;
> @@
> f(..., struct mystruct *s, ...) {
> /* after any variable declarations*/
> + x = s->x;
> ...
> }
>
> I would like it to patch as follows:
>
> int function(struct mystruct *s) {
> + x = s->x;
> return 0;
> }
>
> as well as
>
> int function(struct mystruct *s)
> {
> int y;
> + x = s->x;
> return 0;
> }
>
> or
>
> int function(struct mystruct *s)
> {
> int y, z = 10;
> float f;
> + x = s->x;
> return 0;
> }
>
> Any suggestions on how I could do that?
@@
@@
f(...) {
... when != S
+ new_code
S
... when any
}
julia
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH] scripts: coccicheck: fix troubles on non-English builds
On Tue, 18 May 2021, Mauro Carvalho Chehab wrote: > When LANG is not set to English, the logic which checks the > number of CPUs fail, as the messages can be localized, and > the logic at: > > THREADS_PER_CORE=$(lscpu | grep "Thread(s) per core: " | tr -cd > "[:digit:]") > > will not get the number of threads per core. > > This causes the script to not run properly, as it will produce > a warning: > > $ make coccicheck > COCCI=$PWD/scripts/coccinelle/misc/add_namespace.cocci MODE=report > drivers/media/ > ./scripts/coccicheck: linha 93: [: número excessivo de argumentos > > Fix it by forcing LANG=C when calling lscpu. > > Signed-off-by: Mauro Carvalho Chehab Applied, thanks. julia > --- > scripts/coccicheck | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/scripts/coccicheck b/scripts/coccicheck > index 65fee63aeadb..caba0bff6da7 100755 > --- a/scripts/coccicheck > +++ b/scripts/coccicheck > @@ -87,7 +87,7 @@ else > fi > > # Use only one thread per core by default if hyperthreading is enabled > -THREADS_PER_CORE=$(lscpu | grep "Thread(s) per core: " | tr -cd > "[:digit:]") > +THREADS_PER_CORE=$(LANG=C lscpu | grep "Thread(s) per core: " | tr -cd > "[:digit:]") > if [ -z "$J" ]; then > NPROC=$(getconf _NPROCESSORS_ONLN) > if [ $THREADS_PER_CORE -gt 1 -a $NPROC -gt 4 ] ; then > -- > 2.31.1 > >___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH v6] coccinelle: api: semantic patch to use pm_runtime_resume_and_get
pm_runtime_get_sync keeps a reference count on failure, which can lead
to leaks. pm_runtime_resume_and_get drops the reference count in the
failure case. This rule very conservatively follows the definition of
pm_runtime_resume_and_get to address the cases where the reference
count is unlikely to be needed in the failure case. Specifically, the
change is only done when pm_runtime_get_sync is followed immediately
by an if and when the branch of the if is immediately a call to
pm_runtime_put_noidle (like in the definition of
pm_runtime_resume_and_get) or something that is likely a print
statement followed by a pm_runtime_put_noidle call.
The change is furthermore only done when the ret variable is only used
in a ret < 0 test and possibly in the associated branch. No change is
made if ret is used in the else branch (the rule bad) or after the if.
This is because pm_runtime_resume_and_get only returns 0 (success) or
a negative value (failure), while pm_runtime_get_sync may also return
1 in the success case. Thus more attention is required to make the
change in a case where a 1 value might be observed.
The patch case appears somewhat more complicated than the others,
because it aolso deals with the cases where {}s need to be removed.
pm_runtime_resume_and_get was introduced in
commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to
deal with usage counter")
Signed-off-by: Julia Lawall
Acked-by: Rafael J. Wysocki
---
v6: don't touch code that reuses the value of ret, as suggested by Mauro
Carvalho Chehab
v5: print a message with the new function name, as suggested by Markus Elfring
v4: s/pm_runtime_resume_and_get/pm_runtime_put_noidle/ as noted by John Hovold
v3: add the people who signed off on commit dd8088d5a896, expand the log message
v2: better keyword
scripts/coccinelle/api/pm_runtime_resume_and_get.cocci | 185 +
1 file changed, 185 insertions(+)
diff --git a/scripts/coccinelle/api/pm_runtime_resume_and_get.cocci
b/scripts/coccinelle/api/pm_runtime_resume_and_get.cocci
new file mode 100644
index ..b5abb3783d3d
--- /dev/null
+++ b/scripts/coccinelle/api/pm_runtime_resume_and_get.cocci
@@ -0,0 +1,185 @@
+// SPDX-License-Identifier: GPL-2.0-only
+///
+/// Use pm_runtime_resume_and_get.
+/// pm_runtime_get_sync keeps a reference count on failure,
+/// which can lead to leaks. pm_runtime_resume_and_get
+/// drops the reference count in the failure case.
+/// This rule addresses the cases where the reference count
+/// is unlikely to be needed in the failure case.
+///
+// Confidence: High
+// Copyright: (C) 2021 Julia Lawall, Inria
+// URL: https://coccinelle.gitlabpages.inria.fr/website
+// Options: --include-headers --no-includes
+// Keywords: pm_runtime_get_sync
+
+virtual patch
+virtual context
+virtual org
+virtual report
+
+@bad exists@
+expression ret;
+statement S1;
+position p;
+@@
+
+ ret = pm_runtime_get_sync(...);
+ if@p (ret < 0) S1
+ else {<+... ret ...+>}
+
+@r0 depends on patch && !context && !org && !report@
+expression ret,e,e1;
+@@
+
+- ret = pm_runtime_get_sync(e);
++ ret = pm_runtime_resume_and_get(e);
+- if (ret < 0)
+- pm_runtime_put_noidle(e);
+ ... when != ret
+? ret = e1
+
+@r1 depends on patch && !context && !org && !report@
+expression ret,e,e1;
+statement S1,S2;
+position p != bad.p;
+@@
+
+- ret = pm_runtime_get_sync(e);
++ ret = pm_runtime_resume_and_get(e);
+ if@p (ret < 0)
+- {
+- pm_runtime_put_noidle(e);
+ S1
+- }
+ else S2
+ ... when != ret
+? ret = e1
+
+@r2 depends on patch && !context && !org && !report@
+expression ret,e,e1;
+statement S;
+position p != bad.p;
+@@
+
+- ret = pm_runtime_get_sync(e);
++ ret = pm_runtime_resume_and_get(e);
+ if@p (ret < 0) {
+- pm_runtime_put_noidle(e);
+ ...
+ } else S
+ ... when != ret
+? ret = e1
+
+@r3 depends on patch && !context && !org && !report@
+expression ret,e,e1;
+identifier f;
+constant char[] c;
+position p != bad.p;
+statement S;
+@@
+
+- ret = pm_runtime_get_sync(e);
++ ret = pm_runtime_resume_and_get(e);
+ if@p (ret < 0)
+- {
+ f(...,c,...);
+- pm_runtime_put_noidle(e);
+- }
+ else S
+ ... when != ret
+? ret = e1
+
+@r4 depends on patch && !context && !org && !report@
+expression ret,e,e1;
+identifier f;
+constant char[] c;
+position p != bad.p;
+statement S;
+@@
+
+- ret = pm_runtime_get_sync(e);
++ ret = pm_runtime_resume_and_get(e);
+ if@p (ret < 0) {
+ f(...,c,...);
+- pm_runtime_put_noidle(e);
+ ...
+ } else S
+ ... when != ret
+? ret = e1
+
+//
+
+@
Re: [Cocci] [PATCH v5] coccinelle: api: semantic patch to use pm_runtime_resume_and_get
On Wed, 5 May 2021, Mauro Carvalho Chehab wrote:
> Hi Julia,
>
> Em Thu, 29 Apr 2021 19:43:43 +0200
> Julia Lawall escreveu:
>
> > pm_runtime_get_sync keeps a reference count on failure, which can lead
> > to leaks. pm_runtime_resume_and_get drops the reference count in the
> > failure case. This rule very conservatively follows the definition of
> > pm_runtime_resume_and_get to address the cases where the reference
> > count is unlikely to be needed in the failure case. Specifically, the
> > change is only done when pm_runtime_get_sync is followed immediately
> > by an if and when the branch of the if is immediately a call to
> > pm_runtime_put_noidle (like in the definition of
> > pm_runtime_resume_and_get) or something that is likely a print
> > statement followed by a pm_runtime_put_noidle call. The patch
> > case appears somewhat more complicated, because it also deals with the
> > cases where {}s need to be removed.
> >
> > pm_runtime_resume_and_get was introduced in
> > commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to
> > deal with usage counter")
> >
> > Signed-off-by: Julia Lawall
> > Acked-by: Rafael J. Wysocki
>
> First of all, thanks for doing that! It sounds a lot better to have
> a script doing the check than newbies trying to address it manually,
> as there are several aspects to be considered on such replacement.
>
> >
> > ---
> > v5: print a message with the new function name, as suggested by Markus
> > Elfring
> > v4: s/pm_runtime_resume_and_get/pm_runtime_put_noidle/ as noted by John
> > Hovold
> > v3: add the people who signed off on commit dd8088d5a896, expand the log
> > message
> > v2: better keyword
> >
> > scripts/coccinelle/api/pm_runtime_resume_and_get.cocci | 153
> > +
> > 1 file changed, 153 insertions(+)
> >
> > diff --git a/scripts/coccinelle/api/pm_runtime_resume_and_get.cocci
> > b/scripts/coccinelle/api/pm_runtime_resume_and_get.cocci
> > new file mode 100644
> > index ..3387cb606f9b
> > --- /dev/null
> > +++ b/scripts/coccinelle/api/pm_runtime_resume_and_get.cocci
> > @@ -0,0 +1,153 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +///
> > +/// Use pm_runtime_resume_and_get.
> > +/// pm_runtime_get_sync keeps a reference count on failure,
> > +/// which can lead to leaks. pm_runtime_resume_and_get
> > +/// drops the reference count in the failure case.
> > +/// This rule addresses the cases where the reference count
> > +/// is unlikely to be needed in the failure case.
> > +///
> > +// Confidence: High
>
> Long story short, I got a corner case where the script is doing
> the wrong thing.
>
> ---
>
> A detailed explanation follows:
>
> As you know, I'm doing some manual work to address issues related
> to pm_runtime_get() on media.
>
> There, I found a corner case: There is a functional difference
> between:
>
> ret = pm_runtime_get_sync(>dev);
> if (ret < 0) {
> pm_runtime_put_noidle(>dev);
> return ret;
> }
>
> and:
> ret = pm_runtime_resume_and_get(>dev);
> if (ret < 0)
> return ret;
>
> On success, pm_runtime_get_sync() can return either 0 or 1.
> When 1 is returned, it means that the driver was already resumed.
>
> pm_runtime_resume_and_get(), on the other hand, don't have the same
> behavior. On success, it always return zero.
>
> IMO, this is actually a good thing, as it helps to address a common
> mistake:
>
> ret = pm_runtime_get_sync(>dev);
> /*
>* or, even worse:
>* ret = some_function_that_calls_pm_runtime_get_sync();
>*/
>
> if (ret) {
> pm_runtime_put_noidle(>dev);
> return ret;
> }
>
> FYI, Dan pointed one media driver to me those days with the above
> issue at the imx334 driver, which I'm fixing on my patch series.
>
> -
>
> Anyway, after revisiting my patches, I found several cases that were
> doing things like:
>
> int ret;
>
> ret = pm_runtime_get_sync(dev);
> pm_runtime_put_noidle(dev); /* Or without it, on drivers
> with unbalanced get/put */
>
> return ret > 0 ? 0 : ret;
>
> Which can be replaced by just:
>
> return pm_runtime_resume_and_get(>gsc_dev->pdev->dev);
>
> Yet, I found a single corner case on media where a driver is actually
> using the positive return: the ccs-core camera sensor driver.
>
> There,
[Cocci] [PATCH v4] coccinelle: api: semantic patch to use pm_runtime_resume_and_get
pm_runtime_get_sync keeps a reference count on failure, which can lead
to leaks. pm_runtime_resume_and_get drops the reference count in the
failure case. This rule very conservatively follows the definition of
pm_runtime_resume_and_get to address the cases where the reference
count is unlikely to be needed in the failure case. Specifically, the
change is only done when pm_runtime_get_sync is followed immediately
by an if and when the branch of the if is immediately a call to
pm_runtime_put_noidle (like in the definition of
pm_runtime_resume_and_get) or something that is likely a print
statement followed by a pm_runtime_put_noidle call. The patch
case appears somewhat more complicated, because it also deals with the
cases where {}s need to be removed.
pm_runtime_resume_and_get was introduced in
commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to
deal with usage counter")
Signed-off-by: Julia Lawall
Acked-by: Rafael J. Wysocki
---
v5: print a message with the new function name, as suggested by Markus Elfring
v4: s/pm_runtime_resume_and_get/pm_runtime_put_noidle/ as noted by John Hovold
v3: add the people who signed off on commit dd8088d5a896, expand the log message
v2: better keyword
scripts/coccinelle/api/pm_runtime_resume_and_get.cocci | 153 +
1 file changed, 153 insertions(+)
diff --git a/scripts/coccinelle/api/pm_runtime_resume_and_get.cocci
b/scripts/coccinelle/api/pm_runtime_resume_and_get.cocci
new file mode 100644
index ..3387cb606f9b
--- /dev/null
+++ b/scripts/coccinelle/api/pm_runtime_resume_and_get.cocci
@@ -0,0 +1,153 @@
+// SPDX-License-Identifier: GPL-2.0-only
+///
+/// Use pm_runtime_resume_and_get.
+/// pm_runtime_get_sync keeps a reference count on failure,
+/// which can lead to leaks. pm_runtime_resume_and_get
+/// drops the reference count in the failure case.
+/// This rule addresses the cases where the reference count
+/// is unlikely to be needed in the failure case.
+///
+// Confidence: High
+// Copyright: (C) 2021 Julia Lawall, Inria
+// URL: https://coccinelle.gitlabpages.inria.fr/website
+// Options: --include-headers --no-includes
+// Keywords: pm_runtime_get_sync
+
+virtual patch
+virtual context
+virtual org
+virtual report
+
+@r0 depends on patch && !context && !org && !report@
+expression ret,e;
+@@
+
+- ret = pm_runtime_get_sync(e);
++ ret = pm_runtime_resume_and_get(e);
+- if (ret < 0)
+- pm_runtime_put_noidle(e);
+
+@r1 depends on patch && !context && !org && !report@
+expression ret,e;
+statement S1,S2;
+@@
+
+- ret = pm_runtime_get_sync(e);
++ ret = pm_runtime_resume_and_get(e);
+ if (ret < 0)
+- {
+- pm_runtime_put_noidle(e);
+ S1
+- }
+ else S2
+
+@r2 depends on patch && !context && !org && !report@
+expression ret,e;
+statement S;
+@@
+
+- ret = pm_runtime_get_sync(e);
++ ret = pm_runtime_resume_and_get(e);
+ if (ret < 0) {
+- pm_runtime_put_noidle(e);
+ ...
+ } else S
+
+@r3 depends on patch && !context && !org && !report@
+expression ret,e;
+identifier f;
+constant char[] c;
+statement S;
+@@
+
+- ret = pm_runtime_get_sync(e);
++ ret = pm_runtime_resume_and_get(e);
+ if (ret < 0)
+- {
+ f(...,c,...);
+- pm_runtime_put_noidle(e);
+- }
+ else S
+
+@r4 depends on patch && !context && !org && !report@
+expression ret,e;
+identifier f;
+constant char[] c;
+statement S;
+@@
+
+- ret = pm_runtime_get_sync(e);
++ ret = pm_runtime_resume_and_get(e);
+ if (ret < 0) {
+ f(...,c,...);
+- pm_runtime_put_noidle(e);
+ ...
+ } else S
+
+//
+
+@r2_context depends on !patch && (context || org || report)@
+statement S;
+expression e, ret;
+position j0, j1;
+@@
+
+* ret@j0 = pm_runtime_get_sync(e);
+ if (ret < 0) {
+* pm_runtime_put_noidle@j1(e);
+ ...
+ } else S
+
+@r3_context depends on !patch && (context || org || report)@
+identifier f;
+statement S;
+constant char []c;
+expression e, ret;
+position j0, j1;
+@@
+
+* ret@j0 = pm_runtime_get_sync(e);
+ if (ret < 0) {
+ f(...,c,...);
+* pm_runtime_put_noidle@j1(e);
+ ...
+ } else S
+
+//
+
+@script:python r2_org depends on org@
+j0 << r2_context.j0;
+j1 << r2_context.j1;
+@@
+
+msg = "WARNING: opportunity for pm_runtime_resume_and_get"
+coccilib.org.print_todo(j0[0], msg)
+coccilib.org.print_link(j1[0], "")
+
+@script:python
[Cocci] [PATCH v4] coccinelle: api: semantic patch to use pm_runtime_resume_and_get
pm_runtime_get_sync keeps a reference count on failure, which can lead
to leaks. pm_runtime_resume_and_get drops the reference count in the
failure case. This rule very conservatively follows the definition of
pm_runtime_resume_and_get to address the cases where the reference
count is unlikely to be needed in the failure case. Specifically, the
change is only done when pm_runtime_get_sync is followed immediately
by an if and when the branch of the if is immediately a call to
pm_runtime_put_noidle (like in the definition of
pm_runtime_resume_and_get) or something that is likely a print
statement followed by a pm_runtime_put_noidle call. The patch
case appears somewhat more complicated, because it also deals with the
cases where {}s need to be removed.
pm_runtime_resume_and_get was introduced in
commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to
deal with usage counter")
Signed-off-by: Julia Lawall
Acked-by: Rafael J. Wysocki
---
v4: s/pm_runtime_resume_and_get/pm_runtime_put_noidle/ as noted by John Hovold
v3: add the people who signed off on commit dd8088d5a896, expand the log message
v2: better keyword
scripts/coccinelle/api/pm_runtime_resume_and_get.cocci | 153 +
1 file changed, 153 insertions(+)
diff --git a/scripts/coccinelle/api/pm_runtime_resume_and_get.cocci
b/scripts/coccinelle/api/pm_runtime_resume_and_get.cocci
new file mode 100644
index ..3387cb606f9b
--- /dev/null
+++ b/scripts/coccinelle/api/pm_runtime_resume_and_get.cocci
@@ -0,0 +1,153 @@
+// SPDX-License-Identifier: GPL-2.0-only
+///
+/// Use pm_runtime_resume_and_get.
+/// pm_runtime_get_sync keeps a reference count on failure,
+/// which can lead to leaks. pm_runtime_resume_and_get
+/// drops the reference count in the failure case.
+/// This rule addresses the cases where the reference count
+/// is unlikely to be needed in the failure case.
+///
+// Confidence: High
+// Copyright: (C) 2021 Julia Lawall, Inria
+// URL: https://coccinelle.gitlabpages.inria.fr/website
+// Options: --include-headers --no-includes
+// Keywords: pm_runtime_get_sync
+
+virtual patch
+virtual context
+virtual org
+virtual report
+
+@r0 depends on patch && !context && !org && !report@
+expression ret,e;
+@@
+
+- ret = pm_runtime_get_sync(e);
++ ret = pm_runtime_resume_and_get(e);
+- if (ret < 0)
+- pm_runtime_put_noidle(e);
+
+@r1 depends on patch && !context && !org && !report@
+expression ret,e;
+statement S1,S2;
+@@
+
+- ret = pm_runtime_get_sync(e);
++ ret = pm_runtime_resume_and_get(e);
+ if (ret < 0)
+- {
+- pm_runtime_put_noidle(e);
+ S1
+- }
+ else S2
+
+@r2 depends on patch && !context && !org && !report@
+expression ret,e;
+statement S;
+@@
+
+- ret = pm_runtime_get_sync(e);
++ ret = pm_runtime_resume_and_get(e);
+ if (ret < 0) {
+- pm_runtime_put_noidle(e);
+ ...
+ } else S
+
+@r3 depends on patch && !context && !org && !report@
+expression ret,e;
+identifier f;
+constant char[] c;
+statement S;
+@@
+
+- ret = pm_runtime_get_sync(e);
++ ret = pm_runtime_resume_and_get(e);
+ if (ret < 0)
+- {
+ f(...,c,...);
+- pm_runtime_put_noidle(e);
+- }
+ else S
+
+@r4 depends on patch && !context && !org && !report@
+expression ret,e;
+identifier f;
+constant char[] c;
+statement S;
+@@
+
+- ret = pm_runtime_get_sync(e);
++ ret = pm_runtime_resume_and_get(e);
+ if (ret < 0) {
+ f(...,c,...);
+- pm_runtime_put_noidle(e);
+ ...
+ } else S
+
+//
+
+@r2_context depends on !patch && (context || org || report)@
+statement S;
+expression e, ret;
+position j0, j1;
+@@
+
+* ret@j0 = pm_runtime_get_sync(e);
+ if (ret < 0) {
+* pm_runtime_put_noidle@j1(e);
+ ...
+ } else S
+
+@r3_context depends on !patch && (context || org || report)@
+identifier f;
+statement S;
+constant char []c;
+expression e, ret;
+position j0, j1;
+@@
+
+* ret@j0 = pm_runtime_get_sync(e);
+ if (ret < 0) {
+ f(...,c,...);
+* pm_runtime_put_noidle@j1(e);
+ ...
+ } else S
+
+//
+
+@script:python r2_org depends on org@
+j0 << r2_context.j0;
+j1 << r2_context.j1;
+@@
+
+msg = "WARNING: opportunity for pm_runtime_get_sync"
+coccilib.org.print_todo(j0[0], msg)
+coccilib.org.print_link(j1[0], "")
+
+@script:python r3_org depends on org@
+j0 << r3_context.j0;
+j1 << r3_context.j1;
+@@
+
+msg = "WARNING: opportunity for pm_runtime_get_sync"
+coccilib.org.print_todo(j0[0], msg)
[Cocci] [PATCH v3] coccinelle: api: semantic patch to use pm_runtime_resume_and_get
pm_runtime_get_sync keeps a reference count on failure, which can lead
to leaks. pm_runtime_resume_and_get drops the reference count in the
failure case. This rule very conservatively follows the definition of
pm_runtime_resume_and_get to address the cases where the reference
count is unlikely to be needed in the failure case. Specifically, the
change is only done when pm_runtime_get_sync is followed immediately
by an if and when the branch of the if is immediately a call to
pm_runtime_put_noidle (like in the definition of
pm_runtime_resume_and_get) or something that is likely a print
statement followed by a pm_runtime_resume_and_get call. The patch
case appears somewhat more complicated, because it also deals with the
cases where {}s need to be removed.
pm_runtime_resume_and_get was introduced in
commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to
deal with usage counter")
Signed-off-by: Julia Lawall
---
v3: add the people who signed off on commit dd8088d5a896, expand the log message
v2: better keyword
scripts/coccinelle/api/pm_runtime_resume_and_get.cocci | 153 +
1 file changed, 153 insertions(+)
diff --git a/scripts/coccinelle/api/pm_runtime_resume_and_get.cocci
b/scripts/coccinelle/api/pm_runtime_resume_and_get.cocci
new file mode 100644
index ..3387cb606f9b
--- /dev/null
+++ b/scripts/coccinelle/api/pm_runtime_resume_and_get.cocci
@@ -0,0 +1,153 @@
+// SPDX-License-Identifier: GPL-2.0-only
+///
+/// Use pm_runtime_resume_and_get.
+/// pm_runtime_get_sync keeps a reference count on failure,
+/// which can lead to leaks. pm_runtime_resume_and_get
+/// drops the reference count in the failure case.
+/// This rule addresses the cases where the reference count
+/// is unlikely to be needed in the failure case.
+///
+// Confidence: High
+// Copyright: (C) 2021 Julia Lawall, Inria
+// URL: https://coccinelle.gitlabpages.inria.fr/website
+// Options: --include-headers --no-includes
+// Keywords: pm_runtime_get_sync
+
+virtual patch
+virtual context
+virtual org
+virtual report
+
+@r0 depends on patch && !context && !org && !report@
+expression ret,e;
+@@
+
+- ret = pm_runtime_get_sync(e);
++ ret = pm_runtime_resume_and_get(e);
+- if (ret < 0)
+- pm_runtime_put_noidle(e);
+
+@r1 depends on patch && !context && !org && !report@
+expression ret,e;
+statement S1,S2;
+@@
+
+- ret = pm_runtime_get_sync(e);
++ ret = pm_runtime_resume_and_get(e);
+ if (ret < 0)
+- {
+- pm_runtime_put_noidle(e);
+ S1
+- }
+ else S2
+
+@r2 depends on patch && !context && !org && !report@
+expression ret,e;
+statement S;
+@@
+
+- ret = pm_runtime_get_sync(e);
++ ret = pm_runtime_resume_and_get(e);
+ if (ret < 0) {
+- pm_runtime_put_noidle(e);
+ ...
+ } else S
+
+@r3 depends on patch && !context && !org && !report@
+expression ret,e;
+identifier f;
+constant char[] c;
+statement S;
+@@
+
+- ret = pm_runtime_get_sync(e);
++ ret = pm_runtime_resume_and_get(e);
+ if (ret < 0)
+- {
+ f(...,c,...);
+- pm_runtime_put_noidle(e);
+- }
+ else S
+
+@r4 depends on patch && !context && !org && !report@
+expression ret,e;
+identifier f;
+constant char[] c;
+statement S;
+@@
+
+- ret = pm_runtime_get_sync(e);
++ ret = pm_runtime_resume_and_get(e);
+ if (ret < 0) {
+ f(...,c,...);
+- pm_runtime_put_noidle(e);
+ ...
+ } else S
+
+//
+
+@r2_context depends on !patch && (context || org || report)@
+statement S;
+expression e, ret;
+position j0, j1;
+@@
+
+* ret@j0 = pm_runtime_get_sync(e);
+ if (ret < 0) {
+* pm_runtime_put_noidle@j1(e);
+ ...
+ } else S
+
+@r3_context depends on !patch && (context || org || report)@
+identifier f;
+statement S;
+constant char []c;
+expression e, ret;
+position j0, j1;
+@@
+
+* ret@j0 = pm_runtime_get_sync(e);
+ if (ret < 0) {
+ f(...,c,...);
+* pm_runtime_put_noidle@j1(e);
+ ...
+ } else S
+
+//
+
+@script:python r2_org depends on org@
+j0 << r2_context.j0;
+j1 << r2_context.j1;
+@@
+
+msg = "WARNING: opportunity for pm_runtime_get_sync"
+coccilib.org.print_todo(j0[0], msg)
+coccilib.org.print_link(j1[0], "")
+
+@script:python r3_org depends on org@
+j0 << r3_context.j0;
+j1 << r3_context.j1;
+@@
+
+msg = "WARNING: opportunity for pm_runtime_get_sync"
+coccilib.org.print_todo(j0[0], msg)
+coccilib.org.print_link(j1[0], "")
+
+// --
Re: [Cocci] [PATCH v2] coccinelle: api: semantic patch to use pm_runtime_resume_and_get
On Tue, 27 Apr 2021, Johan Hovold wrote:
> On Mon, Apr 26, 2021 at 08:54:04PM +0200, Julia Lawall wrote:
> > pm_runtime_get_sync keeps a reference count on failure, which can lead
> > to leaks. pm_runtime_resume_and_get drops the reference count in the
> > failure case. This rule very conservatively follows the definition of
> > pm_runtime_resume_and_get to address the cases where the reference
> > count is unlikely to be needed in the failure case.
> >
> > pm_runtime_resume_and_get was introduced in
> > commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to
> > deal with usage counter")
> >
> > Signed-off-by: Julia Lawall
>
> As I've said elsewhere, not sure trying to do a mass conversion of this
> is a good idea. People may not be used to the interface, but it is
> consistent and has its use. The recent flurry of conversions show that
> those also risk introducing new bugs in code that is currently tested
> and correct.
I looked some of the patches you commented on, and this rule would not
have transformed those cases. This rule is very restricted to ensure that
the transformed code follows the behavior of the new function.
>
> By giving the script kiddies another toy like this, the influx of broken
> patches is just bound to increase.
>
> Would also be good to CC the PM maintainer on this issue.
Sure, I can resend with Rafael in CC.
julia
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH v2] coccinelle: api: semantic patch to use pm_runtime_resume_and_get
pm_runtime_get_sync keeps a reference count on failure, which can lead
to leaks. pm_runtime_resume_and_get drops the reference count in the
failure case. This rule very conservatively follows the definition of
pm_runtime_resume_and_get to address the cases where the reference
count is unlikely to be needed in the failure case.
pm_runtime_resume_and_get was introduced in
commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to
deal with usage counter")
Signed-off-by: Julia Lawall
---
v2: better keyword
scripts/coccinelle/api/pm_runtime_resume_and_get.cocci | 153 +
1 file changed, 153 insertions(+)
diff --git a/scripts/coccinelle/api/pm_runtime_resume_and_get.cocci
b/scripts/coccinelle/api/pm_runtime_resume_and_get.cocci
new file mode 100644
index ..3387cb606f9b
--- /dev/null
+++ b/scripts/coccinelle/api/pm_runtime_resume_and_get.cocci
@@ -0,0 +1,153 @@
+// SPDX-License-Identifier: GPL-2.0-only
+///
+/// Use pm_runtime_resume_and_get.
+/// pm_runtime_get_sync keeps a reference count on failure,
+/// which can lead to leaks. pm_runtime_resume_and_get
+/// drops the reference count in the failure case.
+/// This rule addresses the cases where the reference count
+/// is unlikely to be needed in the failure case.
+///
+// Confidence: High
+// Copyright: (C) 2021 Julia Lawall, Inria
+// URL: https://coccinelle.gitlabpages.inria.fr/website
+// Options: --include-headers --no-includes
+// Keywords: pm_runtime_get_sync
+
+virtual patch
+virtual context
+virtual org
+virtual report
+
+@r0 depends on patch && !context && !org && !report@
+expression ret,e;
+@@
+
+- ret = pm_runtime_get_sync(e);
++ ret = pm_runtime_resume_and_get(e);
+- if (ret < 0)
+- pm_runtime_put_noidle(e);
+
+@r1 depends on patch && !context && !org && !report@
+expression ret,e;
+statement S1,S2;
+@@
+
+- ret = pm_runtime_get_sync(e);
++ ret = pm_runtime_resume_and_get(e);
+ if (ret < 0)
+- {
+- pm_runtime_put_noidle(e);
+ S1
+- }
+ else S2
+
+@r2 depends on patch && !context && !org && !report@
+expression ret,e;
+statement S;
+@@
+
+- ret = pm_runtime_get_sync(e);
++ ret = pm_runtime_resume_and_get(e);
+ if (ret < 0) {
+- pm_runtime_put_noidle(e);
+ ...
+ } else S
+
+@r3 depends on patch && !context && !org && !report@
+expression ret,e;
+identifier f;
+constant char[] c;
+statement S;
+@@
+
+- ret = pm_runtime_get_sync(e);
++ ret = pm_runtime_resume_and_get(e);
+ if (ret < 0)
+- {
+ f(...,c,...);
+- pm_runtime_put_noidle(e);
+- }
+ else S
+
+@r4 depends on patch && !context && !org && !report@
+expression ret,e;
+identifier f;
+constant char[] c;
+statement S;
+@@
+
+- ret = pm_runtime_get_sync(e);
++ ret = pm_runtime_resume_and_get(e);
+ if (ret < 0) {
+ f(...,c,...);
+- pm_runtime_put_noidle(e);
+ ...
+ } else S
+
+//
+
+@r2_context depends on !patch && (context || org || report)@
+statement S;
+expression e, ret;
+position j0, j1;
+@@
+
+* ret@j0 = pm_runtime_get_sync(e);
+ if (ret < 0) {
+* pm_runtime_put_noidle@j1(e);
+ ...
+ } else S
+
+@r3_context depends on !patch && (context || org || report)@
+identifier f;
+statement S;
+constant char []c;
+expression e, ret;
+position j0, j1;
+@@
+
+* ret@j0 = pm_runtime_get_sync(e);
+ if (ret < 0) {
+ f(...,c,...);
+* pm_runtime_put_noidle@j1(e);
+ ...
+ } else S
+
+//
+
+@script:python r2_org depends on org@
+j0 << r2_context.j0;
+j1 << r2_context.j1;
+@@
+
+msg = "WARNING: opportunity for pm_runtime_get_sync"
+coccilib.org.print_todo(j0[0], msg)
+coccilib.org.print_link(j1[0], "")
+
+@script:python r3_org depends on org@
+j0 << r3_context.j0;
+j1 << r3_context.j1;
+@@
+
+msg = "WARNING: opportunity for pm_runtime_get_sync"
+coccilib.org.print_todo(j0[0], msg)
+coccilib.org.print_link(j1[0], "")
+
+//
+
+@script:python r2_report depends on report@
+j0 << r2_context.j0;
+j1 << r2_context.j1;
+@@
+
+msg = "WARNING: opportunity for pm_runtime_get_sync on line %s." % (j0[0].line)
+coccilib.report.print_report(j0[0], msg)
+
+@script:python r3_report depends on report@
+j0 << r3_context.j0;
+j1 << r3_context.j1;
+@@
+
+msg = "WARNING: opportunity for pm_runtime_get_sync on %s." % (j0[0].line)
+coccilib.report.print_report(j0[0], msg)
+
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH] coccinelle: api: semantic patch to use pm_runtime_resume_and_get
pm_runtime_get_sync keeps a reference count on failure, which can lead
to leaks. pm_runtime_resume_and_get drops the reference count in the
failure case. This rule very conservatively follows the definition of
pm_runtime_resume_and_get to address the cases where the reference
count is unlikely to be needed in the failure case.
pm_runtime_resume_and_get was introduced in
commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to
deal with usage counter")
Signed-off-by: Julia Lawall
---
scripts/coccinelle/api/pm_runtime_resume_and_get.cocci | 153 +
1 file changed, 153 insertions(+)
diff --git a/scripts/coccinelle/api/pm_runtime_resume_and_get.cocci
b/scripts/coccinelle/api/pm_runtime_resume_and_get.cocci
new file mode 100644
index ..3387cb606f9b
--- /dev/null
+++ b/scripts/coccinelle/api/pm_runtime_resume_and_get.cocci
@@ -0,0 +1,153 @@
+// SPDX-License-Identifier: GPL-2.0-only
+///
+/// Use pm_runtime_resume_and_get.
+/// pm_runtime_get_sync keeps a reference count on failure,
+/// which can lead to leaks. pm_runtime_resume_and_get
+/// drops the reference count in the failure case.
+/// This rule addresses the cases where the reference count
+/// is unlikely to be needed in the failure case.
+///
+// Confidence: High
+// Copyright: (C) 2021 Julia Lawall, Inria
+// URL: https://coccinelle.gitlabpages.inria.fr/website
+// Options: --include-headers --no-includes
+// Keywords: kwd
+
+virtual patch
+virtual context
+virtual org
+virtual report
+
+@r0 depends on patch && !context && !org && !report@
+expression ret,e;
+@@
+
+- ret = pm_runtime_get_sync(e);
++ ret = pm_runtime_resume_and_get(e);
+- if (ret < 0)
+- pm_runtime_put_noidle(e);
+
+@r1 depends on patch && !context && !org && !report@
+expression ret,e;
+statement S1,S2;
+@@
+
+- ret = pm_runtime_get_sync(e);
++ ret = pm_runtime_resume_and_get(e);
+ if (ret < 0)
+- {
+- pm_runtime_put_noidle(e);
+ S1
+- }
+ else S2
+
+@r2 depends on patch && !context && !org && !report@
+expression ret,e;
+statement S;
+@@
+
+- ret = pm_runtime_get_sync(e);
++ ret = pm_runtime_resume_and_get(e);
+ if (ret < 0) {
+- pm_runtime_put_noidle(e);
+ ...
+ } else S
+
+@r3 depends on patch && !context && !org && !report@
+expression ret,e;
+identifier f;
+constant char[] c;
+statement S;
+@@
+
+- ret = pm_runtime_get_sync(e);
++ ret = pm_runtime_resume_and_get(e);
+ if (ret < 0)
+- {
+ f(...,c,...);
+- pm_runtime_put_noidle(e);
+- }
+ else S
+
+@r4 depends on patch && !context && !org && !report@
+expression ret,e;
+identifier f;
+constant char[] c;
+statement S;
+@@
+
+- ret = pm_runtime_get_sync(e);
++ ret = pm_runtime_resume_and_get(e);
+ if (ret < 0) {
+ f(...,c,...);
+- pm_runtime_put_noidle(e);
+ ...
+ } else S
+
+//
+
+@r2_context depends on !patch && (context || org || report)@
+statement S;
+expression e, ret;
+position j0, j1;
+@@
+
+* ret@j0 = pm_runtime_get_sync(e);
+ if (ret < 0) {
+* pm_runtime_put_noidle@j1(e);
+ ...
+ } else S
+
+@r3_context depends on !patch && (context || org || report)@
+identifier f;
+statement S;
+constant char []c;
+expression e, ret;
+position j0, j1;
+@@
+
+* ret@j0 = pm_runtime_get_sync(e);
+ if (ret < 0) {
+ f(...,c,...);
+* pm_runtime_put_noidle@j1(e);
+ ...
+ } else S
+
+//
+
+@script:python r2_org depends on org@
+j0 << r2_context.j0;
+j1 << r2_context.j1;
+@@
+
+msg = "WARNING: opportunity for pm_runtime_get_sync"
+coccilib.org.print_todo(j0[0], msg)
+coccilib.org.print_link(j1[0], "")
+
+@script:python r3_org depends on org@
+j0 << r3_context.j0;
+j1 << r3_context.j1;
+@@
+
+msg = "WARNING: opportunity for pm_runtime_get_sync"
+coccilib.org.print_todo(j0[0], msg)
+coccilib.org.print_link(j1[0], "")
+
+//
+
+@script:python r2_report depends on report@
+j0 << r2_context.j0;
+j1 << r2_context.j1;
+@@
+
+msg = "WARNING: opportunity for pm_runtime_get_sync on line %s." % (j0[0].line)
+coccilib.report.print_report(j0[0], msg)
+
+@script:python r3_report depends on report@
+j0 << r3_context.j0;
+j1 << r3_context.j1;
+@@
+
+msg = "WARNING: opportunity for pm_runtime_get_sync on %s." % (j0[0].line)
+coccilib.report.print_report(j0[0], msg)
+
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH] coccinelle: irqf_oneshot: reduce the severity due to false positives
On Fri, 23 Apr 2021, Krzysztof Kozlowski wrote: > The IRQF_ONESHOT should be present for threaded IRQ using default > primary handler. However intetrupt of many child devices, e.g. children > of MFD, is nested thus the IRQF_ONESHOT is not needed. The coccinelle > message about error misleads submitters and reviewers about the severity > of the issue, so make it a warning and mention possible false positive. > > Signed-off-by: Krzysztof Kozlowski Applied. Thanks for the clarification. julia > --- > scripts/coccinelle/misc/irqf_oneshot.cocci | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/scripts/coccinelle/misc/irqf_oneshot.cocci > b/scripts/coccinelle/misc/irqf_oneshot.cocci > index 7b48287b3dc1..9b6f404d07f2 100644 > --- a/scripts/coccinelle/misc/irqf_oneshot.cocci > +++ b/scripts/coccinelle/misc/irqf_oneshot.cocci > @@ -103,11 +103,11 @@ devm_request_threaded_irq@p(dev, irq, NULL, ...) > @script:python depends on org@ > p << match.p; > @@ > -msg = "ERROR: Threaded IRQ with no primary handler requested without > IRQF_ONESHOT" > +msg = "WARNING: Threaded IRQ with no primary handler requested without > IRQF_ONESHOT (unless it is nested IRQ)" > coccilib.org.print_todo(p[0],msg) > > @script:python depends on report@ > p << match.p; > @@ > -msg = "ERROR: Threaded IRQ with no primary handler requested without > IRQF_ONESHOT" > +msg = "WARNING: Threaded IRQ with no primary handler requested without > IRQF_ONESHOT (unless it is nested IRQ)" > coccilib.report.print_report(p[0],msg) > -- > 2.25.1 > > ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] getting rid of implicit boolean expressions
On Wed, 21 Apr 2021, Akos Pasztory wrote:
> Hi,
>
> I'm trying do the following kind of transformations:
>
> int x, y;
> char *p;
> bool b, c;
>
> -b = x || !y;
> +b = (x != 0) || (y == 0);
>
> -c = !p;
> +c = (p == NULL);
>
> -if (x & 3)
> +if ((x & 3) != 0)
> f();
> // etc
>
> That is: trying to eliminate implicit boolean-ness (and add parentheses as
> well).
>
> I was thinking along the lines of first finding expressions
> that are in "boolean context" (part of a || or && expression,
> or an if/for/while condition, maybe something else too?).
> Then find sub-expressions of those that are not of the form 'E op F'
> where 'op' is a comparison operator (==, !=, <=, ...).
> And finally depending on whether they are pointer or integer and
> whether they are negated, replace them with the above constructs (x != 0,
> etc.)
>
> Is this the right way to think about this? Meaning does it fit the mental
> model
> of Coccinelle, or some other approach is needed? (E.g. it crossed my mind to
> maybe match all expressions and try to filter out "unwanted" ones via
> position p != { ... } constraints but that seemed infeasible.)
I think you can do
A simple approach could be:
@@
idexpression *x;
@@
- x
+ (x != NULL)
|| ...
@@
idexpression x;
@@
- x
+ (x != 0)
|| ...
If you want to do function calls, you could do
@@
expression *e;
identifier f;
expression list es;
@@
- f(es)@e
+ (f(es) != NULL)
|| ...
@@
identifier f;
expression list es;
@@
- f(es)
+ (f(es) != 0)
|| ...
Some explanation:
* For pattern || ... there is an isomorphism that allows the pattern to
appear anywhere in the top level of a chain of ||s, including an empty
chain. So it actually should match any expression in a boolean context.
* In the third rule, there is )@e. That means that e should match the
smallest expression that contains the ), which turns out to the be
function call. That way you can talk about the return type of the
function call. A limitation here is that Coccinelle has to be able to
figure out what the type is (this is also a limitation of the first rule
above). If it can't figure out the type of the variable or the return
type of the function call, then the first/third rule will fail and you
will end up with a != 0 test on a pointer. To try to avoid this, you can
use the options --recursive-includes --use-headers-for-types
--relax-include-path to try to take into account as many header files as
possible.
An alternate approach is indeed to do something with position variables.
So you could do something like:
@ok@
position p;
expression e;
expression x;
@@
(x != 0)@e@p
@@
position p != ok.p;
expression x;
@@
- x@p
+ (x != 0)
|| ...
But the first rule would have to be extended to consider lots of cases.
A binary operator metavariable could be helpful, eg:
binary operator bop = { ==, !=, <, > };
julia___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH] Fix parse error in presence of symbols named far
On Wed, 21 Apr 2021, Fuad Tabba wrote:
> Hi,
>
> I just started using Coccinelle yesterday, and I can already see
> it saving me a lot of time and agony. Thank you.
>
> I ran into a problem, and I think that this patch might fix it,
> hopefully without causing other problems. I did some sanity
> checking on my kernel tree, and it seems to be fine.
Thanks very much for this. I have applied the patch.
To check this sort of thing, you can do spatch --parse-c linux_path > old
then make your change
then spatch --parse-c linux_path > new
Then look at the end of old and new. If you successfully parsed more
files and have fewer bad and passed tokens, then all should be good.
julia
>
> There are quite a few variables named "far" in the kernel, e.g.,
> arch/arm64/kvm/inject_fault.c. Coccinelle has special treatment
> for "far" as being linkage related, which causes parse errors in
> their presence. I've grepped for "far" in the kernel tree, and
> haven't noticed where it's used like that, but I could have
> missed it.
>
> To reproduce:
>
> cat > test.c << EOF
> int main(void)
> {
> int far = 0;
> int x;
> x = 10;
> return x;
> }
> EOF
>
> cat > test.cocci << EOF
> @@
> identifier x;
> @@
> - x
> + y
> EOF
>
> spatch --sp-file test.cocci test.c --debug --verbose-parsing
>
> Signed-off-by: Fuad Tabba
> ---
> scripts/coccicheck/cocci/notand.h | 1 -
> standard.h| 1 -
> 2 files changed, 2 deletions(-)
>
> diff --git a/scripts/coccicheck/cocci/notand.h
> b/scripts/coccicheck/cocci/notand.h
> index 3da8c303..91fa6c96 100644
> --- a/scripts/coccicheck/cocci/notand.h
> +++ b/scripts/coccicheck/cocci/notand.h
> @@ -302,7 +302,6 @@
> #define fastcall
> #define asmlinkage
>
> -#define far
> #define SK_FAR
>
> // pb
> diff --git a/standard.h b/standard.h
> index 7a7f96ea..936b19c3 100644
> --- a/standard.h
> +++ b/standard.h
> @@ -298,7 +298,6 @@
> #define fastcall
> #define asmlinkage
>
> -#define far
> #define SK_FAR
>
> // pb
> --
> 2.31.1.368.gbe11c130af-goog
>
> ___
> Cocci mailing list
> Cocci@systeme.lip6.fr
> https://systeme.lip6.fr/mailman/listinfo/cocci
>___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH] Coccinelle: drop context *s
Context mode is not supported, so the *s are just confusing to people
who use the rule outside of make coccicheck. So, drop the *s.
Fixes: 6dd9379e8f32 ("coccinelle: also catch kzfree() issues")
Reported-by: Fabio M. De Francesco
Signed-off-by: Julia Lawall
---
scripts/coccinelle/free/kfree.cocci | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/scripts/coccinelle/free/kfree.cocci
b/scripts/coccinelle/free/kfree.cocci
index 168568386034..9b6e2037c2a9 100644
--- a/scripts/coccinelle/free/kfree.cocci
+++ b/scripts/coccinelle/free/kfree.cocci
@@ -22,9 +22,9 @@ position p1;
@@
(
-* kfree@p1(E)
+ kfree@p1(E)
|
-* kfree_sensitive@p1(E)
+ kfree_sensitive@p1(E)
)
@print expression@
@@ -66,9 +66,9 @@ position ok;
while (1) { ...
(
-* kfree@ok(E)
+ kfree@ok(E)
|
-* kfree_sensitive@ok(E)
+ kfree_sensitive@ok(E)
)
... when != break;
when != goto l;
@@ -84,9 +84,9 @@ position free.p1!=loop.ok,p2!={print.p,sz.p};
@@
(
-* kfree@p1(E,...)
+ kfree@p1(E,...)
|
-* kfree_sensitive@p1(E,...)
+ kfree_sensitive@p1(E,...)
)
...
(
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] changing of_get_mac_address() to pass a buffer
On Mon, 5 Apr 2021, Michael Walle wrote:
> Hi Mansour,
>
> Am 2021-04-04 19:48, schrieb Mansour Moufid:
> > On Thu, Apr 1, 2021 at 4:13 AM Michael Walle wrote:
> > >
> > > Hi,
> > >
> > > so first I need to say I've never used coccinelle before,
> > > so please bear with me ;)
> > >
> > > To make of_get_mac_address() work with DSA ports (and a nvmem
> > > provider) I'd need to change the semantics of of_get_mac_address().
> > > Right now it returns a pointer to "const char *", I'd need to change
> > > that so a buffer will be passed as a parameter in which the MAC
> > > address gets stored.
> > >
> > > (1) Usually the call is something like:
> > >
> > >const char *mac;
> > >mac = of_get_mac_address(np);
> > >if (!IS_ERR(mac))
> > > ether_addr_copy(ndev->dev_addr, mac);
> > >
> > > This would need to be changed to:
> > >
> > >of_get_mac_address(np, ndev->dev_addr);
> >
> > Here is one possible approach, doing the API change first then
> > handling the conditionals. It seems to work.
> >
> > @a@
> > identifier x;
> > expression y, z;
> > @@
> > - x = of_get_mac_address(y);
> > + x = of_get_mac_address(y, z);
> > <...
> > - ether_addr_copy(z, x);
> > ...>
> >
> > @@
> > identifier a.x;
> > @@
> > - if (<+... x ...+>) {}
> >
> > @@
> > identifier a.x;
> > @@
> > if (<+... x ...+>) {
> > ...
> > }
> > - else {}
> >
> > @@
> > identifier a.x;
> > expression e;
> > @@
> > - if (<+... x ...+>@e)
> > - {}
> > - else
> > + if (!(e))
Maybe try the above line without the parentheses around e?
> > {...}
> >
> > @@
> > expression x, y, z;
> > @@
> > - x = of_get_mac_address(y, z);
> > + of_get_mac_address(y, z);
> > ... when != x
>
> Thanks a lot!
>
> See also
> https://lore.kernel.org/netdev/20210405164643.21130-1-mich...@walle.cc/
>
> There were some "if (!(!IS_ERR(x))", which I needed to simplify
> manually. Didn't noticed that in my previous script. I'm just
> curious, is that also something coccinelle can simplify on its
> own?
You can certainly write another rule for it:
- !(!e)
+ e
julia
>
> -michael
> ___
> Cocci mailing list
> Cocci@systeme.lip6.fr
> https://systeme.lip6.fr/mailman/listinfo/cocci
>
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Checking support for compound expressions (according to #define directives)
On Mon, 5 Apr 2021, Markus Elfring wrote:
> >> I would like to avoid the repetition of parsing efforts as much as
> >> possible.
> >> Under which circumstances can replacement lists be taken better into
> >> account?
> >
> > Why does my suggestion involve a repetition of parsing effort?
>
> The selection of the applied programming interfaces has got significant
> influences
> on the run time behaviour.
>
> See also:
> https://github.com/coccinelle/coccinelle/issues/200#issuecomment-653775288
>
>
> > You want to use a regexp.
>
> This view depends on some factors.
> I would prefer to search for string literals (and their exclusion) by higher
> level means.
>
>
> > I'm asking you to put the regexp in a python function.
>
> How do you think about to improve the following software situation
> besides the application of regular expressions?
>
> @initialize:python@
> @@
> import re
>
> @display@
> identifier i =~ "^(?:[A-Z]+_){3,3}[A-Z]+";
> expression e : script:python() { re.match('"', e) };
> @@
> *#define i e
>
>
> elfring@Sonne:~/Projekte/PipeWire/lokal> spatch
> ~/Projekte/Coccinelle/janitor/show_define_usage7.cocci
> spa/include/spa/node/type-info.h
> …
> File "", line 5
> coccinelle.result = (re . match ( " , e ))
> ^
> SyntaxError: EOL while scanning string literal
This looks like a problem. Thanks for the report.
julia___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Checking support for compound expressions (according to #define directives)
On Mon, 5 Apr 2021, Markus Elfring wrote:
> >> @display@
> >> identifier i =~ "^(?:[A-Z]+_){3,3}[A-Z]+", x;
> >> constant c =~ "\"";
> >> @@
> >> *#define i x c
> >>
> >>
> >> elfring@Sonne:~/Projekte/Coccinelle/janitor> spatch --parse-cocci
> >> show_define_usage6.cocci
> >> …
> >> minus: parse error:
> >> File "show_define_usage6.cocci", line 5, column 13, charpos = 92
> >> around = 'c',
> >> whole content = *#define i x c
> >
> > No. You have to match the expression and then check it using python.
>
> I find this view improvable.
>
> I would like to avoid the repetition of parsing efforts as much as possible.
> Under which circumstances can replacement lists be taken better into account?
Why does my suggestion involve a repetition of parsing effort? You want
to use a regexp. I'm asking you to put the regexp in a python function.
julia___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Checking support for compound expressions (according to #define directives)
On Mon, 5 Apr 2021, Markus Elfring wrote:
> > Thanks for the simpler examples.
>
> Would you like to support another search pattern by the means of
> the semantic patch language?
>
>
> @display@
> identifier i =~ "^(?:[A-Z]+_){3,3}[A-Z]+", x;
> constant c =~ "\"";
> @@
> *#define i x c
>
>
> elfring@Sonne:~/Projekte/Coccinelle/janitor> spatch --parse-cocci
> show_define_usage6.cocci
> …
> minus: parse error:
> File "show_define_usage6.cocci", line 5, column 13, charpos = 92
> around = 'c',
> whole content = *#define i x c
No. You have to match the expression and then check it using python. YOu
can write:
@@
identifier i;
expression e : script:python() { ... python code to ceck e };
@@
#define i e
julia___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] changing of_get_mac_address() to pass a buffer
On Sun, 4 Apr 2021, Mansour Moufid wrote:
> On Thu, Apr 1, 2021 at 4:13 AM Michael Walle wrote:
> >
> > Hi,
> >
> > so first I need to say I've never used coccinelle before,
> > so please bear with me ;)
> >
> > To make of_get_mac_address() work with DSA ports (and a nvmem
> > provider) I'd need to change the semantics of of_get_mac_address().
> > Right now it returns a pointer to "const char *", I'd need to change
> > that so a buffer will be passed as a parameter in which the MAC
> > address gets stored.
> >
> > (1) Usually the call is something like:
> >
> >const char *mac;
> >mac = of_get_mac_address(np);
> >if (!IS_ERR(mac))
> > ether_addr_copy(ndev->dev_addr, mac);
> >
> > This would need to be changed to:
> >
> >of_get_mac_address(np, ndev->dev_addr);
>
> Here is one possible approach, doing the API change first then
> handling the conditionals. It seems to work.
>
> @a@
> identifier x;
> expression y, z;
> @@
> - x = of_get_mac_address(y);
> + x = of_get_mac_address(y, z);
> <...
> - ether_addr_copy(z, x);
> ...>
>
> @@
> identifier a.x;
> @@
> - if (<+... x ...+>) {}
>
> @@
> identifier a.x;
> @@
> if (<+... x ...+>) {
> ...
> }
> - else {}
>
> @@
> identifier a.x;
> expression e;
> @@
> - if (<+... x ...+>@e)
> - {}
> - else
> + if (!(e))
> {...}
>
> @@
> expression x, y, z;
> @@
> - x = of_get_mac_address(y, z);
> + of_get_mac_address(y, z);
> ... when != x
This seems like a good approach. It would also be possible to have
special cases for when the removed call is in a {} by itself, but it seems
like a lot of trouble for little benefit. Presumably the existing code
doesn't contain {}, so the only code that would be affected by the cleanup
rules would be the code that was introduced by the first rule. Thanks for
th suggestion.
julia
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH v3] coccinelle: misc: add swap script
On Sun, 28 Mar 2021, Denis Efremov wrote:
> Ping?
Applied. Thanks.
>
> On 3/5/21 1:09 PM, Denis Efremov wrote:
> > Check for opencoded swap() implementation.
> >
> > Signed-off-by: Denis Efremov
> > ---
> > Changes in v2:
> > - additional patch rule to drop excessive {}
> > - fix indentation in patch mode by anchoring ;
> > Changes in v3:
> > - Rule added for simple (without var init) swap highlighting in !patch
> > mode
> > - "depends on patch && (rpvar || rp)" fixed
> >
> > scripts/coccinelle/misc/swap.cocci | 122 +
> > 1 file changed, 122 insertions(+)
> > create mode 100644 scripts/coccinelle/misc/swap.cocci
> >
> > diff --git a/scripts/coccinelle/misc/swap.cocci
> > b/scripts/coccinelle/misc/swap.cocci
> > new file mode 100644
> > index ..c5e71b7ef7f5
> > --- /dev/null
> > +++ b/scripts/coccinelle/misc/swap.cocci
> > @@ -0,0 +1,122 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +///
> > +/// Check for opencoded swap() implementation.
> > +///
> > +// Confidence: High
> > +// Copyright: (C) 2021 Denis Efremov ISPRAS
> > +// Options: --no-includes --include-headers
> > +//
> > +// Keywords: swap
> > +//
> > +
> > +virtual patch
> > +virtual org
> > +virtual report
> > +virtual context
> > +
> > +@rvar depends on !patch@
> > +identifier tmp;
> > +expression a, b;
> > +type T;
> > +position p;
> > +@@
> > +
> > +(
> > +* T tmp;
> > +|
> > +* T tmp = 0;
> > +|
> > +* T *tmp = NULL;
> > +)
> > +... when != tmp
> > +* tmp = a;
> > +* a = b;@p
> > +* b = tmp;
> > +... when != tmp
> > +
> > +@r depends on !patch@
> > +identifier tmp;
> > +expression a, b;
> > +position p != rvar.p;
> > +@@
> > +
> > +* tmp = a;
> > +* a = b;@p
> > +* b = tmp;
> > +
> > +@rpvar depends on patch@
> > +identifier tmp;
> > +expression a, b;
> > +type T;
> > +@@
> > +
> > +(
> > +- T tmp;
> > +|
> > +- T tmp = 0;
> > +|
> > +- T *tmp = NULL;
> > +)
> > +... when != tmp
> > +- tmp = a;
> > +- a = b;
> > +- b = tmp
> > ++ swap(a, b)
> > + ;
> > +... when != tmp
> > +
> > +@rp depends on patch@
> > +identifier tmp;
> > +expression a, b;
> > +@@
> > +
> > +- tmp = a;
> > +- a = b;
> > +- b = tmp
> > ++ swap(a, b)
> > + ;
> > +
> > +@depends on patch && (rpvar || rp)@
> > +@@
> > +
> > +(
> > + for (...;...;...)
> > +- {
> > + swap(...);
> > +- }
> > +|
> > + while (...)
> > +- {
> > + swap(...);
> > +- }
> > +|
> > + if (...)
> > +- {
> > + swap(...);
> > +- }
> > +)
> > +
> > +
> > +@script:python depends on report@
> > +p << r.p;
> > +@@
> > +
> > +coccilib.report.print_report(p[0], "WARNING opportunity for swap()")
> > +
> > +@script:python depends on org@
> > +p << r.p;
> > +@@
> > +
> > +coccilib.org.print_todo(p[0], "WARNING opportunity for swap()")
> > +
> > +@script:python depends on report@
> > +p << rvar.p;
> > +@@
> > +
> > +coccilib.report.print_report(p[0], "WARNING opportunity for swap()")
> > +
> > +@script:python depends on org@
> > +p << rvar.p;
> > +@@
> > +
> > +coccilib.org.print_todo(p[0], "WARNING opportunity for swap()")
> >
>
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Excluding quotes from strings of #define directives
On Sun, 4 Apr 2021, Markus Elfring wrote:
> >> I hoped that the specified constraint for the metavariable “e” would mean
> >> that expressions which contain a double quotation character should be
> >> excluded
> >> for my source code analysis approach.
> >> Would you like to check the observed software functionality once more?
> >
> > There is perhaps a problem, but it is surely not necessary to have all of
> > this python code around it to see the problem.
>
> I chose this test display so that it can be clearly seen which data were
> processes
> for the metavariable “e”.
>
>
> > Please make a minimal example. A rule with a match and a * in front of it
> > should be sufficient.
>
> Do you find constraints of the following SmPL script variant easier to
> clarify?
>
> @display@
> identifier i =~ "^(?:[A-Z]+_){3,3}[A-Z]+";
> expression e !~ "\"";
> @@
> *#define i e
>
> Test result:
> https://gitlab.freedesktop.org/pipewire/pipewire/-/blob/97b01ed9b01bac7cba68ff11c6bf7ceabcae7f52/spa/include/spa/node/type-info.h#L38
>
> elfring@Sonne:~/Projekte/PipeWire/lokal> spatch
> ~/Projekte/Coccinelle/janitor/show_define_usage4.cocci
> spa/include/spa/node/type-info.h
> …
> @@ -35,8 +35,6 @@ extern "C" {
> #include
> #include
>
> -#define SPA_TYPE_INFO_IO SPA_TYPE_INFO_ENUM_BASE "IO"
> -#define SPA_TYPE_INFO_IO_BASE SPA_TYPE_INFO_IO ":"
>
> static const struct spa_type_info spa_type_io[] = {
> { SPA_IO_Invalid, SPA_TYPE_Int, SPA_TYPE_INFO_IO_BASE "Invalid", NULL
> },
> …
Thanks for the simpler examples.
julia___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Excluding quotes from strings of #define directives
On Sun, 4 Apr 2021, Markus Elfring wrote:
> > The following looks like what one might want to do to find #defines that
> > are near each other.
>
> I have tried another SmPL script variant out.
>
>
> @initialize:python@
> @@
> import sys
>
> records = {}
>
> class integrity_error:
>pass
>
> def store_positions(places, name, text):
> """Add source code positions to an internal table."""
> for place in places:
>key = place.file, place.line, int(place.column) + 1
>
>if key in records:
> sys.stderr.write("\n".join(["-> duplicate data",
> "file:", key[0],
> "function:", place.current_element,
> "line:", str(place.line)]))
> sys.stderr.write("\n")
> raise integrity_error
>else:
> records[key] = name, text, place.current_element
>
> @find@
> identifier i =~ "^(?:[A-Z]+_){3,3}[A-Z]+";
> expression e !~ "\"";
> position p;
> @@
> #define i@p e
>
> @script:python collection@
> i << find.i;
> e << find.e;
> places << find.p;
> @@
> store_positions(places, i, e)
>
> @finalize:python@
> @@
> if len(records) > 0:
>delimiter = "|"
>sys.stdout.write(delimiter.join(['name',
> 'text',
> 'function',
> '"source file"',
> 'line',
> 'column'
> ]))
>sys.stdout.write("\r\n")
>
>for key, value in records.items():
> sys.stdout.write(delimiter.join([value[0],
>value[1],
>value[2],
>key[0],
>key[1],
>str(key[2])
> ]))
> sys.stdout.write("\r\n")
> else:
>sys.stderr.write("No result for this analysis!\n")
>
>
> I wonder about the following test result then.
>
> elfring@Sonne:~/Projekte/PipeWire/lokal> spatch
> ~/Projekte/Coccinelle/janitor/check_define_usage.cocci
> spa/include/spa/node/type-info.h
> …
> name|text|function|"source file"|line|column
> SPA_TYPE_INFO_NodeCommand|SPA_TYPE_INFO_COMMAND_BASE
> "Node"|something_else|spa/include/spa/node/type-info.h|70|1
> SPA_TYPE_INFO_IO_BASE|SPA_TYPE_INFO_IO
> ":"|something_else|spa/include/spa/node/type-info.h|39|1
> SPA_TYPE_INFO_NODE_EVENT_BASE|SPA_TYPE_INFO_NodeEvent
> ":"|something_else|spa/include/spa/node/type-info.h|56|1
> SPA_TYPE_INFO_NodeEvent|SPA_TYPE_INFO_EVENT_BASE
> "Node"|something_else|spa/include/spa/node/type-info.h|55|1
> SPA_TYPE_INFO_IO|SPA_TYPE_INFO_ENUM_BASE
> "IO"|something_else|spa/include/spa/node/type-info.h|38|1
> SPA_TYPE_INFO_NODE_COMMAND_BASE|SPA_TYPE_INFO_NodeCommand
> ":"|something_else|spa/include/spa/node/type-info.h|71|1
>
>
> I hoped that the specified constraint for the metavariable “e” would mean
> that expressions which contain a double quotation character should be excluded
> for my source code analysis approach.
> Would you like to check the observed software functionality once more?
There is perhaps a problem, but it is surely not necessary to have all of
this python code around it to see the problem. Please make a minimal
example. A rule with a match and a * in front of it should be
sufficient.
julia___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Replacing #define directives with the help of SmPL
On Sun, 28 Mar 2021, Markus Elfring wrote:
> >> Would you like to help any more with attempts to achieve support for
> >> a transformation pattern like “#define ⇒ enum” according to the semantic
> >> patch language?
> >> https://gitlab.freedesktop.org/pipewire/pipewire/-/issues/981
> >> https://github.com/issues?q=is%3Aissue+author%3Aelfring+%22+enum%22
> >
> > If you find that something does not work satisfactorily, propose a
> > semantic patch and show what doesn't work. I'm not going to try to solve
> > a problem when I don't know what the problem is.
>
> Please take another look at the following SmPL script variant.
>
>
> @display3@
> expression e;
> identifier i, s;
> @@
> struct s {
> ...
> *#define i e
> ...
> };
>
>
> elfring@Sonne:~/Projekte/Coccinelle/janitor> spatch --parse-cocci
> show_define_usage3.cocci
> init_defs_builtins: /usr/local/bin/../lib/coccinelle/standard.h
> minus: parse error:
> File "show_define_usage3.cocci", line 7, column 1, charpos = 63
> around = '#define i',
> whole content = *#define i e
This is not supported.
julia
>
>
> See also:
> https://gitlab.freedesktop.org/pipewire/pipewire/-/blob/2681b8236b332abdaf707ab8c5017a9be92d1059/src/pipewire/client.h#L46
>
> Regards,
> Markus
>___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Replacing #define directives with the help of SmPL
On Sun, 28 Mar 2021, Markus Elfring wrote:
> > And what is the problem here?
>
> Would you like to discuss further software development ideas
> according to another SmPL script example like the following?
The following looks like what one might want to do to find #defines that
are near each other.
julia
>
>
> @initialize:python@
> @@
> import sys
>
> records = {}
>
> class integrity_error:
>pass
>
> def store_positions(places, name, text):
> """Add source code positions to an internal table."""
> for place in places:
>key = place.file, place.line, int(place.column) + 1
>
>if key in records:
> sys.stderr.write("\n".join(["-> duplicate data",
> "file:", key[0],
> "function:", place.current_element,
> "line:", str(place.line)]))
> sys.stderr.write("\n")
> raise integrity_error
>else:
> records[key] = name, text, place.current_element
>
> @find@
> expression e;
> identifier i;
> position p;
> @@
> #define i@p e
>
> @script:python collection@
> i << find.i;
> e << find.e;
> places << find.p;
> @@
> store_positions(places, i, e)
>
> @finalize:python@
> @@
> if len(records) > 0:
>delimiter = "|"
>sys.stdout.write(delimiter.join(['name',
> 'text',
> 'function',
> '"source file"',
> 'line',
> 'column'
> ]))
>sys.stdout.write("\r\n")
>
>for key, value in records.items():
> sys.stdout.write(delimiter.join([value[0],
>value[1],
>value[2],
>key[0],
>key[1],
>str(key[2])
> ]))
> sys.stdout.write("\r\n")
> else:
>sys.stderr.write("No result for this analysis!\n")
>
>
> elfring@Sonne:~/Projekte/Coccinelle/janitor> spatch list_macros.cocci
> /usr/include/pipewire-0.3/pipewire/client.h
> …
> name|text|function|"source file"|line|column
> …
> PW_CLIENT_METHOD_GET_PERMISSIONS|3|something_else|/usr/include/pipewire-0.3/pipewire/client.h|98|1
> PW_CLIENT_METHOD_UPDATE_PROPERTIES|2|something_else|/usr/include/pipewire-0.3/pipewire/client.h|97|1
> …
>
>
> Regards,
> Markus
>___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Replacing #define directives with the help of SmPL
On Sun, 28 Mar 2021, Markus Elfring wrote: > >> Another SmPL script example: > >> > >> @display2@ > >> identifier i; > >> expression e; > >> @@ > >> *#define i e > >> > >> > >> elfring@Sonne:~/Projekte/Coccinelle/janitor> spatch > >> show_define_usage2.cocci /usr/include/pipewire-0.3/pipewire/client.h > >> … > >> -#define PW_CLIENT_EVENT_INFO 0 > >> … > > > > And what is the problem here? > > Can you help anyhow according to dealing with the longest common substring > problem > in combination with discussed capabilities of the semantic patch language? You can implement whatever you want in python. > > > > This may (or may not) give you the right grouping, but you will still have > > to ensure that the elements end up in the right order, unless you want to > > provide all the values explicitly. > > Some information from #define directives can be imported into symbol tables > for later transformations (on demand). > Source code positions need to be recorded accordingly, don't they? If you want to record the position of something you can use a position variable. julia___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Replacing #define directives with the help of SmPL
On Sun, 28 Mar 2021, Markus Elfring wrote: > >> @display@ > >> @@ > >> *#define > > > > Obviously this doesn't work. Just like > > > > @@ > > @@ > > *if > > > > doesnt' work. > > Can it become possible to find such key words in the source code > (by such SmPL search approaches)? No. As has been explained many times before, Coccinelle works on complete terms - expressions, statements, etc. If you want to find keywords, use grep. julia > > > >> Another SmPL script example: > >> > >> @display2@ > >> identifier i; > >> expression e; > >> @@ > >> *#define i e > >> > >> > >> elfring@Sonne:~/Projekte/Coccinelle/janitor> spatch > >> show_define_usage2.cocci /usr/include/pipewire-0.3/pipewire/client.h > >> … > >> -#define PW_CLIENT_EVENT_INFO 0 > >> … > > > > And what is the problem here? > > The challenge to determine reasonable group criteria. > > > >> I am looking again for the application of algorithms according to > >> longest common text (or prefixes) in used symbols. > > > > This may (or may not) give you the right grouping, but you will still have > > to ensure that the elements end up in the right order, > > Yes, of course. > > > > unless you want to provide all the values explicitly. > > Special identifier combinations can be converted to enumerations, can't they? > > > >>> Furthermore, if this is targeting C code, the benefits will be limited, > >>> because C considers enums to be the same as ints. > >> > >> * Will such a transformation help with software debugging? > > > > Not likely, because the compiler provides no support > > I got informed that some development tools can work better with enumeration > identifiers > (instead of macro names). > > > > and the definitions are typically far from the uses. > > This is usual. > > Regards, > Markus >___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Replacing #define directives with the help of SmPL
On Sun, 28 Mar 2021, Markus Elfring wrote: > > If you find that something does not work satisfactorily, propose a > > semantic patch and show what doesn't work. > > @display@ > @@ > *#define Obviously this doesn't work. Just like @@ @@ *if doesnt' work. > > I'm not going to try to solve a problem when I don't know what the problem > > is. > > Another SmPL script example: > > @display2@ > identifier i; > expression e; > @@ > *#define i e > > > elfring@Sonne:~/Projekte/Coccinelle/janitor> spatch show_define_usage2.cocci > /usr/include/pipewire-0.3/pipewire/client.h > … > -#define PW_CLIENT_EVENT_INFO 0 > … And what is the problem here? > > > > I think that this tranformation would be diffficult to make using > > Coccinelle, > > * I imagine also that it can be challenging to determine which preprocessor > identifiers > can really be converted to enumerations. > > * My programmer eyes were trained for some pattern recognition. > https://gitlab.freedesktop.org/pipewire/pipewire/-/issues/981 > https://github.com/issues?q=is%3Aissue+author%3Aelfring+%22+enum%22 > > > > due to the need to ensure that the enum values are end up in the > > right order, if you want to rely on the implicit values of enums. > > I am looking again for the application of algorithms according to > longest common text (or prefixes) in used symbols. This may (or may not) give you the right grouping, but you will still have to ensure that the elements end up in the right order, unless you want to provide all the values explicitly. > > > Furthermore, if this is targeting C code, the benefits will be limited, > > because C considers enums to be the same as ints. > > * Will such a transformation help with software debugging? Not likely, because the compiler provides no support and the definitions are typically far from the uses. julia > > * Would you like to reuse another named data type? > > Regards, > Markus >___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Searching only for header files with the Coccinelle software
On Sun, 28 Mar 2021, Markus Elfring wrote: > > Probably the simplest is to put the names of the header files in a file, > > eg find . -name "*h" > header_list, and then give the arguement > > --file-groups header_list to Coccinelle. > > How are the chances to add support for another program option like > “--find-headers”? In the last 15 years I have never wanted to do this. And we already have a huge number of command line options, so I'm not going to add this. julia > > Will any other file filters become also relevant here? > > Regards, > Markus >___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Replacing #define directives with the help of SmPL
On Sun, 28 Mar 2021, Markus Elfring wrote: > >> https://github.com/coccinelle/coccinelle/issues/139 > > > > I looked at the link, but there is no concrete example of something that > > does not work, so I have no idea what the problem is. > > You expressed another bit of better understanding of known limitations for > the Coccinelle software already, didn't you? > > Would you like to help any more with attempts to achieve support for > a transformation pattern like “#define ⇒ enum” according to the semantic > patch language? > https://gitlab.freedesktop.org/pipewire/pipewire/-/issues/981 > https://github.com/issues?q=is%3Aissue+author%3Aelfring+%22+enum%22 If you find that something does not work satisfactorily, propose a semantic patch and show what doesn't work. I'm not going to try to solve a problem when I don't know what the problem is. I think that this tranformation would be diffficult to make using Coccinelle, due to the need to ensure that the enum values are end up in the right order, if you want to rely on the implicit values of enums. Furthermore, if this is targeting C code, the benefits will be limited, because C considers enums to be the same as ints. julia___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Replacing #define directives with the help of SmPL
On Sun, 28 Mar 2021, Markus Elfring wrote: > Hello, > > Will the software development interests ever evolve in ways so that #define > directives > can be replaced with the help of the semantic patch language for special > source code > analysis and transformation approaches? > https://github.com/coccinelle/coccinelle/issues/139 I looked at the link, but there is no concrete example of something that does not work, so I have no idea what the problem is. julia ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Searching only for header files with the Coccinelle software
On Sun, 28 Mar 2021, Markus Elfring wrote: > Hello, > > The Coccinelle software can search for header files in addition to source > files > if the option “--include-headers” was specified. > https://github.com/coccinelle/coccinelle/blob/287374196da8c7cfd169e721a2d23f1e462422f1/docs/manual/spatch_options.tex#L43 > > How can it be achieved that only header files will be searched by this tool > for special source code analysis and transformation approaches? Probably the simplest is to put the names of the header files in a file, eg find . -name "*h" > header_list, and then give the arguement --file-groups header_list to Coccinelle. Another option is to use python in te semantic patch to detect the current file and then abort if it doesn't end in .h. But that would be much slower because the undesired files would still be parsed. julia___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH] coccinelle: misc: restrict patch mode in flexible_array.cocci
On Mon, 8 Mar 2021, Denis Efremov wrote:
> Skip patches generation for structs/unions with a single field.
> Changing a zero-length array to a flexible array member in a struct
> with no named members breaks the compilation. However, reporting
> such cases is still valuable, e.g. commit 637464c59e0b
> ("ACPI: NFIT: Fix flexible_array.cocci warnings").
>
> Signed-off-by: Denis Efremov
Applied. I changed the log message to remove "/unions", since the change
doesn't mention unions.
julia
> ---
> scripts/coccinelle/misc/flexible_array.cocci | 23 ++--
> 1 file changed, 21 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/coccinelle/misc/flexible_array.cocci
> b/scripts/coccinelle/misc/flexible_array.cocci
> index 947fbaff82a9..f427fd68ed2d 100644
> --- a/scripts/coccinelle/misc/flexible_array.cocci
> +++ b/scripts/coccinelle/misc/flexible_array.cocci
> @@ -51,21 +51,40 @@ position p : script:python() { relevant(p) };
>};
> )
>
> +@only_field depends on patch@
> +identifier name, array;
> +type T;
> +position q;
> +@@
> +
> +(
> + struct name {@q
> +T array[0];
> + };
> +|
> + struct {@q
> +T array[0];
> + };
> +)
> +
> @depends on patch@
> identifier name, array;
> type T;
> position p : script:python() { relevant(p) };
> +// position @q with rule "only_field" simplifies
> +// handling of bitfields, arrays, etc.
> +position q != only_field.q;
> @@
>
> (
> - struct name {
> + struct name {@q
> ...
> T array@p[
> - 0
> ];
>};
> |
> - struct {
> + struct {@q
> ...
> T array@p[
> - 0
> --
> 2.26.2
>
>
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH] scripts/coccinelle: Add script to detect sign extension
On Fri, 19 Mar 2021, Evan Benn wrote:
> Hello,
>
> I am attempting to create a coccinelle script that will detect possibly buggy
> usage of the bitwise operators where integer promotion may result in bugs,
> usually due to sign extension.
>
> I know this script needs a lot more work, but I am just beginning to learn the
> syntax of coccinelle. At this stage I am mainly looking for advice if this is
> even worth continuing, or if I am on the wrong track entirely.
I'm not really an expert in the problem, so I don't know exactly what are
the kinds of code you want to find. Coccinelle is good at matching the
types of things and the structure of things. If you need to know the
actual values of things, you may want to try smatch. Coccinelle probably
doesn't have complete knowledge of how various operators affect C types.
For example, it would not have known that BIT results in a long.
The best you can do is try some rules and see what the results are, and
try to collect some relevant examples and see if you can match them with
your rules. Please write back if there is some specific code that is not
matched as expected.
julia
>
> Here is an example of the bug I hope to find:
>
> https://lore.kernel.org/lkml/20210317013758.ga134...@roeck-us.net/
>
> Where ints and unsigned are mixed in bitwise operations, and the sizes differ.
>
> Thanks
>
> Evan Benn
>
> Signed-off-by: Evan Benn
> ---
>
> .../coccinelle/tests/int_sign_extend.cocci| 35 +++
> 1 file changed, 35 insertions(+)
> create mode 100644 scripts/coccinelle/tests/int_sign_extend.cocci
>
> diff --git a/scripts/coccinelle/tests/int_sign_extend.cocci
> b/scripts/coccinelle/tests/int_sign_extend.cocci
> new file mode 100644
> index ..bad61e37e4e7
> --- /dev/null
> +++ b/scripts/coccinelle/tests/int_sign_extend.cocci
> @@ -0,0 +1,35 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/// Mixing signed and unsigned types in bitwise operations risks problems
> when
> +/// the 'Usual arithmetic conversions' are applied.
> +/// For example:
> +/// https://lore.kernel.org/lkml/20210317013758.ga134...@roeck-us.net/
> +/// When a signed int and an unsigned int are compared there is no problem.
> +/// But if the unsigned is changed to a unsigned long, for example by using
> BIT
> +/// the signed value will be sign-extended and could result in incorrect
> logic.
> +// Confidence:
> +// Copyright: (C) 2021 Evan Benn
> +// Comments:
> +// Options:
> +
> +virtual context
> +virtual org
> +virtual report
> +
> +@r@
> +position p;
> +{int} s;
> +{unsigned long} u;
> +@@
> +s@p & u
> +
> +@script:python depends on org@
> +p << r.p;
> +@@
> +
> +cocci.print_main("sign extension when comparing bits of signed and unsigned
> values", p)
> +
> +@script:python depends on report@
> +p << r.p;
> +@@
> +
> +coccilib.report.print_report(p[0],"sign extension when comparing bits of
> signed and unsigned values")
> --
> 2.31.0.291.g576ba9dcdaf-goog
>
>
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Removing the last return statement from a void function
On Thu, 18 Mar 2021, Thomas Adam wrote:
> Hello all,
>
> I've another Coccinelle question I'm hoping you can help me with. The
> codebase I'm working on is old, and has some interesting styles which
> by themselves probably don't cause any problems, but newer C compilers
> are now starting to flag them.
>
> In particular, there seems to be a pattern in this code base of using
> explicit `return;` statements at the end of void functions. Here's an
> example:
>
> static void broadcast_mini_icon(FvwmWindow *fw)
> {
> if (!FMiniIconsSupported)
> {
> return;
> }
> if (fw->mini_pixmap_file && fw->mini_icon)
> {
> BroadcastFvwmPicture( M_MINI_ICON, FW_W(fw),
> FW_W_FRAME(fw), (unsigned long)fw,
> fw->mini_icon, fw->mini_pixmap_file);
> }
> return;
> }
>
> Here you can see the last return statement is not necessary.
>
> I'm trying to make coccinelle recognise this and remove such cases.
> Here's what I've tried:
>
> @@
> identifier f;
> @@
>
> void f(...) {
> <...
> - return;
> ...>
>
> }
>
> ... which sort of works, but proceeds to remove *all* `return;`
> statements from void functions, rather than the last occurance in the
> function.
>
> Am I on the right track with this approach, or do I need to do
> something more creative?
The ... in Coccinelle is based on control flow, so it is a bit hard to
find the return at the bottom of the function. Actually, from
Coccinelle's point of view, all returns are at the bottom of the function,
because one leaves the function after a return.
You can try the following:
@r@
position p;
identifier f;
}
f(...) {
<...
{ .. return@p; }
...>
}
@@
position p != r.p;
@@
- return@p;
Basically the first rule collects the position of all returns that are
inside a { }, and then the second rule removes the others.
However there is an isomorphism that makes a pattern with { ... S } match
just S, for any S, which you don't want. So you can make an empty file
called empty.iso, and then run the rule with the command-line argument
--iso-file empty.iso
julia
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Incorrect match with when != condition
On Wed, 17 Mar 2021, Denis Efremov wrote:
> Hi,
>
> I'm trying to write the check to detect the absence of commit
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit?id=51b2ee7d006a736a9126e8111d1f24e4fd0afaa6
> in kernel. The pattern can be:
>
> @err exists@
> identifier namlen, dchild, dparent, exp;
> position p;
> statement S;
> @@
>
> compose_entry_fh(..., int namlen, ...)
> {
> ...
> if (namlen == 2) {
> * dchild =@p dget_parent(dparent);
> ... when != dparent == exp->ex_path.dentry
add when forall here. There does exist a path that does not contain the
dparent == exp->ex_path.dentry test. That is the path that takesthe first
goto out.
julia
> } else S
> ...
> }
>
> But unfortunately, it matches even the fixed source. I think
> that condition "... when != dparent == exp->ex_path.dentry" doesn't
> work as expected. Steps to reproduce:
>
> $ cd linux # latest master branch
> $ wget
> https://raw.githubusercontent.com/evdenis/cvehound/b2d109c959c299dce10274c1806406fc5653e5d0/cvehound/cve/CVE-2021-3178.cocci
> $ spatch -D detect --cocci-file CVE-2021-3178.cocci fs/nfsd/nfs3xdr.c
> fs/nfsd/nfs3xdr.c:935:10-11: ERROR: CVE-2021-3178
> diff =
> --- fs/nfsd/nfs3xdr.c
> +++ /tmp/cocci-output-526900-b87df1-nfs3xdr.c
> @@ -932,7 +932,6 @@ compose_entry_fh(struct nfsd3_readdirres
> if (isdotent(name, namlen)) {
> if (namlen == 2) {
> // !!! shouldn't match because of if (dparent ==
> exp->ex_path.dentry) goto out; check after
> - dchild = dget_parent(dparent);
> /*
> * Don't return filehandle for ".." if we're at
> * the filesystem or export root:
> $ spatch --version
> spatch version 1.1.0 compiled with OCaml version 4.11.1
>
> Thanks,
> Denis
>
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Replacing a struct member with a function call
On Mon, 15 Mar 2021, Mansour Moufid wrote:
> On Sun, Mar 14, 2021, 20:43 Thomas Adam wrote:
> Hello,
>
> I can see I was as clear as mud with my explanation -- apologies
> for
> that, so let me try again.
>
> In my original example:
>
> struct monitor {
> struct {
> int width;
> int height
> } virtual;
> };
>
> ... the members width and height aren't required any more, as
> they're
> actually computable generically, and don't belong in that
> struct.
> Instead, I have separate functions which can provide those
> values.
>
> So where I have in code, statements such as:
>
> struct monitor *m = this_monitor();
> int foo = m->virutal.width;
>
> I want to be able to substitute "m->virtual.width" with a
> function
> call "get_width()" -- which does not involve "struct monitor" at
> all.
> Indeed, the semantic patch I'm trying to apply now looks like
> this:
>
> @@
> struct monitor *m;
> @@
>
> - m->virtual.width;
> + get_width();
>
> ... and although spatch doesn't tell me of any errors, when I
> run it
> over my codebase, no modifications are made. So clearly I'm
> still
> doing something wrong.
>
>
> Remove the semi-colons. ;)
Good catch :)
julia___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Replacing a struct member with a function call
On Mon, 15 Mar 2021, Thomas Adam wrote:
> Hello,
>
> I can see I was as clear as mud with my explanation -- apologies for
> that, so let me try again.
>
> In my original example:
>
> struct monitor {
> struct {
> int width;
> int height
> } virtual;
> };
>
> ... the members width and height aren't required any more, as they're
> actually computable generically, and don't belong in that struct.
> Instead, I have separate functions which can provide those values.
>
> So where I have in code, statements such as:
>
> struct monitor *m = this_monitor();
> int foo = m->virutal.width;
>
> I want to be able to substitute "m->virtual.width" with a function
> call "get_width()" -- which does not involve "struct monitor" at all.
> Indeed, the semantic patch I'm trying to apply now looks like this:
>
> @@
> struct monitor *m;
> @@
>
> - m->virtual.width;
> + get_width();
>
> ... and although spatch doesn't tell me of any errors, when I run it
> over my codebase, no modifications are made. So clearly I'm still
> doing something wrong. At the point, I have some questions:
>
> 1. Given the inner struct "virtual" inside of "struct monitor", is it
> correct that spatch would understand:
>
> m->virtual.width;
>
> ... or do I need to declare "virtual" as some expression or
> identifier? I did try:
>
> @@
> struct monitor *m;
> expression virtual;
> @@
>
> - m->virtual.width;
> + get_width();
>
> ... but this results in an error.
>
> 2. Do I need to declare "virtual" as a struct in its own right
> somehow? If so, it's not immediately obvious if this should be the
> case or how to do it.
>
> I hope I'm making some sense here -- apologies if not, and if I need
> to expand upon anything further, do please let me know.
>
> Essentially, it seems to me as though the inner struct "virtual" isn't
> being declared as a valid type which spatch is understanding, and this
> is why things aren't working how I'd like.
>
> Again, thanks ever so much for you time -- everyone's been very
> helpful to me in the past, and I've found coccinelle to be invaluable
> to making sweeping code changes, as well as bug-fixes on my codebase,
> so thanks to everyone involved in this project. It's invaluable!
spatch doens't care about virtual. Identifier is fine. If the code is
really as simple as you have presented it, ie with the type of m declared
immediately before its use, then there may be a problem with parsing the
code. Run spatch --parse-c on a file that you think should be changed and
see if BAD or bad come out in front of the code that should be
changed.nYOu can also run spatch --parse-c on the entire directory (may
require removing any limits on the stack size). That will conclude by
telling you the tokens it is most unhappy with. You can often get away
with defining a few dummy macros.
If none of that works, please send a small function that illustrates the
problem and the complete semantic patch.
julia
>
> Kindly,
> Thomas
>
> On Sun, 14 Mar 2021 at 09:16, Julia Lawall wrote:
> >
> >
> >
> > On Sun, 14 Mar 2021, Wolfram Sang wrote:
> >
> > >
> > > > @@
> > > > type M;
> > >
> > > This?
> > >
> > > struct monitor* m;
> > >
> > > > @@
> > > > - m->virtual.width;
> > > > + get_monitor_width();
> >
> > I guess that m should be somewhere in teh call to get_monitor_width too?
> >
> > julia
>
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Replacing a struct member with a function call
On Sun, 14 Mar 2021, Wolfram Sang wrote: > > > @@ > > type M; > > This? > > struct monitor* m; > > > @@ > > - m->virtual.width; > > + get_monitor_width(); I guess that m should be somewhere in teh call to get_monitor_width too? julia ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH v4] coccinelle: misc: add minmax script
> +@pmaxif depends on patch@
> +identifier func;
> +expression x, y;
> +expression max_val;
> +binary operator cmp = {>=, >};
> +@@
> +
> +func(...)
> +{
> + <...
> +-if ((x) cmp (y)) {
> +-max_val = (x);
> +-} else {
> +-max_val = (y);
> +-}
> ++max_val = max(x, y);
> + ...>
> +}
Things work better if there are no parentheses in max_val = (x) and
max_val = (y). Leaving them there seems to cause the match to work in two
ways, causing an already tagged token error. An example is in
crypto/jitterentropy.c
The same is true of the pminif rule. Only the patch rules are affected.
Double matches are allowed in the context cas, ince there is no real
transfotmation in that case.
julia
> +
> +@pmin depends on patch@
> +identifier func;
> +expression x, y;
> +binary operator cmp = {<=, <};
> +@@
> +
> +func(...)
> +{
> + <...
> +-((x) cmp (y) ? (x) : (y))
> ++min(x, y)
> + ...>
> +}
> +
> +@pminif depends on patch@
> +identifier func;
> +expression x, y;
> +expression min_val;
> +binary operator cmp = {<=, <};
> +@@
> +
> +func(...)
> +{
> + <...
> +-if ((x) cmp (y)) {
> +-min_val = (x);
> +-} else {
> +-min_val = (y);
> +-}
> ++min_val = min(x, y);
> + ...>
> +}
> +
> +@script:python depends on report@
> +p << rmax.p;
> +@@
> +
> +for p0 in p:
> + coccilib.report.print_report(p0, "WARNING opportunity for max()")
> +
> +@script:python depends on org@
> +p << rmax.p;
> +@@
> +
> +for p0 in p:
> + coccilib.org.print_todo(p0, "WARNING opportunity for max()")
> +
> +@script:python depends on report@
> +p << rmaxif.p;
> +@@
> +
> +for p0 in p:
> + coccilib.report.print_report(p0, "WARNING opportunity for max()")
> +
> +@script:python depends on org@
> +p << rmaxif.p;
> +@@
> +
> +for p0 in p:
> + coccilib.org.print_todo(p0, "WARNING opportunity for max()")
> +
> +@script:python depends on report@
> +p << rmin.p;
> +@@
> +
> +for p0 in p:
> + coccilib.report.print_report(p0, "WARNING opportunity for min()")
> +
> +@script:python depends on org@
> +p << rmin.p;
> +@@
> +
> +for p0 in p:
> + coccilib.org.print_todo(p0, "WARNING opportunity for min()")
> +
> +@script:python depends on report@
> +p << rminif.p;
> +@@
> +
> +for p0 in p:
> + coccilib.report.print_report(p0, "WARNING opportunity for min()")
> +
> +@script:python depends on org@
> +p << rminif.p;
> +@@
> +
> +for p0 in p:
> + coccilib.org.print_todo(p0, "WARNING opportunity for min()")
> --
> 2.26.2
>
>
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] linux-kernel janitorial RFP: Mark static arrays as const
On Sun, 7 Mar 2021, Joe Perches wrote:
> On Sun, 2021-03-07 at 20:14 +0100, Julia Lawall wrote:
> >
> > On Wed, 3 Mar 2021, Joe Perches wrote:
> >
> > > On Wed, 2021-03-03 at 10:41 +0100, Rasmus Villemoes wrote:
> > > > On 02/03/2021 18.42, Joe Perches wrote:
> > > > > Here is a possible opportunity to reduce data usage in the kernel.
> > > > >
> > > > > $ git grep -P -n
> > > > > '^static\s+(?!const|struct)(?:\w+\s+){1,3}\w+\s*\[\s*\]' drivers/ | \
> > > > > grep -v __initdata | \
> > > > > wc -l
> > > > > 3250
> > > > >
> > > > > Meaning there are ~3000 declarations of arrays with what appears to be
> > > > > file static const content that are not marked const.
> > > > >
> > > > > So there are many static arrays that could be marked const to move the
> > > > > compiled object code from data to text minimizing the total amount of
> > > > > exposed r/w data.
> > > >
> > > > You can add const if you like, but it will rarely change the generated
> > > > code. gcc is already smart enough to take a static array whose contents
> > > > are provably never modified within the TU and put it in .rodata:
> > >
> > > At least some or perhaps even most of the time, true, but the gcc compiler
> > > from v5 through at least v10 seems inconsistent about when it does the
> > > appropriate conversion.
> > >
> > > See the example I posted:
> > > https://lore.kernel.org/lkml/6b8b250a06a98ce42120a14824531a8641f5e8aa.ca...@perches.com/
> > >
> > > It was a randomly chosen source file conversion btw, I had no prior
> > > knowledge of whether the text/data use would change.
> > >
> > > I'm unsure about clang consistently moving static but provably const
> > > arrays
> > > from data to text. I rarely use clang. At least for v11 it seems to be
> > > better though. I didn't try 10.1.
> >
> > I tried the relevnt drivers in drivers/input/joystick. I got only one
> > driver that changed with gcc 9.3, which was
> > drivers/input/joystick/analog.c. It actually got larger:
> >
> > original:
> >
> > textdata bss dec hex filename
> > 22607 10560 320 3348782cf drivers/input/joystick/analog.o
> >
> > after adding const:
> >
> > textdata bss dec hex filename
> > 22728 10816 320 338648448 drivers/input/joystick/analog.o
> >
> > This was the only case where bss was not 0, but I don't know if there is a
> > connection.
>
> You really need consider using defconfig so whatever object code
> does not have tracing/debugging support.
>
> For instance, this code with defconfig and analog joystick:
>
> Original:
>
> $ size drivers/input/joystick/analog.o
>text data bss dec hex filename
>8115 261 22486002198 drivers/input/joystick/analog.o
>
> with const:
>
> $ size drivers/input/joystick/analog.o
>text data bss dec hex filename
>8179 201 2248604219c drivers/input/joystick/analog.o
Thanks for the suggestion. It occurred to me that in one case my
transformation was wrong, because the array was multi-level, and a sub
array was being stored in a structure field that was not declared as
const. So the argument that the compiler would figure out to put the
array in .rodata didn't make sense. But I still go the same sizes for
that file. So I'll try the whole thing again.
thanks,
julia___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] linux-kernel janitorial RFP: Mark static arrays as const
On Wed, 3 Mar 2021, Joe Perches wrote:
> On Wed, 2021-03-03 at 10:41 +0100, Rasmus Villemoes wrote:
> > On 02/03/2021 18.42, Joe Perches wrote:
> > > Here is a possible opportunity to reduce data usage in the kernel.
> > >
> > > $ git grep -P -n '^static\s+(?!const|struct)(?:\w+\s+){1,3}\w+\s*\[\s*\]'
> > > drivers/ | \
> > > grep -v __initdata | \
> > > wc -l
> > > 3250
> > >
> > > Meaning there are ~3000 declarations of arrays with what appears to be
> > > file static const content that are not marked const.
> > >
> > > So there are many static arrays that could be marked const to move the
> > > compiled object code from data to text minimizing the total amount of
> > > exposed r/w data.
> >
> > You can add const if you like, but it will rarely change the generated
> > code. gcc is already smart enough to take a static array whose contents
> > are provably never modified within the TU and put it in .rodata:
>
> At least some or perhaps even most of the time, true, but the gcc compiler
> from v5 through at least v10 seems inconsistent about when it does the
> appropriate conversion.
>
> See the example I posted:
> https://lore.kernel.org/lkml/6b8b250a06a98ce42120a14824531a8641f5e8aa.ca...@perches.com/
>
> It was a randomly chosen source file conversion btw, I had no prior
> knowledge of whether the text/data use would change.
>
> I'm unsure about clang consistently moving static but provably const arrays
> from data to text. I rarely use clang. At least for v11 it seems to be
> better though. I didn't try 10.1.
I tried the relevnt drivers in drivers/input/joystick. I got only one
driver that changed with gcc 9.3, which was
drivers/input/joystick/analog.c. It actually got larger:
original:
textdata bss dec hex filename
22607 10560 320 3348782cf drivers/input/joystick/analog.o
after adding const:
textdata bss dec hex filename
22728 10816 320 338648448 drivers/input/joystick/analog.o
This was the only case where bss was not 0, but I don't know if there is a
connection.
julia___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH v2 RESEND] coccinelle: misc: add minmax script
On Fri, 19 Feb 2021, Denis Efremov wrote:
> Check for opencoded min(), max() implementations.
>
> Signed-off-by: Denis Efremov
> ---
>
> Changes in v2:
> - <... ...> instead of ... when any
> - org mode reports fixed
> - patch rule to drop excessive ()
>
> scripts/coccinelle/misc/minmax.cocci | 224 +++
> 1 file changed, 224 insertions(+)
> create mode 100644 scripts/coccinelle/misc/minmax.cocci
>
> diff --git a/scripts/coccinelle/misc/minmax.cocci
> b/scripts/coccinelle/misc/minmax.cocci
> new file mode 100644
> index ..61d6b61fd82c
> --- /dev/null
> +++ b/scripts/coccinelle/misc/minmax.cocci
> @@ -0,0 +1,224 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +///
> +/// Check for opencoded min(), max() implementations.
> +/// Generated patches sometimes require adding a cast to fix compile warning.
> +/// Warnings/patches scope intentionally limited to a function body.
> +///
> +// Confidence: Medium
> +// Copyright: (C) 2021 Denis Efremov ISPRAS
> +// Options: --no-includes --include-headers
> +//
> +// Keywords: min, max
> +//
> +
> +
> +virtual report
> +virtual org
> +virtual context
> +virtual patch
> +
> +@rmax depends on !patch@
> +identifier func;
> +expression x, y;
> +binary operator cmp = {>, >=};
> +position p;
> +@@
> +
> +func(...)
> +{
> + <...
> +*x cmp@p y ? x : y
The rule below indicated with FIXME is supposed to deal with the
possibility of () that are unnecessary when using min and max. It doesn't
work, because <... P ...> allow P to match 0 or more times, and thus
func@p matches every function.
A simpler solution is to just allow arbitrary () in the pattern, eg:
(x) cmp@p (y) ? (x) : (y)
That will allow each occurrence of x and y to occur with and without
parentheses. In the submitted semantic patch, the () issue was only
considered in the patch case. But it actually affects the purely matching
cases too, because () can be used at one occurrence, but not the other.
> +@script:python depends on report@
> +p << rmax.p;
> +@@
> +
> +coccilib.report.print_report(p[0], "WARNING opportunity for max()")
p is an array because it can be bound to different positions on different
control-flow paths. Notably this occurs with <... ...>. If there are
multiple occurrences of the pattern, there will be one match that contains
all of them. Thus the reporting code should be:
for p0 in p:
coccilib.report.print_report(p0, "WARNING opportunity for max()")
julia
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] linux-kernel janitorial RFP: Mark static arrays as const
On Wed, 3 Mar 2021, Mansour Moufid wrote:
> On Tue, Mar 2, 2021 at 4:22 PM Joe Perches wrote:
> >
> > Here is a possible opportunity to reduce data usage in the kernel.
> >
> > $ git grep -P -n '^static\s+(?!const|struct)(?:\w+\s+){1,3}\w+\s*\[\s*\]'
> > drivers/ | \
> > grep -v __initdata | \
> > wc -l
> > 3250
> >
> > Meaning there are ~3000 declarations of arrays with what appears to be
> > file static const content that are not marked const.
> >
> > So there are many static arrays that could be marked const to move the
> > compiled object code from data to text minimizing the total amount of
> > exposed r/w data.
> >
> > However, I do not know of a mechanism using coccinelle to determine
> > whether or not any of these static declarations are ever modified.
>
> I thought it would be a fun exercise but it got tedious quick.
>
> I don't know how to ignore an attribute like __initdata.
>
> Feel free to refine it:
This adds the consts, but it doesn't check that the array is never
updated, or never stored in a field from which it could get updated.
In my attempt, I tried something like this for the first part:
@r disable optional_qualifier@
type T;
identifier x;
position p != ok.p;
@@
static T x@p[] = { ... };
In principle, this should match cases where there is no const. But it
dones't work. It matches some cases with const too.
Then I tried:
@ok@
type T;
identifier x;
position p;
@@
static const T x@p[] = { ... };
@r@
type T;
identifier x;
position p != ok.p;
@@
static T x@p[] = { ... };
The first rule matches the cases with const, and then the second rule
matches all of the cases with the declared variable at a position
different than that of the first rule. It works if the type T is a simple
type like int, but it doesn't work if it is something more complex, like
struct foo *.
Afterwards, I have the following:
@bad@
position p;
identifier r.x;
expression e,y;
@@
(
x@p[y] = e
|
@p[y]
)
@good@
identifier r.x;
expression y;
position p != bad.p;
@@
x@p[y]
@notgood@
identifier r.x;
position p != good.p;
@@
x@p
@depends on good && !notgood@
identifier r.x;
type r.T;
@@
static
+const
T x[] = { ... };
That is,
* rule bad: Give up if we assign an element or take the address of an
element.
* rule good: Things are going well if we access an element of the array.
If there is no access at all, there is something suspicious, perhaps uses
in macros.
* rule notgood: A montion of the array that is not an element access is
not good either
In the end, if we match good and we don't match notgood, then we can make
the change.
I got stuck on the const problem, and didn't try the __initdata issue.
But one could address with a rule like the rule ok above.
The const problem is at least something worth looking into.
julia
>
>
> @@
> type t;
> identifier x;
> @@
> (
> static const struct { ... } x[];
> |
> static
> + const
> struct { ... } x[];
> |
> static const struct s *x[];
> |
> static
> + const
> struct s *x[];
> |
> static const struct s x[];
> |
> static
> + const
> struct s x[];
> |
> static const t *x[];
> |
> static
> + const
> t *x[];
> |
> static const t x[];
> |
> static
> + const
> t x[];
> )
>
> @@
> type t;
> identifier s, x, y, z;
> assignment operator xx;
> @@
> (
> static const struct { ... } x[] = { ... };
> |
> static
> + const
> struct { ... } x[] = { ... };
> |
> static const struct s *x[] = { ... };
> |
> static
> + const
> struct s *x[] = { ... };
> |
> static const struct s x[] = { ... };
> |
> static
> + const
> struct s x[] = { ... };
> |
> static const t *x[] = { ... };
> |
> static
> + const
> t *x[] = { ... };
> |
> static const t x[] = { ... };
> |
> static
> + const
> t x[] = { ... };
> )
> ... when != x.y xx ...
> when != x[...] xx ...
> when != z = x
> ___
> Cocci mailing list
> Cocci@systeme.lip6.fr
> https://systeme.lip6.fr/mailman/listinfo/cocci
>
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH v2] coccinelle: misc: add swap script
On Fri, 19 Feb 2021, Denis Efremov wrote:
> Check for opencoded swap() implementation.
>
> Signed-off-by: Denis Efremov
> ---
> Changes in v2:
> - additional patch rule to drop excessive {}
> - fix indentation in patch mode by anchoring ;
>
> scripts/coccinelle/misc/swap.cocci | 101 +
> 1 file changed, 101 insertions(+)
> create mode 100644 scripts/coccinelle/misc/swap.cocci
>
> diff --git a/scripts/coccinelle/misc/swap.cocci
> b/scripts/coccinelle/misc/swap.cocci
> new file mode 100644
> index ..d5da9888c222
> --- /dev/null
> +++ b/scripts/coccinelle/misc/swap.cocci
> @@ -0,0 +1,101 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +///
> +/// Check for opencoded swap() implementation.
> +///
> +// Confidence: High
> +// Copyright: (C) 2021 Denis Efremov ISPRAS
> +// Options: --no-includes --include-headers
> +//
> +// Keywords: swap
> +//
> +
> +virtual patch
> +virtual org
> +virtual report
> +virtual context
> +
> +@r depends on !patch@
> +identifier tmp;
> +expression a, b;
> +type T;
> +position p;
> +@@
> +
> +(
> +* T tmp;
> +|
> +* T tmp = 0;
> +|
> +* T *tmp = NULL;
> +)
> +... when != tmp
> +* tmp = a;
> +* a = b;@p
> +* b = tmp;
> +... when != tmp
> +
> +@rpvar depends on patch@
> +identifier tmp;
> +expression a, b;
> +type T;
> +@@
> +
> +(
> +- T tmp;
> +|
> +- T tmp = 0;
> +|
> +- T *tmp = NULL;
> +)
> +... when != tmp
> +- tmp = a;
> +- a = b;
> +- b = tmp
> ++ swap(a, b)
> + ;
> +... when != tmp
> +
> +
> +@rp depends on patch@
> +identifier tmp;
> +expression a, b;
> +@@
> +
> +- tmp = a;
> +- a = b;
> +- b = tmp
> ++ swap(a, b)
> + ;
A rule like the above should also appear in the non-patch case.
> +
> +@depends on (rpvar || rp)@
This needs to be depends on patch && (rpvar || rp). It doesn't make much
sense, because rpvar and rp both depend on patch, but at the moment that
information isn't propagate everywhere that it is needed.
thanks,
julia
> +@@
> +
> +(
> + for (...;...;...)
> +- {
> + swap(...);
> +- }
> +|
> + while (...)
> +- {
> + swap(...);
> +- }
> +|
> + if (...)
> +- {
> + swap(...);
> +- }
> +)
> +
> +
> +@script:python depends on report@
> +p << r.p;
> +@@
> +
> +coccilib.report.print_report(p[0], "WARNING opportunity for swap()")
> +
> +@script:python depends on org@
> +p << r.p;
> +@@
> +
> +coccilib.org.print_todo(p[0], "WARNING opportunity for swap()")
> --
> 2.26.2
>
>
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] linux-kernel janitorial RFP: Mark static arrays as const
On Tue, 2 Mar 2021, Bernd Petrovitsch wrote:
> Hi all!
>
> On 02/03/2021 18:42, Joe Perches wrote:
> [...]
> > - For instance: (head -10 of the git grep for file statics)
> >
> > drivers/accessibility/speakup/keyhelp.c:18:static u_short masks[] = { 32,
> > 16, 8, 4, 2, 1 };
> > drivers/accessibility/speakup/keyhelp.c:26:static u_char funcvals[] = {
> > drivers/accessibility/speakup/main.c:2059:static spkup_hand spkup_handler[]
> > = {
> > drivers/accessibility/speakup/speakup_acntpc.c:35:static unsigned int
> > synth_portlist[] = { 0x2a8, 0 };
> > drivers/accessibility/speakup/speakup_decpc.c:133:static int
> > synth_portlist[] = { 0x340, 0x350, 0x240, 0x250, 0 };
> > drivers/accessibility/speakup/speakup_dectlk.c:110:static int ap_defaults[]
> > = {122, 89, 155, 110, 208, 240, 200, 106, 306};
> > drivers/accessibility/speakup/speakup_dectlk.c:111:static int g5_defaults[]
> > = {86, 81, 86, 84, 81, 80, 83, 83, 73};
> > drivers/accessibility/speakup/speakup_dtlk.c:34:static unsigned int
> > synth_portlist[] = {
> > drivers/accessibility/speakup/speakup_keypc.c:34:static unsigned int
> > synth_portlist[] = { 0x2a8, 0 };
> > drivers/acpi/ac.c:137:static enum power_supply_property ac_props[] = {
> >
> > For drivers/accessibility/speakup/keyhelp.c:18:static u_short masks[] = {
> > 32, 16, 8, 4, 2, 1 };
>
> Looking at the examples: Just s/^static /static const / in the lines
> reported by the grep's above and see if it compiles (and save space)?
There is a small risk with compiling that there may be a problem for a
configuration you haven't considered.
julia
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] linux-kernel janitorial RFP: Mark static arrays as const
On Tue, 2 Mar 2021, Joe Perches wrote:
> Here is a possible opportunity to reduce data usage in the kernel.
Does it actually reduce data usage?
julia
>
> $ git grep -P -n '^static\s+(?!const|struct)(?:\w+\s+){1,3}\w+\s*\[\s*\]'
> drivers/ | \
> grep -v __initdata | \
> wc -l
> 3250
>
> Meaning there are ~3000 declarations of arrays with what appears to be
> file static const content that are not marked const.
>
> So there are many static arrays that could be marked const to move the
> compiled object code from data to text minimizing the total amount of
> exposed r/w data.
>
> However, I do not know of a mechanism using coccinelle to determine
> whether or not any of these static declarations are ever modified.
>
> So it appears that each instance of these declarations might need
> manual inspection.
>
> But for arrays declared inside functions, it's much more likely that
> the static declaration without const is done with the intent to modify
> the array:
>
> (note the difference in the git grep with a leading '^\s+')
>
> $ git grep -Pn '^\s+static\s+(?!const|struct)(?:\w+\s+){1,3}\w+\s*\[\s*\]'
> drivers/ | \
> grep -v __initdata | \
> wc -l
> 323
>
> - For instance: (head -10 of the git grep for file statics)
>
> drivers/accessibility/speakup/keyhelp.c:18:static u_short masks[] = { 32, 16,
> 8, 4, 2, 1 };
> drivers/accessibility/speakup/keyhelp.c:26:static u_char funcvals[] = {
> drivers/accessibility/speakup/main.c:2059:static spkup_hand spkup_handler[] =
> {
> drivers/accessibility/speakup/speakup_acntpc.c:35:static unsigned int
> synth_portlist[] = { 0x2a8, 0 };
> drivers/accessibility/speakup/speakup_decpc.c:133:static int synth_portlist[]
> = { 0x340, 0x350, 0x240, 0x250, 0 };
> drivers/accessibility/speakup/speakup_dectlk.c:110:static int ap_defaults[] =
> {122, 89, 155, 110, 208, 240, 200, 106, 306};
> drivers/accessibility/speakup/speakup_dectlk.c:111:static int g5_defaults[] =
> {86, 81, 86, 84, 81, 80, 83, 83, 73};
> drivers/accessibility/speakup/speakup_dtlk.c:34:static unsigned int
> synth_portlist[] = {
> drivers/accessibility/speakup/speakup_keypc.c:34:static unsigned int
> synth_portlist[] = { 0x2a8, 0 };
> drivers/acpi/ac.c:137:static enum power_supply_property ac_props[] = {
>
> For drivers/accessibility/speakup/keyhelp.c:18:static u_short masks[] = { 32,
> 16, 8, 4, 2, 1 };
>
> masks is only used in static function say_key and should be const and
> perhaps the declaration might be better moved into that function.
>
> For drivers/accessibility/speakup/keyhelp.c:26:static u_char funcvals[] = {
>
> funcvals is only used in static function spk_handle_help and should be const
> and perhaps the declaration might be better moved into that function.
>
> For drivers/accessibility/speakup/main.c:2059:static spkup_hand
> spkup_handler[] = {
>
> spkup_handler is only used in static function do_spkup and should be const
> and perhaps the declaration might be better moved into that function.
>
> etc... for speakup
>
> For drivers/acpi/ac.c:137:static enum power_supply_property ac_props[] = {
>
> array ac_props is assigned as a reference in acpi_ac_add as a
> "const enum power_supply_property *" member of a struct power_supply_desc.
>
> - For instance: (head -10 of the git grep for function statics)
>
> drivers/acpi/apei/apei-base.c:781:static u8 whea_uuid_str[] =
> "ed855e0c-6c90-47bf-a62a-26de0fc5ad5c";
> drivers/block/amiflop.c:1051: static unsigned char CRCTable1[] = {
> drivers/block/amiflop.c:1070: static unsigned char CRCTable2[] = {
> drivers/block/drbd/drbd_nl.c:872: static char units[] = { 'K', 'M', 'G',
> 'T', 'P', 'E' };
> drivers/block/drbd/drbd_proc.c:224: static char write_ordering_chars[] = {
> drivers/block/drbd/drbd_receiver.c:4363: static enum drbd_conns c_tab[]
> = {
> drivers/char/pcmcia/synclink_cs.c:3717: static unsigned char patterns[]
> =
> drivers/cpufreq/intel_pstate.c:1515: static int silvermont_freq_table[] = {
> drivers/cpufreq/intel_pstate.c:1530: static int airmont_freq_table[] = {
> drivers/dma/xgene-dma.c:360: static u8 flyby_type[] = {
>
> Some of these could be const, but some could not. For instance:
>
> For drivers/acpi/apei/apei-base.c:781:static u8 whea_uuid_str[] =
> "ed855e0c-6c90-47bf-a62a-26de0fc5ad5c";
>
> whea_uuid_str is assigned as a reference in "int apei_osc_setup(void)"
> a struct acpi_osc_context where .uuid_str is not declared as const char *.
>
>
>
>
> ___
> Cocci mailing list
> Cocci@systeme.lip6.fr
> https://systeme.lip6.fr/mailman/listinfo/cocci
>
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] A few build failures with OCaml 4.12.0
This problem is fixed now in the github version. Thanks again Richard and Markus for the report. julia On Tue, 2 Mar 2021, Richard W.M. Jones wrote: > ocamlfind ocamlopt -c -package result -package seq -bin-annot -no-alias-deps > -I . -alert -deprecated stdcompat__arg_s.mli -o stdcompat__arg_s.cmi > File "stdcompat__arg_s.mli", lines 3-17, characters 0-38: > 3 | type spec = Arg.spec = > 4 | | Unit of (unit -> unit) > 5 | | Bool of (bool -> unit) > 6 | | Set of bool ref > 7 | | Clear of bool ref > ... > 14 | | Tuple of spec list > 15 | | Symbol of string list * (string -> unit) > 16 | | Rest of (string -> unit) > 17 | | Expand of (string -> string array). > Error: This variant or record definition does not match that of type Arg.spec >Constructors number 14 have different names, Rest_all and Expand. > > There is a new Rest_all constructor: > > https://github.com/ocaml/ocaml/blob/500d8dc8296d09305b5413f140c63ffee1de111d/stdlib/arg.mli#L92 > > -- > > ocamlfind ocamlopt -c -package result -package seq -bin-annot -no-alias-deps > -I . -alert -deprecated stdcompat__spacetime_s.mli -o > stdcompat__spacetime_s.cmi > File "stdcompat__spacetime_s.mli", line 3, characters 16-32: > 3 | module Series = Spacetime.Series > > Error: Unbound module Spacetime > > This module was removed in OCaml commit > 540996d21ee3793a1cecce252c81fb76a6b9fd61. > > -- > > ocamlfind ocamlc -c -package result -package seq -bin-annot -no-alias-deps -I > . -alert -deprecated stdcompat__ephemeron.ml -o stdcompat__ephemeron.cmo > File "stdcompat__ephemeron.ml", line 1: > Error: The implementation stdcompat__ephemeron.ml >does not match the interface stdcompat__ephemeron.cmi: >... >At position module type S = >Type declarations do not match: > type 'a t >is not included in > type !'a t >Their variances do not agree. >File "hashtbl.mli", line 335, characters 4-14: Expected declaration >File "stdcompat__ephemeron_s.mli", line 6, characters 4-13: > Actual declaration > > Not sure about this one but AFAICT cocci doesn't use this module. > > -- > > I made a patch to workaround the issues in Fedora, but it's a pure hack: > > https://src.fedoraproject.org/rpms/coccinelle/tree/rawhide > > Rich. > > -- > Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones > Read my programming and virtualization blog: http://rwmj.wordpress.com > libguestfs lets you edit virtual machines. Supports shell scripting, > bindings from many languages. http://libguestfs.org > > ___ > Cocci mailing list > Cocci@systeme.lip6.fr > https://systeme.lip6.fr/mailman/listinfo/cocci > ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] A few build failures with OCaml 4.12.0
On Tue, 2 Mar 2021, Richard W.M. Jones wrote: > ocamlfind ocamlopt -c -package result -package seq -bin-annot -no-alias-deps > -I . -alert -deprecated stdcompat__arg_s.mli -o stdcompat__arg_s.cmi > File "stdcompat__arg_s.mli", lines 3-17, characters 0-38: > 3 | type spec = Arg.spec = > 4 | | Unit of (unit -> unit) > 5 | | Bool of (bool -> unit) > 6 | | Set of bool ref > 7 | | Clear of bool ref > ... > 14 | | Tuple of spec list > 15 | | Symbol of string list * (string -> unit) > 16 | | Rest of (string -> unit) > 17 | | Expand of (string -> string array). > Error: This variant or record definition does not match that of type Arg.spec >Constructors number 14 have different names, Rest_all and Expand. > > There is a new Rest_all constructor: > > https://github.com/ocaml/ocaml/blob/500d8dc8296d09305b5413f140c63ffee1de111d/stdlib/arg.mli#L92 > > -- > > ocamlfind ocamlopt -c -package result -package seq -bin-annot -no-alias-deps > -I . -alert -deprecated stdcompat__spacetime_s.mli -o > stdcompat__spacetime_s.cmi > File "stdcompat__spacetime_s.mli", line 3, characters 16-32: > 3 | module Series = Spacetime.Series > > Error: Unbound module Spacetime > > This module was removed in OCaml commit > 540996d21ee3793a1cecce252c81fb76a6b9fd61. > > -- > > ocamlfind ocamlc -c -package result -package seq -bin-annot -no-alias-deps -I > . -alert -deprecated stdcompat__ephemeron.ml -o stdcompat__ephemeron.cmo > File "stdcompat__ephemeron.ml", line 1: > Error: The implementation stdcompat__ephemeron.ml >does not match the interface stdcompat__ephemeron.cmi: >... >At position module type S = >Type declarations do not match: > type 'a t >is not included in > type !'a t >Their variances do not agree. >File "hashtbl.mli", line 335, characters 4-14: Expected declaration >File "stdcompat__ephemeron_s.mli", line 6, characters 4-13: > Actual declaration > > Not sure about this one but AFAICT cocci doesn't use this module. > > -- > > I made a patch to workaround the issues in Fedora, but it's a pure hack: Thanks for the feedback and the fix attempt. I hope that this can be fixed on our side shortly. julia > > https://src.fedoraproject.org/rpms/coccinelle/tree/rawhide > > Rich. > > -- > Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones > Read my programming and virtualization blog: http://rwmj.wordpress.com > libguestfs lets you edit virtual machines. Supports shell scripting, > bindings from many languages. http://libguestfs.org > > ___ > Cocci mailing list > Cocci@systeme.lip6.fr > https://systeme.lip6.fr/mailman/listinfo/cocci > ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] release of version 1.1.0
On Sat, 27 Feb 2021, Denis Efremov wrote: > > > On 2/27/21 12:14 PM, Julia Lawall wrote: > > > > > > On Sat, 27 Feb 2021, Denis Efremov wrote: > > > >> Hi, > >> > >> Great news! > >> > >> Could you please create release tag on GitHub? > >> This will allow me to add this version to tests and add new opam package > >> version. > >> Currently, I use these versions for tests: > >> https://github.com/evdenis/cvehound/blob/master/.github/workflows/test.yml#L25 > > > > There is no tag at the moment. I have asked why. > > > >> > >> BTW, is there 1.0.9 version? I see that fedora provides 1.0.9 version, > >> but there is not release tag for it and there is no opam package for it. > > > > No, there is no version 1.0.9. We moved on to 1.1 due to the #spatch > > feature. > > > > Seems like there is a mess. There is a commit with 1.0.9 release planned: > https://github.com/coccinelle/coccinelle/commit/842075f77505386a782030ebffb1acf7f4237737#diff-652d5876e7e73c420820682a12ecdec66ecdec029223e24808e129d64493a967 > > Repology says that Fedora provides most recent coccinelle version: > https://repology.org/project/coccinelle/versions > Other distibutives use 1.0.8 version. > > On Fedora: > $ sudo dnf info coccinelle > Installed Packages > Name : coccinelle > Version : 1.0.9 > Release : 0.14.gitd0fd4c7d.fc33.1 // <== d0fd4c7d commit reference > Architecture : x86_64 > Size : 26 M > Source : coccinelle-1.0.9-0.14.gitd0fd4c7d.fc33.1.src.rpm > > $ /usr/bin/spatch --version > spatch version 1.0.8-gc1dbb4f-dirty compiled with OCaml version 4.11.1 > Flags passed to the configure script: --build=x86_64-redhat-linux-gnu > --host=x86_64-redhat-linux-gnu --program-prefix= > --disable-dependency-tracking --prefix=/usr --exec-prefix=/usr > --bindir=/usr/bin --sbindir=/usr/sbin --sysconfdir=/etc --datadir=/usr/share > --includedir=/usr/include --libdir=/usr/lib64 --libexecdir=/usr/libexec > --localstatedir=/var --sharedstatedir=/var/lib --mandir=/usr/share/man > --infodir=/usr/share/info --with-python=/usr/bin/python3 > --with-menhir=/usr/bin/menhir > OCaml scripting support: yes > Python scripting support: yes > Syntax of regular expressions: PCRE > > Fedora 33, 1.0.9 version points to commit: > $ fedpkg clone -a coccinelle > $ cd coccinelle > $ git checkout f33 > $ cat coccinelle.spec | grep commit > %global commit d0fd4c7dfa70763870914eedee7022fa35f700e2 > %global shortcommit d0fd4c7d > > Fedora 34 also uses d0fd4c7d as base for 1.0.9 version. > Fedora 32 points to 111d328fee1303f14a5b9def835301d849e41331. > > Maybe release 1.0.9 wasn't planned, but as my personal opinion adding 1.0.9 > tag is not a bad idea > just to reference some commit (for example, d0fd4c7d) and have 1.0.9 version > to run > backward compatibility tests on it. > https://github.com/coccinelle/coccinelle/releases I donm't think any 1.0.9 was planned. It was probably an experiement with the release system gone wrong. But if someone has picked up on it, we may as well put a tag for it too. Thanks for the feedback. julia > > Thanks, > Denis > > >> > >> On 2/27/21 11:11 AM, Julia Lawall wrote: > >>> A new version 1.1.0 has been released. The most interesting change is the > >>> ability to put spatch command line options in a .cocci file, for example: > >>> > >>> #spatch --very-quiet -j 8 --recursive-includes --include-headers-for-types > >>> > >>> Other specific improvements are an improved handling of attributes and a > >>> more efficient treatment of header files (all thanks to Jaskaran Singh). > >>> > >>> Various other small issues have been addressed. > >>> > >>> All of these changes have already been available on github. > >>> > >>> julia > >>> ___ > >>> Cocci mailing list > >>> Cocci@systeme.lip6.fr > >>> https://systeme.lip6.fr/mailman/listinfo/cocci > >>> > >> ___ > >> Cocci mailing list > >> Cocci@systeme.lip6.fr > >> https://systeme.lip6.fr/mailman/listinfo/cocci > >> > ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] release of version 1.1.0
On Sat, 27 Feb 2021, Denis Efremov wrote: > Hi, > > Great news! > > Could you please create release tag on GitHub? > This will allow me to add this version to tests and add new opam package > version. > Currently, I use these versions for tests: > https://github.com/evdenis/cvehound/blob/master/.github/workflows/test.yml#L25 There is no tag at the moment. I have asked why. > > BTW, is there 1.0.9 version? I see that fedora provides 1.0.9 version, > but there is not release tag for it and there is no opam package for it. No, there is no version 1.0.9. We moved on to 1.1 due to the #spatch feature. julia > Thanks, > Denis > > On 2/27/21 11:11 AM, Julia Lawall wrote: > > A new version 1.1.0 has been released. The most interesting change is the > > ability to put spatch command line options in a .cocci file, for example: > > > > #spatch --very-quiet -j 8 --recursive-includes --include-headers-for-types > > > > Other specific improvements are an improved handling of attributes and a > > more efficient treatment of header files (all thanks to Jaskaran Singh). > > > > Various other small issues have been addressed. > > > > All of these changes have already been available on github. > > > > julia > > ___ > > Cocci mailing list > > Cocci@systeme.lip6.fr > > https://systeme.lip6.fr/mailman/listinfo/cocci > > > ___ > Cocci mailing list > Cocci@systeme.lip6.fr > https://systeme.lip6.fr/mailman/listinfo/cocci > ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] release of version 1.1.0
A new version 1.1.0 has been released. The most interesting change is the ability to put spatch command line options in a .cocci file, for example: #spatch --very-quiet -j 8 --recursive-includes --include-headers-for-types Other specific improvements are an improved handling of attributes and a more efficient treatment of header files (all thanks to Jaskaran Singh). Various other small issues have been addressed. All of these changes have already been available on github. julia ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH] coccinelle: misc: add swap script
On Thu, 18 Feb 2021, Markus Elfring wrote: > > A disjunction basically says "at this node in the cfg, can I match the > > first patter, or can I match the second pattern, etc." Unfortunately in > > this case the two branches start matching at different nodes, so the short > > circuit aspect of a disjunction isn't used, and it matches both patterns. > > > > The solution is to just make two rules. The first for the if case and the > > second for everything else. > > Will such feedback trigger further software development considerations? No. This is never going to change, until someone completely redesigns Coccinelle. julia ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH] coccinelle: misc: add swap script
On Thu, 18 Feb 2021, Denis Efremov wrote:
>
>
> On 2/18/21 1:17 PM, Julia Lawall wrote:
> >
> >
> > On Thu, 18 Feb 2021, Denis Efremov wrote:
> >
> >>
> >>
> >> On 2/18/21 12:31 AM, Julia Lawall wrote:
> >>>> +@depends on patch@
> >>>> +identifier tmp;
> >>>> +expression a, b;
> >>>> +type T;
> >>>> +@@
> >>>> +
> >>>> +(
> >>>> +- T tmp;
> >>>> +|
> >>>> +- T tmp = 0;
> >>>> +|
> >>>> +- T *tmp = NULL;
> >>>> +)
> >>>> +... when != tmp
> >>>> +- tmp = a;
> >>>> +- a = b;
> >>>> +- b = tmp;
> >>>> ++ swap(a, b);
> >>>> +... when != tmp
> >>>
> >>> In this rule and the next one, if you remove the final ; from the b = tmp
> >>> line and from the swap line, and put it into context code afterwards, them
> >>> the generated code looks better on cases like fs/xfs/xfs_inode.c in the
> >>> function xfs_lock_two_inodes where two successive swap calls are
> >>> generated.
> >>>
> >>> There are also some cases such as drivers/net/wireless/ath/ath5k/phy.c in
> >>> the function ath5k_hw_get_median_noise_floor where the swap code makes up
> >>> a whole if branch.
> >>
> >>> In this cases it would be good to remove the {}.
> >>
> >> How this can be handled?
> >>
> >> If I use this pattern:
> >> @depends on patch@
> >> identifier tmp;
> >> expression a, b;
> >> @@
> >>
> >> (
> >> if (...)
> >> - {
> >> - tmp = a;
> >> - a = b;
> >> - b = tmp
> >> + swap(a, b)
> >> ;
> >> - }
> >> |
> >> - tmp = a;
> >> - a = b;
> >> - b = tmp
> >> + swap(a, b)
> >> ;
> >> )
> >>
> >> The tool fails with error:
> >>
> >> EXN: Failure("rule starting on line 58: already tagged token:\nC code
> >> context\nFile \"drivers/net/wireless/ath/ath5k/phy.c\", line 1574,
> >> column 4, charpos = 41650\n around = 'sort',\n whole content =
> >> \t\t\t\tsort[j - 1] = tmp;") in drivers/net/wireless/ath/ath5k/phy.c
> >
> > A disjunction basically says "at this node in the cfg, can I match the
> > first patter, or can I match the second pattern, etc." Unfortunately in
> > this case the two branches start matching at different nodes, so the short
> > circuit aspect of a disjunction isn't used, and it matches both patterns.
> >
> > The solution is to just make two rules. The first for the if case and the
> > second for everything else.
> >
>
> if (...)
> - {
> - tmp = a;
> - a = b;
> - b = tmp
> + swap(a, b)
> ;
> - }
>
>
> This produces "single-line ifs".
> Maybe generated patches can be improved somehow?
> Moving -+; doesn't help in this case.
There is clearly some problem with the management of newlines...
The other alternative is to just have one rule for introducing swap and
another for removing the braces around a swap, ie
if (...)
- {
swap(...);
- }
I don't think it would be motivated to remove the newline in that case.
julia
>
> diff -u -p a/drivers/net/wireless/ath/ath9k/calib.c
> b/drivers/net/wireless/ath/ath9k/calib.c
> --- a/drivers/net/wireless/ath/ath9k/calib.c
> +++ b/drivers/net/wireless/ath/ath9k/calib.c
> @@ -32,11 +32,7 @@ static int16_t ath9k_hw_get_nf_hist_mid(
>
> for (i = 0; i < ATH9K_NF_CAL_HIST_MAX - 1; i++) {
> for (j = 1; j < ATH9K_NF_CAL_HIST_MAX - i; j++) {
> - if (sort[j] > sort[j - 1]) {
> - nfval = sort[j];
> - sort[j] = sort[j - 1];
> - sort[j - 1] = nfval;
> - }
> + if (sort[j] > sort[j - 1]) swap(sort[j], sort[j - 1]);
> }
> }
> nfval = sort[(ATH9K_NF_CAL_HIST_MAX - 1) >> 1];
> diff -u -p a/drivers/net/wireless/ath/ath9k/ar9003_calib.c
> b/drivers/net/wireless/ath/ath9k/ar9003_calib.c
> --- a/drivers/net/wireless/ath/ath9k/ar9003_calib.c
> +++ b/drivers/net/wireless/ath/ath9k/ar9003_calib.c
> @@ -1011,19 +1011,11 @@ static void __ar955x_tx_iq_cal_sort(stru
> for (ix = 0;
Re: [Cocci] Problem with partial patch generation
On Mon, 15 Feb 2021, Denis Efremov wrote: > > > On 2/15/21 2:47 PM, Julia Lawall wrote: > > > > > > On Mon, 15 Feb 2021, Denis Efremov wrote: > > > >> Hi, > >> > >> I wrote a simple rule (swap.cocci): > >> > >> virtual patch > >> > >> @depends on patch@ > >> identifier tmp; > >> expression a, b; > >> type T; > >> @@ > >> > >> ( > >> - T tmp; > >> | > >> - T tmp = 0; > >> | > >> - T *tmp = NULL; > >> ) > >> ... when != tmp > >> - tmp = a; > >> - a = b; > >> - b = tmp; > >> + swap(a, b); > >> ... when != tmp > >> > >> I would expect it to remove a local variable only if there is a match > >> with swap template. > >> > >> However, it generates "partial" patch on 5.11 linux code: > >> $ spatch --version > >> spatch version 1.0.8-00201-g267f9cf8cc82 compiled with OCaml version 4.11.1 > >> $ spatch -D patch --sp-file swap.cocci mm/filemap.c > >> --- mm/filemap.c > >> +++ /tmp/cocci-output-445786-88aa66-filemap.c > >> @@ -2348,7 +2348,7 @@ static int generic_file_buffered_read_ge > >> struct file_ra_state *ra = >f_ra; > >> pgoff_t index = iocb->ki_pos >> PAGE_SHIFT; > >> pgoff_t last_index = (iocb->ki_pos + iter->count + PAGE_SIZE-1) >> > >> PAGE_SHIFT; > >> - int i, j, nr_got, err = 0; > >> + int i, nr_got, err = 0; > >> > >> nr = min_t(unsigned long, last_index - index, nr); > >> find_page: > >> > >> How can I improve the rule? > > > > I don't get a match with the latest version of Coccinelle. > > > > If the latest version of Coccinelle were to become a release, would that > > be good enough for you? Or do you need 1.0.8 to work as well? > > I planned to submit the swap rule to scripts/coccinelle. However, I can > create more strict pattern for the patch mode. Maybe adding "when strict" > is suitable here? If there is a release, then the submitted rule can just have a constraint about the release. You can try when strict. Normally, that is to ensure that error paths aren't excused from having the pattern. I don't see why it would help here, but perhaps it's worth a try. julia ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Problem with partial patch generation
On Mon, 15 Feb 2021, Denis Efremov wrote: > Hi, > > I wrote a simple rule (swap.cocci): > > virtual patch > > @depends on patch@ > identifier tmp; > expression a, b; > type T; > @@ > > ( > - T tmp; > | > - T tmp = 0; > | > - T *tmp = NULL; > ) > ... when != tmp > - tmp = a; > - a = b; > - b = tmp; > + swap(a, b); > ... when != tmp > > I would expect it to remove a local variable only if there is a match > with swap template. > > However, it generates "partial" patch on 5.11 linux code: > $ spatch --version > spatch version 1.0.8-00201-g267f9cf8cc82 compiled with OCaml version 4.11.1 > $ spatch -D patch --sp-file swap.cocci mm/filemap.c > --- mm/filemap.c > +++ /tmp/cocci-output-445786-88aa66-filemap.c > @@ -2348,7 +2348,7 @@ static int generic_file_buffered_read_ge > struct file_ra_state *ra = >f_ra; > pgoff_t index = iocb->ki_pos >> PAGE_SHIFT; > pgoff_t last_index = (iocb->ki_pos + iter->count + PAGE_SIZE-1) >> > PAGE_SHIFT; > - int i, j, nr_got, err = 0; > + int i, nr_got, err = 0; > > nr = min_t(unsigned long, last_index - index, nr); > find_page: > > How can I improve the rule? I don't get a match with the latest version of Coccinelle. If the latest version of Coccinelle were to become a release, would that be good enough for you? Or do you need 1.0.8 to work as well? julia ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Problem with partial patch generation
On Mon, 15 Feb 2021, Denis Efremov wrote: > Hi, > > I wrote a simple rule (swap.cocci): > > virtual patch > > @depends on patch@ > identifier tmp; > expression a, b; > type T; > @@ > > ( > - T tmp; > | > - T tmp = 0; > | > - T *tmp = NULL; > ) > ... when != tmp > - tmp = a; > - a = b; > - b = tmp; > + swap(a, b); > ... when != tmp > > I would expect it to remove a local variable only if there is a match > with swap template. > > However, it generates "partial" patch on 5.11 linux code: > $ spatch --version > spatch version 1.0.8-00201-g267f9cf8cc82 compiled with OCaml version 4.11.1 > $ spatch -D patch --sp-file swap.cocci mm/filemap.c > --- mm/filemap.c > +++ /tmp/cocci-output-445786-88aa66-filemap.c > @@ -2348,7 +2348,7 @@ static int generic_file_buffered_read_ge > struct file_ra_state *ra = >f_ra; > pgoff_t index = iocb->ki_pos >> PAGE_SHIFT; > pgoff_t last_index = (iocb->ki_pos + iter->count + PAGE_SIZE-1) >> > PAGE_SHIFT; > - int i, j, nr_got, err = 0; > + int i, nr_got, err = 0; > > nr = min_t(unsigned long, last_index - index, nr); > find_page: > > How can I improve the rule? Strange. It shouldn't do that. julia ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Missing package for Ubuntu 20.04 LTS?
On Sat, 13 Feb 2021, Taylor Blau wrote: > Hi Julia, > > On Fri, Feb 12, 2021 at 03:06:16PM +0100, Julia Lawall wrote: > > > Is there a planned release of coccinelle that will appear in the focal > > > suite? > > > > Can you use the PPA? > > > > https://launchpad.net/~npalix/+archive/ubuntu/coccinelle > > Yes, we could. But it's an extra step that all users of Ubuntu Focal > will have to take. Note that 20.04 is the image that all users of > GitHub Actions' "runs-on: ubuntu-latest" get, so anybody in that group > who wants to use coccinelle will have to configure and install from the > PPA. > > Is there a reason that 20.04 was skipped when preparing a release for > the official suites? We don't do this. I don't know who does it. julia ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Backward compatibility issue
On Fri, 12 Feb 2021, Denis Efremov wrote: > > > On 2/12/21 5:04 PM, Julia Lawall wrote: > > > > > > On Thu, 11 Feb 2021, Denis Efremov wrote: > > > >> Hi, one of my patterns started to fail tests on latest coccinelle. > >> I've bisected the commit that introduces "error". It's: > >> commit db60e916633d2cb3ae31140364783fdf85ed10f4 > >> "make information about SmPL iterator and declarer names available to the > >> C parser" > >> > >> To reproduce the error: > >> $ cd linux > >> $ git checkout 5b01014759991887b1e450c9def01e58c02ab81b > >> $ wget > >> https://raw.githubusercontent.com/evdenis/cvehound/master/cvehound/cve/CVE-2016-9793.cocci > >> $ spatch -D detect --cocci-file CVE-2016-9793.cocci net/core/sock.c > >> # spatch before db60e916633d2cb3ae31140364783fdf85ed10f4 will find the > >> match > >> net/core/sock.c:718:16-17: ERROR: CVE-2016-9793 > >> net/core/sock.c:754:16-17: ERROR: CVE-2016-9793 > >> ... > >> # spatch >= db60e916633d2cb3ae31140364783fdf85ed10f4 will not match the > >> same code > > > > If you change typedef u32 to symbol u32, it should be good. It is no use > > to Coccinelle to know that u32 is a typedef in this code. > > After changing typedef to symbol, git version starts to match the code, > but coccinelle 1.0.8 starts to fail the detection. Strange, I'll check. > Well, this doesn't > solve the problem for me. > > What's the difference between symbol and typedef? How can I understand > when to use one or another? Typedef allows the thing to be used as a type. But in semantic patch that oyu have, there is no way for Coccinelle to know that u32 is being used as a type. It's just the argument to a macro. It could be any identifier. Symbol just means "this is an identifier I want to match exactly, and I don't want the semantic patch parser to ask me if it should be a metavariable". It actually has no impact on the matching process. julia ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Missing package for Ubuntu 20.04 LTS?
On Tue, 9 Feb 2021, Taylor Blau wrote: > Hi, > > The Git project recently noticed that our static analysis builds were > failing after upgrading to Ubuntu 20.04 due to being unable to find the > coccinelle package in the focal suite. > > https://lore.kernel.org/git/YCGrmsg8J7XT32TM@nand.local/ > > Searching for coccinelle [1] turns up hits in the xenial, bionic, and > groovy suites, but not the focal ones. This appears to have been > discussed on this list a couple of times without any conclusion. > > Is there a planned release of coccinelle that will appear in the focal > suite? Can you use the PPA? https://launchpad.net/~npalix/+archive/ubuntu/coccinelle julia > > Thanks, > Taylor > > [1]: https://packages.ubuntu.com/search?keywords=coccinelle > [2]: http://archive.ubuntu.com/ubuntu/pool/universe/c/coccinelle/ > ___ > Cocci mailing list > Cocci@systeme.lip6.fr > https://systeme.lip6.fr/mailman/listinfo/cocci > ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Backward compatibility issue
On Thu, 11 Feb 2021, Denis Efremov wrote: > Hi, one of my patterns started to fail tests on latest coccinelle. > I've bisected the commit that introduces "error". It's: > commit db60e916633d2cb3ae31140364783fdf85ed10f4 > "make information about SmPL iterator and declarer names available to the C > parser" > > To reproduce the error: > $ cd linux > $ git checkout 5b01014759991887b1e450c9def01e58c02ab81b > $ wget > https://raw.githubusercontent.com/evdenis/cvehound/master/cvehound/cve/CVE-2016-9793.cocci > $ spatch -D detect --cocci-file CVE-2016-9793.cocci net/core/sock.c > # spatch before db60e916633d2cb3ae31140364783fdf85ed10f4 will find the match > net/core/sock.c:718:16-17: ERROR: CVE-2016-9793 > net/core/sock.c:754:16-17: ERROR: CVE-2016-9793 > ... > # spatch >= db60e916633d2cb3ae31140364783fdf85ed10f4 will not match the same > code If you change typedef u32 to symbol u32, it should be good. It is no use to Coccinelle to know that u32 is a typedef in this code. julia > > Thanks, > Denis > ___ > Cocci mailing list > Cocci@systeme.lip6.fr > https://systeme.lip6.fr/mailman/listinfo/cocci > ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Backward compatibility issue
On Thu, 11 Feb 2021, Denis Efremov wrote: > Hi, one of my patterns started to fail tests on latest coccinelle. > I've bisected the commit that introduces "error". It's: > commit db60e916633d2cb3ae31140364783fdf85ed10f4 > "make information about SmPL iterator and declarer names available to the C > parser" > > To reproduce the error: > $ cd linux > $ git checkout 5b01014759991887b1e450c9def01e58c02ab81b > $ wget > https://raw.githubusercontent.com/evdenis/cvehound/master/cvehound/cve/CVE-2016-9793.cocci > $ spatch -D detect --cocci-file CVE-2016-9793.cocci net/core/sock.c > # spatch before db60e916633d2cb3ae31140364783fdf85ed10f4 will find the match > net/core/sock.c:718:16-17: ERROR: CVE-2016-9793 > net/core/sock.c:754:16-17: ERROR: CVE-2016-9793 > ... > # spatch >= db60e916633d2cb3ae31140364783fdf85ed10f4 will not match the same > code Thanks for the report. Maybe sometimes making the parser work better actually makes it work worse... I'll check on it. julia > > Thanks, > Denis > ___ > Cocci mailing list > Cocci@systeme.lip6.fr > https://systeme.lip6.fr/mailman/listinfo/cocci > ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] qualified function rule
On Wed, 27 Jan 2021, James K. Lowden wrote:
> I don't understand how, if it's possible, to qualify a function in a
> rule. I want the class of all functions having a parameter of a
> particular type.
>
> The code I'm working with has hundreds of unnecessary casts. Many
> functions take a void* parameter, but are nonetheless called by casting
> the parameter. For example, the parameters to memcpy(3) often
> have casts applied.
>
> I imagine writing a rule like
>
> @@
> type T, D;
> identifier F(void*);
> identifier D * data;
> @@
>
> - F((T*)data)
> + F(data)
>
> but that doesn't work, and I haven't found anything that does.
>
> In the kmalloc examples, I see things like
>
> - \(kmalloc|kcmalloc\)(...)
> + mumble something
>
> but that forces me to enumerate all such function names. It seems
> vaguely like positions would do the trick, but, well, vaguely.
>
> Is there a way?
In principle, you should be able to specify the type of F. But I'm not at
all sure that that is supported for function names.
Maybe it would suffice to do:
@fn@
identifier F,i;
parameter list[n] ps;
@@
F(ps,void *i,...) { ... }
@@
identifier fn.F;
expression list[fn.n] es;
type T;
expression *e;
@@
F(es,
- (T*)
e, ...)
@ty@
identifier F,i;
parameter list[n] ps;
type t;
@@
t F(ps,void *i,...);
@@
identifier ty.F;
expression list[ty.n] es;
type T;
expression *e;
@@
F(es,
- (T*)
e, ...)
Probably your function prototypes are not in the .c files, but rather in
things that they include. So you would want to use an argument like
--all-includes (include locally mentioned header) or --recursive-includes
(include headers included in other headers). You may want to give some -I
dir arguments to help it find the header files.
julia
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] online talk
Hello, I will give a tutorial talk about Coccinelle on February 2, at 7:30AM PST. It is possible to register at the following link: https://events.linuxfoundation.org/mentorship-session-coccinelle/ julia ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Performance issue with quite simple patch?
On Wed, 13 Jan 2021, Maxime Ripard wrote:
> Hi!
>
> I've been trying to get a patch to rename any variable called "state" in
> a given set of callbacks.
>
> This is the patch that I've come up with:
>
> @ plane_atomic_func @
> identifier helpers;
> identifier func;
> @@
>
> (
> static const struct drm_plane_helper_funcs helpers = {
> ...,
> .atomic_check = func,
> ...,
> };
> |
> static const struct drm_plane_helper_funcs helpers = {
> ...,
> .atomic_disable = func,
> ...,
> };
> |
> static const struct drm_plane_helper_funcs helpers = {
> ...,
> .atomic_update = func,
> ...,
> };
> )
You don't need the ...s in the above. For structure declarations
Coccinelle is happy as long as what you specify is a subset of what is
present. The static and const aren't essential either. If you remove
them, the pattern will match whethe thy are present or not.
>
> @@
> identifier plane_atomic_func.func;
> symbol state;
> expression e;
> type T;
> @@
>
> func(...)
> {
> ...
> - T state = e;
> + T plane_state = e;
> <+...
> - state
> + plane_state
> ...+>
> }
>
> However, it seems like at least on a file (in Linux,
> drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c), it takes quite big
> performance hit with one CPU running at 100% until the timeout is hit.
>
> Replacing <+... by ... makes it work instantly, but doesn't really do
> what I'm expecting, so I guess it's a matter of the patch being
> subobtimal?
>
> Is there a more optimal way of doing it?
In your rule, I donkt think that there is really any essential connection
between the declaration and the use? You just want to change state to
plane_state when it occurs in one of the functions that you detected. So
you could at least try the following and see if it gives any false
positives:
@@
identifier plane_atomic_func.func;
symbol state;
expression e;
type T;
@@
func(...)
{
<...
(
- T state = e;
+ T plane_state = e;
|
- state
+ plane_state
)
...>
}
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] How to match switch cases and their absence with coccinelle?
On Tue, 12 Jan 2021, Denis Efremov wrote:
>
>
> On 1/12/21 7:13 PM, Julia Lawall wrote:
> >
> >
> > On Tue, 12 Jan 2021, Denis Efremov wrote:
> >
> >> Hi,
> >>
> >> Let's suppose I have this pattern:
> >> @fix exists@
> >> position p;
> >> @@
> >>
> >> binder_release_work(...)
> >> {
> >>...
> >>switch (...) {
> >> * case BINDER_WORK_NODE: ... break;@p
> >>}
> >>...
> >
> > Add when any to the outer ...s
>
> Thanks, this helped.
>
> >
> >> }
> >>
> >> and I want to match binder_release_work() function in
> >> drivers/android/binder.c
> >> file (linux kernel, master)
> >>
> >> Seems like the rule is not enough, it gives nothing:
> >> $ spatch --cocci-file binder.cocci drivers/android/binder.c
> >> init_defs_builtins: /usr/lib64/coccinelle/standard.h
> >> HANDLING: drivers/android/binder.c
> >>
> >> 1) What can I do to reliable check that there is a special case in a
> >> switch?
> >> 2) Is it possible to check that there is no case handling with something
> >> like:
> >>switch (...) {
> >>... when != case BINDER_WORK_NODE: ... break;
> >>}
> >
> > I don't know if that will work. But you can do it with two rules. In the
> > first rule, you could put a position variable on the switch, and then in
> > the second rule, you could make a position variable that is required to be
> > different than the first one, and that is also attached to a switch.
>
> Yes, I use this method currently.
>
> Also I faced the problem when I can't use ... in the beginning of enum, i.e.:
> struct binder_work {
> ...
> enum binder_work_type {
> ... // <== will not work
Probably the ... needs a comma.
julia
> * BINDER_WORK_NODE,
> ...
> } type;
> ...
> }
>
> This works:
> struct binder_work {
> ...
> enum binder_work_type {
> // BINDER_WORK_TRANSACTION = ..., // also will not work
> BINDER_WORK_TRANSACTION = 1,
> BINDER_WORK_TRANSACTION_COMPLETE,
> BINDER_WORK_RETURN_ERROR,
> * BINDER_WORK_NODE,
> ...
> } type;
> ...
> }
>
> Thanks,
> Denis
>
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] How to match switch cases and their absence with coccinelle?
On Tue, 12 Jan 2021, Denis Efremov wrote:
> Hi,
>
> Let's suppose I have this pattern:
> @fix exists@
> position p;
> @@
>
> binder_release_work(...)
> {
> ...
> switch (...) {
> * case BINDER_WORK_NODE: ... break;@p
> }
> ...
Add when any to the outer ...s
> }
>
> and I want to match binder_release_work() function in drivers/android/binder.c
> file (linux kernel, master)
>
> Seems like the rule is not enough, it gives nothing:
> $ spatch --cocci-file binder.cocci drivers/android/binder.c
> init_defs_builtins: /usr/lib64/coccinelle/standard.h
> HANDLING: drivers/android/binder.c
>
> 1) What can I do to reliable check that there is a special case in a switch?
> 2) Is it possible to check that there is no case handling with something like:
> switch (...) {
> ... when != case BINDER_WORK_NODE: ... break;
> }
I don't know if that will work. But you can do it with two rules. In the
first rule, you could put a position variable on the switch, and then in
the second rule, you could make a position variable that is required to be
different than the first one, and that is also attached to a switch.
julia
> Thanks,
> Denis
> ___
> Cocci mailing list
> Cocci@systeme.lip6.fr
> https://systeme.lip6.fr/mailman/listinfo/cocci
>
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] How to match function-like macro calls, e.g. RTA_ALIGN(rta->rta_len)?
On Mon, 11 Jan 2021, Denis Efremov wrote:
>
>
> On 1/11/21 11:40 PM, Julia Lawall wrote:
> >
> >
> > On Mon, 11 Jan 2021, Denis Efremov wrote:
> >
> >>
> >>
> >> On 1/11/21 11:23 PM, Julia Lawall wrote:
> >>>
> >>>
> >>> On Mon, 11 Jan 2021, Denis Efremov wrote:
> >>>
> >>>> Hi,
> >>>>
> >>>> Let's suppose I want to match otx_cpt_aead_cbc_aes_sha_setkey() function
> >>>> from
> >>>> drivers/crypto/marvell/octeontx/otx_cptvf_algs.c file (linux kernel).
> >>>>
> >>>> My pattern:
> >>>> @exists@
> >>>> identifier rta, param, key, keylen;
> >>>> position p;
> >>>> @@
> >>>>
> >>>> otx_cpt_aead_cbc_aes_sha_setkey(...,
> >>>> unsigned char *key, unsigned int keylen)
> >>>> {
> >>>> ...
> >>>> *if (RTA_PAYLOAD(rta) < sizeof(*param))@p
> >>>> goto badkey;
> >>>> ...
> >>>> *key += RTA_ALIGN(rta->rta_len);
> >>>> *keylen -= RTA_ALIGN(rta->rta_len);
> >>>> ...
> >>>> }
> >>>>
> >>>> $ spatch --no-includes --include-headers --cocci-file test.cocci
> >>>> drivers/crypto/marvell/octeontx/otx_cptvf_algs.c
> >>>> init_defs_builtins: /usr/lib64/coccinelle/standard.h
> >>>> minus: parse error:
> >>>> File "test.cocci", line 13, column 9, charpos = 219
> >>>> around = 'RTA_ALIGN',
> >>>> whole content = * key += RTA_ALIGN(rta->rta_len);
> >>>>
> >>>>
> >>>> What can I do to match RTA_ALIGN(...) lines?
> >>>
> >>> I don't understand the problem. I took your rule and your command line,
> >>> and everything was fine.
> >>
> >> I use version:
> >> spatch version 1.0.8-gc1dbb4f-dirty compiled with OCaml version 4.11.1
> >> Flags passed to the configure script: --build=x86_64-redhat-linux-gnu
> >> --host=x86_64-redhat-linux-gnu --program-prefix=
> >> --disable-dependency-tracking --prefix=/usr --exec-prefix=/usr
> >> --bindir=/usr/bin --sbindir=/usr/sbin --sysconfdir=/etc
> >> --datadir=/usr/share --includedir=/usr/include --libdir=/usr/lib64
> >> --libexecdir=/usr/libexec --localstatedir=/var --sharedstatedir=/var/lib
> >> --mandir=/usr/share/man --infodir=/usr/share/info
> >> --with-python=/usr/bin/python3 --with-menhir=/usr/bin/menhir
> >> OCaml scripting support: yes
> >> Python scripting support: yes
> >> Syntax of regular expressions: PCRE
> >>
> >> Maybe parsing error is fixed in new versions?
> >
> > I can't imagine what would cause a parsing error here. I don't see what
> > could be special about RTA_ALIGN. Normally Coccinelle doesn't even know
> > that it is a macro. Maybe you can try to simplify the semantic patch a
> > little bit and see if there is some change that causes the problem to
> > disappear? Does the problem disappear if you use a name other than
> > RTA_ALIGN?
>
> Changing:
> key += RTA_ALIGN(rta->rta_len);
> to:
> key = RTA_ALIGN(rta->rta_len);
>
> makes the parsing error disappear.
>
> Using aop instead of += leads to the same parsing error:
> assignment operator aop;
> key aop RTA_ALIGN(rta->rta_len);
> init_defs_builtins: /usr/lib64/coccinelle/standard.h
> minus: parse error:
> File "test.cocci", line 14, column 10, charpos = 245
> around = 'RTA_ALIGN',
> whole content = * key aop RTA_ALIGN(rta->rta_len);
I assume not, but does spatch --parse-cocci test.cocci work?
julia
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] How to match function-like macro calls, e.g. RTA_ALIGN(rta->rta_len)?
On Mon, 11 Jan 2021, Denis Efremov wrote:
>
>
> On 1/11/21 11:23 PM, Julia Lawall wrote:
> >
> >
> > On Mon, 11 Jan 2021, Denis Efremov wrote:
> >
> >> Hi,
> >>
> >> Let's suppose I want to match otx_cpt_aead_cbc_aes_sha_setkey() function
> >> from
> >> drivers/crypto/marvell/octeontx/otx_cptvf_algs.c file (linux kernel).
> >>
> >> My pattern:
> >> @exists@
> >> identifier rta, param, key, keylen;
> >> position p;
> >> @@
> >>
> >> otx_cpt_aead_cbc_aes_sha_setkey(...,
> >>unsigned char *key, unsigned int keylen)
> >> {
> >>...
> >> * if (RTA_PAYLOAD(rta) < sizeof(*param))@p
> >>goto badkey;
> >>...
> >> * key += RTA_ALIGN(rta->rta_len);
> >> * keylen -= RTA_ALIGN(rta->rta_len);
> >>...
> >> }
> >>
> >> $ spatch --no-includes --include-headers --cocci-file test.cocci
> >> drivers/crypto/marvell/octeontx/otx_cptvf_algs.c
> >> init_defs_builtins: /usr/lib64/coccinelle/standard.h
> >> minus: parse error:
> >> File "test.cocci", line 13, column 9, charpos = 219
> >> around = 'RTA_ALIGN',
> >> whole content = * key += RTA_ALIGN(rta->rta_len);
> >>
> >>
> >> What can I do to match RTA_ALIGN(...) lines?
> >
> > I don't understand the problem. I took your rule and your command line,
> > and everything was fine.
>
> I use version:
> spatch version 1.0.8-gc1dbb4f-dirty compiled with OCaml version 4.11.1
> Flags passed to the configure script: --build=x86_64-redhat-linux-gnu
> --host=x86_64-redhat-linux-gnu --program-prefix=
> --disable-dependency-tracking --prefix=/usr --exec-prefix=/usr
> --bindir=/usr/bin --sbindir=/usr/sbin --sysconfdir=/etc --datadir=/usr/share
> --includedir=/usr/include --libdir=/usr/lib64 --libexecdir=/usr/libexec
> --localstatedir=/var --sharedstatedir=/var/lib --mandir=/usr/share/man
> --infodir=/usr/share/info --with-python=/usr/bin/python3
> --with-menhir=/usr/bin/menhir
> OCaml scripting support: yes
> Python scripting support: yes
> Syntax of regular expressions: PCRE
>
> Maybe parsing error is fixed in new versions?
I can't imagine what would cause a parsing error here. I don't see what
could be special about RTA_ALIGN. Normally Coccinelle doesn't even know
that it is a macro. Maybe you can try to simplify the semantic patch a
little bit and see if there is some change that causes the problem to
disappear? Does the problem disappear if you use a name other than
RTA_ALIGN?
julia
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] How to match function-like macro calls, e.g. RTA_ALIGN(rta->rta_len)?
On Mon, 11 Jan 2021, Denis Efremov wrote:
> Hi,
>
> Let's suppose I want to match otx_cpt_aead_cbc_aes_sha_setkey() function from
> drivers/crypto/marvell/octeontx/otx_cptvf_algs.c file (linux kernel).
>
> My pattern:
> @exists@
> identifier rta, param, key, keylen;
> position p;
> @@
>
> otx_cpt_aead_cbc_aes_sha_setkey(...,
> unsigned char *key, unsigned int keylen)
> {
> ...
> * if (RTA_PAYLOAD(rta) < sizeof(*param))@p
> goto badkey;
> ...
> * key += RTA_ALIGN(rta->rta_len);
> * keylen -= RTA_ALIGN(rta->rta_len);
> ...
> }
>
> $ spatch --no-includes --include-headers --cocci-file test.cocci
> drivers/crypto/marvell/octeontx/otx_cptvf_algs.c
> init_defs_builtins: /usr/lib64/coccinelle/standard.h
> minus: parse error:
> File "test.cocci", line 13, column 9, charpos = 219
> around = 'RTA_ALIGN',
> whole content = * key += RTA_ALIGN(rta->rta_len);
>
>
> What can I do to match RTA_ALIGN(...) lines?
I don't understand the problem. I took your rule and your command line,
and everything was fine.
julia
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Matching attributes
> But this fails: > > @@ > type T; > identifier var; > attribute name autofree; > @@ > > T var autofree; > ... > ++ free(var); > return ...; > > with > > minus: parse error: >File "autofree.cocci", line 7, column 17, charpos = 73 >around = ';', >whole content =T var autofree; > > What am I doing wrong here? > I tried both with v1.0.8 and latest git. Thanks for the report. I will look into it. julia ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Python 3.10 again: _Py_fopen deprecated
On Tue, 5 Jan 2021, Richard W.M. Jones wrote: > Firstly a gentle reminder that there's a patch waiting to be applied: > https://systeme.lip6.fr/pipermail/cocci/2020-November/thread.html#8398 > > Different from that patch, but still related to Python 3.10, we've got > another bug report here: > https://bugzilla.redhat.com/show_bug.cgi?id=1912931 > > This time _Py_fopen has been deprecated, replaced by _Py_wfopen or > _Py_fopen_obj. It's unclear which is better. The two functions are > documented here: > https://github.com/python/cpython/blob/master/Python/fileutils.c#L1418 > > What I don't understand from the pyxml code is why we use these > internal Python functions at all, instead of calling regular C > functions like fopen etc. In fact it seems like for Python 2 we did > call fopen ... Everything should be up to date now on github. Thanks for your help. Thierry will contact you directly about the choice of fopen. julia > > Rich. > > -- > Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones > Read my programming and virtualization blog: http://rwmj.wordpress.com > Fedora Windows cross-compiler. Compile Windows programs, test, and > build Windows installers. Over 100 libraries supported. > http://fedoraproject.org/wiki/MinGW > > ___ > Cocci mailing list > Cocci@systeme.lip6.fr > https://systeme.lip6.fr/mailman/listinfo/cocci > ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] cocci: missed strlcpy->strscpy conversion?
On Thu, 31 Dec 2020, Joe Perches wrote: > On Thu, 2020-12-31 at 21:27 +0100, Julia Lawall wrote: > > On Thu, 31 Dec 2020, Joe Perches wrote: > > > On Thu, 2020-12-31 at 11:04 -0800, Joe Perches wrote: > > > > strlcpy is deprecated. see: Documentation/process/deprecated.rst > > > > > > > > Change the calls that do not use the strlcpy return value to the > > > > preferred strscpy. > > > > > > > > Done with cocci script: > > > > > > > > @@ > > > > expression e1, e2, e3; > > > > @@ > > > > > > > > - strlcpy( > > > > + strscpy( > > > > e1, e2, e3); > > > > > > > > This cocci script leaves the instances where the return value is > > > > used unchanged. > > > > > > Hey Julia. > > > > > > After using the cocci script above on a test treewide conversion, > > > there were a few instances with no return use that were not converted. > > > > > > Any idea why these were not converted? > [] > > The parse is not happy about the for_each_possible_cpu. It seems that the > > heuristic for detecting that as a loop expects that the body of the loop > > will have braces. You can see this with the --parse-c option, ie > > > > spatch --parse-c drivers/block/rnbd/rnbd-clt.c > > > > The offending line will have BAD in front of it. > > Thanks. > > Do you consider the for_each heuristic a defect? (I'm not sure I do) It seems that the problem is not really the for_each, but the * in front of a "function call" on the left side of an assignment. Without the *, everything is fine. So it is indeed probably not worth doing anything about. julia ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] cocci: missed strlcpy->strscpy conversion?
On Thu, 31 Dec 2020, Joe Perches wrote: > On Thu, 2020-12-31 at 21:27 +0100, Julia Lawall wrote: > > On Thu, 31 Dec 2020, Joe Perches wrote: > > > On Thu, 2020-12-31 at 11:04 -0800, Joe Perches wrote: > > > > strlcpy is deprecated. see: Documentation/process/deprecated.rst > > > > > > > > Change the calls that do not use the strlcpy return value to the > > > > preferred strscpy. > > > > > > > > Done with cocci script: > > > > > > > > @@ > > > > expression e1, e2, e3; > > > > @@ > > > > > > > > - strlcpy( > > > > + strscpy( > > > > e1, e2, e3); > > > > > > > > This cocci script leaves the instances where the return value is > > > > used unchanged. > > > > > > Hey Julia. > > > > > > After using the cocci script above on a test treewide conversion, > > > there were a few instances with no return use that were not converted. > > > > > > Any idea why these were not converted? > [] > > The parse is not happy about the for_each_possible_cpu. It seems that the > > heuristic for detecting that as a loop expects that the body of the loop > > will have braces. You can see this with the --parse-c option, ie > > > > spatch --parse-c drivers/block/rnbd/rnbd-clt.c > > > > The offending line will have BAD in front of it. > > Thanks. > > Do you consider the for_each heuristic a defect? (I'm not sure I do) It could be improved. I was wondering if the indentation was not correct. If there was a space in the line with the for_each, then the two lines would be equally indented, which would certainly confuse the heuristics. But that is not the case; the indentation is fine. And the file contains a previous for_each_possible_cpu that has braces, so it should have taken note of that and detected that the next one should also be a loop. > > I hope it's time for you to stop working today... > > cheers and happy new year, Joe Thanks! Happy new year :) julia ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] cocci: missed strlcpy->strscpy conversion?
On Thu, 31 Dec 2020, Joe Perches wrote:
> On Thu, 2020-12-31 at 11:04 -0800, Joe Perches wrote:
> > strlcpy is deprecated. see: Documentation/process/deprecated.rst
> >
> > Change the calls that do not use the strlcpy return value to the
> > preferred strscpy.
> >
> > Done with cocci script:
> >
> > @@
> > expression e1, e2, e3;
> > @@
> >
> > - strlcpy(
> > + strscpy(
> > e1, e2, e3);
> >
> > This cocci script leaves the instances where the return value is
> > used unchanged.
>
> Hey Julia.
>
> After using the cocci script above on a test treewide conversion,
> there were a few instances with no return use that were not converted.
>
> Any idea why these were not converted?
> I don't see a pattern.
For a semantic patch like this, where you don't case about type
information and the change is very local, you can use the options:
--no-includes --include-headers
Then the .c files and the .h files will be treated one by one. The
--no-includes options prevents the .h files from being included into the
.c files, which could causetheir code to get transformed at each
inclusion. The --include-headers option causes the .h files to be
considered.
>
> The .h files may be because those are the only uses in .h files in the kernel
> but drivers/block/rnbd/rnbd-clt.c I don't understand at all.
The parse is not happy about the for_each_possible_cpu. It seems that the
heuristic for detecting that as a loop expects that the body of the loop
will have braces. You can see this with the --parse-c option, ie
spatch --parse-c drivers/block/rnbd/rnbd-clt.c
The offending line will have BAD in front of it.
julia
> drivers/block/rnbd/rnbd-clt.c: strlcpy(sess->sessname, sessname,
> sizeof(sess->sessname));
> drivers/input/serio/i8042-x86ia64io.h: strlcpy(dst, "PNP:", dst_size);
> drivers/input/serio/i8042-x86ia64io.h: strlcpy(i8042_pnp_kbd_name, did->id,
> sizeof(i8042_pnp_kbd_name));
> drivers/input/serio/i8042-x86ia64io.h: strlcpy(i8042_pnp_aux_name, did->id,
> sizeof(i8042_pnp_aux_name));
> drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.h: strlcpy(buf,
> bp->acquire_resp.pfdev_info.fw_ver, buf_len);
>
> $ git grep -3 strlcpy drivers/block/rnbd/rnbd-clt.c
> drivers/input/serio/i8042-x86ia64io.h
> drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.h
> drivers/block/rnbd/rnbd-clt.c- sess = kzalloc_node(sizeof(*sess),
> GFP_KERNEL, NUMA_NO_NODE);
> drivers/block/rnbd/rnbd-clt.c- if (!sess)
> drivers/block/rnbd/rnbd-clt.c- return ERR_PTR(-ENOMEM);
> drivers/block/rnbd/rnbd-clt.c: strlcpy(sess->sessname, sessname,
> sizeof(sess->sessname));
> drivers/block/rnbd/rnbd-clt.c- atomic_set(>busy, 0);
> drivers/block/rnbd/rnbd-clt.c- mutex_init(>lock);
> drivers/block/rnbd/rnbd-clt.c- INIT_LIST_HEAD(>devs_list);
> --
> drivers/input/serio/i8042-x86ia64io.h-
> drivers/input/serio/i8042-x86ia64io.h-static void
> i8042_pnp_id_to_string(struct pnp_id *id, char *dst, int dst_size)
> drivers/input/serio/i8042-x86ia64io.h-{
> drivers/input/serio/i8042-x86ia64io.h: strlcpy(dst, "PNP:", dst_size);
> drivers/input/serio/i8042-x86ia64io.h-
> drivers/input/serio/i8042-x86ia64io.h- while (id) {
> drivers/input/serio/i8042-x86ia64io.h- strlcat(dst, " ", dst_size);
> --
> drivers/input/serio/i8042-x86ia64io.h- if (pnp_irq_valid(dev,0))
> drivers/input/serio/i8042-x86ia64io.h- i8042_pnp_kbd_irq =
> pnp_irq(dev, 0);
> drivers/input/serio/i8042-x86ia64io.h-
> drivers/input/serio/i8042-x86ia64io.h: strlcpy(i8042_pnp_kbd_name, did->id,
> sizeof(i8042_pnp_kbd_name));
> drivers/input/serio/i8042-x86ia64io.h- if (strlen(pnp_dev_name(dev))) {
> drivers/input/serio/i8042-x86ia64io.h- strlcat(i8042_pnp_kbd_name,
> ":", sizeof(i8042_pnp_kbd_name));
> drivers/input/serio/i8042-x86ia64io.h- strlcat(i8042_pnp_kbd_name,
> pnp_dev_name(dev), sizeof(i8042_pnp_kbd_name));
> --
> drivers/input/serio/i8042-x86ia64io.h- if (pnp_irq_valid(dev, 0))
> drivers/input/serio/i8042-x86ia64io.h- i8042_pnp_aux_irq =
> pnp_irq(dev, 0);
> drivers/input/serio/i8042-x86ia64io.h-
> drivers/input/serio/i8042-x86ia64io.h: strlcpy(i8042_pnp_aux_name, did->id,
> sizeof(i8042_pnp_aux_name));
> drivers/input/serio/i8042-x86ia64io.h- if (strlen(pnp_dev_name(dev))) {
> drivers/input/serio/i8042-x86ia64io.h- strlcat(i8042_pnp_aux_name,
> ":", sizeof(i8042_pnp_aux_name));
> drivers/input/serio/i8042-x86ia64io.h- strlcat(i8042_pnp_aux_name,
> pnp_dev_name(dev), sizeof(i8042_pnp_aux_name));
> --
> drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.h-static inline void
> bnx2x_vf_fill_fw_str(struct bnx2x *bp, char *buf,
> drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.h-
> size_t buf_len)
> drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.h-{
> drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.h: strlcpy(buf,
> bp->acquire_resp.pfdev_info.fw_ver, buf_len);
> drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.h-}
>
Re: [Cocci] [PATCH v3] scripts: coccicheck: Correct usage of make coccicheck
On Wed, 25 Nov 2020, Sumera Priyadarsini wrote:
> The command "make coccicheck C=1 CHECK=scripts/coccicheck" results in the
> error:
> ./scripts/coccicheck: line 65: -1: shift count out of range
>
> This happens because every time the C variable is specified,
> the shell arguments need to be "shifted" in order to take only
> the last argument, which is the C file to test. These shell arguments
> mostly comprise flags that have been set in the Makefile. However,
> when coccicheck is specified in the make command as a rule, the
> number of shell arguments is zero, thus passing the invalid value -1
> to the shift command, resulting in an error.
>
> Modify coccicheck to print correct usage of make coccicheck so as to
> avoid the error.
Applied, thanks.
julia
>
> Signed-off-by: Sumera Priyadarsini
> ---
> Changes in v2:
> - Move test to only display error message
>
> Changes in v3:
> - Update example with latest file
> ---
> scripts/coccicheck | 12
> 1 file changed, 12 insertions(+)
>
> diff --git a/scripts/coccicheck b/scripts/coccicheck
> index 209bb0427b43..d1aaa1dc0a69 100755
> --- a/scripts/coccicheck
> +++ b/scripts/coccicheck
> @@ -61,6 +61,18 @@ COCCIINCLUDE=${COCCIINCLUDE// -include/ --include}
> if [ "$C" = "1" -o "$C" = "2" ]; then
> ONLINE=1
>
> +if [[ $# -le 0 ]]; then
> + echo ''
> + echo 'Specifying both the variable "C" and rule "coccicheck" in the
> make
> +command results in a shift count error.'
> + echo ''
> + echo 'Try specifying "scripts/coccicheck" as a value for the CHECK
> variable instead.'
> + echo ''
> + echo 'Example: make C=2 CHECK=scripts/coccicheck
> drivers/net/ethernet/ethoc.o'
> + echo ''
> + exit 1
> +fi
> +
> # Take only the last argument, which is the C file to test
> shift $(( $# - 1 ))
> OPTIONS="$COCCIINCLUDE $1"
> --
> 2.25.1
>
> ___
> Cocci mailing list
> Cocci@systeme.lip6.fr
> https://systeme.lip6.fr/mailman/listinfo/cocci
>
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
