[PATCH 0/3 RFC] Coccinelle: completion API checking

[PATCH 0/3 RFC] Coccinelle: completion API checking

[Nicholas Mc Guire]

This little set of semantic patches is for partially checking the
completion API. It seems to be working correctly and has not yet
produced any false-positive.

The cases being detected are:

1/3 - duplicate init_completions.
2/3 - incorrect static initialization of completion on stack.
3/3 - re-initialization of completion with init_completion() rather
      than reinit_completion().

semantic patch                  findings   files  confirmed
duplicate_init_completion.cocci    2         2        2
false_declare_completion.cocci     6         5        3 
false_init_compltion.cocci         9         6        5 

Note: false_declare_completion.cocci runs extremely slow on my system
      roughly a factor 10 slower than the other two - not clear why.

All findings have been submitted based on these scripts. Some have 
not yet been confirmed.

Note sure if it makes sense putting this type of API misuse cleanup
into mainline given the relatively small number of findings.

patch is against linux-next 3.19.0-rc1 (-next-20141226)

[PATCH 1/3 RFC] Coccinelle: drop unecessary duplicated init_compltion calls

[Nicholas Mc Guire]

Reviewed-by: Andreas Platschek <platschek@ict.tuwien.ac.at>
Signed-off-by: Nicholas Mc Guire <der.herr@hofr.at>
---
 .../coccinelle/api/duplicate_init_completion.cocci |   59 ++++++++++++++++++++
 1 file changed, 59 insertions(+)
 create mode 100644 scripts/coccinelle/api/duplicate_init_completion.cocci

diff --git a/scripts/coccinelle/api/duplicate_init_completion.cocci b/scripts/coccinelle/api/duplicate_init_completion.cocci
new file mode 100644
index 0000000..889715a
--- /dev/null
+++ b/scripts/coccinelle/api/duplicate_init_completion.cocci
@@ -0,0 +1,59 @@
+/* drop unnecessary duplicated init_compltion calls
+ * in patch mode one must be check that it is actually
+ * the second init_completion that can be removed and not
+ * the first.
+ * Options: --no-includes --include-headers
+ */
+virtual context
+virtual patch
+virtual org
+virtual report
+
+/* mark first call to init_completion */
+@c@
+expression cmp;
+position p;
+@@
+
+ init_completion@p(cmp)
+
+/* flag duplicate initializations */
+@d depends on patch && !(context || org || report)@
+expression E,c.cmp;
+identifier f;
+position c.p,p1;
+@@
+
+  init_completion@p(cmp)
+  ... when != E = cmp
+      when != E = &cmp
+      when != f(..., cmp, ...)
+      when != f(..., &cmp, ...)
+- init_completion@p1(cmp);
+
+@dp depends on !patch && (context || org || report)@
+identifier f;
+expression E,c.cmp;
+position c.p,p1;
+@@
+
+  init_completion@p(cmp)
+  ... when != E = cmp
+      when != E = &cmp
+      when != f(..., cmp, ...)
+      when != f(..., &cmp, ...)
+  init_completion@p1(cmp);
+
+@script:python depends on org@
+p << dp.p1;
+@@
+
+msg="WARNING: possible duplicate init_completion"
+coccilib.org.print_todo(p[0], msg)
+
+@script:python depends on report@
+p << dp.p1;
+@@
+
+msg="WARNING: possible duplicate init_completion"
+coccilib.report.print_report(p[0], msg)
-- 
1.7.10.4

[PATCH 2/3 RFC] Coccinelle: check for incorrect DECLARE_COMPLETION use

[Nicholas Mc Guire]

Reviewed-by: Andreas Platschek <platschek@ict.tuwien.ac.at>
Signed-off-by: Nicholas Mc Guire <der.herr@hofr.at>
---
 .../coccinelle/api/false_declare_completion.cocci  |   50 ++++++++++++++++++++
 1 file changed, 50 insertions(+)
 create mode 100644 scripts/coccinelle/api/false_declare_completion.cocci

diff --git a/scripts/coccinelle/api/false_declare_completion.cocci b/scripts/coccinelle/api/false_declare_completion.cocci
new file mode 100644
index 0000000..a0e3d6c
--- /dev/null
+++ b/scripts/coccinelle/api/false_declare_completion.cocci
@@ -0,0 +1,50 @@
+/* check for incorrect DECLARE_COMPLETION use within a function
+ *
+ * Options:  --no-includes --include-headers
+ */
+virtual context
+virtual patch
+virtual org
+virtual report
+
+/* flag incorrect initializer*/
+@e depends on patch && !(context || org || report)@
+expression cmp;
+identifier f;
+declarer name DECLARE_COMPLETION;
+position p;
+@@
+
+f(...) {
+  <...
+- DECLARE_COMPLETION@p(cmp);
++ DECLARE_COMPLETION_ONSTACK(cmp);
+  ...>
+}
+
+@ep depends on !patch && (context || org || report)@
+expression cmp;
+identifier f;
+position p;
+@@
+
+f(...) {
+  <...
+* DECLARE_COMPLETION@p(cmp);
+  ...>
+}
+
+@script:python depends on org@
+p << ep.p;
+@@
+
+msg="WARNING: possible incorrect use of DECLARE_COMPLETION"
+coccilib.org.print_todo(p[0], msg)
+
+@script:python depends on report@
+p << ep.p;
+@@
+
+msg="WARNING: possible incorrect use of DECLARE_COMPLETION"
+coccilib.report.print_report(p[0], msg)
+
-- 
1.7.10.4

