Skip to contentSkip to conversation

annoying new gcc 4.6.0 warnings.

DISCUSSION Dave Jones 7 participants 17 messages Apr 21, 2011 – Apr 29, 2011 Kbuild

lore
Share & tools
lore

Dave Jones

author
#0 lore raw

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

#1 lore raw
Quoted 5 lines · Dave Jones

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

#2 lore raw
Quoted 7 lines · Linus Torvalds
Quoted 5 lines · Dave Jones

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 ?

Quoted 8 lines

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

#3 lore raw
Quoted 2 lines · David Daney

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

author
#4 lore raw
Quoted 6 lines · Joe Perches
Quoted 2 lines · David Daney

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

Makefile +2 -1
index b967b96..68f178e 100644
@@ -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__
scripts/Makefile.build +1 -0
index d5f925a..30627ab 100644
@@ -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
#5 lore raw
Quoted 11 lines · Dave Jones
Quoted 6 lines · Joe Perches
Quoted 2 lines · David Daney

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

author
#6 lore raw
Quoted 2 lines · Sam Ravnborg

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

Makefile +2 -0
index b967b96..29e16f2 100644
@@ -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
scripts/Makefile.build +1 -0
index d5f925a..30627ab 100644
@@ -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
#7 lore raw
Quoted 35 lines · Dave Jones
Quoted 2 lines · Sam Ravnborg

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

Makefile +2 -0
index b967b96..29e16f2 100644
@@ -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
scripts/Makefile.build +0 -0
index d5f925a..30627ab 100644

scripts/Makefile.build KBUILD_EXTRA_WARNINGS += -Wpointer-arith · line 82

Quoted 4 lines
scripts/Makefile.build +1 -0
@@ -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

author
#8 lore raw
Quoted 4 lines · David Daney
Quoted 1 line

+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

Signature · 6 lines
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]>
Makefile +2 -0
index b967b96..29e16f2 100644
@@ -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
scripts/Makefile.build +1 -0
index d5f925a..30627ab 100644
@@ -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
Acked-by Tested-by
#9 lore raw
Quoted 7 lines · Dave Jones
Quoted 4 lines · David Daney
Quoted 1 line

+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:

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

author
#10 lore raw
Quoted 5 lines · Sam Ravnborg

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.

Quoted 8 lines

So you can add:

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

Signature · 8 lines
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]>
Makefile +4 -0
index b967b96..08c5484 100644
@@ -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
scripts/Makefile.build +1 -0
index d5f925a..021f7eb 100644
@@ -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
#11 lore raw
Quoted 10 lines · Dave Jones
Quoted 9 lines · Sam Ravnborg

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:

Quoted 6 lines
Quoted 4 lines

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

#13 lore raw
In reply to Dave Jones

Makefile KBUILD_CFLAGS   := -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs \ · line 363

Quoted 7 lines · Dave Jones
Makefile +2 -1
@@ -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

author
#14 lore raw
Quoted 12 lines · Linus Torvalds

Makefile KBUILD_CFLAGS   := -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs \ · line 363

Quoted 7 lines · Dave Jones
Makefile +2 -1
@@ -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?

Quoted 1 line

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

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

Dave

#15 lore raw
In reply to Dave Jones
Quoted 32 lines · Dave Jones

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 ?

Quoted 7 lines

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

Signature · 2 lines
Stratos Psomadakis
<[email protected]>