Re: [tip:x86/build] x86/build: Remove -Wno-sign-compare

Re: [tip:x86/build] x86/build: Remove -Wno-sign-compare

[Ingo Molnar]

* Linus Torvalds <torvalds@linux-foundation.org> wrote:

> On May 11, 2015 05:44, "tip-bot for Ingo Molnar" <tipbot@zytor.com> wrote:
> >
> > So restore the warning and see what happens.
> 
> The gcc sign compare had been *totally* broken, I really don't want 
> to get a "let's see what happens" thing.
> 
> I do "allmodconfig" builds all the time, and actually check 
> warnings. [...]

Yeah, so what I failed to point out in the changelog is that I started 
doing that too a couple of weeks ago in my continuous integration 
kernel testing setup. There is a "baseline warnings count" gathered 
from your tree, so I can monitor changes in the level of warnings 
emitted:

 def64:    #   1, 8e70122d, Tue_May_12_09_38_25_CEST_2015:   0 kernels/hour, [ bzImage...    32 secs, modules...     4 secs, done ] (warns:   0)
 def32:    #   2, 8e70122d, Tue_May_12_09_38_47_CEST_2015: 156 kernels/hour, [ bzImage...    30 secs, modules...     3 secs, done ] (warns:   0)
 allno64:  #   3, 8e70122d, Tue_May_12_09_39_24_CEST_2015: 120 kernels/hour, [ bzImage...    22 secs, modules...     0 secs, done ] (warns:   2)
 allno32:  #   4, 8e70122d, Tue_May_12_09_39_58_CEST_2015: 114 kernels/hour, [ bzImage...    21 secs, modules...     0 secs, done ] (warns:   1)
 allyes64: #   5, 8e70122d, Tue_May_12_09_40_21_CEST_2015: 123 kernels/hour, [ bzImage...   204 secs, modules...     9 secs, done ] (warns:  10)
 allyes32: #   6, 8e70122d, Tue_May_12_09_40_44_CEST_2015: 128 kernels/hour, [ bzImage...   206 secs, modules...     9 secs, done ] (warns:  16)
 allmod64: #   7, 8e70122d, Tue_May_12_09_44_19_CEST_2015:  60 kernels/hour, [ bzImage...    62 secs, modules...   105 secs, done ] (warns:   6)
 allmod32: #   8, 8e70122d, Tue_May_12_09_47_56_CEST_2015:  44 kernels/hour, [ bzImage...    58 secs, modules...   103 secs, done ] (warns:  12)

the 'warns: 16' for example means that there are 16 warnings in that 
build. If a new warning is emitted by any of the trees I maintain, 
then I get a delta like this:

 def64:    #   1, 5e5f191d, Tue_May_12_08_55_14_CEST_2015:   0 kernels/hour, [ bzImage...    32 secs, modules...     5 secs, done ] (warns:   0)
 def32:    #   2, ed602bbb, Tue_May_12_08_55_39_CEST_2015: 138 kernels/hour, [ bzImage...    30 secs, modules...     3 secs, done ] (warns:   0)
 allno64:  #   3, ed602bbb, Tue_May_12_08_56_17_CEST_2015: 112 kernels/hour, [ bzImage...    22 secs, modules...     0 secs, done ] (warns:   2)
 allno32:  #   4, ed602bbb, Tue_May_12_08_56_51_CEST_2015: 110 kernels/hour, [ bzImage...    21 secs, modules...     0 secs, done ] (warns:   1)
 allyes64: #   5, ed602bbb, Tue_May_12_08_57_14_CEST_2015: 119 kernels/hour, [ bzImage...   199 secs, modules...     9 secs, done ] (warns:  11, delta: 1)
 allyes32: #   6, ed602bbb, Tue_May_12_08_57_37_CEST_2015: 125 kernels/hour, [ bzImage...   205 secs, modules...     9 secs, done ] (warns:  17, delta: 1)
 allmod64: #   7, ed602bbb, Tue_May_12_09_01_06_CEST_2015:  61 kernels/hour, [ bzImage...    60 secs, modules...   103 secs, done ] (warns:   7, delta: 1)
 allmod32: #   8, ed602bbb, Tue_May_12_09_04_41_CEST_2015:  44 kernels/hour, [ bzImage...    59 secs, modules...   105 secs, done ] (warns:  13, delta: 1)

(nicely color highligted so I cannot miss it and such.)

