From: Jon Flatley <[email protected]>
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 <[email protected]>
---
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 Sat, Feb 2, 2019 at 6:10 AM <[email protected]> wrote:
>
> From: Jon Flatley <[email protected]>
>
> 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 <[email protected]>
> ---
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
>
--
Best Regards
Masahiro Yamada
On Tue, Feb 05, 2019 at 05:26:05PM +0900, Masahiro Yamada wrote:
> On Sat, Feb 2, 2019 at 6:10 AM <[email protected]> wrote:
> >
> > From: Jon Flatley <[email protected]>
> >
> > 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 <[email protected]>
> > ---
>
> 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 9:32 AM Jon Flatley <[email protected]> wrote:
>
> On Wed, Feb 6, 2019 at 8:45 AM Nathan Chancellor
> <[email protected]> wrote:
> >
> > On Tue, Feb 05, 2019 at 05:26:05PM +0900, Masahiro Yamada wrote:
> > > On Sat, Feb 2, 2019 at 6:10 AM <[email protected]> wrote:
> > > >
> > > > From: Jon Flatley <[email protected]>
> > > >
> > > > 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 <[email protected]>
> > > > ---
> > >
> > > 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 [email protected]?
> 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.
--
Thanks,
~Nick Desaulniers
On Wed, Feb 6, 2019 at 8:45 AM Nathan Chancellor
<[email protected]> wrote:
>
> On Tue, Feb 05, 2019 at 05:26:05PM +0900, Masahiro Yamada wrote:
> > On Sat, Feb 2, 2019 at 6:10 AM <[email protected]> wrote:
> > >
> > > From: Jon Flatley <[email protected]>
> > >
> > > 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 <[email protected]>
> > > ---
> >
> > 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 06, 2019 at 09:36:55AM -0800, Nick Desaulniers wrote:
> On Wed, Feb 6, 2019 at 9:32 AM Jon Flatley <[email protected]> wrote:
> >
> > On Wed, Feb 6, 2019 at 8:45 AM Nathan Chancellor
> > <[email protected]> wrote:
> > >
> > > On Tue, Feb 05, 2019 at 05:26:05PM +0900, Masahiro Yamada wrote:
> > > > On Sat, Feb 2, 2019 at 6:10 AM <[email protected]> wrote:
> > > > >
> > > > > From: Jon Flatley <[email protected]>
> > > > >
> > > > > 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 <[email protected]>
> > > > > ---
> > > >
> > > > 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 [email protected]?
>
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
<[email protected]> 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 <[email protected]> wrote:
> > >
> > > On Wed, Feb 6, 2019 at 8:45 AM Nathan Chancellor
> > > <[email protected]> wrote:
> > > >
> > > > On Tue, Feb 05, 2019 at 05:26:05PM +0900, Masahiro Yamada wrote:
> > > > > On Sat, Feb 2, 2019 at 6:10 AM <[email protected]> wrote:
> > > > > >
> > > > > > From: Jon Flatley <[email protected]>
> > > > > >
> > > > > > 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 <[email protected]>
> > > > > > ---
> > > > >
> > > > > 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 [email protected]?
> >
>
> 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.
--
Best Regards
Masahiro Yamada
On Thu, Feb 7, 2019 at 9:24 AM Masahiro Yamada
<[email protected]> wrote:
>
> On Thu, Feb 7, 2019 at 3:03 AM Nathan Chancellor
> <[email protected]> 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 <[email protected]> wrote:
> > > >
> > > > On Wed, Feb 6, 2019 at 8:45 AM Nathan Chancellor
> > > > <[email protected]> wrote:
> > > > >
> > > > > On Tue, Feb 05, 2019 at 05:26:05PM +0900, Masahiro Yamada wrote:
> > > > > > On Sat, Feb 2, 2019 at 6:10 AM <[email protected]> wrote:
> > > > > > >
> > > > > > > From: Jon Flatley <[email protected]>
> > > > > > >
> > > > > > > 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 <[email protected]>
> > > > > > > ---
> > > > > >
> > > > > > 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 [email protected]?
> > >
> >
> > 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 ...
--
Best Regards
Masahiro Yamada
On Wed, Feb 6, 2019 at 4:28 PM Masahiro Yamada
<[email protected]> 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
On Wed, Feb 06, 2019 at 04:30:21PM -0800, Nick Desaulniers wrote:
> On Wed, Feb 6, 2019 at 4:28 PM Masahiro Yamada
> <[email protected]> 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
<[email protected]> 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
> > <[email protected]> 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.
--
Thanks,
~Nick Desaulniers