2013-08-15 18:41:00

by Sergei Trofimovich

[permalink] [raw]
Subject: [PATCH v2] Makefile: enable -Werror=implicit-int and -Werror=strict-prototypes by default

The common error type found in forward-ported/backported patches is missing headers.
One recent example (files and function names are mangled):

void foo(){}
EXPORT_SYMBOL(foo);

gave only warning

foo.c:12345678:5: warning: function declaration isn't a prototype [-Wstrict-prototypes]
void foo(){}
^

foo.c:12345679:5: warning: data definition has no type or storage class [enabled by default]
EXPORT_SYMBOL(foo);
foo.c:12345679:5: warning: type defaults to 'int' in declaration of 'EXORT_SYMBOL' [-Werror=implicit-int]

Now it's a fata error. Tested on x86_64 allyesconfig.

Signed-off-by: Sergei Trofimovich <[email protected]>
Cc: Michal Marek <[email protected]>
Cc: [email protected]
Cc: Andrew Morton <[email protected]>
---
Makefile | 5 +++++
1 file changed, 5 insertions(+)

Change since v1:
- use 'cc-option' to respect old gccs
- fix typos. Thanks to Oleg Verych

diff --git a/Makefile b/Makefile
index 6e48848..53f4776 100644
--- a/Makefile
+++ b/Makefile
@@ -374,6 +374,11 @@ KBUILD_CFLAGS := -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs \
-Werror-implicit-function-declaration \
-Wno-format-security \
-fno-delete-null-pointer-checks
+
+# not universally available, but nice to have ones
+KBUILD_CFLAGS += $(call cc-option,-Werror=implicit-int) \
+ $(call cc-option,-Werror=strict-prototypes)
+
KBUILD_AFLAGS_KERNEL :=
KBUILD_CFLAGS_KERNEL :=
KBUILD_AFLAGS := -D__ASSEMBLY__
--
1.8.3.2


2013-08-15 21:18:51

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH v2] Makefile: enable -Werror=implicit-int and -Werror=strict-prototypes by default

On Thu, 15 Aug 2013 21:37:31 +0300 Sergei Trofimovich <[email protected]> wrote:

> The common error type found in forward-ported/backported patches is missing headers.
> One recent example (files and function names are mangled):
>
> void foo(){}
> EXPORT_SYMBOL(foo);
>
> gave only warning
>
> foo.c:12345678:5: warning: function declaration isn't a prototype [-Wstrict-prototypes]
> void foo(){}
> ^
>
> foo.c:12345679:5: warning: data definition has no type or storage class [enabled by default]
> EXPORT_SYMBOL(foo);
> foo.c:12345679:5: warning: type defaults to 'int' in declaration of 'EXORT_SYMBOL' [-Werror=implicit-int]
>
> Now it's a fata error. Tested on x86_64 allyesconfig.

Yes, let's try that.

Partly because the build still generates far too many warnings..

2013-08-23 12:18:05

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH v2] Makefile: enable -Werror=implicit-int and -Werror=strict-prototypes by default

On Thu, Aug 15, 2013 at 8:37 PM, Sergei Trofimovich <[email protected]> wrote:
> diff --git a/Makefile b/Makefile
> index 6e48848..53f4776 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -374,6 +374,11 @@ KBUILD_CFLAGS := -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs \
> -Werror-implicit-function-declaration \
> -Wno-format-security \
> -fno-delete-null-pointer-checks
> +
> +# not universally available, but nice to have ones
> +KBUILD_CFLAGS += $(call cc-option,-Werror=implicit-int) \
> + $(call cc-option,-Werror=strict-prototypes)
> +

This should be _below_ the line

include $(srctree)/arch/$(SRCARCH)/Makefile

, together with the other users of cc-option, else it detects the features of
the host compiler instead of the cross-compiler when cross-compiling:

cc1: error: unrecognized command line option "-Werror=implicit-int"
cc1: error: unrecognized command line option "-Werror=strict-prototypes"

See also commit a1f42beb8e287482d1a802731d4fb7e2bdc2c703
("Makefile: fix up CROSS_COMPILE and READABLE_ASM interaction.").

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2013-08-23 17:41:14

