[v2] kbuild, LLVMLinux: Supress warnings unless W=1-3

Commit Message

Behan Webster July 31, 2014, 4:16 a.m. UTC

From: Behan Webster <[e-mail]>

clang has more warnings enabled by default. Turn them off unless W is set.
This patch fixes a logic bug where warnings in clang were disabled when W was set.

Signed-off-by: Behan Webster <[e-mail]>
Signed-off-by: Jan-Simon Möller <[e-mail]>
Signed-off-by: Mark Charlebois <[e-mail]>
Cc: [e-mail]
Cc: [e-mail]
---
 Makefile                   |  1 +
 scripts/Makefile.extrawarn | 22 ++++++++++++----------
 2 files changed, 13 insertions(+), 10 deletions(-)

Comments

Michal Marek July 31, 2014, 8:18 a.m. UTC | #1

Dne 31.7.2014 06:16, [e-mail] napsal(a):
> From: Behan Webster <[e-mail]>
> 
> clang has more warnings enabled by default. Turn them off unless W is set.
> This patch fixes a logic bug where warnings in clang were disabled when W was set.
> 
> Signed-off-by: Behan Webster <[e-mail]>
> Signed-off-by: Jan-Simon Möller <[e-mail]>
> Signed-off-by: Mark Charlebois <[e-mail]>
> Cc: [e-mail]
> Cc: [e-mail]
> ---
>  Makefile                   |  1 +
>  scripts/Makefile.extrawarn | 22 ++++++++++++----------
>  2 files changed, 13 insertions(+), 10 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index f6a7794..f343e17 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -668,6 +668,7 @@ KBUILD_CFLAGS += $(call cc-disable-warning, tautological-compare)
>  # source of a reference will be _MergedGlobals and not on of the whitelisted names.
>  # See modpost pattern 2
>  KBUILD_CFLAGS += $(call cc-option, -mno-global-merge,)
> +KBUILD_CFLAGS += $(call cc-option, -fcatch-undefined-behavior)
>  else
>  
>  # This warning generated too much noise in a regular build.
> diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn
> index 6564350..b5b0751 100644
> --- a/scripts/Makefile.extrawarn
> +++ b/scripts/Makefile.extrawarn
> @@ -26,16 +26,6 @@ warning-1 += $(call cc-option, -Wmissing-include-dirs)
>  warning-1 += $(call cc-option, -Wunused-but-set-variable)
>  warning-1 += $(call cc-disable-warning, missing-field-initializers)
>  
> -# Clang
> -warning-1 += $(call cc-disable-warning, initializer-overrides)
> -warning-1 += $(call cc-disable-warning, unused-value)
> -warning-1 += $(call cc-disable-warning, format)
> -warning-1 += $(call cc-disable-warning, unknown-warning-option)
> -warning-1 += $(call cc-disable-warning, sign-compare)
> -warning-1 += $(call cc-disable-warning, format-zero-length)
> -warning-1 += $(call cc-disable-warning, uninitialized)
> -warning-1 += $(call cc-option, -fcatch-undefined-behavior)
> -

OK.


>  warning-2 := -Waggregate-return
>  warning-2 += -Wcast-align
>  warning-2 += -Wdisabled-optimization
> @@ -55,6 +45,18 @@ warning-3 += -Wswitch-default
>  warning-3 += $(call cc-option, -Wpacked-bitfield-compat)
>  warning-3 += $(call cc-option, -Wvla)
>  
> +ifeq ($(COMPILER),clang)
> +ifndef $(W)
> +KBUILD_CFLAGS += $(call cc-disable-warning, initializer-overrides)
> +KBUILD_CFLAGS += $(call cc-disable-warning, unused-value)
> +KBUILD_CFLAGS += $(call cc-disable-warning, format)
> +KBUILD_CFLAGS += $(call cc-disable-warning, unknown-warning-option)
> +KBUILD_CFLAGS += $(call cc-disable-warning, sign-compare)
> +KBUILD_CFLAGS += $(call cc-disable-warning, format-zero-length)
> +KBUILD_CFLAGS += $(call cc-disable-warning, uninitialized)
> +endif
> +endif
> +

