2011-04-21 19:21:24

by Dave Jones

[permalink] [raw]
Subject: annoying new gcc 4.6.0 warnings.

gcc 4.6.0 enables a new warning by default, which spews ~3000 lines
of extra warnings to my build. The warning looks like ..

arch/x86/kernel/cpu/cpufreq/powernow-k8.c: In function ‘pending_bit_stuck’:
arch/x86/kernel/cpu/cpufreq/powernow-k8.c:108:10: warning: variable ‘hi’ set but not used [-Wunused-but-set-variable]
arch/x86/kernel/cpu/cpufreq/powernow-k8.c: In function ‘core_voltage_pre_transition’:
arch/x86/kernel/cpu/cpufreq/powernow-k8.c:340:14: warning: variable ‘lo’ set but not used [-Wunused-but-set-variable]

In those cases, there's no bug there, the code just doesn't use all the arguments
that get passed to rdmsr... For eg,

static int pending_bit_stuck(void)
{
u32 lo, hi;

if (cpu_family == CPU_HW_PSTATE)
return 0;

rdmsr(MSR_FIDVID_STATUS, lo, hi);
return lo & MSR_S_LO_CHANGE_PENDING ? 1 : 0;
}


gcc's manpage says to dismiss this warning with attribute(unused), but sprinkling thousands
of those through the source seems pretty ugly. As does rewriting functions
like rdmsr to handle NULL as an argument.

There might be some valid bugs found (I think DaveM found a few already) from this new
warning, but it seems like everything I've looked at so far is just noise.

Any thoughts ?

Dave


2011-04-21 19:25:17

by Linus Torvalds

[permalink] [raw]
Subject: Re: annoying new gcc 4.6.0 warnings.

On Thu, Apr 21, 2011 at 12:21 PM, Dave Jones <[email protected]> wrote:
>
> There might be some valid bugs found (I think DaveM found a few already) from this new
> warning, but it seems like everything I've looked at so far is just noise.
>
> Any thoughts ?

Yeah, let's just disable the crazy warning. Maybe some of them are
valid, but it's like the f*cking sign-compare warning: most of them
are just inane noise, and as such the warning is not worth the pain.

I assume (hope) that there is some -Wno-unused-but-set-variable thing
we could do, the same way we do -Wno-trigraphs for another totally
useless gcc warning.

Linus

2011-04-21 19:39:26

by David Daney

[permalink] [raw]
Subject: Re: annoying new gcc 4.6.0 warnings.

On 04/21/2011 12:24 PM, Linus Torvalds wrote:
> On Thu, Apr 21, 2011 at 12:21 PM, Dave Jones<[email protected]> wrote:
>>
>> There might be some valid bugs found (I think DaveM found a few already) from this new
>> warning, but it seems like everything I've looked at so far is just noise.
>>
>> Any thoughts ?
>
> Yeah, let's just disable the crazy warning. Maybe some of them are
> valid, but it's like the f*cking sign-compare warning: most of them
> are just inane noise, and as such the warning is not worth the pain.
>
> I assume (hope) that there is some -Wno-unused-but-set-variable thing
> we could do, the same way we do -Wno-trigraphs for another totally
> useless gcc warning.
>

This particular warning has helped to find quite a bit of dead code.

Would it make any sense to add a config option to enable the spew for
those that wanted to see it?

David Daney

2011-04-21 19:44:49

by Joe Perches

[permalink] [raw]
Subject: Re: annoying new gcc 4.6.0 warnings.

On Thu, 2011-04-21 at 12:39 -0700, David Daney wrote:
> Would it make any sense to add a config option to enable the spew for
> those that wanted to see it?

Add it to make W=1 instead?
See commit 4a5838ad9d2d

2011-04-21 19:58:50

by Dave Jones

[permalink] [raw]
Subject: Re: annoying new gcc 4.6.0 warnings.

On Thu, Apr 21, 2011 at 12:44:46PM -0700, Joe Perches wrote:
> On Thu, 2011-04-21 at 12:39 -0700, David Daney wrote:
> > Would it make any sense to add a config option to enable the spew for
> > those that wanted to see it?
>
> Add it to make W=1 instead?
> See commit 4a5838ad9d2d

This patch does that. Though I question the usefulness of W=1.
I just built powernow-k8.o with W=1, and got 4030 lines of output.
That's just insane.

Dave

--

Disable the new -Wunused-but-set-variable that was added in gcc 4.6.0
It produces more false positives than useful warnings.

This can still be enabled using W=1

Signed-off-by: Dave Jones <[email protected]>

diff --git a/Makefile b/Makefile
index b967b96..68f178e 100644
--- a/Makefile
+++ b/Makefile
@@ -359,7 +359,8 @@ KBUILD_CFLAGS := -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs \
-fno-strict-aliasing -fno-common \
-Werror-implicit-function-declaration \
-Wno-format-security \
- -fno-delete-null-pointer-checks
+ -fno-delete-null-pointer-checks \
+ -Wno-unused-but-set-variable
KBUILD_AFLAGS_KERNEL :=
KBUILD_CFLAGS_KERNEL :=
KBUILD_AFLAGS := -D__ASSEMBLY__
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index d5f925a..30627ab 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -79,6 +79,7 @@ KBUILD_EXTRA_WARNINGS += -Wpointer-arith
KBUILD_EXTRA_WARNINGS += -Wredundant-decls
KBUILD_EXTRA_WARNINGS += -Wshadow
KBUILD_EXTRA_WARNINGS += -Wswitch-default
+KBUILD_EXTRA_WARNINGS += -Wunused-but-set-variable
KBUILD_EXTRA_WARNINGS += $(call cc-option, -Wvla,)
KBUILD_CFLAGS += $(KBUILD_EXTRA_WARNINGS)
endif

2011-04-21 20:08:38

by Sam Ravnborg

[permalink] [raw]
Subject: Re: annoying new gcc 4.6.0 warnings.

On Thu, Apr 21, 2011 at 03:58:02PM -0400, Dave Jones wrote:
> On Thu, Apr 21, 2011 at 12:44:46PM -0700, Joe Perches wrote:
> > On Thu, 2011-04-21 at 12:39 -0700, David Daney wrote:
> > > Would it make any sense to add a config option to enable the spew for
> > > those that wanted to see it?
> >
> > Add it to make W=1 instead?
> > See commit 4a5838ad9d2d
>
> This patch does that. Though I question the usefulness of W=1.
> I just built powernow-k8.o with W=1, and got 4030 lines of output.
> That's just insane.

It was discussed to do:
W=1 - the most usefull warnings
W=2 - the less usefull warnings
W=3 - the noise

Or somthing like that.
But no-one stepped up doing the classification then.
Should be simple to do.

My gcc (4.3.2) barfed over -Wno-unused-but-set-variable.
So your patch needs to check if the option is supported.

Sam

2011-04-21 20:09:37

by Linus Torvalds

[permalink] [raw]
Subject: Re: annoying new gcc 4.6.0 warnings.

On Thu, Apr 21, 2011 at 12:58 PM, Dave Jones <[email protected]> wrote:
> @@ -359,7 +359,8 @@ KBUILD_CFLAGS ? := -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs \
> ? ? ? ? ? ? ? ? ? -fno-strict-aliasing -fno-common \
> ? ? ? ? ? ? ? ? ? -Werror-implicit-function-declaration \
> ? ? ? ? ? ? ? ? ? -Wno-format-security \
> - ? ? ? ? ? ? ? ? ?-fno-delete-null-pointer-checks
> + ? ? ? ? ? ? ? ? ?-fno-delete-null-pointer-checks \
> + ? ? ? ? ? ? ? ? ?-Wno-unused-but-set-variable

Does this work ok for older gcc's? Do they understand that
-Wno-unused-by-set-variable?

gcc-4.5.1 seems to understand it, but what about much older gccs?

Linus

2011-04-21 20:16:19

by Dave Jones

[permalink] [raw]
Subject: Re: annoying new gcc 4.6.0 warnings.

On Thu, Apr 21, 2011 at 01:09:11PM -0700, Linus Torvalds wrote:
> On Thu, Apr 21, 2011 at 12:58 PM, Dave Jones <[email protected]> wrote:
> > @@ -359,7 +359,8 @@ KBUILD_CFLAGS ? := -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs \
> > ? ? ? ? ? ? ? ? ? -fno-strict-aliasing -fno-common \
> > ? ? ? ? ? ? ? ? ? -Werror-implicit-function-declaration \
> > ? ? ? ? ? ? ? ? ? -Wno-format-security \
> > - ? ? ? ? ? ? ? ? ?-fno-delete-null-pointer-checks
> > + ? ? ? ? ? ? ? ? ?-fno-delete-null-pointer-checks \
> > + ? ? ? ? ? ? ? ? ?-Wno-unused-but-set-variable
>
> Does this work ok for older gcc's? Do they understand that
> -Wno-unused-by-set-variable?
>
> gcc-4.5.1 seems to understand it, but what about much older gccs?

aparently not. I'll poke at it some more.

Dave

2011-04-21 20:28:00

by Dave Jones

[permalink] [raw]
Subject: Re: annoying new gcc 4.6.0 warnings.

On Thu, Apr 21, 2011 at 10:08:36PM +0200, Sam Ravnborg wrote:

> My gcc (4.3.2) barfed over -Wno-unused-but-set-variable.
> So your patch needs to check if the option is supported.

I don't have a gcc older than 4.4.0. Can you try this ?

Dave
--

Disable the new -Wunused-but-set-variable that was added in gcc 4.6.0
It produces more false positives than useful warnings.

This can still be enabled using W=1

Signed-off-by: Dave Jones <[email protected]>

diff --git a/Makefile b/Makefile
index b967b96..29e16f2 100644
--- a/Makefile
+++ b/Makefile
@@ -559,6 +559,8 @@ ifndef CONFIG_CC_STACKPROTECTOR
KBUILD_CFLAGS += $(call cc-option, -fno-stack-protector)
endif

+KBUILD_CFLAGS += $(call cc-option, -Wno-unused-but-set-variable)
+
ifdef CONFIG_FRAME_POINTER
KBUILD_CFLAGS += -fno-omit-frame-pointer -fno-optimize-sibling-calls
else
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index d5f925a..30627ab 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -79,6 +79,7 @@ KBUILD_EXTRA_WARNINGS += -Wpointer-arith
KBUILD_EXTRA_WARNINGS += -Wredundant-decls
KBUILD_EXTRA_WARNINGS += -Wshadow
KBUILD_EXTRA_WARNINGS += -Wswitch-default
+KBUILD_EXTRA_WARNINGS += -Wunused-but-set-variable
KBUILD_EXTRA_WARNINGS += $(call cc-option, -Wvla,)
KBUILD_CFLAGS += $(KBUILD_EXTRA_WARNINGS)
endif

2011-04-21 20:30:36

by Stratos Psomadakis

[permalink] [raw]
Subject: Re: annoying new gcc 4.6.0 warnings.

On 21/04/2011 10:21 μμ, Dave Jones wrote:
> gcc 4.6.0 enables a new warning by default, which spews ~3000 lines
> of extra warnings to my build. The warning looks like ..
>
> arch/x86/kernel/cpu/cpufreq/powernow-k8.c: In function ‘pending_bit_stuck’:
> arch/x86/kernel/cpu/cpufreq/powernow-k8.c:108:10: warning: variable ‘hi’ set but not used [-Wunused-but-set-variable]
> arch/x86/kernel/cpu/cpufreq/powernow-k8.c: In function ‘core_voltage_pre_transition’:
> arch/x86/kernel/cpu/cpufreq/powernow-k8.c:340:14: warning: variable ‘lo’ set but not used [-Wunused-but-set-variable]
>
> In those cases, there's no bug there, the code just doesn't use all the arguments
> that get passed to rdmsr... For eg,
>
> static int pending_bit_stuck(void)
> {
> u32 lo, hi;
>
> if (cpu_family == CPU_HW_PSTATE)
> return 0;
>
> rdmsr(MSR_FIDVID_STATUS, lo, hi);
> return lo & MSR_S_LO_CHANGE_PENDING ? 1 : 0;
> }
>
>
> gcc's manpage says to dismiss this warning with attribute(unused), but sprinkling thousands
> of those through the source seems pretty ugly. As does rewriting functions
> like rdmsr to handle NULL as an argument.
>
> There might be some valid bugs found (I think DaveM found a few already) from this new
> warning, but it seems like everything I've looked at so far is just noise.
>
> Any thoughts ?
>
> Dave
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
This new gcc-4.6.0 behavior can also cause build failures for archs
which use -Werror by default (sparc, ppc) [1][2].

[1] https://lkml.org/lkml/2011/4/19/383
[2] https://lkml.org/lkml/2011/4/18/287

--
Stratos Psomadakis
<[email protected]>

2011-04-21 20:37:43

by David Daney