Btw., just some feedback, 'random' kernel configs still generate a ton 
of warnings:

 randconf: #   9, ed602bbb, Tue_May_12_09_07_25_CEST_2015:  39 kernels/hour, [ bzImage...    81 secs, modules...    21 secs, done ] (warns:   6)
 randconf: #  10, ed602bbb, Tue_May_12_09_10_10_CEST_2015:  36 kernels/hour, [ bzImage...    43 secs, modules...    20 secs, done ] (warns:  12)
 randconf: #  11, ed602bbb, Tue_May_12_09_11_52_CEST_2015:  36 kernels/hour, [ bzImage...    71 secs, modules...     0 secs, done ] (warns:   5)
 randconf: #  12, ed602bbb, Tue_May_12_09_12_56_CEST_2015:  37 kernels/hour, [ bzImage...    64 secs, modules...    28 secs, done ] (warns:  16)
 randconf: #  13, ed602bbb, Tue_May_12_09_14_07_CEST_2015:  38 kernels/hour, [ bzImage...   106 secs, modules...     0 secs, done ] (warns: 157)
 randconf: #  14, ed602bbb, Tue_May_12_09_15_40_CEST_2015:  38 kernels/hour, [ bzImage...   100 secs, modules...     0 secs, done ] (warns:  11)
 randconf: #  15, ed602bbb, Tue_May_12_09_17_27_CEST_2015:  37 kernels/hour, [ bzImage...    65 secs, modules...    28 secs, done ] (warns:  26)
 randconf: #  16, ed602bbb, Tue_May_12_09_19_08_CEST_2015:  37 kernels/hour, [ bzImage...    70 secs, modules...     0 secs, done ] (warns: 228)
 randconf: #  17, ed602bbb, Tue_May_12_09_20_42_CEST_2015:  37 kernels/hour, [ bzImage...    79 secs, modules...     0 secs, done ] (warns: 101)
 randconf: #  18, ed602bbb, Tue_May_12_09_21_53_CEST_2015:  38 kernels/hour, [ bzImage...    55 secs, modules...     0 secs, done ] (warns:   6)
 randconf: #  19, ed602bbb, Tue_May_12_09_23_13_CEST_2015:  38 kernels/hour, [ bzImage...    49 secs, modules...     0 secs, done ] (warns:   4)
 randconf: #  20, ed602bbb, Tue_May_12_09_24_08_CEST_2015:  39 kernels/hour, [ bzImage...    43 secs, modules... B  20 secs, done ] (warns:  28)
 randconf: #  21, ed602bbb, Tue_May_12_09_24_58_CEST_2015:  40 kernels/hour, [ bzImage... B  35 secs, modules...     0 secs, done ] (warns:  14)
 randconf: #  22, ed602bbb, Tue_May_12_09_26_02_CEST_2015:  40 kernels/hour, [ bzImage...    58 secs, modules...     0 secs, done ] (warns:  16)
 randconf: #  23, ed602bbb, Tue_May_12_09_26_38_CEST_2015:  42 kernels/hour, [ bzImage...    48 secs, modules... B  23 secs, done ] (warns:  35)
 randconf: #  24, ed602bbb, Tue_May_12_09_27_37_CEST_2015:  42 kernels/hour, [ bzImage...    40 secs, modules... B  15 secs, done ] (warns:  25)
 randconf: #  25, ed602bbb, Tue_May_12_09_28_49_CEST_2015:  42 kernels/hour, [ bzImage...    64 secs, modules...    19 secs, done ] (warns:   3)
 randconf: #  26, ed602bbb, Tue_May_12_09_29_45_CEST_2015:  43 kernels/hour, [ bzImage...    87 secs, modules...     0 secs, done ] (warns:  15)
 randconf: #  27, ed602bbb, Tue_May_12_09_31_09_CEST_2015:  43 kernels/hour, [ bzImage...    87 secs, modules...     0 secs, done ] (warns:   7)

(and 'B' marks known upstream build breakages.)

There are a few hotspots.

But in any case, your policy to police allmodconfig builds is a good 
starting point and has a positive effect.

Before I pushed out this -Wno-sign-compare change I made sure there 
are no extra warnings generated on the 8 key configs I monitor 
explicitly: [def|allno|allyes|allmod]config[64|32].

The "let's see what happens" in the changelog referred to the 
possibility that an older GCC version that what I use (4.9.2) emits so 
much crap that amounts to a regression and that I'll have to zap the 
commit. It did not refer to any random enabling of compiler warnings.

I absolutely failed at pointing out all this background work in the 
changelog though. Do you want me to rebase it to fix the changelog?

> [...] Right now I get something like six warnings, all from old 
> drivers that so dubious things, but that u can live with.

So the current warnings count is 0/0/2/1/10/16/6/12 on the main 
configs on your tree.

I also have a separate build system that uses GCC 5.0.1, there the 
warnings count is substantially higher:

 def64:    #   1, 5e5f191d, Tue_May_12_09_56_23_CEST_2015:   0 kernels/hour, [ bzImage...    47 secs, modules...     3 secs, done ] (warns:   4, delta: 4)
 def32:    #   2, 1e29025d, Tue_May_12_09_56_53_CEST_2015: 116 kernels/hour, [ bzImage...    46 secs, modules...     2 secs, done ] (warns:   4, delta: 4)
 allno64:  #   3, 1e29025d, Tue_May_12_09_57_44_CEST_2015:  87 kernels/hour, [ bzImage...    24 secs, modules...     0 secs, done ] (warns:   4, delta: 2)
 allno32:  #   4, 1e29025d, Tue_May_12_09_58_33_CEST_2015:  82 kernels/hour, [ bzImage...    24 secs, modules...     0 secs, done ] (warns:   3, delta: 2)
 allyes64: #   5, 1e29025d, Tue_May_12_09_58_58_CEST_2015:  92 kernels/hour, [ bzImage...   386 secs, modules...     9 secs, done ] (warns:  31, delta: 21)
 allyes32: #   6, 1e29025d, Tue_May_12_09_59_23_CEST_2015:  99 kernels/hour, [ bzImage...   344 secs, modules...    10 secs, done ] (warns:  36, delta: 20)
 allmod64: #   7, 1e29025d, Tue_May_12_10_05_59_CEST_2015:  37 kernels/hour, [ bzImage...    82 secs, modules...   250 secs, done ] (warns:  27, delta: 21)
 allmod32: #   8, 1e29025d, Tue_May_12_10_11_54_CEST_2015:  27 kernels/hour, [ bzImage...    75 secs, modules...   243 secs, done ] (warns:  32, delta: 20)

Here's the full list of warnings for allmod64:

