[PATCH 1/2] kbuild: move -Werror from KBUILD_CFLAGS to KBUILD_CPPFLAGS

[PATCH 1/2] kbuild: move -Werror from KBUILD_CFLAGS to KBUILD_CPPFLAGS

[Masahiro Yamada]

CONFIG_WERROR turns warnings into errors, which happens only for *.c
files because -Werror is added to KBUILD_CFLAGS.

Adding it to KBUILD_CPPFLAGS makes more sense because preprocessors
understand the -Werror option.

For example, you can put a #warning directive in any preprocessed code.

    warning: #warning "this is a warning message" [-Wcpp]

If -Werror is added, it is promoted to an error.

    error: #warning "this is a warning message" [-Werror=cpp]

This commit moves -Werror to KBUILD_CPPFLAGS so it works in the same way
for *.c, *.S, *.lds.S or whatever needs preprocessing.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
---

 Makefile | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 53fa1a9fba8a..f84b57910667 100644
--- a/Makefile
+++ b/Makefile
@@ -869,7 +869,8 @@ stackp-flags-$(CONFIG_STACKPROTECTOR_STRONG)      := -fstack-protector-strong
 
 KBUILD_CFLAGS += $(stackp-flags-y)
 
-KBUILD_CFLAGS-$(CONFIG_WERROR) += -Werror
+KBUILD_CPPFLAGS-$(CONFIG_WERROR) += -Werror
+KBUILD_CPPFLAGS += $(KBUILD_CPPFLAGS-y)
 KBUILD_CFLAGS-$(CONFIG_CC_NO_ARRAY_BOUNDS) += -Wno-array-bounds
 
 KBUILD_RUSTFLAGS-$(CONFIG_WERROR) += -Dwarnings
-- 
2.34.1

[PATCH 2/2] kbuild: add -Wundef to KBUILD_CPPFLAGS for W=1 builds

[Masahiro Yamada]

The use of an undefined macro in an #if directive is warned, but only
in *.c files. No warning from other files such as *.S, *.lds.S.

Since -Wundef is a preprocessor-related warning, it should be added to
KBUILD_CPPFLAGS instead of KBUILD_CFLAGS.

My previous attempt [1] uncovered several warnings, and could not finish
fixing them all.

This commit adds -Wundef to KBUILD_CPPFLAGS for W=1 builds in order to
block new breakages. (The kbuild test robot tests with W=1)

We can fix the warnings one by one. After we fix all of them, we can
make this default in the top Makefile, and remove -Wundef from
KBUILD_CFLAGS.

[1]: https://lore.kernel.org/all/20221012180118.331005-2-masahiroy@kernel.org/

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 scripts/Makefile.extrawarn | 1 +
 1 file changed, 1 insertion(+)

diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn
index 6bbba36c5969..40cd13eca82e 100644
--- a/scripts/Makefile.extrawarn
+++ b/scripts/Makefile.extrawarn
@@ -38,6 +38,7 @@ KBUILD_CFLAGS += -Wno-sign-compare
 KBUILD_CFLAGS += -Wno-type-limits
 KBUILD_CFLAGS += -Wno-shift-negative-value
 
+KBUILD_CPPFLAGS += -Wundef
 KBUILD_CPPFLAGS += -DKBUILD_EXTRA_WARN1
 
 else
-- 
2.34.1

Re: [PATCH 2/2] kbuild: add -Wundef to KBUILD_CPPFLAGS for W=1 builds

[Nathan Chancellor]

On Tue, Dec 06, 2022 at 01:07:31PM +0900, Masahiro Yamada wrote:
> The use of an undefined macro in an #if directive is warned, but only
> in *.c files. No warning from other files such as *.S, *.lds.S.
> 
> Since -Wundef is a preprocessor-related warning, it should be added to
> KBUILD_CPPFLAGS instead of KBUILD_CFLAGS.
> 
> My previous attempt [1] uncovered several warnings, and could not finish
> fixing them all.
> 
> This commit adds -Wundef to KBUILD_CPPFLAGS for W=1 builds in order to
> block new breakages. (The kbuild test robot tests with W=1)
> 
> We can fix the warnings one by one. After we fix all of them, we can
> make this default in the top Makefile, and remove -Wundef from
> KBUILD_CFLAGS.
> 
> [1]: https://lore.kernel.org/all/20221012180118.331005-2-masahiroy@kernel.org/
> 
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

