[Cocci] [RFC PATCH] coccinelle: misc: add flexible_array.cocci script

2020-08-06 Thread Denis Efremov
Commit 68e4cd17e218 ("docs: deprecated.rst: Add zero-length and one-element
arrays") marks one-element and zero-length arrays as deprecated. Kernel
code should always use "flexible array members" instead.

The script warns about one-element and zero-length arrays in structs.

Cc: Kees Cook 
Cc: Gustavo A. R. Silva 
Signed-off-by: Denis Efremov 
---

Currently, it's just a draft. I've placed a number of questions in the
script and marked them as TODO. Kees, Gustavo, if you could help me with
my questions I think that this rule will be enough to close:
https://github.com/KSPP/linux/issues/76

BTW, I it's possible to not warn about files in uapi folder if
this is relevant. Do I need to do it in the script?

 scripts/coccinelle/misc/flexible_array.cocci | 158 +++
 1 file changed, 158 insertions(+)
 create mode 100644 scripts/coccinelle/misc/flexible_array.cocci

diff --git a/scripts/coccinelle/misc/flexible_array.cocci 
b/scripts/coccinelle/misc/flexible_array.cocci
new file mode 100644
index ..1e7165c79e60
--- /dev/null
+++ b/scripts/coccinelle/misc/flexible_array.cocci
@@ -0,0 +1,158 @@
+// SPDX-License-Identifier: GPL-2.0-only
+///
+/// Zero-length and one-element arrays are deprecated, see
+/// Documentation/process/deprecated.rst
+/// Flexible-array members should be used instead.
+///
+//
+// Confidence: High
+// Copyright: (C) 2020 Denis Efremov ISPRAS.
+// Comments:
+// Options: --no-includes --include-headers
+
+virtual context
+virtual report
+virtual org
+virtual patch
+
+@r depends on !patch@
+identifier name, size, array;
+// TODO: We can additionally restrict size and array to:
+// identifier size =~ ".*(num|len|count|size|ncpus).*";
+// identifier array !~ ".*(pad|reserved).*";
+// Do we need it?
+type TS, TA;
+position p;
+@@
+
+(
+  // This will also match: typedef struct name { ...
+  // However nested structs are not matched, i.e.:
+  //   struct name1 { struct name2 { int s; int a[0]; } st; int i; }
+  // will not be matched. Do we need to handle it?
+  struct name {
+...  // TODO: Maybe simple ... is enough? It will match structs with a
+TS size; // single field, e.g.
+...  // 
https://elixir.bootlin.com/linux/v5.8/source/arch/arm/include/uapi/asm/setup.h#L127
+(
+*TA array@p[0];
+|
+ // TODO: It seems that there are exception cases for array[1], e.g.
+ //  
https://elixir.bootlin.com/linux/v5.8/source/arch/powerpc/boot/rs6000.h#L152
+ //  
https://elixir.bootlin.com/linux/v5.8/source/include/uapi/linux/cdrom.h#L292
+ //  
https://elixir.bootlin.com/linux/v5.8/source/drivers/net/wireless/ath/ath6kl/usb.c#L108
+ // We could either drop array[1] checking from this rule or
+ // restrict array name with regexp and add, for example, an "allowlist"
+ // with struct names where we allow this code pattern.
+ // TODO: How to handle: u8 data[1][MAXLEN_PSTR6]; ?
+*TA array@p[1];
+)
+  };
+|
+  struct {
+...
+TS size;
+...
+(
+*TA array@p[0];
+|
+*TA array@p[1];
+)
+  };
+|
+  // TODO: do we need to handle unions?
+  union name {
+...
+TS size;
+...
+(
+*TA array@p[0];
+|
+*TA array@p[1];
+)
+  };
+|
+  union {
+...
+TS size;
+...
+(
+*TA array@p[0];
+|
+*TA array@p[1];
+)
+  };
+)
+
+// FIXME: Patch mode doesn't work as expected.
+// Coccinelle handles formatting incorrectly.
+// Patch mode in this rule should be disabled until
+// proper formatting will be supported.
+@depends on patch exists@
+identifier name, size, array;
+type TS, TA;
+@@
+
+(
+  struct name {
+...
+TS size;
+...
+(
+-TA array[0];
+|
+-TA array[1];
+)
++TA array[];
+  };
+|
+  struct {
+...
+TS size;
+...
+(
+-TA array[0];
+|
+-TA array[1];
+)
++TA array[];
+  };
+|
+  union name {
+...
+TS size;
+...
+(
+-TA array[0];
+|
+-TA array[1];
+)
++TA array[];
+  };
+|
+  union {
+...
+TS size;
+...
+(
+-TA array[0];
+|
+-TA array[1];
+)
++TA array[];
+  };
+)
+
+@script: python depends on report@
+p << r.p;
+@@
+
+msg = "WARNING: use flexible-array member instead"
+coccilib.report.print_report(p[0], msg)
+
+@script: python depends on org@
+p << r.p;
+@@
+
+msg = "WARNING: use flexible-array member instead"
+coccilib.org.print_todo(p, msg)
-- 
2.26.2

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


Re: [Cocci] [PATCH] scripts: add dummy report mode to add_namespace.cocci

2020-08-06 Thread Shuah Khan

On 8/6/20 1:57 PM, Greg Kroah-Hartman wrote:

On Thu, Aug 06, 2020 at 01:48:26PM -0600, Shuah Khan wrote:

On 6/22/20 10:14 AM, Shuah Khan wrote:

On 6/22/20 9:06 AM, Greg Kroah-Hartman wrote:

On Mon, Jun 22, 2020 at 08:46:18AM -0600, Shuah Khan wrote:

On 6/22/20 2:03 AM, Matthias Maennich wrote:

On Thu, Jun 04, 2020 at 02:39:18PM -0600, Shuah Khan wrote:

On 6/4/20 1:31 PM, Julia Lawall wrote:



On Thu, 4 Jun 2020, Matthias Maennich wrote:


When running `make coccicheck` in report mode using the
add_namespace.cocci file, it will fail for files that contain
MODULE_LICENSE. Those match the replacement precondition, but spatch
errors out as virtual.ns is not set.

In order to fix that, add the virtual rule nsdeps and only
do search and
replace if that rule has been explicitly requested.

In order to make spatch happy in report mode, we also need a
dummy rule,
as otherwise it errors out with "No rules
apply". Using a script:python
rule appears unrelated and odd, but this is the shortest I
could come up
with.

Adjust scripts/nsdeps accordingly to set the nsdeps rule
when run trough
`make nsdeps`.

Suggested-by: Julia Lawall 
Fixes: c7c4e29fb5a4 ("scripts: add_namespace:
Fix coccicheck failed")
Cc: YueHaibing 
Cc: j...@kernel.org
Cc: cocci@systeme.lip6.fr
Cc: sta...@vger.kernel.org
Signed-off-by: Matthias Maennich 


Acked-by: Julia Lawall 

Shuah reported the problem to me, so you could add

Reported-by: Shuah Khan 



Very cool. No errors with this patch. Thanks for fixing it
quickly.


I am happy I could fix that and thanks for confirming. I assume your
Tested-by could be added?


Yes

Tested-by: Shuah Khan 


Is somebody willing to take this patch through their tree?



My guess is that these go through kbuild git??


If you want to take this, that's fine with me.  But as I had the
original file come through my tree, I can take it too.  It's up to you,
either is ok with me.



Great. Please take this through your tree.



Greg! Looks like this one didn't make it in. Can you pick this up?


I think this is 55c7549819e4 ("scripts: add dummy report mode to
add_namespace.cocci") in Linus's tree right now, right?



Yes. It is in Linux 5.9. I was looking in the wrong place on
Linux 5.8. :(


thanks,
-- Shuah

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


Re: [Cocci] [PATCH] scripts: add dummy report mode to add_namespace.cocci

2020-08-06 Thread Shuah Khan

On 6/22/20 10:14 AM, Shuah Khan wrote:

On 6/22/20 9:06 AM, Greg Kroah-Hartman wrote:

On Mon, Jun 22, 2020 at 08:46:18AM -0600, Shuah Khan wrote:

On 6/22/20 2:03 AM, Matthias Maennich wrote:

On Thu, Jun 04, 2020 at 02:39:18PM -0600, Shuah Khan wrote:

On 6/4/20 1:31 PM, Julia Lawall wrote:



On Thu, 4 Jun 2020, Matthias Maennich wrote:


When running `make coccicheck` in report mode using the
add_namespace.cocci file, it will fail for files that contain
MODULE_LICENSE. Those match the replacement precondition, but spatch
errors out as virtual.ns is not set.

In order to fix that, add the virtual rule nsdeps and only
do search and
replace if that rule has been explicitly requested.

In order to make spatch happy in report mode, we also need a
dummy rule,
as otherwise it errors out with "No rules apply". Using a 
script:python

rule appears unrelated and odd, but this is the shortest I
could come up
with.

Adjust scripts/nsdeps accordingly to set the nsdeps rule
when run trough
`make nsdeps`.

Suggested-by: Julia Lawall 
Fixes: c7c4e29fb5a4 ("scripts: add_namespace: Fix coccicheck 
failed")

Cc: YueHaibing 
Cc: j...@kernel.org
Cc: cocci@systeme.lip6.fr
Cc: sta...@vger.kernel.org
Signed-off-by: Matthias Maennich 


Acked-by: Julia Lawall 

Shuah reported the problem to me, so you could add

Reported-by: Shuah Khan 



Very cool. No errors with this patch. Thanks for fixing it
quickly.


I am happy I could fix that and thanks for confirming. I assume your
Tested-by could be added?


Yes

Tested-by: Shuah Khan 


Is somebody willing to take this patch through their tree?



My guess is that these go through kbuild git??


If you want to take this, that's fine with me.  But as I had the
original file come through my tree, I can take it too.  It's up to you,
either is ok with me.



Great. Please take this through your tree.



Greg! Looks like this one didn't make it in. Can you pick this up?

thanks,
-- Shuah

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


Re: [Cocci] [PATCH] scripts: add dummy report mode to add_namespace.cocci

2020-08-06 Thread Greg Kroah-Hartman
On Thu, Aug 06, 2020 at 01:48:26PM -0600, Shuah Khan wrote:
> On 6/22/20 10:14 AM, Shuah Khan wrote:
> > On 6/22/20 9:06 AM, Greg Kroah-Hartman wrote:
> > > On Mon, Jun 22, 2020 at 08:46:18AM -0600, Shuah Khan wrote:
> > > > On 6/22/20 2:03 AM, Matthias Maennich wrote:
> > > > > On Thu, Jun 04, 2020 at 02:39:18PM -0600, Shuah Khan wrote:
> > > > > > On 6/4/20 1:31 PM, Julia Lawall wrote:
> > > > > > > 
> > > > > > > 
> > > > > > > On Thu, 4 Jun 2020, Matthias Maennich wrote:
> > > > > > > 
> > > > > > > > When running `make coccicheck` in report mode using the
> > > > > > > > add_namespace.cocci file, it will fail for files that contain
> > > > > > > > MODULE_LICENSE. Those match the replacement precondition, but 
> > > > > > > > spatch
> > > > > > > > errors out as virtual.ns is not set.
> > > > > > > > 
> > > > > > > > In order to fix that, add the virtual rule nsdeps and only
> > > > > > > > do search and
> > > > > > > > replace if that rule has been explicitly requested.
> > > > > > > > 
> > > > > > > > In order to make spatch happy in report mode, we also need a
> > > > > > > > dummy rule,
> > > > > > > > as otherwise it errors out with "No rules
> > > > > > > > apply". Using a script:python
> > > > > > > > rule appears unrelated and odd, but this is the shortest I
> > > > > > > > could come up
> > > > > > > > with.
> > > > > > > > 
> > > > > > > > Adjust scripts/nsdeps accordingly to set the nsdeps rule
> > > > > > > > when run trough
> > > > > > > > `make nsdeps`.
> > > > > > > > 
> > > > > > > > Suggested-by: Julia Lawall 
> > > > > > > > Fixes: c7c4e29fb5a4 ("scripts: add_namespace:
> > > > > > > > Fix coccicheck failed")
> > > > > > > > Cc: YueHaibing 
> > > > > > > > Cc: j...@kernel.org
> > > > > > > > Cc: cocci@systeme.lip6.fr
> > > > > > > > Cc: sta...@vger.kernel.org
> > > > > > > > Signed-off-by: Matthias Maennich 
> > > > > > > 
> > > > > > > Acked-by: Julia Lawall 
> > > > > > > 
> > > > > > > Shuah reported the problem to me, so you could add
> > > > > > > 
> > > > > > > Reported-by: Shuah Khan 
> > > > > > > 
> > > > > > 
> > > > > > Very cool. No errors with this patch. Thanks for fixing it
> > > > > > quickly.
> > > > > 
> > > > > I am happy I could fix that and thanks for confirming. I assume your
> > > > > Tested-by could be added?
> > > > 
> > > > Yes
> > > > 
> > > > Tested-by: Shuah Khan 
> > > > > 
> > > > > Is somebody willing to take this patch through their tree?
> > > > > 
> > > > 
> > > > My guess is that these go through kbuild git??
> > > 
> > > If you want to take this, that's fine with me.  But as I had the
> > > original file come through my tree, I can take it too.  It's up to you,
> > > either is ok with me.
> > > 
> > 
> > Great. Please take this through your tree.
> > 
> 
> Greg! Looks like this one didn't make it in. Can you pick this up?

I think this is 55c7549819e4 ("scripts: add dummy report mode to
add_namespace.cocci") in Linus's tree right now, right?

thanks,

greg k-h
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci