Re: [Cocci] Checking software behaviour according to selected SmPL code variants

2020-06-09 Thread Markus Elfring
>> If I omit the specification “, ...” from the function call parameters
>> because I could be unsure about the number of arguments in other
>> software situations, I do not get the desired test output as before.
>
> This has been discussed before.

I was just looking for a related update in this area.


> When you put <+... ...+> in an argument list, it doesn't mean an unknown 
> number
> of arguments, it means one arguemnt that has something as a subexpression.

Another SmPL code variant can eventually become interesting then.

*y = x(..., <+... e ...+>, ...)


>> If I omit even the semicolon from the assignment statement in the
>> search pattern, I get an error message.
>>
>> elfring@Sonne:~/Projekte/Coccinelle/janitor> spatch --parse-cocci 
>> show_questionable_brelse_usage6.cocci
>> …
>> minus: parse error:
>>   File "show_questionable_brelse_usage6.cocci", line 6, column 0, charpos = 
>> 67
>>   around = '',
>>   whole content =
>
> That is quite normal.  One statement should be followed by another statement.

I hope that the support will become better also for assignment expressions.

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


Re: [Cocci] Checking software behaviour according to selected SmPL code variants

2020-06-09 Thread Julia Lawall


On Tue, 9 Jun 2020, Markus Elfring wrote:

> Hello,
>
> My software development attention was caught also by a recent patch.
> https://lore.kernel.org/linux-fsdevel/20200608141629.GA1912173@mwanda/
> https://lore.kernel.org/patchwork/patch/1253499/
>
> Thus I have tried another tiny script out for the semantic patch language
> (according to the software combination “Coccinelle 1.0.8-00104-ge06b9156”).
>
>
> @display@
> expression e, x, y;
> @@
> *brelse(e);
> *y = x(<+... e ...+>, ...);
>
>
> An usable output is generated then as expected for a test source file
> like the following.
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/fs/exfat/nls.c?id=b676fdbcf4c8424f3c02ed7f31576d99b963bded#n652
>
> // SPDX-License-Identifier: GPL-2.0-or-later
> // deleted part
> static int exfat_load_upcase_table(struct super_block *sb,
>   sector_t sector, unsigned long long num_sectors,
>   unsigned int utbl_checksum)
> {
>   struct exfat_sb_info *sbi = EXFAT_SB(sb);
>   unsigned int sect_size = sb->s_blocksize;
>   unsigned int i, index = 0;
>   u32 chksum = 0;
> // deleted part
>   while (sector < num_sectors) {
>   struct buffer_head *bh;
>
>   bh = sb_bread(sb, sector);
> // deleted part
>   brelse(bh);
>   chksum = exfat_calc_chksum32(bh->b_data, i, chksum, CS_DEFAULT);
>   }
> // deleted part
> }
> // deleted part
>
>
> If I omit the specification “, ...” from the function call parameters
> because I could be unsure about the number of arguments in other
> software situations, I do not get the desired test output as before.

This has been discussed before.  When you put <+... ...+> in an argument
list, it doesn't mean an unknown number of arguments, it means one
arguemnt that has something as a subexpression.

>
> If I omit even the semicolon from the assignment statement in the
> search pattern, I get an error message.
>
> elfring@Sonne:~/Projekte/Coccinelle/janitor> spatch --parse-cocci 
> show_questionable_brelse_usage6.cocci
> …
> minus: parse error:
>   File "show_questionable_brelse_usage6.cocci", line 6, column 0, charpos = 67
>   around = '',
>   whole content =

That is quite normal.  One statement should be followed by another
statement.

>
> Will such observations influence subsequent software evolution?

No.

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


[Cocci] Checking software behaviour according to selected SmPL code variants

2020-06-09 Thread Markus Elfring
Hello,

My software development attention was caught also by a recent patch.
https://lore.kernel.org/linux-fsdevel/20200608141629.GA1912173@mwanda/
https://lore.kernel.org/patchwork/patch/1253499/

