[PATCH] kbuild: Add extra gcc checks
Borislav Petkov
object.
The idea behind this option is targeted at developers who, in the
process of writing their code, want to do the occasional
make W=1 [target.o]
and let gcc do more extensive code checking for them. Then, they
could eyeball the output for valid gcc warnings about various
bugs/discrepancies which are not reported during the normal build
process.
For more background information and a use case, read through this
thread: http://marc.info/?i=20110218091716.GA4384@bicker
Cc: Ingo Molnar <mi...@elte.hu>
Cc: Michal Marek <mma...@suse.cz>
Cc: linux-...@vger.kernel.org
Signed-off-by: Borislav Petkov <b...@alien8.de>
---
Makefile | 31 +++++++++++++++++++++++++++++++
1 files changed, 31 insertions(+), 0 deletions(-)
diff --git a/Makefile b/Makefile
index c9c8c8f..a783a69 100644
--- a/Makefile
+++ b/Makefile
@@ -102,6 +102,10 @@ ifeq ("$(origin O)", "command line")
KBUILD_OUTPUT := $(O)
endif
+ifeq ("$(origin W)", "command line")
+ KBUILD_ENABLE_EXTRA_WARNINGS = 1
+endif
+
# That's our default target when none is given on the command line
PHONY := _all
_all:
@@ -363,6 +367,32 @@ KBUILD_AFLAGS_MODULE := -DMODULE
KBUILD_CFLAGS_MODULE := -DMODULE
KBUILD_LDFLAGS_MODULE := -T $(srctree)/scripts/module-common.lds
+ifdef KBUILD_ENABLE_EXTRA_WARNINGS
+KBUILD_CFLAGS += -Wextra -Wno-unused
+KBUILD_CFLAGS += -Waggregate-return
+KBUILD_CFLAGS += -Wbad-function-cast
+KBUILD_CFLAGS += -Wcast-qual
+KBUILD_CFLAGS += -Wcast-align
+KBUILD_CFLAGS += -Wconversion
+KBUILD_CFLAGS += -Wdisabled-optimization
+KBUILD_CFLAGS += -Wlogical-op
+KBUILD_CFLAGS += -Wmissing-declarations
+KBUILD_CFLAGS += -Wmissing-format-attribute
+KBUILD_CFLAGS += -Wmissing-include-dirs
+KBUILD_CFLAGS += -Wmissing-prototypes
+KBUILD_CFLAGS += -Wnested-externs
+KBUILD_CFLAGS += -Wold-style-definition
+KBUILD_CFLAGS += -Woverlength-strings
+KBUILD_CFLAGS += -Wpacked
+KBUILD_CFLAGS += -Wpacked-bitfield-compat
+KBUILD_CFLAGS += -Wpadded
+KBUILD_CFLAGS += -Wpointer-arith
+KBUILD_CFLAGS += -Wredundant-decls
+KBUILD_CFLAGS += -Wshadow
+KBUILD_CFLAGS += -Wswitch-default
+KBUILD_CFLAGS += -Wvla
+endif
+
# Read KERNELRELEASE from include/config/kernel.release (if it exists)
KERNELRELEASE = $(shell cat include/config/kernel.release 2> /dev/null)
KERNELVERSION = $(VERSION).$(PATCHLEVEL).$(SUBLEVEL)$(EXTRAVERSION)
@@ -1262,6 +1292,7 @@ help:
@echo ' make O=dir [targets] Locate all output files in "dir", including .config'
@echo ' make C=1 [targets] Check all c source with $$CHECK (sparse by default)'
@echo ' make C=2 [targets] Force check of all c source with $$CHECK'
+ @echo ' make W=1 [targets] Enable extra gcc checks'
@echo ''
@echo 'Execute "make" or "make all" to build all targets marked with [*] '
@echo 'For further info see the ./README file'
--
1.7.4.1.48.g5673d
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Ingo Molnar
* Borislav Petkov <b...@alien8.de> wrote:
> Add a 'W=1' Makefile switch which adds additional checking per build
> object.
>
> The idea behind this option is targeted at developers who, in the
> process of writing their code, want to do the occasional
>
> make W=1 [target.o]
>
> and let gcc do more extensive code checking for them. Then, they
> could eyeball the output for valid gcc warnings about various
> bugs/discrepancies which are not reported during the normal build
> process.
>
> For more background information and a use case, read through this
> thread: http://marc.info/?i=20110218091716.GA4384@bicker
>
> Cc: Ingo Molnar <mi...@elte.hu>
> Cc: Michal Marek <mma...@suse.cz>
> Cc: linux-...@vger.kernel.org
> Signed-off-by: Borislav Petkov <b...@alien8.de>
Nice.
Acked-by: Ingo Molnar <mi...@elte.hu>
We enable a lot of GCC warnings in tools/perf/ as well, and while there are false
positives occasionally, the general effect on code quality is positive. We combine
it with -Werror to make sure warnings do not accumulate.
Thanks,
Ingo
Sam Ravnborg
> Add a 'W=1' Makefile switch which adds additional checking per build
> object.
>
> The idea behind this option is targeted at developers who, in the
> process of writing their code, want to do the occasional
>
> make W=1 [target.o]
>
> and let gcc do more extensive code checking for them. Then, they
> could eyeball the output for valid gcc warnings about various
> bugs/discrepancies which are not reported during the normal build
> process.
>
> For more background information and a use case, read through this
> thread: http://marc.info/?i=20110218091716.GA4384@bicker
>
> Cc: Ingo Molnar <mi...@elte.hu>
> Cc: Michal Marek <mma...@suse.cz>
> Cc: linux-...@vger.kernel.org
> Signed-off-by: Borislav Petkov <b...@alien8.de>
> ---
> Makefile | 31 +++++++++++++++++++++++++++++++
> 1 files changed, 31 insertions(+), 0 deletions(-)
I like the idea.
But can we avoid polluting the top-level Makefile?
That file is full of unreadable stuff, and adding more
is not good.
Makefile.lib or Makefile.build seems like a better place for this.
Sam
Borislav Petkov
> I like the idea.
> But can we avoid polluting the top-level Makefile?
> That file is full of unreadable stuff, and adding more
> is not good.
>
> Makefile.lib or Makefile.build seems like a better place for this.
You're right, ./Makefile shouldn't get any fatter than it is right now :).
Here's a second version; Ingo, I've left your ack in even though it was
referencing the initial version since the idea is kept the same - only
the code placement is different.
Thanks.
--
From: Borislav Petkov <b...@alien8.de>
Date: Sun, 20 Feb 2011 17:18:40 +0100
Subject: [PATCH] kbuild: Add extra gcc checks
Add a 'W=1' Makefile switch which adds additional checking per build
object.
The idea behind this option is targeted at developers who, in the
process of writing their code, want to do the occasional
make W=1 [target.o]
and let gcc do more extensive code checking for them. Then, they
could eyeball the output for valid gcc warnings about various
bugs/discrepancies which are not reported during the normal build
process.
For more background information and a use case, read through this
thread: http://marc.info/?l=kernel-janitors&m=129802065918147&w=2
Cc: Michal Marek <mma...@suse.cz>
Cc: Sam Ravnborg <s...@ravnborg.org>
Cc: linux-...@vger.kernel.org
Acked-by: Ingo Molnar <mi...@elte.hu>
Signed-off-by: Borislav Petkov <b...@alien8.de>
---
Makefile | 10 ++++++++++
scripts/Makefile.build | 28 +++++++++++++++++++++++++++-
2 files changed, 37 insertions(+), 1 deletions(-)
diff --git a/Makefile b/Makefile
index c9c8c8f..03989a1 100644
--- a/Makefile
+++ b/Makefile
@@ -102,6 +102,15 @@ ifeq ("$(origin O)", "command line")
KBUILD_OUTPUT := $(O)
endif
+ifeq ("$(origin W)", "command line")
+ KBUILD_ENABLE_EXTRA_GCC_CHECKS = 1
+endif
+ifndef KBUILD_ENABLE_EXTRA_GCC_CHECKS
+ KBUILD_ENABLE_EXTRA_GCC_CHECKS = 0
+endif
+
+export KBUILD_ENABLE_EXTRA_GCC_CHECKS
+
# That's our default target when none is given on the command line
PHONY := _all
_all:
@@ -1262,6 +1271,7 @@ help:
@echo ' make O=dir [targets] Locate all output files in "dir", including .config'
@echo ' make C=1 [targets] Check all c source with $$CHECK (sparse by default)'
@echo ' make C=2 [targets] Force check of all c source with $$CHECK'
+ @echo ' make W=1 [targets] Enable extra gcc checks'
@echo ''
@echo 'Execute "make" or "make all" to build all targets marked with [*] '
@echo 'For further info see the ./README file'
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 4eb99ab..5bf9f40 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -30,6 +30,33 @@ ldflags-y :=
subdir-asflags-y :=
subdir-ccflags-y :=
+# make W=1 settings
+ifdef KBUILD_ENABLE_EXTRA_GCC_CHECKS
+EXTRA_CFLAGS += -Wextra -Wno-unused
+EXTRA_CFLAGS += -Waggregate-return
+EXTRA_CFLAGS += -Wbad-function-cast
+EXTRA_CFLAGS += -Wcast-qual
+EXTRA_CFLAGS += -Wcast-align
+EXTRA_CFLAGS += -Wconversion
+EXTRA_CFLAGS += -Wdisabled-optimization
+EXTRA_CFLAGS += -Wlogical-op
+EXTRA_CFLAGS += -Wmissing-declarations
+EXTRA_CFLAGS += -Wmissing-format-attribute
+EXTRA_CFLAGS += -Wmissing-include-dirs
+EXTRA_CFLAGS += -Wmissing-prototypes
+EXTRA_CFLAGS += -Wnested-externs
+EXTRA_CFLAGS += -Wold-style-definition
+EXTRA_CFLAGS += -Woverlength-strings
+EXTRA_CFLAGS += -Wpacked
+EXTRA_CFLAGS += -Wpacked-bitfield-compat
+EXTRA_CFLAGS += -Wpadded
+EXTRA_CFLAGS += -Wpointer-arith
+EXTRA_CFLAGS += -Wredundant-decls
+EXTRA_CFLAGS += -Wshadow
+EXTRA_CFLAGS += -Wswitch-default
+EXTRA_CFLAGS += -Wvla
+endif
+
# Read auto.conf if it exists, otherwise ignore
-include include/config/auto.conf
@@ -403,7 +430,6 @@ ifneq ($(cmd_files),)
include $(cmd_files)
endif
-
# Declare the contents of the .PHONY variable as phony. We keep that
# information in a variable se we can use it in if_changed and friends.
--
1.7.4.1.48.g5673d
--
Regards/Gruss,
Boris.
Joe Perches
> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> +# make W=1 settings
> +ifdef KBUILD_ENABLE_EXTRA_GCC_CHECKS
ifeq ($(KBUILD_ENABLE_EXTRA_GCC_CHECKS),1)
> +EXTRA_CFLAGS += -Wextra -Wno-unused
Why add -Wno-unused ?
If it's because of verbosity, maybe
ifeq ($(KBUILD_ENABLE_EXTRA_GCC_CHECKS),2)
EXTRA_CFLAGS += -Wunused
endif
Sam Ravnborg
> diff --git a/Makefile b/Makefile
> index c9c8c8f..03989a1 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -102,6 +102,15 @@ ifeq ("$(origin O)", "command line")
> KBUILD_OUTPUT := $(O)
> endif
>
> +ifeq ("$(origin W)", "command line")
> + KBUILD_ENABLE_EXTRA_GCC_CHECKS = 1
> +endif
> +ifndef KBUILD_ENABLE_EXTRA_GCC_CHECKS
> + KBUILD_ENABLE_EXTRA_GCC_CHECKS = 0
> +endif
I do not see the purpose of setting KBUILD_ENABLE_EXTRA_GCC_CHECKS
equal to 0.
> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> index 4eb99ab..5bf9f40 100644
> --- a/scripts/Makefile.build
> +++ b/scripts/Makefile.build
> @@ -30,6 +30,33 @@ ldflags-y :=
> subdir-asflags-y :=
> subdir-ccflags-y :=
>
> +# make W=1 settings
> +ifdef KBUILD_ENABLE_EXTRA_GCC_CHECKS
The symbol KBUILD_ENABLE_EXTRA_GCC_CHECKS is always defined,
as you set it to "0" in the top-level Makefile.
So you will always have the extra checks enabled.
You also need to move the assignments down.
As it is now any kbuild file that assign
EXTRA_CFLAGS with
EXTRA_CFLAGS := -D DEBUG
Will drop all the extra warnings.
From Makefile.build:
# If the save-* variables changed error out
ifeq ($(KBUILD_NOPEDANTIC),)
ifneq ("$(save-cflags)","$(CFLAGS)")
$(error CFLAGS was changed in "$(kbuild-file)". Fix it to use EXTRA_CFLAGS)
endif
endif
<<<<< Here would be the better place to add this.
include scripts/Makefile.lib
> +EXTRA_CFLAGS += -Wextra -Wno-unused
Use of EXTRA_CFLAGS is deprecated - so that is not the right choice.
I suggest to use KBUILD_CFLAGS that is an KBUILD internal variable.
Sam
Jesper Juhl
> Add a 'W=1' Makefile switch which adds additional checking per build
> object.
>
> The idea behind this option is targeted at developers who, in the
> process of writing their code, want to do the occasional
>
> make W=1 [target.o]
>
> and let gcc do more extensive code checking for them. Then, they
> could eyeball the output for valid gcc warnings about various
> bugs/discrepancies which are not reported during the normal build
> process.
>
I like this. I often modify the Makefile to add something similar to this
for my local builds. This will save me having to do that in the future.
--
Jesper Juhl <j...@chaosbits.net> http://www.chaosbits.net/
Plain text mails only, please.
Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
Borislav Petkov
.
> Use of EXTRA_CFLAGS is deprecated - so that is not the right choice.
> I suggest to use KBUILD_CFLAGS that is an KBUILD internal variable.
Hey Sam,
thanks for the suggestions/review, here's hopefully a better version:
--
From: Borislav Petkov <b...@alien8.de>
Date: Sun, 20 Feb 2011 17:18:40 +0100
Subject: [PATCH -v3] kbuild: Add extra gcc checks
Add a 'W=1' Makefile switch which adds additional checking per build
object.
The idea behind this option is targeted at developers who, in the
process of writing their code, want to do the occasional
make W=1 [target.o]
and let gcc do more extensive code checking for them. Then, they
could eyeball the output for valid gcc warnings about various
bugs/discrepancies which are not reported during the normal build
process.
For more background information and a use case, read through this
thread: http://marc.info/?l=kernel-janitors&m=129802065918147&w=2
Cc: Michal Marek <mma...@suse.cz>
Cc: Sam Ravnborg <s...@ravnborg.org>
Cc: linux-...@vger.kernel.org
Acked-by: Ingo Molnar <mi...@elte.hu>
Signed-off-by: Borislav Petkov <b...@alien8.de>
---
Makefile | 6 ++++++
scripts/Makefile.build | 29 ++++++++++++++++++++++++++++-
2 files changed, 34 insertions(+), 1 deletions(-)
diff --git a/Makefile b/Makefile
index c9c8c8f..c3bca9c 100644
--- a/Makefile
+++ b/Makefile
@@ -102,6 +102,11 @@ ifeq ("$(origin O)", "command line")
KBUILD_OUTPUT := $(O)
endif
+ifeq ("$(origin W)", "command line")
+ KBUILD_ENABLE_EXTRA_GCC_CHECKS = 1
+ export KBUILD_ENABLE_EXTRA_GCC_CHECKS
+endif
+
# That's our default target when none is given on the command line
PHONY := _all
_all:
@@ -1262,6 +1267,7 @@ help:
@echo ' make O=dir [targets] Locate all output files in "dir", including .config'
@echo ' make C=1 [targets] Check all c source with $$CHECK (sparse by default)'
@echo ' make C=2 [targets] Force check of all c source with $$CHECK'
+ @echo ' make W=1 [targets] Enable extra gcc checks'
@echo ''
@echo 'Execute "make" or "make all" to build all targets marked with [*] '
@echo 'For further info see the ./README file'
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 4eb99ab..b4d7661 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -49,6 +49,34 @@ ifeq ($(KBUILD_NOPEDANTIC),)
$(error CFLAGS was changed in "$(kbuild-file)". Fix it to use EXTRA_CFLAGS)
endif
endif
+
+# make W=1 settings
+ifdef KBUILD_ENABLE_EXTRA_GCC_CHECKS
include scripts/Makefile.lib
ifdef host-progs
@@ -403,7 +431,6 @@ ifneq ($(cmd_files),)
include $(cmd_files)
endif
-
# Declare the contents of the .PHONY variable as phony. We keep that
# information in a variable se we can use it in if_changed and friends.
--
1.7.4.1.48.g5673d
--
Regards/Gruss,
Boris.
Borislav Petkov
> > +EXTRA_CFLAGS += -Wextra -Wno-unused
>
> Why add -Wno-unused ?
>
> If it's because of verbosity, maybe
Nah, it's because it is too noisy and spits too many false positives.
For example, it reports the arguments of all those stubs from the
headers which are provided for the else-branch of a CONFIG_* option,
etc.
--
Regards/Gruss,
Boris.
Arnaud Lacombe
On Sun, Feb 20, 2011 at 9:23 PM, Borislav Petkov <b...@alien8.de> wrote:
> On Sun, Feb 20, 2011 at 08:52:22PM +0100, Sam Ravnborg wrote:
>
> ..
You never check CC is gcc. How can you assert this ? Moreover, can you
certify that all the compiler supported to build Linux do support the
set of new warnings ? What about icc ? old gcc ? LLVM/clang ?
> # That's our default target when none is given on the command line
> PHONY := _all
> _all:
> @@ -1262,6 +1267,7 @@ help:
> @echo ' make O=dir [targets] Locate all output files in "dir", including .config'
> @echo ' make C=1 [targets] Check all c source with $$CHECK (sparse by default)'
> @echo ' make C=2 [targets] Force check of all c source with $$CHECK'
> + @echo ' make W=1 [targets] Enable extra gcc checks'
same as above.
Why not making it simple at the beginning by:
1) s/GCC/CC/
2) using `cc-option' provided by Kbuild for each new option. As an
example, for `-Wvla':
KBUILD_CFLAGS += $(call cc-option, -Wvla)
so so on.
- Arnaud
> include scripts/Makefile.lib
>
> ifdef host-progs
> @@ -403,7 +431,6 @@ ifneq ($(cmd_files),)
> include $(cmd_files)
> endif
>
> -
> # Declare the contents of the .PHONY variable as phony. We keep that
> # information in a variable se we can use it in if_changed and friends.
>
> --
> 1.7.4.1.48.g5673d
>
>
>
> --
> Regards/Gruss,
> Boris.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
Arnaud Lacombe
On Sun, Feb 20, 2011 at 9:27 PM, Borislav Petkov <b...@alien8.de> wrote:
> On Sun, Feb 20, 2011 at 12:00:47PM -0800, Joe Perches wrote:
>> > +EXTRA_CFLAGS += -Wextra -Wno-unused
>>
>> Why add -Wno-unused ?
>>
>> If it's because of verbosity, maybe
>
> Nah, it's because it is too noisy and spits too many false positives.
>
"too noisy" is a subjective point of view.
> For example, it reports the arguments of all those stubs from the
> headers which are provided for the else-branch of a CONFIG_* option,
> etc.
>
and by the same way, you silence function marked with
`warn_unused_result', unless I misread the manpage. If you want to
silence something specific, why not just the `no' variant of the thing
you do not want ?
Btw, could you not take the same approach as the one taken by the BSD,
which is 3 or 4 different level of new warnings. That way, you keep
the noisy stuff for the highest warning level.
- Arnaud
> --
> Regards/Gruss,
> Boris.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
Borislav Petkov
> > The idea behind this option is targeted at developers who, in the
> > process of writing their code, want to do the occasional
> >
> > make W=1 [target.o]
> >
> > and let gcc do more extensive code checking for them. Then, they
> > could eyeball the output for valid gcc warnings about various
> > bugs/discrepancies which are not reported during the normal build
> > process.
> >
[..]
> > +ifeq ("$(origin W)", "command line")
> > + KBUILD_ENABLE_EXTRA_GCC_CHECKS = 1
> > + export KBUILD_ENABLE_EXTRA_GCC_CHECKS
> > +endif
> > +
> You never check CC is gcc. How can you assert this ?
Hmm, I somehow knew that the other compilers are going to come into the
discussion :).
> Moreover, can you certify that all the compiler supported to build
> Linux do support the set of new warnings ?
Well, as you've probably already read in the commit message, this
option is for devs only, in case they want to do a build-check whether
the couple of lines they just added to a .c file trigger new compiler
warnings. So, no, I cannot certify this and I don't have to - this is
not meant for production use anyway.
> What about icc ?
I don't know, is anyone using it to build the kernel? Even if so, icc
might have a completely different set of -W options (totally guessing
here) so you shouldn't use 'make W=1' with it.
> old gcc?
Yes, cc-option should be used in that case, I'll redo the patch.
> LLVM/clang ?
Can you even build the kernel with it?
But to make sure we're on the realistic side of things. First of all,
the purpose of this is to quickly get gcc scream out while building your
changes. Secondly, let's face it, the majority of developers, if not
all, use gcc, the kernel code is full of gcc-isms so accomodating all
the compilers to such a quick-and-dirty test option is just plain too
much. Look at it this way: it is cheaper to upgrade your dev environment
than add unnecessary and ugly code to kbuild. Even the argument with
older gcc versions cannot weigh in enough in favor of the cc-option -
simply upgrade your gcc.
Thanks.
--
Regards/Gruss,
Boris.
--
Borislav Petkov
> Hi,
>
> On Sun, Feb 20, 2011 at 9:27 PM, Borislav Petkov <b...@alien8.de> wrote:
> > On Sun, Feb 20, 2011 at 12:00:47PM -0800, Joe Perches wrote:
> >> > +EXTRA_CFLAGS += -Wextra -Wno-unused
> >>
> >> Why add -Wno-unused ?
> >>
> >> If it's because of verbosity, maybe
> >
> > Nah, it's because it is too noisy and spits too many false positives.
> >
> "too noisy" is a subjective point of view.
Ok, does "too many false positives" objectify it a bit more to your
taste?
> > For example, it reports the arguments of all those stubs from the
> > headers which are provided for the else-branch of a CONFIG_* option,
> > etc.
> >
> and by the same way, you silence function marked with
> `warn_unused_result', unless I misread the manpage.
Can you point me to that passage, I cannot find it in my gcc manpage.
> If you want to silence something specific, why not just the `no'
> variant of the thing you do not want ?
Yes, '-Wunused -Wno-unused-parameter' looks better.
> Btw, could you not take the same approach as the one taken by the BSD,
> which is 3 or 4 different level of new warnings. That way, you keep
> the noisy stuff for the highest warning level.
Nope, because there's no reason for it. I want to have one switch that
craps out all the possible warnings gcc can spit, I catch the build
output, fix the bugs and that's it.
--
Regards/Gruss,
Boris.
--
Arnaud Lacombe
People _will_ end up using it in production.
>> What about icc ?
>
> I don't know, is anyone using it to build the kernel?
then what would be the point of, say `include/linux/compiler-intel.h' ?
> Even if so, icc
> might have a completely different set of -W options (totally guessing
> here) so you shouldn't use 'make W=1' with it.
>
>> old gcc?
>
> Yes, cc-option should be used in that case, I'll redo the patch.
>
>> LLVM/clang ?
>
> Can you even build the kernel with it?
>
At first sight, partly[0].
> But to make sure we're on the realistic side of things.
Whose realistic side ? yours ? If not, are you speaking for all the
Linux community ? I do not think so.
> First of all,
> the purpose of this is to quickly get gcc scream out while building your
> changes. Secondly, let's face it, the majority of developers, if not
> all, use gcc the kernel code is full of gcc-isms so accomodating all
> the compilers to such a quick-and-dirty test option is just plain too
> much.
That's a statement I would not make. Lots of compiler dependent stuff
is buried in <linux/compiler.h> for a good reason.
> Look at it this way: it is cheaper to upgrade your dev environment
> than add unnecessary and ugly code to kbuild. Even the argument with
> older gcc versions cannot weigh in enough in favor of the cc-option -
> simply upgrade your gcc.
>
Do you speak for users, in the embedded world, of BSP whose supporting
company either went defunct or is no longer maintaining the toolchain
for a dark platform, or say an architecture, in kernel, which do not
have support in mainstream binutils and support's patches are tied
with a given version of binutils/gcc ?
- Arnaud
[0]: http://lists.cs.uiuc.edu/pipermail/cfe-dev/2010-October/011711.html
Arnaud Lacombe
On Sun, Feb 20, 2011 at 11:54 PM, Borislav Petkov <b...@alien8.de> wrote:
> On Sun, Feb 20, 2011 at 10:34:57PM -0500, Arnaud Lacombe wrote:
>> Hi,
>>
>> On Sun, Feb 20, 2011 at 9:27 PM, Borislav Petkov <b...@alien8.de> wrote:
>> > On Sun, Feb 20, 2011 at 12:00:47PM -0800, Joe Perches wrote:
>> >> > +EXTRA_CFLAGS += -Wextra -Wno-unused
>> >>
>> >> Why add -Wno-unused ?
>> >>
>> >> If it's because of verbosity, maybe
>> >
>> > Nah, it's because it is too noisy and spits too many false positives.
>> >
>> "too noisy" is a subjective point of view.
>
> Ok, does "too many false positives" objectify it a bit more to your
> taste?
>
The degree of acceptable verbosity should be left to the end user.
>> > For example, it reports the arguments of all those stubs from the
>> > headers which are provided for the else-branch of a CONFIG_* option,
>> > etc.
>> >
>> and by the same way, you silence function marked with
>> `warn_unused_result', unless I misread the manpage.
>
> Can you point me to that passage, I cannot find it in my gcc manpage.
>
my mistake, I just checked, `-Wno-unused' does not affect the
`warn_unused_result' warning. `-Wno-unused-result' is required to
silent that one.
>> If you want to silence something specific, why not just the `no'
>> variant of the thing you do not want ?
>
> Yes, '-Wunused -Wno-unused-parameter' looks better.
>
>> Btw, could you not take the same approach as the one taken by the BSD,
>> which is 3 or 4 different level of new warnings. That way, you keep
>> the noisy stuff for the highest warning level.
>
> Nope, because there's no reason for it. I want to have one switch that
> craps out all the possible warnings gcc can spit, I catch the build
> output, fix the bugs and that's it.
>
Looks like I'll submit the patch implementing multiple warnings level then ;-)
- Arnaud
Borislav Petkov
Add a 'W=1' Makefile switch which adds additional checking per build
object.
The idea behind this option is targeted at developers who, in the
process of writing their code, want to do the occasional
make W=1 [target.o]
and let gcc do more extensive code checking for them. Then, they
could eyeball the output for valid gcc warnings about various
bugs/discrepancies which are not reported during the normal build
process.
For more background information and a use case, read through this
thread: http://marc.info/?l=kernel-janitors&m=129802065918147&w=2
-v4: Enable this for gcc only. Also, check for options which are not
supported by every gcc version we use to build the kernel. Adding those
checks for _every_ option slows down the build noticeably so be sensible
here and add it only when your gcc version chokes on a particular
option.
-v[2..3]: move to Makefile.build
Cc: Michal Marek <mma...@suse.cz>
Cc: Sam Ravnborg <s...@ravnborg.org>
Cc: linux-...@vger.kernel.org
Acked-by: Ingo Molnar <mi...@elte.hu>
Signed-off-by: Borislav Petkov <b...@alien8.de>
---
Makefile | 6 ++++++
scripts/Makefile.build | 37 ++++++++++++++++++++++++++++++++++++-
2 files changed, 42 insertions(+), 1 deletions(-)
diff --git a/Makefile b/Makefile
index c9c8c8f..c3bca9c 100644
--- a/Makefile
+++ b/Makefile
@@ -102,6 +102,11 @@ ifeq ("$(origin O)", "command line")
KBUILD_OUTPUT := $(O)
endif
+ifeq ("$(origin W)", "command line")
+ KBUILD_ENABLE_EXTRA_GCC_CHECKS = 1
+ export KBUILD_ENABLE_EXTRA_GCC_CHECKS
+endif
+
# That's our default target when none is given on the command line
PHONY := _all
_all:
@@ -1262,6 +1267,7 @@ help:
@echo ' make O=dir [targets] Locate all output files in "dir", including .config'
@echo ' make C=1 [targets] Check all c source with $$CHECK (sparse by default)'
@echo ' make C=2 [targets] Force check of all c source with $$CHECK'
+ @echo ' make W=1 [targets] Enable extra gcc checks'
@echo ''
@echo 'Execute "make" or "make all" to build all targets marked with [*] '
@echo 'For further info see the ./README file'
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 4eb99ab..fa4b40d 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -49,6 +49,42 @@ ifeq ($(KBUILD_NOPEDANTIC),)
$(error CFLAGS was changed in "$(kbuild-file)". Fix it to use EXTRA_CFLAGS)
endif
endif
+
+#
+# make W=1 settings
+#
+# $(call cc-option... ) handles gcc -W.. options which
+# are not supported by all versions of the compiler
+ifdef KBUILD_ENABLE_EXTRA_GCC_CHECKS
+ifneq ($(call cc-version),)
+KBUILD_EXTRA_WARNINGS := -Wextra
+KBUILD_EXTRA_WARNINGS += -Wunused -Wno-unused-parameter
+KBUILD_EXTRA_WARNINGS += -Waggregate-return
+KBUILD_EXTRA_WARNINGS += -Wbad-function-cast
+KBUILD_EXTRA_WARNINGS += -Wcast-qual
+KBUILD_EXTRA_WARNINGS += -Wcast-align
+KBUILD_EXTRA_WARNINGS += -Wconversion
+KBUILD_EXTRA_WARNINGS += -Wdisabled-optimization
+KBUILD_EXTRA_WARNINGS += -Wlogical-op
+KBUILD_EXTRA_WARNINGS += -Wmissing-declarations
+KBUILD_EXTRA_WARNINGS += -Wmissing-format-attribute
+KBUILD_EXTRA_WARNINGS += $(call cc-option, -Wmissing-include-dirs,)
+KBUILD_EXTRA_WARNINGS += -Wmissing-prototypes
+KBUILD_EXTRA_WARNINGS += -Wnested-externs
+KBUILD_EXTRA_WARNINGS += -Wold-style-definition
+KBUILD_EXTRA_WARNINGS += $(call cc-option, -Woverlength-strings,)
+KBUILD_EXTRA_WARNINGS += -Wpacked
+KBUILD_EXTRA_WARNINGS += -Wpacked-bitfield-compat
+KBUILD_EXTRA_WARNINGS += -Wpadded
+KBUILD_EXTRA_WARNINGS += -Wpointer-arith
+KBUILD_EXTRA_WARNINGS += -Wredundant-decls
+KBUILD_EXTRA_WARNINGS += -Wshadow
+KBUILD_EXTRA_WARNINGS += -Wswitch-default
+KBUILD_EXTRA_WARNINGS += $(call cc-option, -Wvla,)
+KBUILD_CFLAGS += $(KBUILD_EXTRA_WARNINGS)
+endif
+endif
+
include scripts/Makefile.lib
ifdef host-progs
@@ -403,7 +439,6 @@ ifneq ($(cmd_files),)
include $(cmd_files)
endif
-
# Declare the contents of the .PHONY variable as phony. We keep that
# information in a variable se we can use it in if_changed and friends.
--
1.7.4.1.48.g5673d
--
Regards/Gruss,
Boris.
--
Borislav Petkov
>
> Add a 'W=1' Makefile switch which adds additional checking per build
> object.
>
> The idea behind this option is targeted at developers who, in the
> process of writing their code, want to do the occasional
>
> make W=1 [target.o]
>
> and let gcc do more extensive code checking for them. Then, they
> could eyeball the output for valid gcc warnings about various
> bugs/discrepancies which are not reported during the normal build
> process.
>
> For more background information and a use case, read through this
> thread: http://marc.info/?l=kernel-janitors&m=129802065918147&w=2
>
> -v4: Enable this for gcc only. Also, check for options which are not
> supported by every gcc version we use to build the kernel. Adding those
> checks for _every_ option slows down the build noticeably so be sensible
> here and add it only when your gcc version chokes on a particular
> option.
>
> -v[2..3]: move to Makefile.build
>
> Cc: Michal Marek <mma...@suse.cz>
> Cc: Sam Ravnborg <s...@ravnborg.org>
> Cc: linux-...@vger.kernel.org
> Acked-by: Ingo Molnar <mi...@elte.hu>
> Signed-off-by: Borislav Petkov <b...@alien8.de>
Ping?
Michal, Sam,
any news here? FWIW, I've been build-testing amd64_edac.c with it and
have caught a bunch of buglets with it. IMHO, it really helps to enable
more extensive gcc checking in debugging builds.
Thanks.
Sam Ravnborg
>
It is simpler to just write:
export KBUILD_ENABLE_EXTRA_GCC_CHECKS := 1
> +#
> +# make W=1 settings
> +#
> +# $(call cc-option... ) handles gcc -W.. options which
> +# are not supported by all versions of the compiler
> +ifdef KBUILD_ENABLE_EXTRA_GCC_CHECKS
> +ifneq ($(call cc-version),)
This check looks redundant.
You introduce a new environment variable "KBUILD_ENABLE_EXTRA_GCC_CHECKS".
We actually ty to document these in Documentation/kbuild/kbuild.txt
Fix the above and you can add my:
Acked-by: Sam Ravnborg <s...@ravnborg.org>
Sam
Arnd Bergmann
> Add a 'W=1' Makefile switch which adds additional checking per build
> object.
>
> The idea behind this option is targeted at developers who, in the
> process of writing their code, want to do the occasional
>
> make W=1 [target.o]
Great stuff, I really like the idea!
I would be a little more selective here. Maybe we can have two levels
W=1 and W=2, with the full set getting enabled by W=2, and the smaller
set getting enabled in W=1. The reason is that many of the warnings
are pointless or even hurting code quality, while others (e.g.
-Wmissing-declarations) are generally useful and the only reason for
not enabling them is that they cause too many warnings with existing
code.
Arnd
Borislav Petkov
> On Monday 21 February 2011 12:03:22 Borislav Petkov wrote:
> > Add a 'W=1' Makefile switch which adds additional checking per build
> > object.
> >
> > The idea behind this option is targeted at developers who, in the
> > process of writing their code, want to do the occasional
> >
> > make W=1 [target.o]
>
> Great stuff, I really like the idea!
Thanks. :)
My intention was not to have multiple levels of warnings because then
you have to go and enable the different levels and have to remember
which level you used last, etc, etc.
Instead I am thinking along with the following lines:
make W=1 [path/to/kernel/file.o] 2>w.log
and then take a look at w.log and start fixing warnings.
You can selectively ignore some of the warnings since, as you say
yourself above, some simply make you write ugly code like enforcing
casts just for the sake of shutting up the compiler. A great deal of the
warnings come from includes which are hard to fix or gcc is issuing the
warning wrong since we do sick sh*t with C in the kernel and that's OK
:).
But in all cases you have all the warnings in one single file and that's
it. If a certain -W option is useless, we should rather remove it since
it doesn't help anyway. The selection above is clearly not complete so
I'd rather drop some instead of including different W=x levels.
Hmm... ?
--
Regards/Gruss,
Boris.
Borislav Petkov
object.
The idea behind this option is targeted at developers who, in the
process of writing their code, want to do the occasional
make W=1 [target.o]
and let gcc do more extensive code checking for them. Then, they
could eyeball the output for valid gcc warnings about various
bugs/discrepancies which are not reported during the normal build
process.
For more background information and a use case, read through this
thread: http://marc.info/?l=kernel-janitors&m=129802065918147&w=2
-v5: Document internal kbuild variable which is introduced by this.
Simplify assignment.
-v4: Enable this for gcc only. Also, check for options which are not
supported by every gcc version we use to build the kernel. Adding those
checks for _every_ option slows down the build noticeably so be sensible
here and add it only when your gcc version chokes on a particular
option.
-v[2..3]: move to Makefile.build
Cc: Michal Marek <mma...@suse.cz>
Cc: linux-...@vger.kernel.org
Acked-by: Sam Ravnborg <s...@ravnborg.org>
Acked-by: Ingo Molnar <mi...@elte.hu>
Signed-off-by: Borislav Petkov <b...@alien8.de>
---
Documentation/kbuild/kbuild.txt | 5 +++++
Makefile | 5 +++++
scripts/Makefile.build | 35 ++++++++++++++++++++++++++++++++++-
3 files changed, 44 insertions(+), 1 deletions(-)
diff --git a/Documentation/kbuild/kbuild.txt b/Documentation/kbuild/kbuild.txt
index 4a99031..376538c 100644
--- a/Documentation/kbuild/kbuild.txt
+++ b/Documentation/kbuild/kbuild.txt
@@ -196,3 +196,8 @@ to be included in the databases, separated by blank space. E.g.:
To get all available archs you can also specify all. E.g.:
$ make ALLSOURCE_ARCHS=all tags
+
+KBUILD_ENABLE_EXTRA_GCC_CHECKS
+--------------------------------------------------
+If enabled over the make command line with "W=1", it turns on additional
+gcc -W... options for more extensive build-time checking.
diff --git a/Makefile b/Makefile
index c9c8c8f..8c4614f 100644
--- a/Makefile
+++ b/Makefile
@@ -102,6 +102,10 @@ ifeq ("$(origin O)", "command line")
KBUILD_OUTPUT := $(O)
endif
+ifeq ("$(origin W)", "command line")
+ export KBUILD_ENABLE_EXTRA_GCC_CHECKS := 1
+endif
+
# That's our default target when none is given on the command line
PHONY := _all
_all:
@@ -1262,6 +1266,7 @@ help:
@echo ' make O=dir [targets] Locate all output files in "dir", including .config'
@echo ' make C=1 [targets] Check all c source with $$CHECK (sparse by default)'
@echo ' make C=2 [targets] Force check of all c source with $$CHECK'
+ @echo ' make W=1 [targets] Enable extra gcc checks'
@echo ''
@echo 'Execute "make" or "make all" to build all targets marked with [*] '
@echo 'For further info see the ./README file'
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 4eb99ab..d5f925a 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -49,6 +49,40 @@ ifeq ($(KBUILD_NOPEDANTIC),)
$(error CFLAGS was changed in "$(kbuild-file)". Fix it to use EXTRA_CFLAGS)
endif
endif
+
+#
+# make W=1 settings
+#
+# $(call cc-option... ) handles gcc -W.. options which
+# are not supported by all versions of the compiler
+ifdef KBUILD_ENABLE_EXTRA_GCC_CHECKS
+
include scripts/Makefile.lib
ifdef host-progs
@@ -403,7 +437,6 @@ ifneq ($(cmd_files),)
include $(cmd_files)
endif
-
# Declare the contents of the .PHONY variable as phony. We keep that
# information in a variable se we can use it in if_changed and friends.
--
1.7.4.1.48.g5673d
Arnd Bergmann
> My intention was not to have multiple levels of warnings because then
> you have to go and enable the different levels and have to remember
> which level you used last, etc, etc.
I wasn't suggesting more than two, so the two would have very distinct
definitions:
W=1: Warnings that we would like to fix all over the tree, patches to
remove these are always welcome and you can build the entire kernel
with it. Once they are all fixed, we can make the warnings the default.
W=2: Warnings that we know we don't always want to fix, meant for what
you describe here -- you build a single file and decide what to
do based on common sense.
> Instead I am thinking along with the following lines:
>
> make W=1 [path/to/kernel/file.o] 2>w.log
>
> and then take a look at w.log and start fixing warnings.
>
> You can selectively ignore some of the warnings since, as you say
> yourself above, some simply make you write ugly code like enforcing
> casts just for the sake of shutting up the compiler. A great deal of the
> warnings come from includes which are hard to fix or gcc is issuing the
> warning wrong since we do sick sh*t with C in the kernel and that's OK
> :).
Yes, I understand that.
> But in all cases you have all the warnings in one single file and that's
> it. If a certain -W option is useless, we should rather remove it since
> it doesn't help anyway. The selection above is clearly not complete so
> I'd rather drop some instead of including different W=x levels.
>
> Hmm... ?
I'd have to see the output myself. My feeling is that a lot of the
-Wextra warnings are not that useful, but I don't know which ones
are included in -Wextra these days.
Arnd
Américo Wang
>On Monday 28 February 2011, Borislav Petkov wrote:
>> My intention was not to have multiple levels of warnings because then
>> you have to go and enable the different levels and have to remember
>> which level you used last, etc, etc.
>
>I wasn't suggesting more than two, so the two would have very distinct
>definitions:
>
>W=1: Warnings that we would like to fix all over the tree, patches to
> remove these are always welcome and you can build the entire kernel
> with it. Once they are all fixed, we can make the warnings the default.
>
>W=2: Warnings that we know we don't always want to fix, meant for what
> you describe here -- you build a single file and decide what to
> do based on common sense.
>
Right, this makes sense. Borislav, could you implement this?
Thanks.
Borislav Petkov
> On Tue, Mar 01, 2011 at 12:35:00PM +0100, Arnd Bergmann wrote:
> >On Monday 28 February 2011, Borislav Petkov wrote:
> >> My intention was not to have multiple levels of warnings because then
> >> you have to go and enable the different levels and have to remember
> >> which level you used last, etc, etc.
> >
> >I wasn't suggesting more than two, so the two would have very distinct
> >definitions:
> >
> >W=1: Warnings that we would like to fix all over the tree, patches to
> > remove these are always welcome and you can build the entire kernel
> > with it. Once they are all fixed, we can make the warnings the default.
> >
> >W=2: Warnings that we know we don't always want to fix, meant for what
> > you describe here -- you build a single file and decide what to
> > do based on common sense.
> >
>
> Right, this makes sense. Borislav, could you implement this?
Yeah, I could try to come up with a sensible choice for mutual-exclusive
sets of -W.. options. Any preferences?
--
Regards/Gruss,
Boris.
Borislav Petkov
> On Tue, Mar 01, 2011 at 09:35:29AM +0100, Borislav Petkov wrote:
> > Add a 'W=1' Makefile switch which adds additional checking per build
> > object.
> >
> > The idea behind this option is targeted at developers who, in the
> > process of writing their code, want to do the occasional
> >
> > make W=1 [target.o]
> >
> > and let gcc do more extensive code checking for them. Then, they
> > could eyeball the output for valid gcc warnings about various
> > bugs/discrepancies which are not reported during the normal build
> > process.
> >
> > For more background information and a use case, read through this
> > thread: http://marc.info/?l=kernel-janitors&m=129802065918147&w=2
> >
> > -v5: Document internal kbuild variable which is introduced by this.
> > Simplify assignment.
>
> good selection of warnings for W=1 and W=2, we can change it later.
Yeah, I was going to do that but am terribly busy at the moment. I'll do
an initial proposal against your kbuild branch when I get a chance.
Thanks.
--
Regards/Gruss,
Boris.
Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
General Managers: Alberto Bozzo, Andrew Bowd
Registration: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632
Ingo Molnar
* Michal Marek <mma...@suse.cz> wrote:
> On Tue, Mar 01, 2011 at 09:35:29AM +0100, Borislav Petkov wrote:
> > Add a 'W=1' Makefile switch which adds additional checking per build
> > object.
> >
> > The idea behind this option is targeted at developers who, in the
> > process of writing their code, want to do the occasional
> >
> > make W=1 [target.o]
> >
> > and let gcc do more extensive code checking for them. Then, they
> > could eyeball the output for valid gcc warnings about various
> > bugs/discrepancies which are not reported during the normal build
> > process.
> >
> > For more background information and a use case, read through this
> > thread: http://marc.info/?l=kernel-janitors&m=129802065918147&w=2
> >
> > -v5: Document internal kbuild variable which is introduced by this.
> > Simplify assignment.
>
> Thanks, applied to kbuild-2.6.git#packaging. If someone comes up with a
> good selection of warnings for W=1 and W=2, we can change it later.
Another, related, very nice kbuild feature would be to allow for arch maintainers to
mark certain files as "should only build fine without warnings" - i.e. -Werror
should be the default. There would be a Kconfig feature to opt out of this,
CONFIG_CC_IGNORE_WARNINGS=y or so. This would allow for people to still build the
kernel with old (or buggy) versions of GCC.
arch/x86/ would start using this by gradually marking more and more files as -Werror
by default. Some architectures like arch/sparc/ and arch/powerpc/ already build with
-Werror enabled - but for arch/x86/ we cannot force this (we cannot break the build
for who knows how many people) so we want to do it gradually and with a way out for
users with buggy compilers.
Thanks,
Ingo
Sam Ravnborg
> mark certain files as "should only build fine without warnings" - i.e. -Werror
> should be the default. There would be a Kconfig feature to opt out of this,
> CONFIG_CC_IGNORE_WARNINGS=y or so. This would allow for people to still build the
> kernel with old (or buggy) versions of GCC.
To add -Werror for all files conditionally you can do:
ccflags-$(CONFIG_WERROR) += -Werror
For individual files we can then drop -Werror like this:
CFLAGS_REMOVE_foobar.o := -Werror
So it should be doable with the existing infrastructure.
Sam
Ingo Molnar
* Sam Ravnborg <s...@ravnborg.org> wrote:
> > Another, related, very nice kbuild feature would be to allow for arch maintainers to
> > mark certain files as "should only build fine without warnings" - i.e. -Werror
> > should be the default. There would be a Kconfig feature to opt out of this,
> > CONFIG_CC_IGNORE_WARNINGS=y or so. This would allow for people to still build the
> > kernel with old (or buggy) versions of GCC.
>
> To add -Werror for all files conditionally you can do:
>
> ccflags-$(CONFIG_WERROR) += -Werror
>
> For individual files we can then drop -Werror like this:
>
> CFLAGS_REMOVE_foobar.o := -Werror
>
> So it should be doable with the existing infrastructure.
Stupid question: is there an existing kbuild rule that i could use to enable -Werror
for a single .o file, such as kernel/sched.o - without affecting other files in
kernel/ that i do not maintain?
Also, adding 3 lines per object file is pretty ugly - would it be possible to create
a nicer, compact, single-line way to someone condense a CONFIG_ERROR opt-out
mechanism, the -Werror default and the single-object-file into a single rule?
Something like:
obj-werror-y += sched.o
Although i'm not sure if it is wise to mix build details into the object build tree
hierarchy like that ...
Maybe we can live with some multi-line definition after all - until all of the
kernel is covered by such a mechanism. But the per object file rule would still be
important, for multi-maintainenace-boundaries directories like kernel/*.o.
Thanks,
Ingo
Michal Marek
>
> * Sam Ravnborg <s...@ravnborg.org> wrote:
>
>>> Another, related, very nice kbuild feature would be to allow for arch maintainers to
>>> mark certain files as "should only build fine without warnings" - i.e. -Werror
>>> should be the default. There would be a Kconfig feature to opt out of this,
>>> CONFIG_CC_IGNORE_WARNINGS=y or so. This would allow for people to still build the
>>> kernel with old (or buggy) versions of GCC.
>>
>> To add -Werror for all files conditionally you can do:
>>
>> ccflags-$(CONFIG_WERROR) += -Werror
>>
>> For individual files we can then drop -Werror like this:
>>
>> CFLAGS_REMOVE_foobar.o := -Werror
>>
>> So it should be doable with the existing infrastructure.
>
> Stupid question: is there an existing kbuild rule that i could use to enable -Werror
> for a single .o file, such as kernel/sched.o - without affecting other files in
> kernel/ that i do not maintain?
CFLAGS_sched.o := -Werror
> Also, adding 3 lines per object file is pretty ugly - would it be possible to create
> a nicer, compact, single-line way to someone condense a CONFIG_ERROR opt-out
> mechanism, the -Werror default and the single-object-file into a single rule?
>
> Something like:
>
> obj-werror-y += sched.o
>
> Although i'm not sure if it is wise to mix build details into the object build tree
> hierarchy like that ...
One way without extending the current rules could be:
ifdef CONFIG_CC_WERROR
Werror := -Werror
endif
CFLAGS_sched.o := $(Werror)
CFLAGS_another.o := $(Werror)
What do you think?
Borislav Petkov
>
> * Sam Ravnborg <s...@ravnborg.org> wrote:
>
> > > Another, related, very nice kbuild feature would be to allow for arch maintainers to
> > > mark certain files as "should only build fine without warnings" - i.e. -Werror
> > > should be the default. There would be a Kconfig feature to opt out of this,
> > > CONFIG_CC_IGNORE_WARNINGS=y or so. This would allow for people to still build the
> > > kernel with old (or buggy) versions of GCC.
> >
> > To add -Werror for all files conditionally you can do:
> >
> > ccflags-$(CONFIG_WERROR) += -Werror
> >
> > For individual files we can then drop -Werror like this:
> >
> > CFLAGS_REMOVE_foobar.o := -Werror
> >
> > So it should be doable with the existing infrastructure.
>
> Stupid question: is there an existing kbuild rule that i could use to enable -Werror
> for a single .o file, such as kernel/sched.o - without affecting other files in
> kernel/ that i do not maintain?
>
> Also, adding 3 lines per object file is pretty ugly - would it be possible to create
> a nicer, compact, single-line way to someone condense a CONFIG_ERROR opt-out
> mechanism, the -Werror default and the single-object-file into a single rule?
>
> Something like:
>
> obj-werror-y += sched.o
I believe you can do
CFLAGS_sched.o = -Werror
or similar but you'll have to try it out. I remember doing a more
involved per-object flags fumbling with the popcnt stuff, i.e. you might
want to take a look at d61931d89be506372d01a90d1755f6d0a9fafe2d where
we pass it an arch-specific config option so you can have per-object,
per-arch build flags :).
HTH.
--
Regards/Gruss,
Boris.
Sam Ravnborg
>
> ifdef CONFIG_CC_WERROR
> Werror := -Werror
> endif
>
> CFLAGS_sched.o := $(Werror)
> CFLAGS_another.o := $(Werror)
>
> What do you think?
We could extend kbuild to support something like this:
ccflags-<filename>-y := -Werror
Sample:
ccflags-sched.o-$(CONFIG_WERROR) := -Werror
The above is a nice counterpart to the existing ccflags-y.
And we could then deprecate CLFAGS_<filename> as we have today.
To remove options we could use:
ccflags-<filename>-remove-y := -pg
This is a replacement of the undocumented CFLAGS_REMOVE_<filename> variant.
Today:
ifdef CONFIG_FUNCTION_TRACER
CFLAGS_REMOVE_lockdep.o = -pg
endif
Could be replaced with:
ccflags-lockdep.o-remove-$(CONFIG_FUNCTION_TRACER) := -pg
The ifdef often cover more than one file so we save less lines in
reality than the example says.
All looks rather trivial to implement.
But it is maybe a bit too cryptic?
Sam
Arnd Bergmann
> Also, adding 3 lines per object file is pretty ugly - would it be possible to create
> a nicer, compact, single-line way to someone condense a CONFIG_ERROR opt-out
> mechanism, the -Werror default and the single-object-file into a single rule?
>
> Something like:
>
> obj-werror-y += sched.o
I haven't tried it, but it could work (or be made to work) recursively,
so you can link multiple objects together and have them use the same flag e.g.
obj-y += obj_werror.o
ccflags-obj_werror.o += -Werror
obj-werror-y += sched.o module.o user.o
Arnd