Build failure: -Wno-unused-const-variable DNE on old GCC

Hi,

I'm using a GCC 4.6.3 compiler for some compile tests, and I noticed that commit 2cd55c68c0a4 ("cxl: Fix build failure due to -Wunused-variable behaviour change") breaks my builds, because the -Wno-unused-const-variable doesn't exist on GCC 4.6.3.

  drivers/misc/cxl/base.c: At top level:
  cc1: error: unrecognized command line option "-Wno-unused-const-variable" [-Werror]

Any thoughts on how to best fix this? I'd like not to have to scrounge up a new cross compiler just for build tests.

Quoted 7 lines

Hi,

I'm using a GCC 4.6.3 compiler for some compile tests, and I noticed that commit 2cd55c68c0a4 ("cxl: Fix build failure due to -Wunused-variable behaviour change") breaks my builds, because the -Wno-unused-const-variable doesn't exist on GCC 4.6.3.

Quoted 5 lines

drivers/misc/cxl/base.c: At top level:
cc1: error: unrecognized command line option "-Wno-unused-const-variable" [-Werror]

Any thoughts on how to best fix this? I'd like not to have to scrounge up a new cross compiler just for build tests.

drivers/misc/cxl/Makefile:ccflags-y := -Werror -Wno-unused-const-variable

You could take that -Wno-unused-const-variable out of the Makefile or maybe add something like:

$(call cc-ifversion, -ge, 0530, -Wno-unused-const-variable)

or whatever gcc version actually added that unused-const-variable check

Dne 7.1.2016 v 20:37 Joe Perches napsal(a):

Quoted 13 lines · Brian Norris

Quoted 8 lines

I'm using a GCC 4.6.3 compiler for some compile tests, and I noticed that commit 2cd55c68c0a4 ("cxl: Fix build failure due to -Wunused-variable behaviour change") breaks my builds, because the -Wno-unused-const-variable doesn't exist on GCC 4.6.3. drivers/misc/cxl/base.c: At top level:
  cc1: error: unrecognized command line option "-Wno-unused-const-variable" [-Werror] Any thoughts on how to best fix this? I'd like not to have to scrounge up a new cross compiler just for build tests.

drivers/misc/cxl/Makefile:ccflags-y := -Werror -Wno-unused-const-variable You could take that -Wno-unused-const-variable out of the Makefile or maybe add something like:

Quoted 3 lines

$(call cc-ifversion, -ge, 0530, -Wno-unused-const-variable)

or whatever gcc version actually added that unused-const-variable check

We have cc-disable-warning for this.

Quoted 14 lines · Michal Marek

Dne 7.1.2016 v 20:37 Joe Perches napsal(a):

Quoted 13 lines · Brian Norris

Quoted 8 lines

I'm using a GCC 4.6.3 compiler for some compile tests, and I noticed that commit 2cd55c68c0a4 ("cxl: Fix build failure due to -Wunused-variable behaviour change") breaks my builds, because the -Wno-unused-const-variable doesn't exist on GCC 4.6.3. drivers/misc/cxl/base.c: At top level:
  cc1: error: unrecognized command line option "-Wno-unused-const-variable" [-Werror] Any thoughts on how to best fix this? I'd like not to have to scrounge up a new cross compiler just for build tests.

drivers/misc/cxl/Makefile:ccflags-y := -Werror -Wno-unused-const-variable You could take that -Wno-unused-const-variable out of the Makefile or maybe add something like:

Quoted 5 lines

Quoted 3 lines

$(call cc-ifversion, -ge, 0530, -Wno-unused-const-variable)

or whatever gcc version actually added that unused-const-variable check

We have cc-disable-warning for this.

Thanks Michal.

Perhaps most uses of -Werror without some CONFIG_<FOO> guard should be removed or replaced by some other mechanism.

$ git grep -E "=\s*\-Werror" | grep -v CONFIG
[...]
arch/alpha/lib/Makefile:ccflags-y := -Werror
arch/alpha/mm/Makefile:ccflags-y := -Werror
arch/alpha/oprofile/Makefile:ccflags-y := -Werror -Wno-sign-compare
arch/metag/oprofile/Makefile:ccflags-y	+= -Werror
arch/mips/Kbuild:subdir-ccflags-y := -Werror

arch/sh/cchips/hd6446x/Makefile:ccflags-y := -Werror arch/sh/kernel/Makefile:ccflags-y := -Werror arch/sh/lib/Makefile:ccflags-y := -Werror arch/sh/mm/Makefile:ccflags-y := -Werror arch/sparc/kernel/Makefile:ccflags-y := -Werror arch/sparc/lib/Makefile:ccflags-y := -Werror arch/sparc/mm/Makefile:ccflags-y := -Werror arch/sparc/prom/Makefile:ccflags := -Werror
drivers/gpu/drm/tilcdc/Makefile: ccflags-y += -Werror
drivers/misc/cxl/Makefile:ccflags-y := -Werror -Wno-unused-const-variable drivers/scsi/aic7xxx/Makefile:ccflags-y += -Werror drivers/scsi/lpfc/Makefile:ccflags-y += -Werror

Quoted 4 lines · Joe Perches

Quoted 1 line · Michal Marek

We have cc-disable-warning for this.

Thanks Michal.

Cool, thanks! I'll send a patch to use that instead.

GCC 4.6.3 does not support -Wno-unused-const-variable. Instead, use the kbuild infrastructure that checks if this options exists.

Also drop -Werror, since it's harmful, if forced on the user. New GCC's, or higher warning verbosities (e.g., W=1) can easily kill the build where they shouldn't.

Quoted 2 lines

Perhaps most uses of -Werror without some CONFIG_<FOO> guard should be removed or replaced by some other mechanism.

+1000. I'd personally like to see all one-off uses of -Werror removed.

Quoted 7 lines

$ git grep -E "=\s*\-Werror" | grep -v CONFIG
[...]
arch/alpha/lib/Makefile:ccflags-y := -Werror
arch/alpha/mm/Makefile:ccflags-y := -Werror
arch/alpha/oprofile/Makefile:ccflags-y := -Werror -Wno-sign-compare
arch/metag/oprofile/Makefile:ccflags-y	+= -Werror
arch/mips/Kbuild:subdir-ccflags-y := -Werror

^^ I always patch this one out when build-testing MIPS, since I like to turn up warning levels (e.g., W=1), but not kill the build entirely.

Quoted 13 lines · Brian Norris

Quoted 2 lines

Perhaps most uses of -Werror without some CONFIG_ guard should be removed or replaced by some other mechanism.

+1000. I'd personally like to see all one-off uses of -Werror removed.

Quoted 7 lines

$ git grep -E "=\s*\-Werror" | grep -v CONFIG
[...]
arch/alpha/lib/Makefile:ccflags-y := -Werror
arch/alpha/mm/Makefile:ccflags-y := -Werror
arch/alpha/oprofile/Makefile:ccflags-y := -Werror -Wno-sign-compare
arch/metag/oprofile/Makefile:ccflags-y	+= -Werror
arch/mips/Kbuild:subdir-ccflags-y := -Werror

Quoted 2 lines

^^ I always patch this one out when build-testing MIPS, since I like to turn up warning levels (e.g., W=1), but not kill the build entirely.

 The MIPS port switched on -Werror years ago, because people submitted awful code and couldn't be bothered unless the build crashed.  You're welcome to patch your own tree, however I maintain it was a very good decision, and TBH I think -Werror should be on globally.

I'm using a GCC 4.6.3 compiler for some compile tests, and I noticed that commit 2cd55c68c0a4 ("cxl: Fix build failure due to -Wunused-variable behaviour change") breaks my builds, because the -Wno-unused-const-variable doesn't exist on GCC 4.6.3.

drivers/misc/cxl/base.c: At top level:
cc1: error: unrecognized command line option "-Wno-unused-const-variable" [-Werror]

Any thoughts on how to best fix this? I'd like not to have to scrounge up a new cross compiler just for build tests.

Alternatively, remove the -Werror. We occasionally get people that add this flag to a Makefile, but it tends to cause more trouble whenever a new gcc version arrives.

Quoted 4 lines

Alternatively, remove the -Werror. We occasionally get people that add this flag to a Makefile, but it tends to cause more trouble whenever a new gcc version arrives.

Speaking up as the person who added -Werror to cxl, I'd really rather it stayed. There are a number of reasons I think this. Here's the first three that came to mind.