Thus I have tried another tiny script out for the semantic patch language
(according to the software combination “Coccinelle 1.0.8-00104-ge06b9156”).


@display@
expression e, x, y;
@@
*brelse(e);
*y = x(<+... e ...+>, ...);


An usable output is generated then as expected for a test source file
like the following.
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/fs/exfat/nls.c?id=b676fdbcf4c8424f3c02ed7f31576d99b963bded#n652

// SPDX-License-Identifier: GPL-2.0-or-later
// deleted part
static int exfat_load_upcase_table(struct super_block *sb,
sector_t sector, unsigned long long num_sectors,
unsigned int utbl_checksum)
{
struct exfat_sb_info *sbi = EXFAT_SB(sb);
unsigned int sect_size = sb->s_blocksize;
unsigned int i, index = 0;
u32 chksum = 0;
// deleted part
while (sector < num_sectors) {
struct buffer_head *bh;

bh = sb_bread(sb, sector);
// deleted part
brelse(bh);
chksum = exfat_calc_chksum32(bh->b_data, i, chksum, CS_DEFAULT);
}
// deleted part
}
// deleted part


If I omit the specification “, ...” from the function call parameters
because I could be unsure about the number of arguments in other
software situations, I do not get the desired test output as before.

If I omit even the semicolon from the assignment statement in the
search pattern, I get an error message.

elfring@Sonne:~/Projekte/Coccinelle/janitor> spatch --parse-cocci 
show_questionable_brelse_usage6.cocci
…
minus: parse error:
  File "show_questionable_brelse_usage6.cocci", line 6, column 0, charpos = 67
  around = '',
  whole content =


Will such observations influence subsequent software evolution?

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


Re: [Cocci] [PATCH v2 4/4] coccinelle: api: add selfcheck for memdup_user rule

2020-06-09 Thread Julia Lawall



On Mon, 8 Jun 2020, Denis Efremov wrote:

> Check that the rule matches vmemdup_user implementation.
> memdup_user is out of scope because we are not matching
> kmalloc_track_caller() function.

Is this a bit over-enginered?  More precisely, even if it is nice to check
that the API definition has the expected behavior, does it make sense to
do it in one case but not the other?

julia

>
> Signed-off-by: Denis Efremov 
> ---
>  scripts/coccinelle/api/memdup_user.cocci | 46 ++--
>  1 file changed, 44 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/coccinelle/api/memdup_user.cocci 
> b/scripts/coccinelle/api/memdup_user.cocci
> index 8621bd98be1e..78fded83b197 100644
> --- a/scripts/coccinelle/api/memdup_user.cocci
> +++ b/scripts/coccinelle/api/memdup_user.cocci
> @@ -14,13 +14,24 @@ virtual patch
>  virtual context
>  virtual org
>  virtual report
> +virtual selfcheck
>
>  @initialize:python@
>  @@
> -filter = frozenset(['memdup_user', 'vmemdup_user'])
> +
> +definitions = {
> +'memdup_user': 'mm/util.c',
> +'vmemdup_user': 'mm/util.c',
> +}
> +
> +filter = frozenset(definitions.keys())
> +coccinelle.filtered = set()
> +coccinelle.checked_files = set()
>
>  def relevant(p):
> -return not (filter & {el.current_element for el in p})
> +found = filter & {el.current_element for el in p}
> +coccinelle.filtered |= found
> +return not found
>
>  @depends on patch@
>  expression from,to,size;
> @@ -117,3 +128,34 @@ p << rv.p;
>  @@
>
>  coccilib.report.print_report(p[0], "WARNING opportunity for vmemdup_user")
> +
> +@script:python depends on selfcheck@
> +@@
> +coccinelle.checked_files |= set(definitions.values()) & set(cocci.files())
> +
> +@finalize:python depends on selfcheck@
> +filtered << merge.filtered;
> +checked_files << merge.checked_files;
> +@@
> +
> +# Don't check memdup_user because the pattern is not capturing
> +# kmalloc_track_caller() calls
> +del definitions['memdup_user']
> +
> +# mapping between checked files and filtered definitions
> +found_defns = {}
> +for files, funcs in zip(checked_files, filtered):
> +   for file in files:
> +  found_defns[file] = funcs
> +
> +# reverse mapping of definitions
> +expected_defns = {v : set() for v in definitions.values()}
> +for k, v in definitions.items():
> +expected_defns[v] |= {k}
> +
> +for efile, efuncs in expected_defns.items():
> +if efile in found_defns:
> +not_found = efuncs - found_defns[efile]
> +if not_found:
> +print('SELF-CHECK: the pattern no longer matches ' \
> +   'definitions {} in file {}'.format(not_found, efile))
> --
> 2.26.2
>
> ___
> 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 1/4] coccinelle: api: extend memdup_user transformation with GFP_USER

