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

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

[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

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

[Denis Efremov]

On 6/9/20 7:22 PM, Julia Lawall wrote:
> 
> 
> 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?

Last patch it's just a PoC. Patches 1-3 are independent from 4.

> 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?

Yes, I also don't like it. However, I doubt that we need to match
kmalloc_track_caller.

Thanks,
Denis
_______________________________________________
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

[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 <efremov@linux.com>
> ---
>  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 4/4] coccinelle: api: add selfcheck for memdup_user rule

[Markus Elfring]

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

I find this change description improvable.
Will it become helpful (for example) to mention that you would like to
add another operation mode?


> +@finalize:python depends on selfcheck@
> +filtered << merge.filtered;
> +checked_files << merge.checked_files;
> +@@

Are we looking for better software documentation for such functionality?

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

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

[Denis Efremov]

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

Signed-off-by: Denis Efremov <efremov@linux.com>
---
 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