[permalink] [raw]
Subject: Re: annoying new gcc 4.6.0 warnings.

On 04/21/2011 01:27 PM, Dave Jones wrote:
> On Thu, Apr 21, 2011 at 10:08:36PM +0200, Sam Ravnborg wrote:
>
> > My gcc (4.3.2) barfed over -Wno-unused-but-set-variable.
> > So your patch needs to check if the option is supported.
>
> I don't have a gcc older than 4.4.0. Can you try this ?
>
> Dave
> --
>
> Disable the new -Wunused-but-set-variable that was added in gcc 4.6.0
> It produces more false positives than useful warnings.
>
> This can still be enabled using W=1
>
> Signed-off-by: Dave Jones<[email protected]>
>
> diff --git a/Makefile b/Makefile
> index b967b96..29e16f2 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -559,6 +559,8 @@ ifndef CONFIG_CC_STACKPROTECTOR
> KBUILD_CFLAGS += $(call cc-option, -fno-stack-protector)
> endif
>
> +KBUILD_CFLAGS += $(call cc-option, -Wno-unused-but-set-variable)
> +
> ifdef CONFIG_FRAME_POINTER
> KBUILD_CFLAGS += -fno-omit-frame-pointer -fno-optimize-sibling-calls
> else
> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> index d5f925a..30627ab 100644
> --- a/scripts/Makefile.build
> +++ b/scripts/Makefile.build
> @@ -79,6 +79,7 @@ KBUILD_EXTRA_WARNINGS += -Wpointer-arith
> KBUILD_EXTRA_WARNINGS += -Wredundant-decls
> KBUILD_EXTRA_WARNINGS += -Wshadow
> KBUILD_EXTRA_WARNINGS += -Wswitch-default
> +KBUILD_EXTRA_WARNINGS += -Wunused-but-set-variable

Well I didn't test it, but presumably if you need the $(call cc-option,
-Wno-unused-but-set-variable), you would need similar here.

David Daney

2011-04-21 20:46:36

by Dave Jones

[permalink] [raw]
Subject: Re: annoying new gcc 4.6.0 warnings.

On Thu, Apr 21, 2011 at 01:37:36PM -0700, David Daney wrote:
> > +KBUILD_EXTRA_WARNINGS += -Wunused-but-set-variable
>
> Well I didn't test it, but presumably if you need the $(call cc-option,
> -Wno-unused-but-set-variable), you would need similar here.

Third time's the charm ?

Dave

--

Disable the new -Wunused-but-set-variable that was added in gcc 4.6.0
It produces more false positives than useful warnings.

This can still be enabled using W=1

Signed-off-by: Dave Jones <[email protected]>

diff --git a/Makefile b/Makefile
index b967b96..29e16f2 100644
--- a/Makefile
+++ b/Makefile
@@ -559,6 +559,8 @@ ifndef CONFIG_CC_STACKPROTECTOR
KBUILD_CFLAGS += $(call cc-option, -fno-stack-protector)
endif

+KBUILD_CFLAGS += $(call cc-option, -Wno-unused-but-set-variable)
+
ifdef CONFIG_FRAME_POINTER
KBUILD_CFLAGS += -fno-omit-frame-pointer -fno-optimize-sibling-calls
else
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index d5f925a..30627ab 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -79,6 +79,7 @@ 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, -Wunused-but-set-variable)
KBUILD_EXTRA_WARNINGS += $(call cc-option, -Wvla,)
KBUILD_CFLAGS += $(KBUILD_EXTRA_WARNINGS)
endif

2011-04-21 20:56:16

by Sam Ravnborg

[permalink] [raw]
Subject: Re: annoying new gcc 4.6.0 warnings.

On Thu, Apr 21, 2011 at 04:45:49PM -0400, Dave Jones wrote:
> On Thu, Apr 21, 2011 at 01:37:36PM -0700, David Daney wrote:
> > > +KBUILD_EXTRA_WARNINGS += -Wunused-but-set-variable
> >
> > Well I didn't test it, but presumably if you need the $(call cc-option,
> > -Wno-unused-but-set-variable), you would need similar here.
>
> Third time's the charm ?

Seems soo. It semi works now.
"make W=1" is broken for reasons that is not related to this patch - will submit
a fix in a minute for that.

So you can add:
Acked-by: Sam Ravnborg <[email protected]>
Tested-by: Sam Ravnborg <[email protected]>

