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

From: Ingo Molnar
Date: Tue May 12 2015 - 04:44:30 EST


* Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:

> On May 11, 2015 05:44, "tip-bot for Ingo Molnar" <tipbot@xxxxxxxxx> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/