| Message ID | 20190201210853.244043-1-jflat@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | kbuild, LLVMLinux: Don't suppress format warnings | expand |
On Sat, Feb 2, 2019 at 6:10 AM <jflat@chromium.org> wrote: > > From: Jon Flatley <jflat@chromium.org> > > gcc produces format warnings that clang suppresses. To keep behavior > consistent between gcc and clang, don't suppress format warnings in > clang. > > Signed-off-by: Jon Flatley <jflat@chromium.org> > --- Applied to linux-kbuild. Thanks. > scripts/Makefile.extrawarn | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn > index 768306add591..94daf4e1fc73 100644 > --- a/scripts/Makefile.extrawarn > +++ b/scripts/Makefile.extrawarn > @@ -68,7 +68,6 @@ else > ifdef CONFIG_CC_IS_CLANG > KBUILD_CFLAGS += $(call cc-disable-warning, initializer-overrides) > KBUILD_CFLAGS += $(call cc-disable-warning, unused-value) > -KBUILD_CFLAGS += $(call cc-disable-warning, format) > KBUILD_CFLAGS += $(call cc-disable-warning, sign-compare) > KBUILD_CFLAGS += $(call cc-disable-warning, format-zero-length) > KBUILD_CFLAGS += $(call cc-disable-warning, uninitialized) > -- > 2.20.1.611.gfbb209baf1-goog >
On Tue, Feb 05, 2019 at 05:26:05PM +0900, Masahiro Yamada wrote: > On Sat, Feb 2, 2019 at 6:10 AM <jflat@chromium.org> wrote: > > > > From: Jon Flatley <jflat@chromium.org> > > > > gcc produces format warnings that clang suppresses. To keep behavior > > consistent between gcc and clang, don't suppress format warnings in > > clang. > > > > Signed-off-by: Jon Flatley <jflat@chromium.org> > > --- > > Applied to linux-kbuild. > Thanks. > > Hi Jon and Masahiro, Just as a heads up, this introduces a ton of warnings (duh). Isn't the typical plan behind turning on warnings that were disabled to build with 'W=', fix them all, then turn them on so as not to pollute the build? Log file: https://gist.github.com/443db156e56cd3c0f6b21d9d77728d80 Note a big chunk of them come from one scnprintf call in include/linux/usb/wusb.h but still, there are many other warnings that make quite a bit of noise. Some seem relatively easy to fix, which I suppose I will try to tackle soon. Thanks, Nathan > > > > scripts/Makefile.extrawarn | 1 - > > 1 file changed, 1 deletion(-) > > > > diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn > > index 768306add591..94daf4e1fc73 100644 > > --- a/scripts/Makefile.extrawarn > > +++ b/scripts/Makefile.extrawarn > > @@ -68,7 +68,6 @@ else > > ifdef CONFIG_CC_IS_CLANG > > KBUILD_CFLAGS += $(call cc-disable-warning, initializer-overrides) > > KBUILD_CFLAGS += $(call cc-disable-warning, unused-value) > > -KBUILD_CFLAGS += $(call cc-disable-warning, format) > > KBUILD_CFLAGS += $(call cc-disable-warning, sign-compare) > > KBUILD_CFLAGS += $(call cc-disable-warning, format-zero-length) > > KBUILD_CFLAGS += $(call cc-disable-warning, uninitialized) > > -- > > 2.20.1.611.gfbb209baf1-goog > > > > > -- > Best Regards > Masahiro Yamada
On Wed, Feb 6, 2019 at 8:45 AM Nathan Chancellor <natechancellor@gmail.com> wrote: > > On Tue, Feb 05, 2019 at 05:26:05PM +0900, Masahiro Yamada wrote: > > On Sat, Feb 2, 2019 at 6:10 AM <jflat@chromium.org> wrote: > > > > > > From: Jon Flatley <jflat@chromium.org> > > > > > > gcc produces format warnings that clang suppresses. To keep behavior > > > consistent between gcc and clang, don't suppress format warnings in > > > clang. > > > > > > Signed-off-by: Jon Flatley <jflat@chromium.org> > > > --- > > > > Applied to linux-kbuild. > > Thanks. > > > > > > Hi Jon and Masahiro, > > Just as a heads up, this introduces a ton of warnings (duh). Isn't the > typical plan behind turning on warnings that were disabled to build with > 'W=', fix them all, then turn them on so as not to pollute the build? > > Log file: https://gist.github.com/443db156e56cd3c0f6b21d9d77728d80 > > Note a big chunk of them come from one scnprintf call in > include/linux/usb/wusb.h but still, there are many other warnings that > make quite a bit of noise. Some seem relatively easy to fix, which I > suppose I will try to tackle soon. > > Thanks, > Nathan > Hi Nathan, This was definitely not my intention. I noticed the added warnings this morning and was considering calling for a revert on this patch. The intent was to match the behavior of gcc, as it has -Wformat enabled. It was rather naive of me to assume the behavior of -Wformat would be the same in both gcc and clang. Indeed, it seems gcc is more permissive about what format substitutions it allows. For example passing int to the "%hu" format specifier is fine in gcc under -Wformat but produces a warning in clang. Maybe this was the motivation for adding -Wno-format to clang in the first place. This difference is puzzling to me, and I wonder if it's by design. Considering the whole point of this patch was to sync up this behavior between gcc and clang I am OK with reverting this. Thanks, Jon > > > > > > > scripts/Makefile.extrawarn | 1 - > > > 1 file changed, 1 deletion(-) > > > > > > diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn > > > index 768306add591..94daf4e1fc73 100644 > > > --- a/scripts/Makefile.extrawarn > > > +++ b/scripts/Makefile.extrawarn > > > @@ -68,7 +68,6 @@ else > > > ifdef CONFIG_CC_IS_CLANG > > > KBUILD_CFLAGS += $(call cc-disable-warning, initializer-overrides) > > > KBUILD_CFLAGS += $(call cc-disable-warning, unused-value) > > > -KBUILD_CFLAGS += $(call cc-disable-warning, format) > > > KBUILD_CFLAGS += $(call cc-disable-warning, sign-compare) > > > KBUILD_CFLAGS += $(call cc-disable-warning, format-zero-length) > > > KBUILD_CFLAGS += $(call cc-disable-warning, uninitialized) > > > -- > > > 2.20.1.611.gfbb209baf1-goog > > > > > > > > > -- > > Best Regards > > Masahiro Yamada
On Wed, Feb 6, 2019 at 9:32 AM Jon Flatley <jflat@chromium.org> wrote: > > On Wed, Feb 6, 2019 at 8:45 AM Nathan Chancellor > <natechancellor@gmail.com> wrote: > > > > On Tue, Feb 05, 2019 at 05:26:05PM +0900, Masahiro Yamada wrote: > > > On Sat, Feb 2, 2019 at 6:10 AM <jflat@chromium.org> wrote: > > > > > > > > From: Jon Flatley <jflat@chromium.org> > > > > > > > > gcc produces format warnings that clang suppresses. To keep behavior > > > > consistent between gcc and clang, don't suppress format warnings in > > > > clang. > > > > > > > > Signed-off-by: Jon Flatley <jflat@chromium.org> > > > > --- > > > > > > Applied to linux-kbuild. > > > Thanks. > > > > > > > > > > Hi Jon and Masahiro, > > > > Just as a heads up, this introduces a ton of warnings (duh). Isn't the > > typical plan behind turning on warnings that were disabled to build with > > 'W=', fix them all, then turn them on so as not to pollute the build? > > > > Log file: https://gist.github.com/443db156e56cd3c0f6b21d9d77728d80 Oh boy, that's a lot. Too many to fix quickly IMO. > > > > Note a big chunk of them come from one scnprintf call in > > include/linux/usb/wusb.h but still, there are many other warnings that > > make quite a bit of noise. Some seem relatively easy to fix, which I > > suppose I will try to tackle soon. > > > > Thanks, > > Nathan > > > > Hi Nathan, > > This was definitely not my intention. > I noticed the added warnings this morning and was considering calling > for a revert on this patch. > > The intent was to match the behavior of gcc, as it has -Wformat enabled. > It was rather naive of me to assume the behavior of -Wformat would be > the same in both gcc and clang. > Indeed, it seems gcc is more permissive about what format > substitutions it allows. > > For example passing int to the "%hu" format specifier is fine in gcc > under -Wformat but produces a warning in clang. > Maybe this was the motivation for adding -Wno-format to clang in the > first place. Sorry, I'm late to this thread. What is it reverting; who authored the original patch? Was it mka@chromium.org? > This difference is puzzling to me, and I wonder if it's by design. Probably; internally let's sync up with the Clang devs to understand this difference more. > > Considering the whole point of this patch was to sync up this behavior > between gcc and clang I am OK with reverting this. Is this patch in -next, or has it already hit mainline? I think it's better to revert, then start upstreaming fixes, then re-land it once we're warning free.
On Wed, Feb 06, 2019 at 09:36:55AM -0800, Nick Desaulniers wrote: > On Wed, Feb 6, 2019 at 9:32 AM Jon Flatley <jflat@chromium.org> wrote: > > > > On Wed, Feb 6, 2019 at 8:45 AM Nathan Chancellor > > <natechancellor@gmail.com> wrote: > > > > > > On Tue, Feb 05, 2019 at 05:26:05PM +0900, Masahiro Yamada wrote: > > > > On Sat, Feb 2, 2019 at 6:10 AM <jflat@chromium.org> wrote: > > > > > > > > > > From: Jon Flatley <jflat@chromium.org> > > > > > > > > > > gcc produces format warnings that clang suppresses. To keep behavior > > > > > consistent between gcc and clang, don't suppress format warnings in > > > > > clang. > > > > > > > > > > Signed-off-by: Jon Flatley <jflat@chromium.org> > > > > > --- > > > > > > > > Applied to linux-kbuild. > > > > Thanks. > > > > > > > > > > > > > > Hi Jon and Masahiro, > > > > > > Just as a heads up, this introduces a ton of warnings (duh). Isn't the > > > typical plan behind turning on warnings that were disabled to build with > > > 'W=', fix them all, then turn them on so as not to pollute the build? > > > > > > Log file: https://gist.github.com/443db156e56cd3c0f6b21d9d77728d80 > > Oh boy, that's a lot. Too many to fix quickly IMO. > > > > > > > Note a big chunk of them come from one scnprintf call in > > > include/linux/usb/wusb.h but still, there are many other warnings that > > > make quite a bit of noise. Some seem relatively easy to fix, which I > > > suppose I will try to tackle soon. > > > > > > Thanks, > > > Nathan > > > > > > > Hi Nathan, > > > > This was definitely not my intention. > > I noticed the added warnings this morning and was considering calling > > for a revert on this patch. > > > > The intent was to match the behavior of gcc, as it has -Wformat enabled. > > It was rather naive of me to assume the behavior of -Wformat would be > > the same in both gcc and clang. > > Indeed, it seems gcc is more permissive about what format > > substitutions it allows. > > My guess is that it has something to do with how the compilers internally handle certain specifier promotions (GCC probably just silently ignores the 'h' part of the specifier whereas Clang warns) but I didn't do any actual research into the matter. Probably should before looking into all of the warnings :) > > For example passing int to the "%hu" format specifier is fine in gcc > > under -Wformat but produces a warning in clang. > > Maybe this was the motivation for adding -Wno-format to clang in the > > first place. > > Sorry, I'm late to this thread. What is it reverting; who authored > the original patch? Was it mka@chromium.org? > This patch is turning on '-Wformat' for Clang, which was disabled in commit 3d3d6b847420 ("kbuild: LLVMLinux: Adapt warnings for compilation with clang"). > > This difference is puzzling to me, and I wonder if it's by design. > > Probably; internally let's sync up with the Clang devs to understand > this difference more. > Yes, I do think it would be a good idea to turn this on eventually but it'd be wise to understand why Clang emits a warning but GCC doesn't. > > > > Considering the whole point of this patch was to sync up this behavior > > between gcc and clang I am OK with reverting this. > > Is this patch in -next, or has it already hit mainline? I think it's > better to revert, then start upstreaming fixes, then re-land it once > we're warning free. > It's in linux-kbuild/kbuild, it hasn't hit -next yet. Nathan > -- > Thanks, > ~Nick Desaulniers
On Thu, Feb 7, 2019 at 3:03 AM Nathan Chancellor <natechancellor@gmail.com> wrote: > > On Wed, Feb 06, 2019 at 09:36:55AM -0800, Nick Desaulniers wrote: > > On Wed, Feb 6, 2019 at 9:32 AM Jon Flatley <jflat@chromium.org> wrote: > > > > > > On Wed, Feb 6, 2019 at 8:45 AM Nathan Chancellor > > > <natechancellor@gmail.com> wrote: > > > > > > > > On Tue, Feb 05, 2019 at 05:26:05PM +0900, Masahiro Yamada wrote: > > > > > On Sat, Feb 2, 2019 at 6:10 AM <jflat@chromium.org> wrote: > > > > > > > > > > > > From: Jon Flatley <jflat@chromium.org> > > > > > > > > > > > > gcc produces format warnings that clang suppresses. To keep behavior > > > > > > consistent between gcc and clang, don't suppress format warnings in > > > > > > clang. > > > > > > > > > > > > Signed-off-by: Jon Flatley <jflat@chromium.org> > > > > > > --- > > > > > > > > > > Applied to linux-kbuild. > > > > > Thanks. > > > > > > > > > > > > > > > > > > Hi Jon and Masahiro, > > > > > > > > Just as a heads up, this introduces a ton of warnings (duh). Isn't the > > > > typical plan behind turning on warnings that were disabled to build with > > > > 'W=', fix them all, then turn them on so as not to pollute the build? > > > > > > > > Log file: https://gist.github.com/443db156e56cd3c0f6b21d9d77728d80 > > > > Oh boy, that's a lot. Too many to fix quickly IMO. > > > > > > > > > > Note a big chunk of them come from one scnprintf call in > > > > include/linux/usb/wusb.h but still, there are many other warnings that > > > > make quite a bit of noise. Some seem relatively easy to fix, which I > > > > suppose I will try to tackle soon. > > > > > > > > Thanks, > > > > Nathan > > > > > > > > > > Hi Nathan, > > > > > > This was definitely not my intention. > > > I noticed the added warnings this morning and was considering calling > > > for a revert on this patch. > > > > > > The intent was to match the behavior of gcc, as it has -Wformat enabled. > > > It was rather naive of me to assume the behavior of -Wformat would be > > > the same in both gcc and clang. > > > Indeed, it seems gcc is more permissive about what format > > > substitutions it allows. > > > > > My guess is that it has something to do with how the compilers > internally handle certain specifier promotions (GCC probably just > silently ignores the 'h' part of the specifier whereas Clang warns) but > I didn't do any actual research into the matter. Probably should before > looking into all of the warnings :) > > > > For example passing int to the "%hu" format specifier is fine in gcc > > > under -Wformat but produces a warning in clang. > > > Maybe this was the motivation for adding -Wno-format to clang in the > > > first place. > > > > Sorry, I'm late to this thread. What is it reverting; who authored > > the original patch? Was it mka@chromium.org? > > > > This patch is turning on '-Wformat' for Clang, which was disabled in > commit 3d3d6b847420 ("kbuild: LLVMLinux: Adapt warnings for compilation > with clang"). > > > > This difference is puzzling to me, and I wonder if it's by design. > > > > Probably; internally let's sync up with the Clang devs to understand > > this difference more. > > > > Yes, I do think it would be a good idea to turn this on eventually but > it'd be wise to understand why Clang emits a warning but GCC doesn't. > > > > > > > Considering the whole point of this patch was to sync up this behavior > > > between gcc and clang I am OK with reverting this. > > > > Is this patch in -next, or has it already hit mainline? I think it's > > better to revert, then start upstreaming fixes, then re-land it once > > we're warning free. > > > > It's in linux-kbuild/kbuild, it hasn't hit -next yet. Right, it is just staying in my branch. I will drop it. I should have tested this carefully. Sorry.
On Thu, Feb 7, 2019 at 9:24 AM Masahiro Yamada <yamada.masahiro@socionext.com> wrote: > > On Thu, Feb 7, 2019 at 3:03 AM Nathan Chancellor > <natechancellor@gmail.com> wrote: > > > > On Wed, Feb 06, 2019 at 09:36:55AM -0800, Nick Desaulniers wrote: > > > On Wed, Feb 6, 2019 at 9:32 AM Jon Flatley <jflat@chromium.org> wrote: > > > > > > > > On Wed, Feb 6, 2019 at 8:45 AM Nathan Chancellor > > > > <natechancellor@gmail.com> wrote: > > > > > > > > > > On Tue, Feb 05, 2019 at 05:26:05PM +0900, Masahiro Yamada wrote: > > > > > > On Sat, Feb 2, 2019 at 6:10 AM <jflat@chromium.org> wrote: > > > > > > > > > > > > > > From: Jon Flatley <jflat@chromium.org> > > > > > > > > > > > > > > gcc produces format warnings that clang suppresses. To keep behavior > > > > > > > consistent between gcc and clang, don't suppress format warnings in > > > > > > > clang. > > > > > > > > > > > > > > Signed-off-by: Jon Flatley <jflat@chromium.org> > > > > > > > --- > > > > > > > > > > > > Applied to linux-kbuild. > > > > > > Thanks. > > > > > > > > > > > > > > > > > > > > > > Hi Jon and Masahiro, > > > > > > > > > > Just as a heads up, this introduces a ton of warnings (duh). Isn't the > > > > > typical plan behind turning on warnings that were disabled to build with > > > > > 'W=', fix them all, then turn them on so as not to pollute the build? > > > > > > > > > > Log file: https://gist.github.com/443db156e56cd3c0f6b21d9d77728d80 > > > > > > Oh boy, that's a lot. Too many to fix quickly IMO. > > > > > > > > > > > > > Note a big chunk of them come from one scnprintf call in > > > > > include/linux/usb/wusb.h but still, there are many other warnings that > > > > > make quite a bit of noise. Some seem relatively easy to fix, which I > > > > > suppose I will try to tackle soon. > > > > > > > > > > Thanks, > > > > > Nathan > > > > > > > > > > > > > Hi Nathan, > > > > > > > > This was definitely not my intention. > > > > I noticed the added warnings this morning and was considering calling > > > > for a revert on this patch. > > > > > > > > The intent was to match the behavior of gcc, as it has -Wformat enabled. > > > > It was rather naive of me to assume the behavior of -Wformat would be > > > > the same in both gcc and clang. > > > > Indeed, it seems gcc is more permissive about what format > > > > substitutions it allows. > > > > > > > > My guess is that it has something to do with how the compilers > > internally handle certain specifier promotions (GCC probably just > > silently ignores the 'h' part of the specifier whereas Clang warns) but > > I didn't do any actual research into the matter. Probably should before > > looking into all of the warnings :) > > > > > > For example passing int to the "%hu" format specifier is fine in gcc > > > > under -Wformat but produces a warning in clang. > > > > Maybe this was the motivation for adding -Wno-format to clang in the > > > > first place. > > > > > > Sorry, I'm late to this thread. What is it reverting; who authored > > > the original patch? Was it mka@chromium.org? > > > > > > > This patch is turning on '-Wformat' for Clang, which was disabled in > > commit 3d3d6b847420 ("kbuild: LLVMLinux: Adapt warnings for compilation > > with clang"). > > > > > > This difference is puzzling to me, and I wonder if it's by design. > > > > > > Probably; internally let's sync up with the Clang devs to understand > > > this difference more. > > > > > > > Yes, I do think it would be a good idea to turn this on eventually but > > it'd be wise to understand why Clang emits a warning but GCC doesn't. > > > > > > > > > > Considering the whole point of this patch was to sync up this behavior > > > > between gcc and clang I am OK with reverting this. > > > > > > Is this patch in -next, or has it already hit mainline? I think it's > > > better to revert, then start upstreaming fixes, then re-land it once > > > we're warning free. > > > > > > > It's in linux-kbuild/kbuild, it hasn't hit -next yet. > > > Right, it is just staying in my branch. > I will drop it. > > > I should have tested this carefully. Sorry. BTW, it would be nice if kbuild test robot could test building with clang ...
On Wed, Feb 6, 2019 at 4:28 PM Masahiro Yamada <yamada.masahiro@socionext.com> wrote: > BTW, it would be nice if kbuild test robot > could test building with clang ... Is "kbuild test robot" different than 0day, kernelCI, or ClangBuiltLinux's CI?
On Wed, Feb 06, 2019 at 04:30:21PM -0800, Nick Desaulniers wrote: > On Wed, Feb 6, 2019 at 4:28 PM Masahiro Yamada > <yamada.masahiro@socionext.com> wrote: > > BTW, it would be nice if kbuild test robot > > could test building with clang ... > > Is "kbuild test robot" different than 0day, kernelCI, or ClangBuiltLinux's CI? > -- > Thanks, > ~Nick Desaulniers kbuild test robot is 0day (that's the name the emails come under when reporting issues). Nathan
On Wed, Feb 6, 2019 at 4:37 PM Nathan Chancellor <natechancellor@gmail.com> wrote: > > On Wed, Feb 06, 2019 at 04:30:21PM -0800, Nick Desaulniers wrote: > > On Wed, Feb 6, 2019 at 4:28 PM Masahiro Yamada > > <yamada.masahiro@socionext.com> wrote: > > > BTW, it would be nice if kbuild test robot > > > could test building with clang ... > > > > Is "kbuild test robot" different than 0day, kernelCI, or ClangBuiltLinux's CI? > > -- > > Thanks, > > ~Nick Desaulniers > > kbuild test robot is 0day (that's the name the emails come under when > reporting issues). Ah, ok, I've email the 0day team about a year ago. Once we land the asm goto patches and get everything working again on x86_64, then I'll contact them again.
diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn index 768306add591..94daf4e1fc73 100644 --- a/scripts/Makefile.extrawarn +++ b/scripts/Makefile.extrawarn @@ -68,7 +68,6 @@ else ifdef CONFIG_CC_IS_CLANG KBUILD_CFLAGS += $(call cc-disable-warning, initializer-overrides) KBUILD_CFLAGS += $(call cc-disable-warning, unused-value) -KBUILD_CFLAGS += $(call cc-disable-warning, format) KBUILD_CFLAGS += $(call cc-disable-warning, sign-compare) KBUILD_CFLAGS += $(call cc-disable-warning, format-zero-length) KBUILD_CFLAGS += $(call cc-disable-warning, uninitialized)