make bzImage:

 include/linux/blkdev.h:624:26: warning: switch condition has boolean value [-Wswitch-bool]
 ./arch/x86/include/asm/qspinlock.h:28:2: warning: implicit declaration of function ?pv_queued_spin_lock_slowpath? [-Wimplicit-function-declaration]
 ./arch/x86/include/asm/qspinlock.h:33:2: warning: implicit declaration of function ?pv_queued_spin_unlock? [-Wimplicit-function-declaration]
 ./arch/x86/include/asm/qspinlock.h:28:2: warning: implicit declaration of function ?pv_queued_spin_lock_slowpath? [-Wimplicit-function-declaration]
 ./arch/x86/include/asm/qspinlock.h:33:2: warning: implicit declaration of function ?pv_queued_spin_unlock? [-Wimplicit-function-declaration]
 ./arch/x86/include/asm/qspinlock.h:28:2: warning: implicit declaration of function ?pv_queued_spin_lock_slowpath? [-Wimplicit-function-declaration]
 ./arch/x86/include/asm/qspinlock.h:33:2: warning: implicit declaration of function ?pv_queued_spin_unlock? [-Wimplicit-function-declaration]
 ./arch/x86/include/asm/qspinlock.h:28:2: warning: implicit declaration of function ?pv_queued_spin_lock_slowpath? [-Wimplicit-function-declaration]
 ./arch/x86/include/asm/qspinlock.h:33:2: warning: implicit declaration of function ?pv_queued_spin_unlock? [-Wimplicit-function-declaration]

make modules:

 drivers/ata/pata_hpt366.c:376:9: warning: assignment discards ?const? qualifier from pointer target type [-Wdiscarded-array-qualifiers]
 drivers/ata/pata_hpt366.c:379:9: warning: assignment discards ?const? qualifier from pointer target type [-Wdiscarded-array-qualifiers]
 drivers/ata/pata_hpt366.c:382:9: warning: assignment discards ?const? qualifier from pointer target type [-Wdiscarded-array-qualifiers]
 drivers/hid/hid-input.c:1160:67: warning: logical not is only applied to the left hand side of comparison [-Wlogical-not-parentheses]
 drivers/md/md.c:6375:26: warning: logical not is only applied to the left hand side of comparison [-Wlogical-not-parentheses]
 drivers/gpu/drm/gma500/cdv_intel_dp.c:869:2: warning: ?i2c_dp_aux_add_bus? is deprecated [-Wdeprecated-declarations]
 drivers/mmc/host/sh_mmcif.c:401:4: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
 drivers/mmc/host/sh_mmcif.c:402:4: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
 drivers/isdn/hardware/eicon/message.c:6115:1: warning: the frame size of 2352 bytes is larger than 2048 bytes [-Wframe-larger-than=]
 drivers/scsi/advansys.c:71:2: warning: #warning this driver is still not properly converted to the DMA API [-Wcpp]
 drivers/scsi/be2iscsi/be_main.c:3168:18: warning: logical not is only applied to the left hand side of comparison [-Wlogical-not-parentheses]
 sound/pci/oxygen/oxygen_mixer.c:91:43: warning: logical not is only applied to the left hand side of comparison [-Wlogical-not-parentheses]
 drivers/usb/renesas_usbhs/common.c:492:25: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
 drivers/media/platform/s3c-camif/camif-capture.c:118:10: warning: logical not is only applied to the left hand side of comparison [-Wlogical-not-parentheses]
 drivers/media/platform/s3c-camif/camif-capture.c:134:10: warning: logical not is only applied to the left hand side of comparison [-Wlogical-not-parentheses]
 drivers/net/wireless/brcm80211/brcmsmac/phy/phy_n.c:16065:1: warning: the frame size of 3200 bytes is larger than 2048 bytes [-Wframe-larger-than=]
 drivers/media/usb/cx231xx/cx231xx-cards.c:1110:1: warning: the frame size of 2064 bytes is larger than 2048 bytes [-Wframe-larger-than=]
 drivers/net/wireless/brcm80211/brcmsmac/phy/phy_n.c:17138:1: warning: the frame size of 2768 bytes is larger than 2048 bytes [-Wframe-larger-than=]

No new warnings were triggered by -Wno-sign-compare.

Six of the new warnings are generated due to -Wlogical-not-parentheses,
which looks like false positives in the cases of:

  drivers/hid/hid-input.c:1160
  drivers/md/md.c:6375

but it's actually pointing out what appears to be a real bug here:

  drivers/scsi/be2iscsi/be_main.c:3168

and I'm unsure about:

  sound/pci/oxygen/oxygen_mixer.c:91

but it sure looks weird. These are probably correct but are
looking weird:

  drivers/media/platform/s3c-camif/camif-capture.c:118
  drivers/media/platform/s3c-camif/camif-capture.c:134

and any kernel code that should be routine but makes reviewers
looks twice probably needs improving.

In any case, yes, I fully agree with your point and general policy 
that by default we cannot trust GCC's warnings, so I have tested this 
change as much as I could.

> Does this add to that? Because if it does, I'm not pulling the end 
> result.

Yeah, absolutely. I'll even zap the commit if it generates noise for 
older but still widely used GCC versions.

Thanks,

	Ingo

Re: [tip:x86/build] x86/build: Remove -Wno-sign-compare

[Ingo Molnar]

* Ingo Molnar <mingo@kernel.org> wrote:

> Here's the full list of warnings for allmod64:
> 
> make bzImage:
> 
>  include/linux/blkdev.h:624:26: warning: switch condition has boolean value [-Wswitch-bool]
>  ./arch/x86/include/asm/qspinlock.h:28:2: warning: implicit declaration of function ?pv_queued_spin_lock_slowpath? [-Wimplicit-function-declaration]
>  ./arch/x86/include/asm/qspinlock.h:33:2: warning: implicit declaration of function ?pv_queued_spin_unlock? [-Wimplicit-function-declaration]
>  ./arch/x86/include/asm/qspinlock.h:28:2: warning: implicit declaration of function ?pv_queued_spin_lock_slowpath? [-Wimplicit-function-declaration]
>  ./arch/x86/include/asm/qspinlock.h:33:2: warning: implicit declaration of function ?pv_queued_spin_unlock? [-Wimplicit-function-declaration]
>  ./arch/x86/include/asm/qspinlock.h:28:2: warning: implicit declaration of function ?pv_queued_spin_lock_slowpath? [-Wimplicit-function-declaration]
>  ./arch/x86/include/asm/qspinlock.h:33:2: warning: implicit declaration of function ?pv_queued_spin_unlock? [-Wimplicit-function-declaration]
>  ./arch/x86/include/asm/qspinlock.h:28:2: warning: implicit declaration of function ?pv_queued_spin_lock_slowpath? [-Wimplicit-function-declaration]
>  ./arch/x86/include/asm/qspinlock.h:33:2: warning: implicit declaration of function ?pv_queued_spin_unlock? [-Wimplicit-function-declaration]

And that comes from tip:locking/core, but those warnings do not show 
up with GCC 4.9.2: so it's either a GCC 5.0.1 bug, or we missed 
something with the WIP queued pv spinlocks changes that newer GCC is 
able to notice.

Thanks,

	Ingo

Re: [tip:x86/build] x86/build: Remove -Wno-sign-compare

[Waiman Long]

On 05/12/2015 04:53 AM, Ingo Molnar wrote:
> * Ingo Molnar<mingo@kernel.org>  wrote:
>
>> Here's the full list of warnings for allmod64:
>>
>> make bzImage:
>>
>>   include/linux/blkdev.h:624:26: warning: switch condition has boolean value [-Wswitch-bool]
>>   ./arch/x86/include/asm/qspinlock.h:28:2: warning: implicit declaration of function ?pv_queued_spin_lock_slowpath? [-Wimplicit-function-declaration]
>>   ./arch/x86/include/asm/qspinlock.h:33:2: warning: implicit declaration of function ?pv_queued_spin_unlock? [-Wimplicit-function-declaration]
>>   ./arch/x86/include/asm/qspinlock.h:28:2: warning: implicit declaration of function ?pv_queued_spin_lock_slowpath? [-Wimplicit-function-declaration]
>>   ./arch/x86/include/asm/qspinlock.h:33:2: warning: implicit declaration of function ?pv_queued_spin_unlock? [-Wimplicit-function-declaration]
>>   ./arch/x86/include/asm/qspinlock.h:28:2: warning: implicit declaration of function ?pv_queued_spin_lock_slowpath? [-Wimplicit-function-declaration]
>>   ./arch/x86/include/asm/qspinlock.h:33:2: warning: implicit declaration of function ?pv_queued_spin_unlock? [-Wimplicit-function-declaration]
>>   ./arch/x86/include/asm/qspinlock.h:28:2: warning: implicit declaration of function ?pv_queued_spin_lock_slowpath? [-Wimplicit-function-declaration]
>>   ./arch/x86/include/asm/qspinlock.h:33:2: warning: implicit declaration of function ?pv_queued_spin_unlock? [-Wimplicit-function-declaration]
> And that comes from tip:locking/core, but those warnings do not show
> up with GCC 4.9.2: so it's either a GCC 5.0.1 bug, or we missed
> something with the WIP queued pv spinlocks changes that newer GCC is
> able to notice.
>
> Thanks,
>
> 	Ingo

The only way this can happen is when __ASSEMBLY__ is defined and 
asm/qspinlock.h is included. Could you apply the following patch to see 
if it can fix the compilation warnings? I don't have a gcc 5 compiler on 
hand to verify that.

Cheers,
Longman


---
  arch/x86/include/asm/qspinlock.h |    5 ++++-
  1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/arch/x86/include/asm/qspinlock.h 
b/arch/x86/include/asm/qspinlock.h
index 9d51fae..3ea1c57 100644
--- a/arch/x86/include/asm/qspinlock.h
+++ b/arch/x86/include/asm/qspinlock.h
@@ -17,7 +17,10 @@ static inline void native_queued_spin_unlock(struct 
qspinlock *lock)
      smp_store_release((u8 *)lock, 0);
  }

-#ifdef CONFIG_PARAVIRT_SPINLOCKS
+/*
+ * Disable the PV code for assembly to prevent compilation warnings.
+ */
+#if defined(CONFIG_PARAVIRT_SPINLOCKS) && !defined(__ASSEMBLY__)
  extern void native_queued_spin_lock_slowpath(struct qspinlock *lock, 
u32 val);
  extern void __pv_init_lock_hash(void);
  extern void __pv_queued_spin_lock_slowpath(struct qspinlock *lock, u32 
val);
-- 

Re: [tip:x86/build] x86/build: Remove -Wno-sign-compare

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 15309 bytes --]

On Tue, 12 May 2015 10:44:15 +0200, Ingo Molnar said:

> Btw., just some feedback, 'random' kernel configs still generate a ton
> of warnings:
>
>  randconf: #   9, ed602bbb, Tue_May_12_09_07_25_CEST_2015:  39 kernels/hour,
[ bzImage...    81 secs, modules...    21 secs, done ] (warns:   6)
>  randconf: #  10, ed602bbb, Tue_May_12_09_10_10_CEST_2015:  36 kernels/hour,
[ bzImage...    43 secs, modules...    20 secs, done ] (warns:  12)
>  randconf: #  11, ed602bbb, Tue_May_12_09_11_52_CEST_2015:  36 kernels/hour,
[ bzImage...    71 secs, modules...     0 secs, done ] (warns:   5)
>  randconf: #  12, ed602bbb, Tue_May_12_09_12_56_CEST_2015:  37 kernels/hour,
[ bzImage...    64 secs, modules...    28 secs, done ] (warns:  16)
>  randconf: #  13, ed602bbb, Tue_May_12_09_14_07_CEST_2015:  38 kernels/hour,
[ bzImage...   106 secs, modules...     0 secs, done ] (warns: 157)
>  randconf: #  14, ed602bbb, Tue_May_12_09_15_40_CEST_2015:  38 kernels/hour,
[ bzImage...   100 secs, modules...     0 secs, done ] (warns:  11)
>  randconf: #  15, ed602bbb, Tue_May_12_09_17_27_CEST_2015:  37 kernels/hour,
[ bzImage...    65 secs, modules...    28 secs, done ] (warns:  26)
>  randconf: #  16, ed602bbb, Tue_May_12_09_19_08_CEST_2015:  37 kernels/hour,
[ bzImage...    70 secs, modules...     0 secs, done ] (warns: 228)

> Before I pushed out this -Wno-sign-compare change I made sure there
> are no extra warnings generated on the 8 key configs I monitor
> explicitly: [def|allno|allyes|allmod]config[64|32].

It makes my config totally explode on next-20150512

% gcc --version
gcc (GCC) 5.1.1 20150422 (Red Hat 5.1.1-1)

% grep "warning: comparison between signed and unsigned integer expressions" build.default | wc -l
64148

A *lot*of them appear to be exploding inside likely().  Here's all the
exploding for *one file*...

  CC      init/main.o
In file included from include/uapi/linux/stddef.h:1:0,
                 from include/linux/stddef.h:4,
                 from ./include/uapi/linux/posix_types.h:4,
                 from include/uapi/linux/types.h:13,
                 from include/linux/types.h:5,
                 from init/main.c:14:
include/asm-generic/qrwlock.h: In function 'queue_write_trylock':
include/asm-generic/qrwlock.h:93:35: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
          cnts, cnts | _QW_LOCKED) == cnts);
                                   ^
include/linux/compiler.h:132:43: note: in definition of macro 'likely'
 #  define likely(x) (__builtin_constant_p(x) ? !!(x) : __branch_check__(x, 1))
                                           ^
include/asm-generic/qrwlock.h:93:35: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
          cnts, cnts | _QW_LOCKED) == cnts);
                                   ^
include/linux/compiler.h:108:47: note: in definition of macro 'likely_notrace'
 #define likely_notrace(x) __builtin_expect(!!(x), 1)
                                               ^
include/linux/compiler.h:132:56: note: in expansion of macro '__branch_check__'
 #  define likely(x) (__builtin_constant_p(x) ? !!(x) : __branch_check__(x, 1))
                                                        ^
include/asm-generic/qrwlock.h:92:9: note: in expansion of macro 'likely'
  return likely(atomic_cmpxchg(&lock->cnts,
         ^
In file included from include/uapi/linux/stddef.h:1:0,
                 from include/linux/stddef.h:4,
                 from ./include/uapi/linux/posix_types.h:4,
                 from include/uapi/linux/types.h:13,
                 from include/linux/types.h:5,
                 from init/main.c:14:
./arch/x86/include/asm/uaccess.h: In function 'copy_from_user':
./arch/x86/include/asm/uaccess.h:712:26: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
  if (likely(sz < 0 || sz >= n))
                          ^
include/linux/compiler.h:132:43: note: in definition of macro 'likely'
 #  define likely(x) (__builtin_constant_p(x) ? !!(x) : __branch_check__(x, 1))
                                           ^
./arch/x86/include/asm/uaccess.h:712:26: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
  if (likely(sz < 0 || sz >= n))
                          ^
include/linux/compiler.h:132:51: note: in definition of macro 'likely'
 #  define likely(x) (__builtin_constant_p(x) ? !!(x) : __branch_check__(x, 1))
                                                   ^
./arch/x86/include/asm/uaccess.h:712:26: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
  if (likely(sz < 0 || sz >= n))
                          ^
include/linux/compiler.h:108:47: note: in definition of macro 'likely_notrace'
 #define likely_notrace(x) __builtin_expect(!!(x), 1)
                                               ^
include/linux/compiler.h:132:56: note: in expansion of macro '__branch_check__'
 #  define likely(x) (__builtin_constant_p(x) ? !!(x) : __branch_check__(x, 1))
                                                        ^
./arch/x86/include/asm/uaccess.h:712:6: note: in expansion of macro 'likely'
  if (likely(sz < 0 || sz >= n))
      ^
./arch/x86/include/asm/uaccess.h: In function 'copy_to_user':
./arch/x86/include/asm/uaccess.h:730:26: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
  if (likely(sz < 0 || sz >= n))
                          ^
include/linux/compiler.h:132:43: note: in definition of macro 'likely'
 #  define likely(x) (__builtin_constant_p(x) ? !!(x) : __branch_check__(x, 1))
                                           ^
./arch/x86/include/asm/uaccess.h:730:26: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
  if (likely(sz < 0 || sz >= n))
                          ^
include/linux/compiler.h:132:51: note: in definition of macro 'likely'
 #  define likely(x) (__builtin_constant_p(x) ? !!(x) : __branch_check__(x, 1))
                                                   ^
./arch/x86/include/asm/uaccess.h:730:26: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
  if (likely(sz < 0 || sz >= n))
                          ^
include/linux/compiler.h:108:47: note: in definition of macro 'likely_notrace'
 #define likely_notrace(x) __builtin_expect(!!(x), 1)
                                               ^
include/linux/compiler.h:132:56: note: in expansion of macro '__branch_check__'
 #  define likely(x) (__builtin_constant_p(x) ? !!(x) : __branch_check__(x, 1))
                                                        ^
./arch/x86/include/asm/uaccess.h:730:6: note: in expansion of macro 'likely'
  if (likely(sz < 0 || sz >= n))
      ^
In file included from ./arch/x86/include/asm/preempt.h:5:0,
                 from include/linux/preempt.h:18,
                 from include/linux/spinlock.h:50,
                 from include/linux/seqlock.h:35,
                 from include/linux/time.h:5,
                 from include/linux/stat.h:18,
                 from include/linux/module.h:10,
                 from init/main.c:15:
include/linux/percpu-refcount.h: In function 'percpu_ref_get_many':
./arch/x86/include/asm/percpu.h:130:31: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
          ((val) == 1 || (val) == -1)) ?  \
                               ^
./arch/x86/include/asm/percpu.h:419:34: note: in expansion of macro 'percpu_add_op'
 #define this_cpu_add_1(pcp, val) percpu_add_op((pcp), val)
                                  ^
include/linux/percpu-defs.h:364:11: note: in expansion of macro 'this_cpu_add_1'
   case 1: stem##1(variable, __VA_ARGS__);break;  \
           ^
include/linux/percpu-defs.h:498:33: note: in expansion of macro '__pcpu_size_call'
 #define this_cpu_add(pcp, val)  __pcpu_size_call(this_cpu_add_, pcp, val)
                                 ^
include/linux/percpu-refcount.h:177:3: note: in expansion of macro 'this_cpu_add'
   this_cpu_add(*percpu_count, nr);
   ^
./arch/x86/include/asm/percpu.h:130:31: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
          ((val) == 1 || (val) == -1)) ?  \
                               ^