- cxl is powerpc specific (and always will be for deep seated hardware reasons), and is handled through the powerpc tree. arch/powerpc compiles with -Werror, and as part of the powerpc ecosystem, cxl should too.

- It forces cxl developers to a higher standard. cxl has already had more than it's fair share of incredibly difficult to debug issues, so any way we can reduce the risk of errors going in makes our lives (and our end-users lives) better.

- I am (and I'm quite confident the other cxl people are) quite happy to send patches to fix build-breaking issues such as this. Indeed, I would have, except you sent it during the Australian night :)

If it's really super-duper important we can consider putting it behind a config guard, but I'd really rather not.

Quoted 4 lines

- It forces cxl developers to a higher standard. cxl has already had more than it's fair share of incredibly difficult to debug issues, so any way we can reduce the risk of errors going in makes our lives (and our end-users lives) better.

One problem with this point: not all warnings are under the purview of cxl developers. For instance, if I turn up warning verbosity (W=1), then the *header* files start producing plenty of warnings. Should this break the build? Your code didn't change, and you can't fix those errors.

That is a real use case for me daily: I turn the warning verbosity up on my compile tests, then (smart)diff the build logs before and after new patches. That way, I can see what new warnings (even potentially false positive ones) are introduced. I can't do that if every random developer wants to stick -Werror in their Makefile.

I think there are plenty of reasons to either remove -Werror, or make it configurable. Some of them are detailed above.

Maybe you can gate the -Werror on CONFIG_PPC_WERROR, just like the rest of PowerPC?

Excerpts from Brian Norris's message of 2016-01-08 10:02:25 +1100:

Quoted 7 lines

Quoted 4 lines

- It forces cxl developers to a higher standard. cxl has already had more than it's fair share of incredibly difficult to debug issues, so any way we can reduce the risk of errors going in makes our lives (and our end-users lives) better.

One problem with this point: not all warnings are under the purview of cxl developers. For instance, if I turn up warning verbosity (W=1), then

Quoted 2 lines

the *header* files start producing plenty of warnings. Should this break the build? Your code didn't change, and you can't fix those errors.

That's a good point, but the specific warnings that we suppressed in the new compiler are in drivers/misc/cxl/cxl.h, which is an internal header that should only ever be included by the cxl driver. We do have some headers elsewhere which are included by other drivers, the generic ppc architecture code and userspace, but these are all warning free and won't be affected by the -Werror when included from elsewhere.

Quoted 8 lines · Ian Munsie

Excerpts from Brian Norris's message of 2016-01-08 10:02:25 +1100:

Quoted 7 lines

Quoted 4 lines

- It forces cxl developers to a higher standard. cxl has already had more than it's fair share of incredibly difficult to debug issues, so any way we can reduce the risk of errors going in makes our lives (and our end-users lives) better.

One problem with this point: not all warnings are under the purview of cxl developers. For instance, if I turn up warning verbosity (W=1), then

(BTW, I think most of these files are currently clean with W=1, but not W=2)

Quoted 9 lines

Quoted 2 lines

the *header* files start producing plenty of warnings. Should this break the build? Your code didn't change, and you can't fix those errors.

That's a good point, but the specific warnings that we suppressed in the new compiler are in drivers/misc/cxl/cxl.h, which is an internal header that should only ever be included by the cxl driver. We do have some headers elsewhere which are included by other drivers, the generic ppc architecture code and userspace, but these are all warning free and won't be affected by the -Werror when included from elsewhere.

I was referring to when extra warnings are turned on, not just the default. So this fails spectacularly:

  $ export ARCH=powerpc
  $ make ppc64_defconfig
  $ make drivers/misc/cxl/api.o W=2
    CC [M]  drivers/misc/cxl/api.o
  In file included from include/linux/bitops.h:36:0,
                   from include/linux/kernel.h:10,
                   from include/linux/list.h:8,
                   from include/linux/pci.h:25,
                   from drivers/misc/cxl/api.c:10:
  ./arch/powerpc/include/asm/bitops.h:226:94: error: declaration of 'ffs' shadows a built-in function [-Werror=shadow]

I doubt you plan to fix all those. So making -Werror configurable seems like the only way forward, then. (Glad you agreed!)