Please remove this part, it has no effect. I assume that if it works for
you, these warning are not as annoying so they do not need to be disabled?

Thanks,
Michal
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Behan Webster July 31, 2014, 4:12 p.m. UTC | #2

On 07/31/14 01:18, Michal Marek wrote:
> Dne 31.7.2014 06:16, [e-mail] napsal(a):
>> From: Behan Webster <[e-mail]>
>>
>> clang has more warnings enabled by default. Turn them off unless W is set.
>> This patch fixes a logic bug where warnings in clang were disabled when W was set.
>>
>> Signed-off-by: Behan Webster <[e-mail]>
>> Signed-off-by: Jan-Simon Möller <[e-mail]>
>> Signed-off-by: Mark Charlebois <[e-mail]>
>> Cc: [e-mail]
>> Cc: [e-mail]
>> ---
>>   Makefile                   |  1 +
>>   scripts/Makefile.extrawarn | 22 ++++++++++++----------
>>   2 files changed, 13 insertions(+), 10 deletions(-)
>>
>> diff --git a/Makefile b/Makefile
>> index f6a7794..f343e17 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -668,6 +668,7 @@ KBUILD_CFLAGS += $(call cc-disable-warning, tautological-compare)
>>   # source of a reference will be _MergedGlobals and not on of the whitelisted names.
>>   # See modpost pattern 2
>>   KBUILD_CFLAGS += $(call cc-option, -mno-global-merge,)
>> +KBUILD_CFLAGS += $(call cc-option, -fcatch-undefined-behavior)
>>   else
>>   
>>   # This warning generated too much noise in a regular build.
>> diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn
>> index 6564350..b5b0751 100644
>> --- a/scripts/Makefile.extrawarn
>> +++ b/scripts/Makefile.extrawarn
>> @@ -26,16 +26,6 @@ warning-1 += $(call cc-option, -Wmissing-include-dirs)
>>   warning-1 += $(call cc-option, -Wunused-but-set-variable)
>>   warning-1 += $(call cc-disable-warning, missing-field-initializers)
>>   
>> -# Clang
>> -warning-1 += $(call cc-disable-warning, initializer-overrides)
>> -warning-1 += $(call cc-disable-warning, unused-value)
>> -warning-1 += $(call cc-disable-warning, format)
>> -warning-1 += $(call cc-disable-warning, unknown-warning-option)
>> -warning-1 += $(call cc-disable-warning, sign-compare)
>> -warning-1 += $(call cc-disable-warning, format-zero-length)
>> -warning-1 += $(call cc-disable-warning, uninitialized)
>> -warning-1 += $(call cc-option, -fcatch-undefined-behavior)
>> -
> OK.
>
>
>>   warning-2 := -Waggregate-return
>>   warning-2 += -Wcast-align
>>   warning-2 += -Wdisabled-optimization
>> @@ -55,6 +45,18 @@ warning-3 += -Wswitch-default
>>   warning-3 += $(call cc-option, -Wpacked-bitfield-compat)
>>   warning-3 += $(call cc-option, -Wvla)
>>   
>> +ifeq ($(COMPILER),clang)
>> +ifndef $(W)
>> +KBUILD_CFLAGS += $(call cc-disable-warning, initializer-overrides)
>> +KBUILD_CFLAGS += $(call cc-disable-warning, unused-value)
>> +KBUILD_CFLAGS += $(call cc-disable-warning, format)
>> +KBUILD_CFLAGS += $(call cc-disable-warning, unknown-warning-option)
>> +KBUILD_CFLAGS += $(call cc-disable-warning, sign-compare)
>> +KBUILD_CFLAGS += $(call cc-disable-warning, format-zero-length)
>> +KBUILD_CFLAGS += $(call cc-disable-warning, uninitialized)
>> +endif
>> +endif
>> +
> Please remove this part, it has no effect. I assume that if it works for
> you, these warning are not as annoying so they do not need to be disabled?
Actually they are annoying, that's why they're disabled normally. Most 
of them complain about practices which are relatively common in kernel code.