./arch/x86/include/asm/percpu.h:420:34: note: in expansion of macro 'percpu_add_op'
 #define this_cpu_add_2(pcp, val) percpu_add_op((pcp), val)
                                  ^
include/linux/percpu-defs.h:365:11: note: in expansion of macro 'this_cpu_add_2'
   case 2: stem##2(variable, __VA_ARGS__);break;  \
           ^
include/linux/percpu-defs.h:498:33: note: in expansion of macro '__pcpu_size_call'
 #define this_cpu_add(pcp, val)  __pcpu_size_call(this_cpu_add_, pcp, val)
                                 ^
include/linux/percpu-refcount.h:177:3: note: in expansion of macro 'this_cpu_add'
   this_cpu_add(*percpu_count, nr);
   ^
./arch/x86/include/asm/percpu.h:130:31: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
          ((val) == 1 || (val) == -1)) ?  \
                               ^
./arch/x86/include/asm/percpu.h:421:34: note: in expansion of macro 'percpu_add_op'
 #define this_cpu_add_4(pcp, val) percpu_add_op((pcp), val)
                                  ^
include/linux/percpu-defs.h:366:11: note: in expansion of macro 'this_cpu_add_4'
   case 4: stem##4(variable, __VA_ARGS__);break;  \
           ^
include/linux/percpu-defs.h:498:33: note: in expansion of macro '__pcpu_size_call'
 #define this_cpu_add(pcp, val)  __pcpu_size_call(this_cpu_add_, pcp, val)
                                 ^
include/linux/percpu-refcount.h:177:3: note: in expansion of macro 'this_cpu_add'
   this_cpu_add(*percpu_count, nr);
   ^
./arch/x86/include/asm/percpu.h:130:31: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
          ((val) == 1 || (val) == -1)) ?  \
                               ^
./arch/x86/include/asm/percpu.h:478:35: note: in expansion of macro 'percpu_add_op'
 #define this_cpu_add_8(pcp, val)  percpu_add_op((pcp), val)
                                   ^
include/linux/percpu-defs.h:367:11: note: in expansion of macro 'this_cpu_add_8'
   case 8: stem##8(variable, __VA_ARGS__);break;  \
           ^
include/linux/percpu-defs.h:498:33: note: in expansion of macro '__pcpu_size_call'
 #define this_cpu_add(pcp, val)  __pcpu_size_call(this_cpu_add_, pcp, val)
                                 ^
include/linux/percpu-refcount.h:177:3: note: in expansion of macro 'this_cpu_add'
   this_cpu_add(*percpu_count, nr);
   ^
include/linux/percpu-refcount.h: In function 'percpu_ref_put_many':
./arch/x86/include/asm/percpu.h:130:31: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
          ((val) == 1 || (val) == -1)) ?  \
                               ^
./arch/x86/include/asm/percpu.h:419:34: note: in expansion of macro 'percpu_add_op'
 #define this_cpu_add_1(pcp, val) percpu_add_op((pcp), val)
                                  ^
include/linux/percpu-defs.h:364:11: note: in expansion of macro 'this_cpu_add_1'
   case 1: stem##1(variable, __VA_ARGS__);break;  \
           ^
include/linux/percpu-defs.h:498:33: note: in expansion of macro '__pcpu_size_call'
 #define this_cpu_add(pcp, val)  __pcpu_size_call(this_cpu_add_, pcp, val)
                                 ^
include/linux/percpu-defs.h:508:33: note: in expansion of macro 'this_cpu_add'
 #define this_cpu_sub(pcp, val)  this_cpu_add(pcp, -(typeof(pcp))(val))
                                 ^
include/linux/percpu-refcount.h:276:3: note: in expansion of macro 'this_cpu_sub'
   this_cpu_sub(*percpu_count, nr);
   ^
./arch/x86/include/asm/percpu.h:130:31: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
          ((val) == 1 || (val) == -1)) ?  \
                               ^
./arch/x86/include/asm/percpu.h:420:34: note: in expansion of macro 'percpu_add_op'
 #define this_cpu_add_2(pcp, val) percpu_add_op((pcp), val)
                                  ^
include/linux/percpu-defs.h:365:11: note: in expansion of macro 'this_cpu_add_2'
   case 2: stem##2(variable, __VA_ARGS__);break;  \
           ^
include/linux/percpu-defs.h:498:33: note: in expansion of macro '__pcpu_size_call'
 #define this_cpu_add(pcp, val)  __pcpu_size_call(this_cpu_add_, pcp, val)
                                 ^
include/linux/percpu-defs.h:508:33: note: in expansion of macro 'this_cpu_add'
 #define this_cpu_sub(pcp, val)  this_cpu_add(pcp, -(typeof(pcp))(val))
                                 ^
include/linux/percpu-refcount.h:276:3: note: in expansion of macro 'this_cpu_sub'
   this_cpu_sub(*percpu_count, nr);
   ^
./arch/x86/include/asm/percpu.h:130:31: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
          ((val) == 1 || (val) == -1)) ?  \
                               ^