[PATCH 3/3 RFC] Coccinelle: incorrect use of multiple init_completion

[Nicholas Mc Guire]

Reviewed-by: Andreas Platschek <platschek@ict.tuwien.ac.at>
Signed-off-by: Nicholas Mc Guire <der.herr@hofr.at>
---
 scripts/coccinelle/api/false_init_compltion.cocci |   74 +++++++++++++++++++++
 1 file changed, 74 insertions(+)
 create mode 100644 scripts/coccinelle/api/false_init_compltion.cocci

diff --git a/scripts/coccinelle/api/false_init_compltion.cocci b/scripts/coccinelle/api/false_init_compltion.cocci
new file mode 100644
index 0000000..a97dfcf
--- /dev/null
+++ b/scripts/coccinelle/api/false_init_compltion.cocci
@@ -0,0 +1,74 @@
+/* check for incorrect use of multiple init_completion on the
+ * same struct completion and switch the reinitializations to use
+ * reinit_completion()
+ *
+ * Options: --no-includes --include-headers
+ */
+virtual context
+virtual patch
+virtual org
+virtual report
+
+/* mark first call to init_completion */
+@c@
+expression cmp;
+position p;
+@@
+
+ init_completion@p(cmp)
+
+/* flag incorrect reinitializations */
+@d depends on patch && !(context || org || report)@
+expression E,c.cmp;
+identifier f;
+position c.p,p1;
+@@
+
+  init_completion@p(cmp)
+  <+...
+(
+  E = cmp
+|
+  E = &cmp
+|
+  f(..., cmp,...)
+|
+  f(..., &cmp,...)
+)
+  ...+>
+- init_completion@p1(cmp)
++ reinit_completion1(cmp)
+
+@dp depends on !patch && (context || org || report)@
+identifier f;
+expression E,c.cmp;
+position c.p,p1;
+@@
+
+  init_completion@p(cmp)
+  <+...
+(
+  E = cmp
+|
+  E = &cmp
+|
+  f(..., cmp,...)
+|
+  f(..., &cmp,...)
+)
+  ...+>
+  init_completion@p1(cmp)
+
+@script:python depends on org@
+p << dp.p1;
+@@
+
+msg="WARNING: possible reinit by init_completion"
+coccilib.org.print_todo(p[0], msg)
+
+@script:python depends on report@
+p << dp.p1;
+@@
+
+msg="WARNING: possible reinit by init_completion"
+coccilib.report.print_report(p[0], msg)
-- 
1.7.10.4

Re: [PATCH 0/3 RFC] Coccinelle: completion API checking

[Julia Lawall]

On Fri, 2 Jan 2015, Nicholas Mc Guire wrote:

>
> This little set of semantic patches is for partially checking the
> completion API. It seems to be working correctly and has not yet
> produced any false-positive.
>
> The cases being detected are:
>
> 1/3 - duplicate init_completions.
> 2/3 - incorrect static initialization of completion on stack.
> 3/3 - re-initialization of completion with init_completion() rather
>       than reinit_completion().
>
> semantic patch                  findings   files  confirmed
> duplicate_init_completion.cocci    2         2        2
> false_declare_completion.cocci     6         5        3
> false_init_compltion.cocci         9         6        5
>
> Note: false_declare_completion.cocci runs extremely slow on my system
>       roughly a factor 10 slower than the other two - not clear why.

The problem is the use of <... ...>.  That is good because it allows
DECLARE_COMPLETION not to appear on some execution paths, but it is not so
good because it allows DECLARE_COMPLETION to appear on no execution paths
at all.  So the semantic patch is considered to be relevant to every file,
and every file has to be parsed, which takes a lot of time.

To fix the problem, you can first check whether the call actually exists
in each function:

@pre_e exists@
identifier f;
declarer name DECLARE_COMPLETION;
@@

f(...) {
  <+...
  DECLARE_COMPLETION(...);
  ...+>
}

Then only run the second rules on functions for which pre_e is satisfied.
That is, in the rule e, instead of putting identifier f, you can put
identifier pre_e.f.  You can do the same in the rule ep.  Ep is already
looking for the existence of an execution path, so for ep you could also
just replace the <... ...> by <+... ...+> instead of relying on pre_e.

julia