by Sergei Trofimovich

[permalink] [raw]
Subject: Re: [PATCH v2] Makefile: enable -Werror=implicit-int and -Werror=strict-prototypes by default

On Fri, 23 Aug 2013 14:18:02 +0200
Geert Uytterhoeven <[email protected]> wrote:

> On Thu, Aug 15, 2013 at 8:37 PM, Sergei Trofimovich <[email protected]> wrote:
> > diff --git a/Makefile b/Makefile
> > index 6e48848..53f4776 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -374,6 +374,11 @@ KBUILD_CFLAGS := -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs \
> > -Werror-implicit-function-declaration \
> > -Wno-format-security \
> > -fno-delete-null-pointer-checks
> > +
> > +# not universally available, but nice to have ones
> > +KBUILD_CFLAGS += $(call cc-option,-Werror=implicit-int) \
> > + $(call cc-option,-Werror=strict-prototypes)
> > +
>
> This should be _below_ the line
>
> include $(srctree)/arch/$(SRCARCH)/Makefile
>
> , together with the other users of cc-option, else it detects the features of
> the host compiler instead of the cross-compiler when cross-compiling:
>
> cc1: error: unrecognized command line option "-Werror=implicit-int"
> cc1: error: unrecognized command line option "-Werror=strict-prototypes"

Oh, it was not obvious.

Does it mean the code right before is buggy as well?

> ifdef CONFIG_CC_OPTIMIZE_FOR_SIZE
> KBUILD_CFLAGS<->+= -Os $(call cc-disable-warning,maybe-uninitialized,)
^^^
> else
> KBUILD_CFLAGS<->+= -O2
> endif

include $(srctree)/arch/$(SRCARCH)/Makefile

Will respin fixed patch in a while.

Thanks!

--

Sergei


Attachments:
signature.asc (198.00 B)

2013-08-23 17:59:43

by Sergei Trofimovich

[permalink] [raw]
Subject: [PATCH v3] Makefile: enable -Werror=implicit-int and -Werror=strict-prototypes by default

The common error fount in forward-ported/backported patches is missing headers.
One recent example (files and function names are mangled):

void foo(){}
EXPORT_SYMBOL(foo);

gave only warning

foo.c:12345678:5: warning: function declaration isn't a prototype [-Wstrict-prototypes]
void foo(){}
^

foo.c:12345679:5: warning: data definition has no type or storage class [enabled by default]
EXPORT_SYMBOL(foo);
foo.c:12345679:5: warning: type defaults to 'int' in declaration of 'EXORT_SYMBOL' [-Werror=implicit-int]

Now it's a fatal error. Tested on x86_64 allyesconfig.

Signed-off-by: Sergei Trofimovich <[email protected]>
Cc: Michal Marek <[email protected]>
Cc: [email protected]
Cc: Andrew Morton <[email protected]>
Cc: Geert Uytterhoeven <[email protected]>
---
Makefile | 6 +++++++
1 file changed, 6 insertions(+)

Change since v2:
- moved CFLAGS checks lower to catch $(CROSS) case
as suggested by Geert
- added comments as other warning options do

Change since v1:
- use 'cc-option' to respect old gccs
- fix typos. Thanks to Oleg Verych

diff --git a/Makefile b/Makefile
index a5a55f4..f5f36e1 100644
--- a/Makefile
+++ b/Makefile
@@ -659,6 +660,12 @@ KBUILD_CFLAGS += $(call cc-option,-fno-strict-overflow)
# conserve stack if available
KBUILD_CFLAGS += $(call cc-option,-fconserve-stack)

+# disallog errors like 'EXPORT_GPL(foo);' with missing header
+KBUILD_CFLAGS += $(call cc-option,-Werror=implicit-int)
+
+# require functions to have argumens in prototypes, not empty 'int foo()'
+KBUILD_CFLAGS += $(call cc-option,-Werror=strict-prototypes)
+
# use the deterministic mode of AR if available
KBUILD_ARFLAGS := $(call ar-option,D)

--
1.8.3.2