./arch/x86/include/asm/percpu.h:421:34: note: in expansion of macro 'percpu_add_op'
 #define this_cpu_add_4(pcp, val) percpu_add_op((pcp), val)
                                  ^
include/linux/percpu-defs.h:366:11: note: in expansion of macro 'this_cpu_add_4'
   case 4: stem##4(variable, __VA_ARGS__);break;  \
           ^
include/linux/percpu-defs.h:498:33: note: in expansion of macro '__pcpu_size_call'
 #define this_cpu_add(pcp, val)  __pcpu_size_call(this_cpu_add_, pcp, val)
                                 ^
include/linux/percpu-defs.h:508:33: note: in expansion of macro 'this_cpu_add'
 #define this_cpu_sub(pcp, val)  this_cpu_add(pcp, -(typeof(pcp))(val))
                                 ^
include/linux/percpu-refcount.h:276:3: note: in expansion of macro 'this_cpu_sub'
   this_cpu_sub(*percpu_count, nr);
   ^
./arch/x86/include/asm/percpu.h:130:31: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
          ((val) == 1 || (val) == -1)) ?  \
                               ^
./arch/x86/include/asm/percpu.h:478:35: note: in expansion of macro 'percpu_add_op'
 #define this_cpu_add_8(pcp, val)  percpu_add_op((pcp), val)
                                   ^
include/linux/percpu-defs.h:367:11: note: in expansion of macro 'this_cpu_add_8'
   case 8: stem##8(variable, __VA_ARGS__);break;  \
           ^
include/linux/percpu-defs.h:498:33: note: in expansion of macro '__pcpu_size_call'
 #define this_cpu_add(pcp, val)  __pcpu_size_call(this_cpu_add_, pcp, val)
                                 ^
include/linux/percpu-defs.h:508:33: note: in expansion of macro 'this_cpu_add'
 #define this_cpu_sub(pcp, val)  this_cpu_add(pcp, -(typeof(pcp))(val))
                                 ^
include/linux/percpu-refcount.h:276:3: note: in expansion of macro 'this_cpu_sub'
   this_cpu_sub(*percpu_count, nr);
   ^
In file included from include/linux/blkdev.h:18:0,
                 from init/main.c:75:
include/linux/bio.h: In function 'bio_next_split':
include/linux/bio.h:388:14: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
  if (sectors >= bio_sectors(bio))
              ^
init/main.c: In function 'do_initcalls':
init/main.c:860:24: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
  for (level = 0; level < ARRAY_SIZE(initcall_levels) - 1; level++)
                        ^