clang warns about a lot more things than gcc does. It means that code 
which compiles cleanly in gcc often doesn't with clang. This cuts out 
the warnings which are unlikely to to be fixed in kernel code anytime 
soon, but which are probably worth exposing when W=1 is used.

This part of the patch explicitly deals with complaints from some in the 
kernel community that clang is too noisy with kernel code.

This part of the patch needs to be somewhere. This seemed the best place.

Behan

Michal Marek July 31, 2014, 8:46 p.m. UTC | #3

Dne 31.7.2014 18:12, Behan Webster napsal(a):
> On 07/31/14 01:18, Michal Marek wrote:
>> Dne 31.7.2014 06:16, [e-mail] napsal(a):
>>> @@ -55,6 +45,18 @@ warning-3 += -Wswitch-default
>>>   warning-3 += $(call cc-option, -Wpacked-bitfield-compat)
>>>   warning-3 += $(call cc-option, -Wvla)
>>>   +ifeq ($(COMPILER),clang)
>>> +ifndef $(W)
>>> +KBUILD_CFLAGS += $(call cc-disable-warning, initializer-overrides)
>>> +KBUILD_CFLAGS += $(call cc-disable-warning, unused-value)
>>> +KBUILD_CFLAGS += $(call cc-disable-warning, format)
>>> +KBUILD_CFLAGS += $(call cc-disable-warning, unknown-warning-option)
>>> +KBUILD_CFLAGS += $(call cc-disable-warning, sign-compare)
>>> +KBUILD_CFLAGS += $(call cc-disable-warning, format-zero-length)
>>> +KBUILD_CFLAGS += $(call cc-disable-warning, uninitialized)
>>> +endif
>>> +endif
>>> +
>> Please remove this part, it has no effect. I assume that if it works for
>> you, these warning are not as annoying so they do not need to be
>> disabled?
> Actually they are annoying, that's why they're disabled normally. Most
> of them complain about practices which are relatively common in kernel
> code.
> 
> clang warns about a lot more things than gcc does. It means that code
> which compiles cleanly in gcc often doesn't with clang. This cuts out
> the warnings which are unlikely to to be fixed in kernel code anytime
> soon, but which are probably worth exposing when W=1 is used.
> 
> This part of the patch explicitly deals with complaints from some in the
> kernel community that clang is too noisy with kernel code.
> 
> This part of the patch needs to be somewhere. This seemed the best place.

You placed it inside a branch that is only evaluated when W= is given.

Michal
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Behan Webster Aug. 1, 2014, 4:07 a.m. UTC | #4

On 07/31/14 13:46, Michal Marek wrote:
> Dne 31.7.2014 18:12, Behan Webster napsal(a):
>> On 07/31/14 01:18, Michal Marek wrote:
>>> Dne 31.7.2014 06:16, [e-mail] napsal(a):
>>>> @@ -55,6 +45,18 @@ warning-3 += -Wswitch-default
>>>>    warning-3 += $(call cc-option, -Wpacked-bitfield-compat)
>>>>    warning-3 += $(call cc-option, -Wvla)
>>>>    +ifeq ($(COMPILER),clang)
>>>> +ifndef $(W)
>>>> +KBUILD_CFLAGS += $(call cc-disable-warning, initializer-overrides)
>>>> +KBUILD_CFLAGS += $(call cc-disable-warning, unused-value)
>>>> +KBUILD_CFLAGS += $(call cc-disable-warning, format)
>>>> +KBUILD_CFLAGS += $(call cc-disable-warning, unknown-warning-option)
>>>> +KBUILD_CFLAGS += $(call cc-disable-warning, sign-compare)
>>>> +KBUILD_CFLAGS += $(call cc-disable-warning, format-zero-length)
>>>> +KBUILD_CFLAGS += $(call cc-disable-warning, uninitialized)
>>>> +endif
>>>> +endif
>>>> +
>>> Please remove this part, it has no effect. I assume that if it works for
>>> you, these warning are not as annoying so they do not need to be
>>> disabled?
>> Actually they are annoying, that's why they're disabled normally. Most
>> of them complain about practices which are relatively common in kernel
>> code.
>>
>> clang warns about a lot more things than gcc does. It means that code
>> which compiles cleanly in gcc often doesn't with clang. This cuts out
>> the warnings which are unlikely to to be fixed in kernel code anytime
>> soon, but which are probably worth exposing when W=1 is used.
>>
>> This part of the patch explicitly deals with complaints from some in the
>> kernel community that clang is too noisy with kernel code.
>>
>> This part of the patch needs to be somewhere. This seemed the best place.
> You placed it inside a branch that is only evaluated when W= is given.
Hmm. You're right. Will fix.

