lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <20150512084415.GA6535@gmail.com>
Date:	Tue, 12 May 2015 10:44:15 +0200
From:	Ingo Molnar <mingo@...nel.org>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
Cc:	Peter Anvin <hpa@...or.com>, Brian Gerst <brgerst@...il.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Borislav Petkov <bp@...en8.de>,
	Peter Zijlstra <peterz@...radead.org>,
	Andy Lutomirski <luto@...capital.net>,
	Louis Langholtz <lou_langholtz@...com>,
	Denys Vlasenko <dvlasenk@...hat.com>,
	linux-kernel@...r.kernel.org
Subject: Re: [tip:x86/build] x86/build: Remove -Wno-sign-compare


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

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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