[-- Attachment #2: Type: application/pgp-signature, Size: 848 bytes --]

Re: [tip:x86/build] x86/build: Remove -Wno-sign-compare

[Ingo Molnar]

* Valdis.Kletnieks@vt.edu <Valdis.Kletnieks@vt.edu> wrote:

> On Tue, 12 May 2015 10:44:15 +0200, Ingo Molnar said:
> 
> > Btw., just some feedback, 'random' kernel configs still generate a ton
> > of warnings:
> >
> >  randconf: #   9, ed602bbb, Tue_May_12_09_07_25_CEST_2015:  39 kernels/hour,
> [ bzImage...    81 secs, modules...    21 secs, done ] (warns:   6)
> >  randconf: #  10, ed602bbb, Tue_May_12_09_10_10_CEST_2015:  36 kernels/hour,
> [ bzImage...    43 secs, modules...    20 secs, done ] (warns:  12)
> >  randconf: #  11, ed602bbb, Tue_May_12_09_11_52_CEST_2015:  36 kernels/hour,
> [ bzImage...    71 secs, modules...     0 secs, done ] (warns:   5)
> >  randconf: #  12, ed602bbb, Tue_May_12_09_12_56_CEST_2015:  37 kernels/hour,
> [ bzImage...    64 secs, modules...    28 secs, done ] (warns:  16)
> >  randconf: #  13, ed602bbb, Tue_May_12_09_14_07_CEST_2015:  38 kernels/hour,
> [ bzImage...   106 secs, modules...     0 secs, done ] (warns: 157)
> >  randconf: #  14, ed602bbb, Tue_May_12_09_15_40_CEST_2015:  38 kernels/hour,
> [ bzImage...   100 secs, modules...     0 secs, done ] (warns:  11)
> >  randconf: #  15, ed602bbb, Tue_May_12_09_17_27_CEST_2015:  37 kernels/hour,
> [ bzImage...    65 secs, modules...    28 secs, done ] (warns:  26)
> >  randconf: #  16, ed602bbb, Tue_May_12_09_19_08_CEST_2015:  37 kernels/hour,
> [ bzImage...    70 secs, modules...     0 secs, done ] (warns: 228)
> 
> > Before I pushed out this -Wno-sign-compare change I made sure there
> > are no extra warnings generated on the 8 key configs I monitor
> > explicitly: [def|allno|allyes|allmod]config[64|32].
> 
> It makes my config totally explode on next-20150512
> 
> % gcc --version
> gcc (GCC) 5.1.1 20150422 (Red Hat 5.1.1-1)
> 
> % grep "warning: comparison between signed and unsigned integer expressions" build.default | wc -l
> 64148
> 
> A *lot*of them appear to be exploding inside likely().  Here's all the
> exploding for *one file*...

That's a prerelease of GCC, right? So I think GCC gets constant 
propagation from various builtins wrong or so.

But ... the output looks horrible, and for years we didn't have the 
warnings, still after reintroducing it we didn't get any new warnings 
about truly bogus code.

So it does not seem to have much utility, and seems to be horribly 
broken on fresh GCC.

I'll remove the commit.

Thanks,

	Ingo

Re: [tip:x86/build] x86/build: Remove -Wno-sign-compare

[Louis Langholtz]

On May 13, 2015, at 4:22 AM, Ingo Molnar <mingo@kernel.org> wrote:

> * Valdis.Kletnieks@vt.edu <Valdis.Kletnieks@vt.edu> wrote:
> 
>> On Tue, 12 May 2015 10:44:15 +0200, Ingo Molnar said:
>> 
>>> ...
>>> Before I pushed out this -Wno-sign-compare change I made sure there
>>> are no extra warnings generated on the 8 key configs I monitor
>>> explicitly: [def|allno|allyes|allmod]config[64|32].
>> 
>> It makes my config totally explode on next-20150512


I note that not all architectures add the no-sign-compare option to their builds.
Specifically (according to grep of arch/*/Makefile), s390, sparc, and x86 do add
this option while none of the others do (well alpha may but not in that same
Makefile).

Are warnings about sign-comparisons an architecture specific issue? I can
imagine that for some architectures the sign comparison is a bigger concern
than for others. I can also imagine that this could be architecture independent
enough to warrant being considered an option for all the kernel builds. Do devs
for the other architectures just live with all these warnings? Or 'clean' them up?

>> 
>> % gcc --version
>> gcc (GCC) 5.1.1 20150422 (Red Hat 5.1.1-1)
>> 
>> % grep "warning: comparison between signed and unsigned integer expressions" build.default | wc -l
>> 64148
>> 
>> A *lot*of them appear to be exploding inside likely().  Here's all the
>> exploding for *one file*...
> 

A lot of the explosion appears to be due to the sign compare issue being in
code that's inline or macro code in header files that get included in multiple C
files. (Is that what you're saying in your next sentence?)

> That's a prerelease of GCC, right? So I think GCC gets constant 
> propagation from various builtins wrong or so.

GCC seems to issue a warning on every use of code; not just the first. I won't
call that wrong. It does decrease the signal-to-noise ratio however and in that
sense I do find it less desirable. OTOH, that does help to prioritize include files
that are more responsible for sign-compare issues.

> 
> But ... the output looks horrible, and for years we didn't have the 
> warnings, still after reintroducing it we didn't get any new warnings 
> about truly bogus code.

I'd hate to have to go through every sign compare that was flagged to see
whether the mixed sign-unsigned comparison actually introduced a potential
real problem. OTOH, there's a lot of places as we're seeing where the
comparison occurs and a lot of these places don't appear to have defensive
code to handle a potentially real comparison flaw.

> So it does not seem to have much utility, and seems to be horribly 
> broken on fresh GCC.

I use gcc 4.9.2 and I see the above described explosion (so not just gcc 5.1.1+).

> 
> I'll remove the commit.
> 
> Thanks,
> 
> 	Ingo

When I submitted the patch that I did (that addresses sign comparisons in
the arch/x86/include/asm/uaccess.h file) that appears to have started this thread,
I considered submitting the change to the Makefile (like you made). While I'd love
to see this change *someday*, I decided against it for now because it seemed like
this would introduce a more significant change that requires a lot more analysis
that probably isn't worth the trouble. Individual developers can remove the
option and address flagged code in the meantime without bothering more people
that aren't interested in potential sign comparison issues in other people's code.

I believe Linus brought up the concern too about how sign-unsigned comparisons
might get addressed. I think there's sometimes a clear-cut correct "fix" but often
there isn't. So I could see a lot of patches coming in to quiet the compiler that also
caused more problems like hiding problems that would be better addressed more
comprehensively.

Lou

Re: [tip:x86/build] x86/build: Remove -Wno-sign-compare

[Linus Torvalds]

On Wed, May 13, 2015 at 10:33 AM, Louis Langholtz <lou_langholtz@me.com> wrote:
>
> Are warnings about sign-comparisons an architecture specific issue?

No, I think it's a combination of

 (a) some architectures don't end up caring too much about warnings (I
guess you could call this an "architecture specific issue")

 (b) the real problems tend to be in gcc versions that do f*cking
insane things and warn for code that cannot possibly be written better
any other way. Some architectures use a very constrained set of
compilers, and their set may not have the particular problem.

The classic example of (b) is the whole "comparison with a constant".
Gcc has gotten this wrong so many times that it's not even funny.

The fact is, we often use constants with the "wrong" sign. Tough. Part
of it is that signed constants are simply the only sane case, even if
you then compare against unsigned variables. A compiler that complains
about that is a shit compiler. It's that simple.

This is why "-Wno-sign-compare" was added in the first place. People
who think that you should always compare unsigned variables against
unsigned constants are simply wrong.

And yes, we have a lot of them inside macros. We often use macros as a
way to do generics in C, so code like this:

    #define percpu_add_op(var, val)                                         \
    do {                                                                    \
        ...
            const int pao_ID__ = (__builtin_constant_p(val) &&              \
                                  ((val) == 1 || (val) == -1)) ?            \

where we do magic things if "val" is 1 or -1 (generating "inc" and
"dec" respectively) is not insane. But because this is a generic
thing, sometimes we pass unsigned things around, and you get that
"compare 'unsigned' val against '-1'" thing. Tough.

Sure, we can add random casts in these things. In this particular
example, we could do things like cast both (val) and -1 to
"typeof(var)" and it wouldn't be "wrong".  After all, it's really just
meant for a heuristic (in this case it's a "let's see if this case of
addition is a trivial special case of adding -1, and we'll turn it
into a smaller instruction").

So those kinds of cast things don't actually improve the code, and in
many cases they actively make it less obvious. They aren't worth it.

So it's a tradeoff. Which one is better: get rid of stupid warnings by
using "-Wno-sign-compare" or by polluting the source code with
pointless type casts?

If -Wno-sign-compare had higher quality, I'd be perfectly willing to
add a few pointless casts. But the problem with the gcc sign-compare
warning is that it has traditionally (and apparenrly _still_) had the
quality of undiluted horse-shit.

Which makes it an easy decision to make: "-Wno-sign-compare" is the
right solution. Shut up the crap warnings, without making the source
worse.

                        Linus