Behan

Patch

diff --git a/Makefile b/Makefile
index f6a7794..f343e17 100644
--- a/Makefile
+++ b/Makefile
@@ -668,6 +668,7 @@  KBUILD_CFLAGS += $(call cc-disable-warning, tautological-compare)
 # source of a reference will be _MergedGlobals and not on of the whitelisted names.
 # See modpost pattern 2
 KBUILD_CFLAGS += $(call cc-option, -mno-global-merge,)
+KBUILD_CFLAGS += $(call cc-option, -fcatch-undefined-behavior)
 else
 
 # This warning generated too much noise in a regular build.
diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn
index 6564350..b5b0751 100644
--- a/scripts/Makefile.extrawarn
+++ b/scripts/Makefile.extrawarn
@@ -26,16 +26,6 @@  warning-1 += $(call cc-option, -Wmissing-include-dirs)
 warning-1 += $(call cc-option, -Wunused-but-set-variable)
 warning-1 += $(call cc-disable-warning, missing-field-initializers)
 
-# Clang
-warning-1 += $(call cc-disable-warning, initializer-overrides)
-warning-1 += $(call cc-disable-warning, unused-value)
-warning-1 += $(call cc-disable-warning, format)
-warning-1 += $(call cc-disable-warning, unknown-warning-option)
-warning-1 += $(call cc-disable-warning, sign-compare)
-warning-1 += $(call cc-disable-warning, format-zero-length)
-warning-1 += $(call cc-disable-warning, uninitialized)
-warning-1 += $(call cc-option, -fcatch-undefined-behavior)
-
 warning-2 := -Waggregate-return
 warning-2 += -Wcast-align
 warning-2 += -Wdisabled-optimization
@@ -55,6 +45,18 @@  warning-3 += -Wswitch-default
 warning-3 += $(call cc-option, -Wpacked-bitfield-compat)
 warning-3 += $(call cc-option, -Wvla)
 
+ifeq ($(COMPILER),clang)
+ifndef $(W)
+KBUILD_CFLAGS += $(call cc-disable-warning, initializer-overrides)
+KBUILD_CFLAGS += $(call cc-disable-warning, unused-value)
+KBUILD_CFLAGS += $(call cc-disable-warning, format)
+KBUILD_CFLAGS += $(call cc-disable-warning, unknown-warning-option)
+KBUILD_CFLAGS += $(call cc-disable-warning, sign-compare)
+KBUILD_CFLAGS += $(call cc-disable-warning, format-zero-length)
+KBUILD_CFLAGS += $(call cc-disable-warning, uninitialized)
+endif
+endif
+
 warning := $(warning-$(findstring 1, $(KBUILD_ENABLE_EXTRA_GCC_CHECKS)))
 warning += $(warning-$(findstring 2, $(KBUILD_ENABLE_EXTRA_GCC_CHECKS)))
 warning += $(warning-$(findstring 3, $(KBUILD_ENABLE_EXTRA_GCC_CHECKS)))