Looks like the robot already found a few so this clearly works as
intended.

> ---
> 
>  scripts/Makefile.extrawarn | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn
> index 6bbba36c5969..40cd13eca82e 100644
> --- a/scripts/Makefile.extrawarn
> +++ b/scripts/Makefile.extrawarn
> @@ -38,6 +38,7 @@ KBUILD_CFLAGS += -Wno-sign-compare
>  KBUILD_CFLAGS += -Wno-type-limits
>  KBUILD_CFLAGS += -Wno-shift-negative-value
>  
> +KBUILD_CPPFLAGS += -Wundef
>  KBUILD_CPPFLAGS += -DKBUILD_EXTRA_WARN1
>  
>  else
> -- 
> 2.34.1
> 
> 

Re: [PATCH 2/2] kbuild: add -Wundef to KBUILD_CPPFLAGS for W=1 builds

[Nick Desaulniers]

On Mon, Dec 5, 2022 at 8:07 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> The use of an undefined macro in an #if directive is warned, but only
> in *.c files. No warning from other files such as *.S, *.lds.S.
>
> Since -Wundef is a preprocessor-related warning, it should be added to
> KBUILD_CPPFLAGS instead of KBUILD_CFLAGS.
>
> My previous attempt [1] uncovered several warnings, and could not finish
> fixing them all.
>
> This commit adds -Wundef to KBUILD_CPPFLAGS for W=1 builds in order to
> block new breakages. (The kbuild test robot tests with W=1)
>
> We can fix the warnings one by one. After we fix all of them, we can
> make this default in the top Makefile, and remove -Wundef from
> KBUILD_CFLAGS.

I like that approach.
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

>
> [1]: https://lore.kernel.org/all/20221012180118.331005-2-masahiroy@kernel.org/
>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---
>
>  scripts/Makefile.extrawarn | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn
> index 6bbba36c5969..40cd13eca82e 100644
> --- a/scripts/Makefile.extrawarn
> +++ b/scripts/Makefile.extrawarn
> @@ -38,6 +38,7 @@ KBUILD_CFLAGS += -Wno-sign-compare
>  KBUILD_CFLAGS += -Wno-type-limits
>  KBUILD_CFLAGS += -Wno-shift-negative-value
>
> +KBUILD_CPPFLAGS += -Wundef
>  KBUILD_CPPFLAGS += -DKBUILD_EXTRA_WARN1
>
>  else
> --
> 2.34.1
>


-- 
Thanks,
~Nick Desaulniers

Re: [PATCH 1/2] kbuild: move -Werror from KBUILD_CFLAGS to KBUILD_CPPFLAGS

[Nathan Chancellor]

On Tue, Dec 06, 2022 at 01:07:30PM +0900, Masahiro Yamada wrote:
> CONFIG_WERROR turns warnings into errors, which happens only for *.c
> files because -Werror is added to KBUILD_CFLAGS.
> 
> Adding it to KBUILD_CPPFLAGS makes more sense because preprocessors
> understand the -Werror option.
> 
> For example, you can put a #warning directive in any preprocessed code.
> 
>     warning: #warning "this is a warning message" [-Wcpp]
> 
> If -Werror is added, it is promoted to an error.
> 
>     error: #warning "this is a warning message" [-Werror=cpp]
> 
> This commit moves -Werror to KBUILD_CPPFLAGS so it works in the same way
> for *.c, *.S, *.lds.S or whatever needs preprocessing.
> 
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

This seems reasonable, as people should not really be polluting the
build with messages through pragmas anyways, since the build should
always be clean.

