[PATCH v2 00/11] getting back -Wmaybe-uninitialized
Hi Linus, It took a while for some patches to make it into mainline through maintainer trees, but the 28-patch series is now reduced to 10, with one tiny patch added at the end. I hope this can still make it into v4.9. Aside from patches that are no longer required, I did these changes compared to version 1: - Dropped "iio: maxim_thermocouple: detect invalid storage size in read()", which is currently in linux-next as commit 32cb7d27e65d. This is the only remaining warning I see for a couple of corner cases (kbuild bot reports it on blackfin, kernelci bot and arm-soc bot both report it on arm64) - Dropped "brcmfmac: avoid maybe-uninitialized warning in brcmf_cfg80211_start_ap", which is currently in net/master merge pending. - Dropped two x86 patches, "x86: math-emu: possible uninitialized variable use" and "x86: mark target address as output in 'insb' asm" as they do not seem to trigger for a default build, and I got no feedback on them. Both of these are ancient issues and seem harmless, I will send them again to the x86 maintainers once the rest is merged. - Dropped "rbd: false-postive gcc-4.9 -Wmaybe-uninitialized" based on feedback from Ilya Dryomov, who already has a different fix queued up for v4.10. The kbuild bot reports this as a warning for xtensa. - Replaced "crypto: aesni: avoid -Wmaybe-uninitialized warning" with a simpler patch, this one always triggers but my first solution would not be safe for linux-4.9 any more at this point. I'll follow up with the larger patch as a cleanup for 4.10. - Replaced "dib0700: fix nec repeat handling" with a better one, contributed by Sean Young. Please merge these directly if you are happy with the result. As the minimum, I'd hope to see the first patch get in soon, but the individual bugfixes are hopefully now all appropriate as well. If you see any regressions with the final patch, just leave that one out and let me know what problems remain. Arnd Arnd Bergmann (10): Kbuild: enable -Wmaybe-uninitialized warning for "make W=1" NFSv4.1: work around -Wmaybe-uninitialized warning x86: apm: avoid uninitialized data nios2: fix timer initcall return value s390: pci: don't print uninitialized data for debugging [media] rc: print correct variable for z8f0811 crypto: aesni: shut up -Wmaybe-uninitialized warning infiniband: shut up a maybe-uninitialized warning pcmcia: fix return value of soc_pcmcia_regulator_set Kbuild: enable -Wmaybe-uninitialized warnings by default Sean Young (1): [media] dib0700: fix nec repeat handling Makefile | 10 +++--- arch/arc/Makefile| 4 ++- arch/nios2/kernel/time.c | 1 + arch/s390/pci/pci_dma.c | 2 +- arch/x86/crypto/aesni-intel_glue.c | 4 +-- arch/x86/kernel/apm_32.c | 5 ++- drivers/infiniband/core/cma.c| 54 +--- drivers/media/i2c/ir-kbd-i2c.c | 2 +- drivers/media/usb/dvb-usb/dib0700_core.c | 5 +-- drivers/pcmcia/soc_common.c | 2 +- fs/nfs/nfs4session.c | 10 +++--- scripts/Makefile.extrawarn | 1 + scripts/Makefile.ubsan | 4 +++ 13 files changed, 61 insertions(+), 43 deletions(-) -- 2.9.0 Cc: Anna Schumaker Cc: "David S. Miller" Cc: Herbert Xu Cc: Ilya Dryomov Cc: Javier Martinez Canillas Cc: Jiri Kosina Cc: Jonathan Cameron Cc: Ley Foon Tan Cc: Luis R. Rodriguez Cc: Martin Schwidefsky Cc: Mauro Carvalho Chehab Cc: Michal Marek Cc: Russell King Cc: Sean Young Cc: Sebastian Ott Cc: Trond Myklebust Cc: x...@kernel.org Cc: linux-kbu...@vger.kernel.org Cc: linux-ker...@vger.kernel.org Cc: linux-snps-...@lists.infradead.org Cc: nios2-...@lists.rocketboards.org Cc: linux-s...@vger.kernel.org Cc: linux-crypto@vger.kernel.org Cc: linux-me...@vger.kernel.org Cc: linux-...@vger.kernel.org -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 00/11] getting back -Wmaybe-uninitialized
On Thu, Nov 10, 2016 at 8:44 AM, Arnd Bergmann wrote: > > Please merge these directly if you are happy with the result. I will take this. I do see two warnings, but they both seem to be valid and recent, though, so I have no issues with the spurious cases. Warning #1: sound/soc/qcom/lpass-platform.c: In function ‘lpass_platform_pcmops_open’: sound/soc/qcom/lpass-platform.c:83:29: warning: ‘dma_ch’ may be used uninitialized in this function [-Wmaybe-uninitialized] drvdata->substream[dma_ch] = substream; ~~~^~~ and 'dma_ch' usage there really is crazy and wrong. Broken by 022d00ee0b55 ("ASoC: lpass-platform: Fix broken pcm data usage") Warning #2 is not a real bug, but it's reasonable that gcc doesn't know that storage_bytes (chip->read_size) has to be 2/4. Again, introduced recently by commit 231147ee77f3 ("iio: maxim_thermocouple: Align 16 bit big endian value of raw reads"), so you didn't see it. drivers/iio/temperature/maxim_thermocouple.c: In function ‘maxim_thermocouple_read_raw’: drivers/iio/temperature/maxim_thermocouple.c:141:5: warning: ‘ret’ may be used uninitialized in this function [-Wmaybe-uninitialized] if (ret) ^ drivers/iio/temperature/maxim_thermocouple.c:128:6: note: ‘ret’ was declared here int ret; ^~~ and I guess that code can just initialize 'ret' to '-EINVAL' or something to just make the theoretical "somehow we had a wrong chip->read_size" case error out cleanly. Linus -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 00/11] getting back -Wmaybe-uninitialized
On Friday, November 11, 2016 9:13:00 AM CET Linus Torvalds wrote: > On Thu, Nov 10, 2016 at 8:44 AM, Arnd Bergmann wrote: > > > > Please merge these directly if you are happy with the result. > > I will take this. Thanks a lot! > I do see two warnings, but they both seem to be valid and recent, > though, so I have no issues with the spurious cases. Ok, both of them should have my fixes coming your way already. > Warning #1: > > sound/soc/qcom/lpass-platform.c: In function ‘lpass_platform_pcmops_open’: > sound/soc/qcom/lpass-platform.c:83:29: warning: ‘dma_ch’ may be used > uninitialized in this function [-Wmaybe-uninitialized] > drvdata->substream[dma_ch] = substream; > ~~~^~~ > > and 'dma_ch' usage there really is crazy and wrong. Broken by > 022d00ee0b55 ("ASoC: lpass-platform: Fix broken pcm data usage") Right, the patches crossed here, the bugfix patch that introduced this came into linux-next over the kernel summit, and the fix I sent on Tuesday made it into Mark Brown's tree on Wednesday but not before you pulled alsa tree. It should be fixed the next time you pull from the alsa tree, the commit is 3b89e4b77ef9 ("ASoC: lpass-platform: initialize dma channel number") > Warning #2 is not a real bug, but it's reasonable that gcc doesn't > know that storage_bytes (chip->read_size) has to be 2/4. Again, > introduced recently by commit 231147ee77f3 ("iio: maxim_thermocouple: > Align 16 bit big endian value of raw reads"), so you didn't see it. This is the one I mentioned in the commit message as one that is fixed in linux-next and that should make it in soon. > drivers/iio/temperature/maxim_thermocouple.c: In function > ‘maxim_thermocouple_read_raw’: > drivers/iio/temperature/maxim_thermocouple.c:141:5: warning: ‘ret’ > may be used uninitialized in this function [-Wmaybe-uninitialized] > if (ret) >^ > drivers/iio/temperature/maxim_thermocouple.c:128:6: note: ‘ret’ was > declared here > int ret; > ^~~ > > and I guess that code can just initialize 'ret' to '-EINVAL' or > something to just make the theoretical "somehow we had a wrong > chip->read_size" case error out cleanly. Right, that was my conclusion too. I sent the bugfix on Oct 25 for linux-next but it didn't make it in until this Monday, after you pulled the patch that introduced it on Oct 29. The commit in staging-testing is 32cb7d27e65d ("iio: maxim_thermocouple: detect invalid storage size in read()") Greg and Jonathan, I see now that this is part of the 'iio-for-4.10b' branch, so I suspect you were not planning to send this before the merge window. Could you make sure this ends up in v4.9 so we get a clean build when -Wmaybe-uninitialized gets enabled again? Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 00/11] getting back -Wmaybe-uninitialized
On 11/11/16 19:49, Arnd Bergmann wrote: > On Friday, November 11, 2016 9:13:00 AM CET Linus Torvalds wrote: >> On Thu, Nov 10, 2016 at 8:44 AM, Arnd Bergmann wrote: >>> >>> Please merge these directly if you are happy with the result. >> >> I will take this. > > Thanks a lot! > >> I do see two warnings, but they both seem to be valid and recent, >> though, so I have no issues with the spurious cases. > > Ok, both of them should have my fixes coming your way already. > >> Warning #1: >> >> sound/soc/qcom/lpass-platform.c: In function ‘lpass_platform_pcmops_open’: >> sound/soc/qcom/lpass-platform.c:83:29: warning: ‘dma_ch’ may be used >> uninitialized in this function [-Wmaybe-uninitialized] >> drvdata->substream[dma_ch] = substream; >> ~~~^~~ >> >> and 'dma_ch' usage there really is crazy and wrong. Broken by >> 022d00ee0b55 ("ASoC: lpass-platform: Fix broken pcm data usage") > > Right, the patches crossed here, the bugfix patch that introduced > this came into linux-next over the kernel summit, and the fix I > sent on Tuesday made it into Mark Brown's tree on Wednesday but not > before you pulled alsa tree. It should be fixed the next time you > pull from the alsa tree, the commit is > > 3b89e4b77ef9 ("ASoC: lpass-platform: initialize dma channel number") > >> Warning #2 is not a real bug, but it's reasonable that gcc doesn't >> know that storage_bytes (chip->read_size) has to be 2/4. Again, >> introduced recently by commit 231147ee77f3 ("iio: maxim_thermocouple: >> Align 16 bit big endian value of raw reads"), so you didn't see it. > > This is the one I mentioned in the commit message as one that > is fixed in linux-next and that should make it in soon. > >> drivers/iio/temperature/maxim_thermocouple.c: In function >> ‘maxim_thermocouple_read_raw’: >> drivers/iio/temperature/maxim_thermocouple.c:141:5: warning: ‘ret’ >> may be used uninitialized in this function [-Wmaybe-uninitialized] >> if (ret) >>^ >> drivers/iio/temperature/maxim_thermocouple.c:128:6: note: ‘ret’ was >> declared here >> int ret; >> ^~~ >> >> and I guess that code can just initialize 'ret' to '-EINVAL' or >> something to just make the theoretical "somehow we had a wrong >> chip->read_size" case error out cleanly. > > Right, that was my conclusion too. I sent the bugfix on Oct 25 > for linux-next but it didn't make it in until this Monday, after > you pulled the patch that introduced it on Oct 29. > > The commit in staging-testing is > 32cb7d27e65d ("iio: maxim_thermocouple: detect invalid storage size in > read()") > > Greg and Jonathan, I see now that this is part of the 'iio-for-4.10b' > branch, so I suspect you were not planning to send this before the > merge window. Could you make sure this ends up in v4.9 so we get > a clean build when -Wmaybe-uninitialized gets enabled again? I'll queue this up and send a pull to Greg tomorrow. Was highly doubtful that a false warning suppression (be it an understandable one) was worth sending mid cycle, hence it was taking the slow route. Jonathan > > Arnd > -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 00/11] getting back -Wmaybe-uninitialized
On Sat, Nov 12, 2016 at 01:27:12PM +, Jonathan Cameron wrote: > On 11/11/16 19:49, Arnd Bergmann wrote: > > On Friday, November 11, 2016 9:13:00 AM CET Linus Torvalds wrote: > >> On Thu, Nov 10, 2016 at 8:44 AM, Arnd Bergmann wrote: > >>> > >>> Please merge these directly if you are happy with the result. > >> > >> I will take this. > > > > Thanks a lot! > > > >> I do see two warnings, but they both seem to be valid and recent, > >> though, so I have no issues with the spurious cases. > > > > Ok, both of them should have my fixes coming your way already. > > > >> Warning #1: > >> > >> sound/soc/qcom/lpass-platform.c: In function > >> ‘lpass_platform_pcmops_open’: > >> sound/soc/qcom/lpass-platform.c:83:29: warning: ‘dma_ch’ may be used > >> uninitialized in this function [-Wmaybe-uninitialized] > >> drvdata->substream[dma_ch] = substream; > >> ~~~^~~ > >> > >> and 'dma_ch' usage there really is crazy and wrong. Broken by > >> 022d00ee0b55 ("ASoC: lpass-platform: Fix broken pcm data usage") > > > > Right, the patches crossed here, the bugfix patch that introduced > > this came into linux-next over the kernel summit, and the fix I > > sent on Tuesday made it into Mark Brown's tree on Wednesday but not > > before you pulled alsa tree. It should be fixed the next time you > > pull from the alsa tree, the commit is > > > > 3b89e4b77ef9 ("ASoC: lpass-platform: initialize dma channel number") > > > >> Warning #2 is not a real bug, but it's reasonable that gcc doesn't > >> know that storage_bytes (chip->read_size) has to be 2/4. Again, > >> introduced recently by commit 231147ee77f3 ("iio: maxim_thermocouple: > >> Align 16 bit big endian value of raw reads"), so you didn't see it. > > > > This is the one I mentioned in the commit message as one that > > is fixed in linux-next and that should make it in soon. > > > >> drivers/iio/temperature/maxim_thermocouple.c: In function > >> ‘maxim_thermocouple_read_raw’: > >> drivers/iio/temperature/maxim_thermocouple.c:141:5: warning: ‘ret’ > >> may be used uninitialized in this function [-Wmaybe-uninitialized] > >> if (ret) > >>^ > >> drivers/iio/temperature/maxim_thermocouple.c:128:6: note: ‘ret’ was > >> declared here > >> int ret; > >> ^~~ > >> > >> and I guess that code can just initialize 'ret' to '-EINVAL' or > >> something to just make the theoretical "somehow we had a wrong > >> chip->read_size" case error out cleanly. > > > > Right, that was my conclusion too. I sent the bugfix on Oct 25 > > for linux-next but it didn't make it in until this Monday, after > > you pulled the patch that introduced it on Oct 29. > > > > The commit in staging-testing is > > 32cb7d27e65d ("iio: maxim_thermocouple: detect invalid storage size in > > read()") > > > > Greg and Jonathan, I see now that this is part of the 'iio-for-4.10b' > > branch, so I suspect you were not planning to send this before the > > merge window. Could you make sure this ends up in v4.9 so we get > > a clean build when -Wmaybe-uninitialized gets enabled again? > I'll queue this up and send a pull to Greg tomorrow. > > Was highly doubtful that a false warning suppression (be it an > understandable one) was worth sending mid cycle, hence it was > taking the slow route. I can just cherry-pick this, no need to send a separate pull request. greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 00/11] getting back -Wmaybe-uninitialized
On Sun, Nov 13, 2016 at 09:47:41AM +0100, Greg KH wrote: > On Sat, Nov 12, 2016 at 01:27:12PM +, Jonathan Cameron wrote: > > On 11/11/16 19:49, Arnd Bergmann wrote: > > > On Friday, November 11, 2016 9:13:00 AM CET Linus Torvalds wrote: > > >> On Thu, Nov 10, 2016 at 8:44 AM, Arnd Bergmann wrote: > > >>> > > >>> Please merge these directly if you are happy with the result. > > >> > > >> I will take this. > > > > > > Thanks a lot! > > > > > >> I do see two warnings, but they both seem to be valid and recent, > > >> though, so I have no issues with the spurious cases. > > > > > > Ok, both of them should have my fixes coming your way already. > > > > > >> Warning #1: > > >> > > >> sound/soc/qcom/lpass-platform.c: In function > > >> ‘lpass_platform_pcmops_open’: > > >> sound/soc/qcom/lpass-platform.c:83:29: warning: ‘dma_ch’ may be used > > >> uninitialized in this function [-Wmaybe-uninitialized] > > >> drvdata->substream[dma_ch] = substream; > > >> ~~~^~~ > > >> > > >> and 'dma_ch' usage there really is crazy and wrong. Broken by > > >> 022d00ee0b55 ("ASoC: lpass-platform: Fix broken pcm data usage") > > > > > > Right, the patches crossed here, the bugfix patch that introduced > > > this came into linux-next over the kernel summit, and the fix I > > > sent on Tuesday made it into Mark Brown's tree on Wednesday but not > > > before you pulled alsa tree. It should be fixed the next time you > > > pull from the alsa tree, the commit is > > > > > > 3b89e4b77ef9 ("ASoC: lpass-platform: initialize dma channel number") > > > > > >> Warning #2 is not a real bug, but it's reasonable that gcc doesn't > > >> know that storage_bytes (chip->read_size) has to be 2/4. Again, > > >> introduced recently by commit 231147ee77f3 ("iio: maxim_thermocouple: > > >> Align 16 bit big endian value of raw reads"), so you didn't see it. > > > > > > This is the one I mentioned in the commit message as one that > > > is fixed in linux-next and that should make it in soon. > > > > > >> drivers/iio/temperature/maxim_thermocouple.c: In function > > >> ‘maxim_thermocouple_read_raw’: > > >> drivers/iio/temperature/maxim_thermocouple.c:141:5: warning: ‘ret’ > > >> may be used uninitialized in this function [-Wmaybe-uninitialized] > > >> if (ret) > > >>^ > > >> drivers/iio/temperature/maxim_thermocouple.c:128:6: note: ‘ret’ was > > >> declared here > > >> int ret; > > >> ^~~ > > >> > > >> and I guess that code can just initialize 'ret' to '-EINVAL' or > > >> something to just make the theoretical "somehow we had a wrong > > >> chip->read_size" case error out cleanly. > > > > > > Right, that was my conclusion too. I sent the bugfix on Oct 25 > > > for linux-next but it didn't make it in until this Monday, after > > > you pulled the patch that introduced it on Oct 29. > > > > > > The commit in staging-testing is > > > 32cb7d27e65d ("iio: maxim_thermocouple: detect invalid storage size in > > > read()") > > > > > > Greg and Jonathan, I see now that this is part of the 'iio-for-4.10b' > > > branch, so I suspect you were not planning to send this before the > > > merge window. Could you make sure this ends up in v4.9 so we get > > > a clean build when -Wmaybe-uninitialized gets enabled again? > > I'll queue this up and send a pull to Greg tomorrow. > > > > Was highly doubtful that a false warning suppression (be it an > > understandable one) was worth sending mid cycle, hence it was > > taking the slow route. > > I can just cherry-pick this, no need to send a separate pull request. Now done and sent to Linus, so all should be good here. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html