LinuxLists.cc - [RFC PATCH] coccinelle: misc: add uninitialized

[permalink] [raw]

Subject: Re: [RFC PATCH] coccinelle: misc: add uninitialized_var.cocci script

On Wed, 12 Aug 2020, Markus Elfring wrote:

> > +@r@
> > +identifier var;
> > +type T;
> > +position p;
> > +@@
> > +
> > +(
> > +* T var@p = var;
> > +|
> > +* T var@p = *(&(var));
>
> I suggest to simplify such code for the semantic patch language a bit.
> Can an other variant be more succinct for the application of a SmPL disjunction?
>
> +*T var@p = $ var \| *(&(var)) $;

It's fine as is.

julia

2020-08-29 19:37:41

[permalink] [raw]

Subject: Re: [RFC PATCH] coccinelle: misc: add uninitialized_var.cocci script

On Wed, 12 Aug 2020, Denis Efremov wrote:

> Commit 63a0895d960a ("compiler: Remove uninitialized_var() macro") and
> commit 4b19bec97c88 ("docs: deprecated.rst: Add uninitialized_var()")
> removed uninitialized_var() and deprecated it.
>
> The purpose of this script is to prevent new occurrences of open-coded
> variants of uninitialized_var().
>
> Cc: Kees Cook <[email protected]>
> Cc: Gustavo A. R. Silva <[email protected]>
> Signed-off-by: Denis Efremov <[email protected]>

Applied, without the commented out part.

I only got three warnings, though. Perhaps the others have been fixed?

lib/glob.c:48:31-39: WARNING: this kind of initialization is deprecated
drivers/gpu/drm/nouveau/nvkm/subdev/instmem/nv50.c:316:7-10: WARNING: this kind of initialization is deprecated
tools/testing/selftests/vm/userfaultfd.c:349:15-22: WARNING: this kind of initialization is deprecated

julia

> ---
> List of warnings:
> ./lib/glob.c:48:31-39: WARNING: this kind of initialization is deprecated
> ./tools/testing/selftests/vm/userfaultfd.c:349:15-22: WARNING: this kind of initialization is deprecated
> ./drivers/block/drbd/drbd_vli.h:330:5-9: WARNING: this kind of initialization is deprecated
> ./drivers/char/hw_random/intel-rng.c:333:15-18: WARNING: this kind of initialization is deprecated
> ./drivers/gpu/drm/nouveau/nvkm/subdev/instmem/nv50.c:316:7-10: WARNING: this kind of initialization is deprecated
> ./arch/x86/include/asm/paravirt_types.h:455:15-20: WARNING: this kind of initialization is deprecated
> ./arch/x86/include/asm/paravirt_types.h:455:30-35: WARNING: this kind of initialization is deprecated
> ./arch/x86/include/asm/paravirt_types.h:455:45-50: WARNING: this kind of initialization is deprecated
> ./arch/x86/include/asm/paravirt_types.h:475:15-20: WARNING: this kind of initialization is deprecated
> ./arch/x86/include/asm/paravirt_types.h:475:30-35: WARNING: this kind of initialization is deprecated
> ./arch/x86/include/asm/paravirt_types.h:476:2-7: WARNING: this kind of initialization isdeprecated
> ./arch/x86/include/asm/paravirt_types.h:476:17-22: WARNING: this kind of initialization is deprecated
> ./arch/x86/include/asm/paravirt_types.h:476:32-37: WARNING: this kind of initialization is deprecated
>
> .../coccinelle/misc/uninitialized_var.cocci | 51 +++++++++++++++++++
> 1 file changed, 51 insertions(+)
> create mode 100644 scripts/coccinelle/misc/uninitialized_var.cocci
>
> diff --git a/scripts/coccinelle/misc/uninitialized_var.cocci b/scripts/coccinelle/misc/uninitialized_var.cocci
> new file mode 100644
> index 000000000000..e4787bc6ab9c
> --- /dev/null
> +++ b/scripts/coccinelle/misc/uninitialized_var.cocci
> @@ -0,0 +1,51 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +///
> +/// uninitialized_var() and its open-coded variations are
> +/// deprecated. For details, see:
> +/// Documentation/process/deprecated.rst
> +///
> +// Confidence: High
> +// Copyright: (C) 2020 Denis Efremov ISPRAS
> +// Options: --no-includes --include-headers
> +//
> +
> +virtual context
> +virtual report
> +virtual org
> +
> +@r@
> +identifier var;
> +type T;
> +position p;
> +@@
> +
> +(
> +* T var@p = var;
> +|
> +* T var@p = *(&(var));
> +//|
> +// TODO: Actually, I'm not sure about this pattern.
> +// Looks like it's used in wireless drivers to determine
> +// whether data belongs to the driver or not.
> +// Here are all matches:
> +// https://elixir.bootlin.com/linux/latest/source/net/mac802154/util.c#L14
> +// https://elixir.bootlin.com/linux/latest/source/drivers/staging/wlan-ng/cfg80211.c#L48
> +// https://elixir.bootlin.com/linux/latest/source/drivers/net/wireless/intersil/orinoco/cfg.c#L21
> +// https://elixir.bootlin.com/linux/latest/source/net/mac80211/util.c#L37
> +// https://elixir.bootlin.com/linux/latest/source/drivers/net/wireless/rndis_wlan.c#L544
> +// * T *var@p = &var;
> +)
> +
> +@script:python depends on report@
> +p << r.p;
> +@@
> +
> +coccilib.report.print_report(p[0],
> + "WARNING: this kind of initialization is deprecated")
> +
> +@script:python depends on org@
> +p << r.p;
> +@@
> +
> +coccilib.org.print_todo(p[0],
> + "WARNING: this kind of initialization is deprecated")
> --
> 2.26.2
>
>

2020-08-29 19:41:45

[permalink] [raw]

Subject: Re: [RFC PATCH] coccinelle: misc: add uninitialized_var.cocci script

On Sat, 2020-08-29 at 21:36 +0200, Julia Lawall wrote:
>
> On Wed, 12 Aug 2020, Denis Efremov wrote:
>
> > Commit 63a0895d960a ("compiler: Remove uninitialized_var() macro") and
> > commit 4b19bec97c88 ("docs: deprecated.rst: Add uninitialized_var()")
> > removed uninitialized_var() and deprecated it.
> >
> > The purpose of this script is to prevent new occurrences of open-coded
> > variants of uninitialized_var().

> > Cc: Kees Cook <[email protected]>
> > Cc: Gustavo A. R. Silva <[email protected]>
> > Signed-off-by: Denis Efremov <[email protected]>
>
> Applied, without the commented out part.
>
> I only got three warnings, though. Perhaps the others have been fixed?

uninitialized_var does not exist in -next

2020-08-29 19:50:29

[permalink] [raw]

Subject: Re: [RFC PATCH] coccinelle: misc: add uninitialized_var.cocci script

On Sat, 29 Aug 2020, Joe Perches wrote:

> On Sat, 2020-08-29 at 21:36 +0200, Julia Lawall wrote:
> >
> > On Wed, 12 Aug 2020, Denis Efremov wrote:
> >
> > > Commit 63a0895d960a ("compiler: Remove uninitialized_var() macro") and
> > > commit 4b19bec97c88 ("docs: deprecated.rst: Add uninitialized_var()")
> > > removed uninitialized_var() and deprecated it.
> > >
> > > The purpose of this script is to prevent new occurrences of open-coded
> > > variants of uninitialized_var().
>
> > > Cc: Kees Cook <[email protected]>
> > > Cc: Gustavo A. R. Silva <[email protected]>
> > > Signed-off-by: Denis Efremov <[email protected]>
> >
> > Applied, without the commented out part.
> >
> > I only got three warnings, though. Perhaps the others have been fixed?
>
> uninitialized_var does not exist in -next

OK, if it seems better, I can remove it. Out of the threee reported, one
was a completely unnecessary initialization.

julia

2020-08-29 20:16:11

[permalink] [raw]

Subject: Re: [RFC PATCH] coccinelle: misc: add uninitialized_var.cocci script

On 8/29/20 10:48 PM, Julia Lawall wrote:
>
>
> On Sat, 29 Aug 2020, Joe Perches wrote:
>
>> On Sat, 2020-08-29 at 21:36 +0200, Julia Lawall wrote:
>>>
>>> On Wed, 12 Aug 2020, Denis Efremov wrote:
>>>
>>>> Commit 63a0895d960a ("compiler: Remove uninitialized_var() macro") and
>>>> commit 4b19bec97c88 ("docs: deprecated.rst: Add uninitialized_var()")
>>>> removed uninitialized_var() and deprecated it.
>>>>
>>>> The purpose of this script is to prevent new occurrences of open-coded
>>>> variants of uninitialized_var().
>>
>>>> Cc: Kees Cook <[email protected]>
>>>> Cc: Gustavo A. R. Silva <[email protected]>
>>>> Signed-off-by: Denis Efremov <[email protected]>
>>>
>>> Applied, without the commented out part.
>>>
>>> I only got three warnings, though. Perhaps the others have been fixed?
>>
>> uninitialized_var does not exist in -next

Yes, and this rule checks for not introducing these initializations once again.

i.e, checks for:

int a = a;

int a = *(&a);

>
> OK, if it seems better, I can remove it. Out of the threee reported, one
> was a completely unnecessary initialization.
>

I would like send v2 with better description and link to the documentation because it's
now available online:
https://www.kernel.org/doc/html/latest/process/deprecated.html#uninitialized-var

Thanks,
Denis

2020-08-29 20:29:42

[permalink] [raw]

Subject: Re: [RFC PATCH] coccinelle: misc: add uninitialized_var.cocci script

On Sat, 29 Aug 2020, Denis Efremov wrote:

>
>
> On 8/29/20 10:48 PM, Julia Lawall wrote:
> >
> >
> > On Sat, 29 Aug 2020, Joe Perches wrote:
> >
> >> On Sat, 2020-08-29 at 21:36 +0200, Julia Lawall wrote:
> >>>
> >>> On Wed, 12 Aug 2020, Denis Efremov wrote:
> >>>
> >>>> Commit 63a0895d960a ("compiler: Remove uninitialized_var() macro") and
> >>>> commit 4b19bec97c88 ("docs: deprecated.rst: Add uninitialized_var()")
> >>>> removed uninitialized_var() and deprecated it.
> >>>>
> >>>> The purpose of this script is to prevent new occurrences of open-coded
> >>>> variants of uninitialized_var().
> >>
> >>>> Cc: Kees Cook <[email protected]>
> >>>> Cc: Gustavo A. R. Silva <[email protected]>
> >>>> Signed-off-by: Denis Efremov <[email protected]>
> >>>
> >>> Applied, without the commented out part.
> >>>
> >>> I only got three warnings, though. Perhaps the others have been fixed?
> >>
> >> uninitialized_var does not exist in -next
>
> Yes, and this rule checks for not introducing these initializations once again.
>
> i.e, checks for:
>
> int a = a;
>
> int a = *(&a);
>
> >
> > OK, if it seems better, I can remove it. Out of the threee reported, one
> > was a completely unnecessary initialization.
> >
>
> I would like send v2 with better description and link to the documentation because it's
> now available online:
> https://www.kernel.org/doc/html/latest/process/deprecated.html#uninitialized-var

OK, thanks.

julia

2020-09-01 07:19:22

[permalink] [raw]

Subject: [PATCH v2] coccinelle: misc: add uninitialized_var.cocci script

Commit 63a0895d960a ("compiler: Remove uninitialized_var() macro") and
commit 4b19bec97c88 ("docs: deprecated.rst: Add uninitialized_var()")
removed uninitialized_var() and deprecated it.

The purpose of this script is to prevent new occurrences of open-coded
variants of uninitialized_var().

Cc: Kees Cook <[email protected]>
Cc: Gustavo A. R. Silva <[email protected]>
Signed-off-by: Denis Efremov <[email protected]>
---
Changes in v2:
- Documentation cited in the script's description
- kernel.org link added to the diagnostics messages
- "T *var = &var;" pattern removed
- "var =@p var", "var =@p *(&(var))" patterns added

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

diff --git a/scripts/coccinelle/misc/uninitialized_var.cocci b/scripts/coccinelle/misc/uninitialized_var.cocci
new file mode 100644
index 000000000000..8fa845cefe11
--- /dev/null
+++ b/scripts/coccinelle/misc/uninitialized_var.cocci
@@ -0,0 +1,51 @@
+// SPDX-License-Identifier: GPL-2.0-only
+///
+/// Please, don't reintroduce uninitialized_var().
+/// From Documentation/process/deprecated.rst:
+/// For any compiler warnings about uninitialized variables, just add
+/// an initializer. Using warning-silencing tricks is dangerous as it
+/// papers over real bugs (or can in the future), and suppresses unrelated
+/// compiler warnings (e.g. "unused variable"). If the compiler thinks it
+/// is uninitialized, either simply initialize the variable or make compiler
+/// changes. Keep in mind that in most cases, if an initialization is
+/// obviously redundant, the compiler's dead-store elimination pass will make
+/// sure there are no needless variable writes.
+///
+// Confidence: High
+// Copyright: (C) 2020 Denis Efremov ISPRAS
+// Options: --no-includes --include-headers
+//
+
+virtual context
+virtual report
+virtual org
+
+@r@
+identifier var;
+type T;
+position p;
+@@
+
+(
+* T var =@p var;
+|
+* T var =@p *(&(var));
+|
+* var =@p var
+|
+* var =@p *(&(var))
+)
+
+@script:python depends on report@
+p << r.p;
+@@
+
+coccilib.report.print_report(p[0],
+ "WARNING this kind of initialization is deprecated (https://www.kernel.org/doc/html/latest/process/deprecated.html#uninitialized-var)")
+
+@script:python depends on org@
+p << r.p;
+@@
+
+coccilib.org.print_todo(p[0],
+ "WARNING this kind of initialization is deprecated (https://www.kernel.org/doc/html/latest/process/deprecated.html#uninitialized-var)")
--
2.26.2

2020-09-01 09:09:06

[permalink] [raw]

Subject: Re: [PATCH v2] coccinelle: misc: add uninitialized_var.cocci script

On Tue, 1 Sep 2020, Denis Efremov wrote:

> Commit 63a0895d960a ("compiler: Remove uninitialized_var() macro") and
> commit 4b19bec97c88 ("docs: deprecated.rst: Add uninitialized_var()")
> removed uninitialized_var() and deprecated it.

I'm not really sure to understand the above. How can something that has
already been removed be deprecated, since it doesn't exist any more?
Maybe the commits should be mentioned in the opposite order?

Personally, I would find the sentence a lot easier to read without the
commit subject lines in parentheses in the middle of it. It inspires me
to just ignore the paragraph completely. I wonder if it could be allowed
to just mention the commit ids in the sentence and then put the ids
followed by the subject line afterwards?

julia

>
> The purpose of this script is to prevent new occurrences of open-coded
> variants of uninitialized_var().
>
> Cc: Kees Cook <[email protected]>
> Cc: Gustavo A. R. Silva <[email protected]>
> Signed-off-by: Denis Efremov <[email protected]>
> ---
> Changes in v2:
> - Documentation cited in the script's description
> - kernel.org link added to the diagnostics messages
> - "T *var = &var;" pattern removed
> - "var =@p var", "var =@p *(&(var))" patterns added
>
> .../coccinelle/misc/uninitialized_var.cocci | 51 +++++++++++++++++++
> 1 file changed, 51 insertions(+)
> create mode 100644 scripts/coccinelle/misc/uninitialized_var.cocci
>
> diff --git a/scripts/coccinelle/misc/uninitialized_var.cocci b/scripts/coccinelle/misc/uninitialized_var.cocci
> new file mode 100644
> index 000000000000..8fa845cefe11
> --- /dev/null
> +++ b/scripts/coccinelle/misc/uninitialized_var.cocci
> @@ -0,0 +1,51 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +///
> +/// Please, don't reintroduce uninitialized_var().
> +/// From Documentation/process/deprecated.rst:
> +/// For any compiler warnings about uninitialized variables, just add
> +/// an initializer. Using warning-silencing tricks is dangerous as it
> +/// papers over real bugs (or can in the future), and suppresses unrelated
> +/// compiler warnings (e.g. "unused variable"). If the compiler thinks it
> +/// is uninitialized, either simply initialize the variable or make compiler
> +/// changes. Keep in mind that in most cases, if an initialization is
> +/// obviously redundant, the compiler's dead-store elimination pass will make
> +/// sure there are no needless variable writes.
> +///
> +// Confidence: High
> +// Copyright: (C) 2020 Denis Efremov ISPRAS
> +// Options: --no-includes --include-headers
> +//
> +
> +virtual context
> +virtual report
> +virtual org
> +
> +@r@
> +identifier var;
> +type T;
> +position p;
> +@@
> +
> +(
> +* T var =@p var;
> +|
> +* T var =@p *(&(var));
> +|
> +* var =@p var
> +|
> +* var =@p *(&(var))
> +)
> +
> +@script:python depends on report@
> +p << r.p;
> +@@
> +
> +coccilib.report.print_report(p[0],
> + "WARNING this kind of initialization is deprecated (https://www.kernel.org/doc/html/latest/process/deprecated.html#uninitialized-var)")
> +
> +@script:python depends on org@
> +p << r.p;
> +@@
> +
> +coccilib.org.print_todo(p[0],
> + "WARNING this kind of initialization is deprecated (https://www.kernel.org/doc/html/latest/process/deprecated.html#uninitialized-var)")
> --
> 2.26.2
>
>

2020-09-01 09:49:50

[permalink] [raw]

Subject: [PATCH v3] coccinelle: misc: add uninitialized_var.cocci script

uninitialized_var() macro was removed from the sources [1] and
other warning-silencing tricks were deprecated [2]. The purpose of this
cocci script is to prevent new occurrences of uninitialized_var()
open-coded variants.

[1] commit 63a0895d960a ("compiler: Remove uninitialized_var() macro")
[2] commit 4b19bec97c88 ("docs: deprecated.rst: Add uninitialized_var()")

Cc: Kees Cook <[email protected]>
Cc: Gustavo A. R. Silva <[email protected]>
Signed-off-by: Denis Efremov <[email protected]>
---
Changes in v2:
- Documentation cited in the script's description
- kernel.org link added to the diagnostics messages
- "T *var = &var;" pattern removed
- "var =@p var", "var =@p *(&(var))" patterns added
Changes in v3:
- commit's description changed

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

diff --git a/scripts/coccinelle/misc/uninitialized_var.cocci b/scripts/coccinelle/misc/uninitialized_var.cocci
new file mode 100644
index 000000000000..8fa845cefe11
--- /dev/null
+++ b/scripts/coccinelle/misc/uninitialized_var.cocci
@@ -0,0 +1,51 @@
+// SPDX-License-Identifier: GPL-2.0-only
+///
+/// Please, don't reintroduce uninitialized_var().
+/// From Documentation/process/deprecated.rst:
+/// For any compiler warnings about uninitialized variables, just add
+/// an initializer. Using warning-silencing tricks is dangerous as it
+/// papers over real bugs (or can in the future), and suppresses unrelated
+/// compiler warnings (e.g. "unused variable"). If the compiler thinks it
+/// is uninitialized, either simply initialize the variable or make compiler
+/// changes. Keep in mind that in most cases, if an initialization is
+/// obviously redundant, the compiler's dead-store elimination pass will make
+/// sure there are no needless variable writes.
+///
+// Confidence: High
+// Copyright: (C) 2020 Denis Efremov ISPRAS
+// Options: --no-includes --include-headers
+//
+
+virtual context
+virtual report
+virtual org
+
+@r@
+identifier var;
+type T;
+position p;
+@@
+
+(
+* T var =@p var;
+|
+* T var =@p *(&(var));
+|
+* var =@p var
+|
+* var =@p *(&(var))
+)
+
+@script:python depends on report@
+p << r.p;
+@@
+
+coccilib.report.print_report(p[0],
+ "WARNING this kind of initialization is deprecated (https://www.kernel.org/doc/html/latest/process/deprecated.html#uninitialized-var)")
+
+@script:python depends on org@
+p << r.p;
+@@
+
+coccilib.org.print_todo(p[0],
+ "WARNING this kind of initialization is deprecated (https://www.kernel.org/doc/html/latest/process/deprecated.html#uninitialized-var)")
--
2.26.2

2020-09-01 10:11:58

[permalink] [raw]

Subject: Re: [PATCH v3] coccinelle: misc: add uninitialized_var.cocci script

On Tue, 1 Sep 2020, Denis Efremov wrote:

> uninitialized_var() macro was removed from the sources [1] and
> other warning-silencing tricks were deprecated [2]. The purpose of this
> cocci script is to prevent new occurrences of uninitialized_var()
> open-coded variants.
>
> [1] commit 63a0895d960a ("compiler: Remove uninitialized_var() macro")
> [2] commit 4b19bec97c88 ("docs: deprecated.rst: Add uninitialized_var()")

Thanks! I find that much more understandable.

julia

>
> Cc: Kees Cook <[email protected]>
> Cc: Gustavo A. R. Silva <[email protected]>
> Signed-off-by: Denis Efremov <[email protected]>
> ---
> Changes in v2:
> - Documentation cited in the script's description
> - kernel.org link added to the diagnostics messages
> - "T *var = &var;" pattern removed
> - "var =@p var", "var =@p *(&(var))" patterns added
> Changes in v3:
> - commit's description changed
>
> .../coccinelle/misc/uninitialized_var.cocci | 51 +++++++++++++++++++
> 1 file changed, 51 insertions(+)
> create mode 100644 scripts/coccinelle/misc/uninitialized_var.cocci
>
> diff --git a/scripts/coccinelle/misc/uninitialized_var.cocci b/scripts/coccinelle/misc/uninitialized_var.cocci
> new file mode 100644
> index 000000000000..8fa845cefe11
> --- /dev/null
> +++ b/scripts/coccinelle/misc/uninitialized_var.cocci
> @@ -0,0 +1,51 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +///
> +/// Please, don't reintroduce uninitialized_var().
> +/// From Documentation/process/deprecated.rst:
> +/// For any compiler warnings about uninitialized variables, just add
> +/// an initializer. Using warning-silencing tricks is dangerous as it
> +/// papers over real bugs (or can in the future), and suppresses unrelated
> +/// compiler warnings (e.g. "unused variable"). If the compiler thinks it
> +/// is uninitialized, either simply initialize the variable or make compiler
> +/// changes. Keep in mind that in most cases, if an initialization is
> +/// obviously redundant, the compiler's dead-store elimination pass will make
> +/// sure there are no needless variable writes.
> +///
> +// Confidence: High
> +// Copyright: (C) 2020 Denis Efremov ISPRAS
> +// Options: --no-includes --include-headers
> +//
> +
> +virtual context
> +virtual report
> +virtual org
> +
> +@r@
> +identifier var;
> +type T;
> +position p;
> +@@
> +
> +(
> +* T var =@p var;
> +|
> +* T var =@p *(&(var));
> +|
> +* var =@p var
> +|
> +* var =@p *(&(var))
> +)
> +
> +@script:python depends on report@
> +p << r.p;
> +@@
> +
> +coccilib.report.print_report(p[0],
> + "WARNING this kind of initialization is deprecated (https://www.kernel.org/doc/html/latest/process/deprecated.html#uninitialized-var)")
> +
> +@script:python depends on org@
> +p << r.p;
> +@@
> +
> +coccilib.org.print_todo(p[0],
> + "WARNING this kind of initialization is deprecated (https://www.kernel.org/doc/html/latest/process/deprecated.html#uninitialized-var)")
> --
> 2.26.2
>
>

2020-09-01 14:40:45

[permalink] [raw]

Subject: checkpatch? (was: Re: [PATCH v3] coccinelle: misc: add uninitialized_var.cocci script)

On Tue, 2020-09-01 at 12:48 +0300, Denis Efremov wrote:
> uninitialized_var() macro was removed from the sources [1] and
> other warning-silencing tricks were deprecated [2]. The purpose of this
> cocci script is to prevent new occurrences of uninitialized_var()
> open-coded variants.

> +(
> +* T var =@p var;
> +|
> +* T var =@p *(&(var));
> +|
> +* var =@p var
> +|
> +* var =@p *(&(var))
> +)

Adding a checkpatch test might be a good thing too.

---
scripts/checkpatch.pl | 11 +++++++++++
1 file changed, 11 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 149518d2a6a7..300b2659aab3 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3901,6 +3901,17 @@ sub process {
#ignore lines not being added
next if ($line =~ /^[^\+]/);

+# check for self assigments used to avoid compiler warnings
+# e.g.: int foo = foo, *bar = NULL;
+# struct foo bar = *(&(bar));
+ if ($line =~ /^\+\s*(?:$Declare)?([A-Za-z_][A-Za-z\d_]*)\s*=/) {
+ my $var = $1;
+ if ($line =~ /^\+\s*(?:$Declare)?$var\s*=\s*(?:$var|\*\s*$?\s*&\s*\(?\s*$var\s*$?\s*\)?)\s*[;,]/) {
+ WARN("SELF_ASSIGNMENT",
+ "Do not use self-assignments to avoid compiler warnings\n" . $herecurr);
+ }
+ }
+
# check for dereferences that span multiple lines
if ($prevline =~ /^\+.*$Lval\s*(?:\.|->)\s*$/ &&
$line =~ /^\+\s*(?!\#\s*(?!define\s+|if))\s*$Lval/) {

2020-09-02 05:18:16

[permalink] [raw]

Subject: Re: checkpatch? (was: Re: [PATCH v3] coccinelle: misc: add uninitialized_var.cocci script)

On 9/1/20 5:37 PM, Joe Perches wrote:
> On Tue, 2020-09-01 at 12:48 +0300, Denis Efremov wrote:
>> uninitialized_var() macro was removed from the sources [1] and
>> other warning-silencing tricks were deprecated [2]. The purpose of this
>> cocci script is to prevent new occurrences of uninitialized_var()
>> open-coded variants.
>
>> +(
>> +* T var =@p var;
>> +|
>> +* T var =@p *(&(var));
>> +|
>> +* var =@p var
>> +|
>> +* var =@p *(&(var))
>> +)
>
> Adding a checkpatch test might be a good thing too.
>
> ---
> scripts/checkpatch.pl | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 149518d2a6a7..300b2659aab3 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -3901,6 +3901,17 @@ sub process {
> #ignore lines not being added
> next if ($line =~ /^[^\+]/);
>
> +# check for self assigments used to avoid compiler warnings
> +# e.g.: int foo = foo, *bar = NULL;
> +# struct foo bar = *(&(bar));
> + if ($line =~ /^\+\s*(?:$Declare)?([A-Za-z_][A-Za-z\d_]*)\s*=/) {
> + my $var = $1;
> + if ($line =~ /^\+\s*(?:$Declare)?$var\s*=\s*(?:$var|\*\s*$?\s*&\s*\(?\s*$var\s*$?\s*\)?)\s*[;,]/) {
> + WARN("SELF_ASSIGNMENT",
> + "Do not use self-assignments to avoid compiler warnings\n" . $herecurr);
> + }
> + }
> +
> # check for dereferences that span multiple lines
> if ($prevline =~ /^\+.*$Lval\s*(?:\.|->)\s*$/ &&
> $line =~ /^\+\s*(?!\#\s*(?!define\s+|if))\s*$Lval/) {

Looks good. I also faced this kind of assignments after declarations.
https://lkml.org/lkml/2020/8/31/85

I'm not sure if they are used to suppress compiler warnings, through.

Denis

2020-09-05 17:19:25

[permalink] [raw]

Subject: Re: [PATCH v3] coccinelle: misc: add uninitialized_var.cocci script

On Tue, 1 Sep 2020, Denis Efremov wrote:

> uninitialized_var() macro was removed from the sources [1] and
> other warning-silencing tricks were deprecated [2]. The purpose of this
> cocci script is to prevent new occurrences of uninitialized_var()
> open-coded variants.
>
> [1] commit 63a0895d960a ("compiler: Remove uninitialized_var() macro")
> [2] commit 4b19bec97c88 ("docs: deprecated.rst: Add uninitialized_var()")
>
> Cc: Kees Cook <[email protected]>
> Cc: Gustavo A. R. Silva <[email protected]>
> Signed-off-by: Denis Efremov <[email protected]>

Applied.

julia

> ---
> Changes in v2:
> - Documentation cited in the script's description
> - kernel.org link added to the diagnostics messages
> - "T *var = &var;" pattern removed
> - "var =@p var", "var =@p *(&(var))" patterns added
> Changes in v3:
> - commit's description changed
>
> .../coccinelle/misc/uninitialized_var.cocci | 51 +++++++++++++++++++
> 1 file changed, 51 insertions(+)
> create mode 100644 scripts/coccinelle/misc/uninitialized_var.cocci
>
> diff --git a/scripts/coccinelle/misc/uninitialized_var.cocci b/scripts/coccinelle/misc/uninitialized_var.cocci
> new file mode 100644
> index 000000000000..8fa845cefe11
> --- /dev/null
> +++ b/scripts/coccinelle/misc/uninitialized_var.cocci
> @@ -0,0 +1,51 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +///
> +/// Please, don't reintroduce uninitialized_var().
> +/// From Documentation/process/deprecated.rst:
> +/// For any compiler warnings about uninitialized variables, just add
> +/// an initializer. Using warning-silencing tricks is dangerous as it
> +/// papers over real bugs (or can in the future), and suppresses unrelated
> +/// compiler warnings (e.g. "unused variable"). If the compiler thinks it
> +/// is uninitialized, either simply initialize the variable or make compiler
> +/// changes. Keep in mind that in most cases, if an initialization is
> +/// obviously redundant, the compiler's dead-store elimination pass will make
> +/// sure there are no needless variable writes.
> +///
> +// Confidence: High
> +// Copyright: (C) 2020 Denis Efremov ISPRAS
> +// Options: --no-includes --include-headers
> +//
> +
> +virtual context
> +virtual report
> +virtual org
> +
> +@r@
> +identifier var;
> +type T;
> +position p;
> +@@
> +
> +(
> +* T var =@p var;
> +|
> +* T var =@p *(&(var));
> +|
> +* var =@p var
> +|
> +* var =@p *(&(var))
> +)
> +
> +@script:python depends on report@
> +p << r.p;
> +@@
> +
> +coccilib.report.print_report(p[0],
> + "WARNING this kind of initialization is deprecated (https://www.kernel.org/doc/html/latest/process/deprecated.html#uninitialized-var)")
> +
> +@script:python depends on org@
> +p << r.p;
> +@@
> +
> +coccilib.org.print_todo(p[0],
> + "WARNING this kind of initialization is deprecated (https://www.kernel.org/doc/html/latest/process/deprecated.html#uninitialized-var)")
> --
> 2.26.2
>
>

2020-09-05 17:59:35

[permalink] [raw]

Subject: [PATCH] checkpatch: Warn on self-assignments

The uninitialized_var() macro was removed recently via
commit 63a0895d960a ("compiler: Remove uninitialized_var() macro")
as it's not a particularly useful warning and its use can
"paper over real bugs".

Add a checkpatch test to warn on self-assignments as a means
to avoid compiler warnings and as a back-door mechanism to
reproduce the old uninitialized_var macro behavior.

Signed-off-by: Joe Perches <[email protected]>
---
scripts/checkpatch.pl | 11 +++++++++++
1 file changed, 11 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 149518d2a6a7..300b2659aab3 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3901,6 +3901,17 @@ sub process {
#ignore lines not being added
next if ($line =~ /^[^\+]/);

+# check for self assigments used to avoid compiler warnings
+# e.g.: int foo = foo, *bar = NULL;
+# struct foo bar = *(&(bar));
+ if ($line =~ /^\+\s*(?:$Declare)?([A-Za-z_][A-Za-z\d_]*)\s*=/) {
+ my $var = $1;
+ if ($line =~ /^\+\s*(?:$Declare)?$var\s*=\s*(?:$var|\*\s*$?\s*&\s*\(?\s*$var\s*$?\s*\)?)\s*[;,]/) {
+ WARN("SELF_ASSIGNMENT",
+ "Do not use self-assignments to avoid compiler warnings\n" . $herecurr);
+ }
+ }
+
# check for dereferences that span multiple lines
if ($prevline =~ /^\+.*$Lval\s*(?:\.|->)\s*$/ &&
$line =~ /^\+\s*(?!\#\s*(?!define\s+|if))\s*$Lval/) {

2020-09-10 19:52:20

by Kees Cook

[permalink] [raw]

Subject: Re: [PATCH] checkpatch: Warn on self-assignments

On Sat, Sep 05, 2020 at 10:58:29AM -0700, Joe Perches wrote:
> The uninitialized_var() macro was removed recently via
> commit 63a0895d960a ("compiler: Remove uninitialized_var() macro")
> as it's not a particularly useful warning and its use can
> "paper over real bugs".
>
> Add a checkpatch test to warn on self-assignments as a means
> to avoid compiler warnings and as a back-door mechanism to
> reproduce the old uninitialized_var macro behavior.
>
> Signed-off-by: Joe Perches <[email protected]>

I like it! :)

Can you add a section to code style and include a link in the checkpatch
warning to it? (Feel free to just reuse the text removed from
deprecated.rst)

--
Kees Cook

2020-09-10 21:38:10