2020-06-09 Thread Markus Elfring
…
> +++ b/scripts/coccinelle/api/memdup_user.cocci
> @@ -20,7 +20,9 @@ expression from,to,size;
…
> +-  to = \(kmalloc\|kzalloc\)
> + (size,\(GFP_KERNEL\|GFP_USER\|
> +   \(GFP_KERNEL\|GFP_USER\)|__GFP_NOWARN\));

I got the impression that this SmPL code needs another correction also
according to the proposed SmPL disjunction.

+-to = \( kmalloc \| kzalloc \) (size, \( GFP_KERNEL \| GFP_USER \) \( | 
__GFP_NOWARN \| \) );


Would you like to express by any other approach that a specific flag
is an optional source code transformation parameter?

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


Re: [Cocci] [PATCH 0/4] parsing_c: parser: Add end_attributes_opt rule

2020-06-09 Thread Julia Lawall



On Mon, 8 Jun 2020, Jaskaran Singh wrote:

> Patches for improving the C parsing of attributes[1] introduced a lot
> redundant code in the C parser. This patch series resolves this by adding
> a rule for optional end attributes and merging the redundant code
> together.

All are applied, thanks.

julia

>
> [1]
> [PATCH v2 00/25] cocci: Improve C parsing of attributes
> https://lore.kernel.org/cocci/20200528122428.4212-1-jaskaransingh7654...@gmail.com/
>
> Jaskaran Singh (4):
>   parsing_c: parser: Add end_attributes_opt rule
>   parsing_c: parser: Use end_attributes_opt in decl2
>   parsing_c: parser: Use end_attributes_opt in field_declaration
>   parsing_c: parser: Use end_attributes_opt in cpp_other
>
>  parser_c.mly |   96 
> +--
>  1 file changed, 9 insertions(+), 87 deletions(-)
>
>
>
>
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci


Re: [Cocci] [PATCH v2 4/4] coccinelle: api: add selfcheck for memdup_user rule

2020-06-09 Thread Markus Elfring
…
> +++ b/scripts/coccinelle/api/memdup_user.cocci
> @@ -14,13 +14,24 @@ virtual patch
>  virtual context
>  virtual org
>  virtual report
> +virtual selfcheck

Would you like to avoid the repetition of a SmPL key word here?

+virtual patch, context, org, report, selfcheck


> @@ -117,3 +128,34 @@ p << rv.p;
>  @@
>
>  coccilib.report.print_report(p[0], "WARNING opportunity for vmemdup_user")
> +
> +@script:python depends on selfcheck@
> +@@
> +coccinelle.checked_files |= set(definitions.values()) & set(cocci.files())

I suggest to reconsider the usage of the function “cocci.files()”.
Can such a script rule determine for which file it should perform data 
processing?


> +print('SELF-CHECK: the pattern no longer matches ' \
> +   'definitions {} in file {}'.format(not_found, efile))

Can the following code variant be a bit nicer?

+sys.stdout.write('SELF-CHECK: The pattern does not match 
definitions {} in file {} any more.\n' \
+ .format(not_found, efile))


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