on the patch.

And if you really want to make me happy then you add a comment about _why_
we disable the warning.

Something like:

# This warning generated too much noise in a regular build.
# Use make W=1 to enable this warning (see scripts/Makefile.build)
+KBUILD_CFLAGS += $(call cc-option, -Wno-unused-but-set-variable)
+
ifdef CONFIG_FRAME_POINTER


Sam

2011-04-21 21:10:13

by Sam Ravnborg

[permalink] [raw]
Subject: Re: annoying new gcc 4.6.0 warnings.

> This new gcc-4.6.0 behavior can also cause build failures for archs
> which use -Werror by default (sparc, ppc) [1][2].
>
> [1] https://lkml.org/lkml/2011/4/19/383
> [2] https://lkml.org/lkml/2011/4/18/287

sparc was easy to fix.
alpha dropped -Werror for now.

Sam

2011-04-21 21:28:35

by Dave Jones

[permalink] [raw]
Subject: Re: annoying new gcc 4.6.0 warnings.

On Thu, Apr 21, 2011 at 10:56:12PM +0200, Sam Ravnborg wrote:
>
> Seems soo. It semi works now.
> "make W=1" is broken for reasons that is not related to this patch - will submit
> a fix in a minute for that.
>
> So you can add:
> Acked-by: Sam Ravnborg <[email protected]>
> Tested-by: Sam Ravnborg <[email protected]>
>
> on the patch.
>
> And if you really want to make me happy then you add a comment about _why_
> we disable the warning.

ok, last time for reals.

Dave

--

Disable the new -Wunused-but-set-variable that was added in gcc 4.6.0
It produces more false positives than useful warnings.

This can still be enabled using W=1

Signed-off-by: Dave Jones <[email protected]>
Acked-by: Sam Ravnborg <[email protected]>
Tested-by: Sam Ravnborg <[email protected]>

diff --git a/Makefile b/Makefile
index b967b96..08c5484 100644
--- a/Makefile
+++ b/Makefile
@@ -559,6 +559,10 @@ ifndef CONFIG_CC_STACKPROTECTOR
KBUILD_CFLAGS += $(call cc-option, -fno-stack-protector)
endif

+# This warning generated too much noise in a regular build.
+# Use make W=1 to enable this warning (see scripts/Makefile.build)
+KBUILD_CFLAGS += $(call cc-option, -Wno-unused-but-set-variable)
+
ifdef CONFIG_FRAME_POINTER
KBUILD_CFLAGS += -fno-omit-frame-pointer -fno-optimize-sibling-calls
else
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index d5f925a..021f7eb 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -79,6 +79,7 @@ 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, -Wunused-but-set-variable)
KBUILD_EXTRA_WARNINGS += $(call cc-option, -Wvla,)
KBUILD_CFLAGS += $(KBUILD_EXTRA_WARNINGS)
endif


2011-04-21 21:42:46

by Sam Ravnborg

[permalink] [raw]
Subject: Re: annoying new gcc 4.6.0 warnings.

On Thu, Apr 21, 2011 at 05:28:13PM -0400, Dave Jones wrote:
> On Thu, Apr 21, 2011 at 10:56:12PM +0200, Sam Ravnborg wrote:
> >
> > Seems soo. It semi works now.
> > "make W=1" is broken for reasons that is not related to this patch - will submit
> > a fix in a minute for that.
> >
> > So you can add:
> > Acked-by: Sam Ravnborg <[email protected]>
> > Tested-by: Sam Ravnborg <[email protected]>
> >
> > on the patch.
> >
> > And if you really want to make me happy then you add a comment about _why_
> > we disable the warning.
>
> ok, last time for reals.
Thanks - I'm happy now :-)

Sam

2011-04-29 15:02:23

by Michal Marek

[permalink] [raw]
Subject: Re: annoying new gcc 4.6.0 warnings.

On Thu, Apr 21, 2011 at 05:28:13PM -0400, Dave Jones wrote:
> Disable the new -Wunused-but-set-variable that was added in gcc 4.6.0
> It produces more false positives than useful warnings.
>
> This can still be enabled using W=1
>
> Signed-off-by: Dave Jones <[email protected]>
> Acked-by: Sam Ravnborg <[email protected]>
> Tested-by: Sam Ravnborg <[email protected]>

Applied to kbuild-2.6.git#kbuild, thanks.

Michal