> ---
> 
>  Makefile | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/Makefile b/Makefile
> index 53fa1a9fba8a..f84b57910667 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -869,7 +869,8 @@ stackp-flags-$(CONFIG_STACKPROTECTOR_STRONG)      := -fstack-protector-strong
>  
>  KBUILD_CFLAGS += $(stackp-flags-y)
>  
> -KBUILD_CFLAGS-$(CONFIG_WERROR) += -Werror
> +KBUILD_CPPFLAGS-$(CONFIG_WERROR) += -Werror
> +KBUILD_CPPFLAGS += $(KBUILD_CPPFLAGS-y)
>  KBUILD_CFLAGS-$(CONFIG_CC_NO_ARRAY_BOUNDS) += -Wno-array-bounds
>  
>  KBUILD_RUSTFLAGS-$(CONFIG_WERROR) += -Dwarnings
> -- 
> 2.34.1
> 
> 

[PATCH 1/2] kbuild: move -Werror from KBUILD_CFLAGS to KBUILD_CPPFLAGS

[Masahiro Yamada]

CONFIG_WERROR makes warnings into errors, but it only happens for *.c
files because -Werror is added to KBUILD_CFLAGS.

For example, you can put a #warning directive in any preprocessed
source file:

    #warning "blah blah ..."

If it is placed in a *.c file, it emits a warning by default, and it
is promoted to an error when CONFIG_WERROR is enabled:

    error: #warning "blah blah ..." [-Werror=cpp]

If it is placed in a *.S file, it is still a warning.

Move it to KBUILD_CPPFLAGS, so it works in the same way for *.c,
*.S, *.lds.S or whatever needs preprocessing.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 Makefile | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index ebcb75442d7f..027d9163eff6 100644
--- a/Makefile
+++ b/Makefile
@@ -788,7 +788,8 @@ stackp-flags-$(CONFIG_STACKPROTECTOR_STRONG)      := -fstack-protector-strong
 
 KBUILD_CFLAGS += $(stackp-flags-y)
 
-KBUILD_CFLAGS-$(CONFIG_WERROR) += -Werror
+KBUILD_CPPFLAGS-$(CONFIG_WERROR) += -Werror
+KBUILD_CPPFLAGS += $(KBUILD_CPPFLAGS-y)
 KBUILD_CFLAGS-$(CONFIG_CC_NO_ARRAY_BOUNDS) += -Wno-array-bounds
 KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) $(CONFIG_CC_IMPLICIT_FALLTHROUGH)
 
-- 
2.34.1

Re: [PATCH 1/2] kbuild: move -Werror from KBUILD_CFLAGS to KBUILD_CPPFLAGS

[Nick Desaulniers]

On Mon, Sep 5, 2022 at 1:37 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> CONFIG_WERROR makes warnings into errors, but it only happens for *.c
> files because -Werror is added to KBUILD_CFLAGS.
>
> For example, you can put a #warning directive in any preprocessed
> source file:
>
>     #warning "blah blah ..."
>
> If it is placed in a *.c file, it emits a warning by default, and it
> is promoted to an error when CONFIG_WERROR is enabled:
>
>     error: #warning "blah blah ..." [-Werror=cpp]
>
> If it is placed in a *.S file, it is still a warning.
>
> Move it to KBUILD_CPPFLAGS, so it works in the same way for *.c,
> *.S, *.lds.S or whatever needs preprocessing.
>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>

Thanks for the patch. I see in lore you sent many more cleanups but
didn't cc me explicitly...I should probably subscribe to that mailing
list! I probably won't have time to review many patches until after
Linux Plumbers Conf next week.

Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

> ---
>
>  Makefile | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/Makefile b/Makefile
> index ebcb75442d7f..027d9163eff6 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -788,7 +788,8 @@ stackp-flags-$(CONFIG_STACKPROTECTOR_STRONG)      := -fstack-protector-strong
>
>  KBUILD_CFLAGS += $(stackp-flags-y)
>
> -KBUILD_CFLAGS-$(CONFIG_WERROR) += -Werror
> +KBUILD_CPPFLAGS-$(CONFIG_WERROR) += -Werror
> +KBUILD_CPPFLAGS += $(KBUILD_CPPFLAGS-y)
>  KBUILD_CFLAGS-$(CONFIG_CC_NO_ARRAY_BOUNDS) += -Wno-array-bounds
>  KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) $(CONFIG_CC_IMPLICIT_FALLTHROUGH)
>
> --
> 2.34.1
>


-- 
Thanks,
~Nick Desaulniers