cocci.inria.fr archive mirror
 help / color / mirror / Atom feed
From: Julia Lawall <julia.lawall@inria.fr>
To: Mansour Moufid <mansourmoufid@gmail.com>
Cc: Joe Perches <joe@perches.com>, cocci <cocci@systeme.lip6.fr>
Subject: Re: [Cocci] linux-kernel janitorial RFP: Mark static arrays as const
Date: Wed, 3 Mar 2021 18:21:23 +0100 (CET)	[thread overview]
Message-ID: <alpine.DEB.2.22.394.2103031811410.2980@hadrien> (raw)
In-Reply-To: <CALogXGVMCiZcMRovK+9gJVKQPDJJdWUuXRPXVZ0fxmAXyq4Uig@mail.gmail.com>



On Wed, 3 Mar 2021, Mansour Moufid wrote:

> On Tue, Mar 2, 2021 at 4:22 PM Joe Perches <joe@perches.com> 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
|
 &x@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

      reply	other threads:[~2021-03-03 17:21 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-02 17:42 [Cocci] linux-kernel janitorial RFP: Mark static arrays as const Joe Perches
2021-03-02 21:41 ` Julia Lawall
2021-03-03  2:47   ` Joe Perches
2021-03-02 22:18 ` Bernd Petrovitsch
2021-03-03  8:36   ` Julia Lawall
2021-03-04  8:34   ` Andy Shevchenko
2021-03-03  9:41 ` Rasmus Villemoes
2021-03-03 14:51   ` Joe Perches
2021-03-07 19:14     ` Julia Lawall
2021-03-08  5:38       ` Joe Perches
2021-03-08  6:54         ` Julia Lawall
2021-03-03 17:06 ` Mansour Moufid
2021-03-03 17:21   ` Julia Lawall [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.DEB.2.22.394.2103031811410.2980@hadrien \
    --to=julia.lawall@inria.fr \
    --cc=cocci@systeme.lip6.fr \
    --cc=joe@perches.com \
    --cc=mansourmoufid@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).