2022-07-20 23:46:35

by Justin Stitt

[permalink] [raw]
Subject: [PATCH] Makefile.extrawarn: re-enable -Wformat for clang

There's been an ongoing mission to re-enable the -Wformat warning for
Clang. A previous attempt at enabling the warning showed that there were
many instances of this warning throughout the codebase. The sheer amount
of these warnings really polluted builds and thus -Wno-format was added
to _temporarily_ toggle them off.

After many patches the warning has largely been eradicated for x86,
x86_64, arm, and arm64 on a variety of configs. The time to enable the
warning has never been better as it seems for the first time we are
ahead of them and can now solve them as they appear rather than tackling
from a backlog.

As to the root cause of this large backlog of warnings, Clang seems to
pickup on some more nuanced cases of format warnings caused by implicit
integer conversion as well as default argument promotions from
printf-like functions.


Link: https://github.com/ClangBuiltLinux/linux/issues/378
Suggested-by: Nick Desaulniers <[email protected]>
Signed-off-by: Justin Stitt <[email protected]>
---
Previous attempt: (https://patchwork.kernel.org/project/linux-kbuild/patch/[email protected]/)

Note:
For this patch to land on its feet, the plethora of supporting patches that
fixed various -Wformat warnings need to be picked up. Thanfully, a lot
of them have!

Here are the patches still waiting to be picked up:
* https://lore.kernel.org/all/[email protected]/
* https://lore.kernel.org/all/[email protected]/

scripts/Makefile.extrawarn | 1 -
1 file changed, 1 deletion(-)

diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn
index f5f0d6f09053..9bbaf7112a9b 100644
--- a/scripts/Makefile.extrawarn
+++ b/scripts/Makefile.extrawarn
@@ -47,7 +47,6 @@ else

ifdef CONFIG_CC_IS_CLANG
KBUILD_CFLAGS += -Wno-initializer-overrides
-KBUILD_CFLAGS += -Wno-format
KBUILD_CFLAGS += -Wno-sign-compare
KBUILD_CFLAGS += -Wno-format-zero-length
KBUILD_CFLAGS += $(call cc-disable-warning, pointer-to-enum-cast)
--
2.37.0.170.g444d1eabd0-goog


2022-07-21 14:57:47

by Nick Desaulniers

[permalink] [raw]
Subject: Re: [PATCH] Makefile.extrawarn: re-enable -Wformat for clang

On Wed, Jul 20, 2022 at 4:23 PM Justin Stitt <[email protected]> wrote:
>
> There's been an ongoing mission to re-enable the -Wformat warning for
> Clang. A previous attempt at enabling the warning showed that there were
> many instances of this warning throughout the codebase. The sheer amount
> of these warnings really polluted builds and thus -Wno-format was added
> to _temporarily_ toggle them off.
>
> After many patches the warning has largely been eradicated for x86,
> x86_64, arm, and arm64 on a variety of configs. The time to enable the
> warning has never been better as it seems for the first time we are
> ahead of them and can now solve them as they appear rather than tackling
> from a backlog.
>
> As to the root cause of this large backlog of warnings, Clang seems to
> pickup on some more nuanced cases of format warnings caused by implicit
> integer conversion as well as default argument promotions from
> printf-like functions.
>
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/378
> Suggested-by: Nick Desaulniers <[email protected]>
> Signed-off-by: Justin Stitt <[email protected]>
> ---
> Previous attempt: (https://patchwork.kernel.org/project/linux-kbuild/patch/[email protected]/)
>
> Note:
> For this patch to land on its feet, the plethora of supporting patches that
> fixed various -Wformat warnings need to be picked up. Thanfully, a lot
> of them have!
>
> Here are the patches still waiting to be picked up:
> * https://lore.kernel.org/all/[email protected]/
> * https://lore.kernel.org/all/[email protected]/

Hi Masahiro, Nathan, and Tom,
What are your thoughts for _when_ in the release cycle this should be
picked up? I worry that if we don't remove this soon, we will
backslide, and more -Wformat issues will crop up making removing this
in the future like digging in sand. Justin has chased down many
instances of this warning, and I'm happy to help clean up fallout from
landing this.

>
> scripts/Makefile.extrawarn | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn
> index f5f0d6f09053..9bbaf7112a9b 100644
> --- a/scripts/Makefile.extrawarn
> +++ b/scripts/Makefile.extrawarn
> @@ -47,7 +47,6 @@ else
>
> ifdef CONFIG_CC_IS_CLANG
> KBUILD_CFLAGS += -Wno-initializer-overrides
> -KBUILD_CFLAGS += -Wno-format
> KBUILD_CFLAGS += -Wno-sign-compare
> KBUILD_CFLAGS += -Wno-format-zero-length
> KBUILD_CFLAGS += $(call cc-disable-warning, pointer-to-enum-cast)
> --
> 2.37.0.170.g444d1eabd0-goog
>


--
Thanks,
~Nick Desaulniers

2022-07-21 15:25:01

by Nick Desaulniers

[permalink] [raw]
Subject: Re: [PATCH] Makefile.extrawarn: re-enable -Wformat for clang

On Thu, Jul 21, 2022 at 7:27 AM Nick Desaulniers
<[email protected]> wrote:
>
> On Wed, Jul 20, 2022 at 4:23 PM Justin Stitt <[email protected]> wrote:
> >
> > There's been an ongoing mission to re-enable the -Wformat warning for
> > Clang. A previous attempt at enabling the warning showed that there were
> > many instances of this warning throughout the codebase. The sheer amount
> > of these warnings really polluted builds and thus -Wno-format was added
> > to _temporarily_ toggle them off.
> >
> > After many patches the warning has largely been eradicated for x86,
> > x86_64, arm, and arm64 on a variety of configs. The time to enable the
> > warning has never been better as it seems for the first time we are
> > ahead of them and can now solve them as they appear rather than tackling
> > from a backlog.
> >
> > As to the root cause of this large backlog of warnings, Clang seems to
> > pickup on some more nuanced cases of format warnings caused by implicit
> > integer conversion as well as default argument promotions from
> > printf-like functions.
> >
> >
> > Link: https://github.com/ClangBuiltLinux/linux/issues/378
> > Suggested-by: Nick Desaulniers <[email protected]>
> > Signed-off-by: Justin Stitt <[email protected]>
> > ---
> > Previous attempt: (https://patchwork.kernel.org/project/linux-kbuild/patch/[email protected]/)
> >
> > Note:
> > For this patch to land on its feet, the plethora of supporting patches that
> > fixed various -Wformat warnings need to be picked up. Thanfully, a lot
> > of them have!
> >
> > Here are the patches still waiting to be picked up:
> > * https://lore.kernel.org/all/[email protected]/
> > * https://lore.kernel.org/all/[email protected]/
>
> Hi Masahiro, Nathan, and Tom,
> What are your thoughts for _when_ in the release cycle this should be
> picked up? I worry that if we don't remove this soon, we will
> backslide, and more -Wformat issues will crop up making removing this
> in the future like digging in sand. Justin has chased down many
> instances of this warning, and I'm happy to help clean up fallout from
> landing this.

Otherwise, we probably want to look at picking up Youngmin's patch ASAP.
https://lore.kernel.org/all/[email protected]/

(This thread, for context for Youngmin:
https://lore.kernel.org/llvm/[email protected]/)

>
> >
> > scripts/Makefile.extrawarn | 1 -
> > 1 file changed, 1 deletion(-)
> >
> > diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn
> > index f5f0d6f09053..9bbaf7112a9b 100644
> > --- a/scripts/Makefile.extrawarn
> > +++ b/scripts/Makefile.extrawarn
> > @@ -47,7 +47,6 @@ else
> >
> > ifdef CONFIG_CC_IS_CLANG
> > KBUILD_CFLAGS += -Wno-initializer-overrides
> > -KBUILD_CFLAGS += -Wno-format
> > KBUILD_CFLAGS += -Wno-sign-compare
> > KBUILD_CFLAGS += -Wno-format-zero-length
> > KBUILD_CFLAGS += $(call cc-disable-warning, pointer-to-enum-cast)
> > --
> > 2.37.0.170.g444d1eabd0-goog
> >
>
>
> --
> Thanks,
> ~Nick Desaulniers



--
Thanks,
~Nick Desaulniers

2022-07-21 15:54:45

by Nathan Chancellor

[permalink] [raw]
Subject: Re: [PATCH] Makefile.extrawarn: re-enable -Wformat for clang

On Thu, Jul 21, 2022 at 07:27:34AM -0700, Nick Desaulniers wrote:
> On Wed, Jul 20, 2022 at 4:23 PM Justin Stitt <[email protected]> wrote:
> >
> > There's been an ongoing mission to re-enable the -Wformat warning for
> > Clang. A previous attempt at enabling the warning showed that there were
> > many instances of this warning throughout the codebase. The sheer amount
> > of these warnings really polluted builds and thus -Wno-format was added
> > to _temporarily_ toggle them off.
> >
> > After many patches the warning has largely been eradicated for x86,
> > x86_64, arm, and arm64 on a variety of configs. The time to enable the
> > warning has never been better as it seems for the first time we are
> > ahead of them and can now solve them as they appear rather than tackling
> > from a backlog.
> >
> > As to the root cause of this large backlog of warnings, Clang seems to
> > pickup on some more nuanced cases of format warnings caused by implicit
> > integer conversion as well as default argument promotions from
> > printf-like functions.
> >
> >
> > Link: https://github.com/ClangBuiltLinux/linux/issues/378
> > Suggested-by: Nick Desaulniers <[email protected]>
> > Signed-off-by: Justin Stitt <[email protected]>
> > ---
> > Previous attempt: (https://patchwork.kernel.org/project/linux-kbuild/patch/[email protected]/)
> >
> > Note:
> > For this patch to land on its feet, the plethora of supporting patches that
> > fixed various -Wformat warnings need to be picked up. Thanfully, a lot
> > of them have!
> >
> > Here are the patches still waiting to be picked up:
> > * https://lore.kernel.org/all/[email protected]/
> > * https://lore.kernel.org/all/[email protected]/
>
> Hi Masahiro, Nathan, and Tom,
> What are your thoughts for _when_ in the release cycle this should be
> picked up? I worry that if we don't remove this soon, we will
> backslide, and more -Wformat issues will crop up making removing this
> in the future like digging in sand. Justin has chased down many
> instances of this warning, and I'm happy to help clean up fallout from
> landing this.

Let me do a series of builds with the two patches above against
next-20220721 to see if there are any instances of this warning across
the less frequently tested architectures then I will review/ack this.

I don't think we need to worry much about backslide, as -Wformat is
enabled with W=1, which the 0day folks already test with, so new
instances of this warning should get reported to the authors when they
are introduced so they can be fixed immediately. However, I would still
like to see this applied sooner rather than later, although I would also
like us to be completely warning clean before doing so, especially with
-Werror now being selected with all{mod,yes}config. -rc8 is this Sunday
and final should be July 31st so I think this could be applied at some
point between those two dates then maybe sent to Linus for a late pull
request once all other trees have been merged but that is ultimately up
to Masahiro.

Cheers,
Nathan

> >
> > scripts/Makefile.extrawarn | 1 -
> > 1 file changed, 1 deletion(-)
> >
> > diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn
> > index f5f0d6f09053..9bbaf7112a9b 100644
> > --- a/scripts/Makefile.extrawarn
> > +++ b/scripts/Makefile.extrawarn
> > @@ -47,7 +47,6 @@ else
> >
> > ifdef CONFIG_CC_IS_CLANG
> > KBUILD_CFLAGS += -Wno-initializer-overrides
> > -KBUILD_CFLAGS += -Wno-format
> > KBUILD_CFLAGS += -Wno-sign-compare
> > KBUILD_CFLAGS += -Wno-format-zero-length
> > KBUILD_CFLAGS += $(call cc-disable-warning, pointer-to-enum-cast)
> > --
> > 2.37.0.170.g444d1eabd0-goog
> >
>
>
> --
> Thanks,
> ~Nick Desaulniers

2022-07-21 20:06:11

by Nathan Chancellor

[permalink] [raw]
Subject: Re: [PATCH] Makefile.extrawarn: re-enable -Wformat for clang

On Thu, Jul 21, 2022 at 08:10:27AM -0700, Nathan Chancellor wrote:
> On Thu, Jul 21, 2022 at 07:27:34AM -0700, Nick Desaulniers wrote:
> > On Wed, Jul 20, 2022 at 4:23 PM Justin Stitt <[email protected]> wrote:
> > >
> > > There's been an ongoing mission to re-enable the -Wformat warning for
> > > Clang. A previous attempt at enabling the warning showed that there were
> > > many instances of this warning throughout the codebase. The sheer amount
> > > of these warnings really polluted builds and thus -Wno-format was added
> > > to _temporarily_ toggle them off.
> > >
> > > After many patches the warning has largely been eradicated for x86,
> > > x86_64, arm, and arm64 on a variety of configs. The time to enable the
> > > warning has never been better as it seems for the first time we are
> > > ahead of them and can now solve them as they appear rather than tackling
> > > from a backlog.
> > >
> > > As to the root cause of this large backlog of warnings, Clang seems to
> > > pickup on some more nuanced cases of format warnings caused by implicit
> > > integer conversion as well as default argument promotions from
> > > printf-like functions.
> > >
> > >
> > > Link: https://github.com/ClangBuiltLinux/linux/issues/378
> > > Suggested-by: Nick Desaulniers <[email protected]>
> > > Signed-off-by: Justin Stitt <[email protected]>
> > > ---
> > > Previous attempt: (https://patchwork.kernel.org/project/linux-kbuild/patch/[email protected]/)
> > >
> > > Note:
> > > For this patch to land on its feet, the plethora of supporting patches that
> > > fixed various -Wformat warnings need to be picked up. Thanfully, a lot
> > > of them have!
> > >
> > > Here are the patches still waiting to be picked up:
> > > * https://lore.kernel.org/all/[email protected]/
> > > * https://lore.kernel.org/all/[email protected]/
> >
> > Hi Masahiro, Nathan, and Tom,
> > What are your thoughts for _when_ in the release cycle this should be
> > picked up? I worry that if we don't remove this soon, we will
> > backslide, and more -Wformat issues will crop up making removing this
> > in the future like digging in sand. Justin has chased down many
> > instances of this warning, and I'm happy to help clean up fallout from
> > landing this.
>
> Let me do a series of builds with the two patches above against
> next-20220721 to see if there are any instances of this warning across
> the less frequently tested architectures then I will review/ack this.

Alright, against next-20220721, I applied:

* https://lore.kernel.org/[email protected]/ (applied to net-next, just not in this -next release)
* https://lore.kernel.org/[email protected]/ (not picked up)
* https://lore.kernel.org/[email protected]/ (not picked up)

I still see the following warnings. I have suggested fixes, which I am happy to
send unless Justin wants to.

========================================================================

ARCH=arm allmodconfig:

../drivers/iommu/msm_iommu.c:603:6: error: format specifies type 'unsigned short' but the argument has type 'int' [-Werror,-Wformat]
sid);
^~~
../include/linux/dev_printk.h:146:70: note: expanded from macro 'dev_warn'
dev_printk_index_wrap(_dev_warn, KERN_WARNING, dev, dev_fmt(fmt), ##__VA_ARGS__)
~~~ ^~~~~~~~~~~
../include/linux/dev_printk.h:110:23: note: expanded from macro 'dev_printk_index_wrap'
_p_func(dev, fmt, ##__VA_ARGS__); \
~~~ ^~~~~~~~~~~
1 error generated.

Introduced by commit f78ebca8ff3d ("iommu/msm: Add support for generic master
bindings").

diff --git a/drivers/iommu/msm_iommu.c b/drivers/iommu/msm_iommu.c
index 428919a474c1..6a24aa804ea3 100644
--- a/drivers/iommu/msm_iommu.c
+++ b/drivers/iommu/msm_iommu.c
@@ -599,7 +599,7 @@ static int insert_iommu_master(struct device *dev,

for (sid = 0; sid < master->num_mids; sid++)
if (master->mids[sid] == spec->args[0]) {
- dev_warn(dev, "Stream ID 0x%hx repeated; ignoring\n",
+ dev_warn(dev, "Stream ID 0x%x repeated; ignoring\n",
sid);
return 0;
}

========================================================================

ARCH=hexagon allmodconfig + CONFIG_FRAME_WARN=0:

../drivers/misc/lkdtm/bugs.c:107:3: error: format specifies type 'unsigned long' but the argument has type 'int' [-Werror,-Wformat]
REC_STACK_SIZE, recur_count);
^~~~~~~~~~~~~~
../include/linux/printk.h:537:34: note: expanded from macro 'pr_info'
printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
~~~ ^~~~~~~~~~~
../include/linux/printk.h:464:60: note: expanded from macro 'printk'
#define printk(fmt, ...) printk_index_wrap(_printk, fmt, ##__VA_ARGS__)
~~~ ^~~~~~~~~~~
../include/linux/printk.h:436:19: note: expanded from macro 'printk_index_wrap'
_p_func(_fmt, ##__VA_ARGS__); \
~~~~ ^~~~~~~~~~~
../drivers/misc/lkdtm/bugs.c:32:24: note: expanded from macro 'REC_STACK_SIZE'
#define REC_STACK_SIZE (THREAD_SIZE / 8)
^~~~~~~~~~~~~~~~~
1 error generated.

Introduced by commit 24cccab42c41 ("lkdtm/bugs: Adjust recursion test to avoid
elision").

diff --git a/drivers/misc/lkdtm/bugs.c b/drivers/misc/lkdtm/bugs.c
index 009239ad1d8a..6381255aaecc 100644
--- a/drivers/misc/lkdtm/bugs.c
+++ b/drivers/misc/lkdtm/bugs.c
@@ -29,7 +29,7 @@ struct lkdtm_list {
#if defined(CONFIG_FRAME_WARN) && (CONFIG_FRAME_WARN > 0)
#define REC_STACK_SIZE (_AC(CONFIG_FRAME_WARN, UL) / 2)
#else
-#define REC_STACK_SIZE (THREAD_SIZE / 8)
+#define REC_STACK_SIZE ((unsigned long)(THREAD_SIZE / 8))
#endif
#define REC_NUM_DEFAULT ((THREAD_SIZE / REC_STACK_SIZE) * 2)


========================================================================

ARCH=arm allmodconfig:

../drivers/nvme/target/auth.c:492:18: error: format specifies type 'unsigned long' but the argument has type 'size_t' (aka 'unsigned int') [-Werror,-Wformat]
ctrl->cntlid, ctrl->dh_keysize, buf_size);
^~~~~~~~~~~~~~~~
../include/linux/printk.h:517:37: note: expanded from macro 'pr_warn'
printk(KERN_WARNING pr_fmt(fmt), ##__VA_ARGS__)
~~~ ^~~~~~~~~~~
../include/linux/printk.h:464:60: note: expanded from macro 'printk'
#define printk(fmt, ...) printk_index_wrap(_printk, fmt, ##__VA_ARGS__)
~~~ ^~~~~~~~~~~
../include/linux/printk.h:436:19: note: expanded from macro 'printk_index_wrap'
_p_func(_fmt, ##__VA_ARGS__); \
~~~~ ^~~~~~~~~~~
1 error generated.

Introduced by commit 71ebe3842ebe ("nvmet-auth: Diffie-Hellman key exchange
support").

This one is not clang specific and already has a fix pending:

https://lore.kernel.org/[email protected]/

========================================================================

Pretty much every allmodconfig:

../sound/soc/sof/ipc3-topology.c:2343:4: error: format specifies type 'unsigned char' but the argument has type 'int' [-Werror,-Wformat]
SOF_ABI_MAJOR, SOF_ABI_MINOR, SOF_ABI_PATCH);
^~~~~~~~~~~~~
../include/linux/dev_printk.h:150:67: note: expanded from macro 'dev_info'
dev_printk_index_wrap(_dev_info, KERN_INFO, dev, dev_fmt(fmt), ##__VA_ARGS__)
~~~ ^~~~~~~~~~~
../include/linux/dev_printk.h:110:23: note: expanded from macro 'dev_printk_index_wrap'
_p_func(dev, fmt, ##__VA_ARGS__); \
~~~ ^~~~~~~~~~~
../include/uapi/sound/sof/abi.h:30:23: note: expanded from macro 'SOF_ABI_MAJOR'
#define SOF_ABI_MAJOR 3
^
../sound/soc/sof/ipc3-topology.c:2343:19: error: format specifies type 'unsigned char' but the argument has type 'int' [-Werror,-Wformat]
SOF_ABI_MAJOR, SOF_ABI_MINOR, SOF_ABI_PATCH);
^~~~~~~~~~~~~
../include/linux/dev_printk.h:150:67: note: expanded from macro 'dev_info'
dev_printk_index_wrap(_dev_info, KERN_INFO, dev, dev_fmt(fmt), ##__VA_ARGS__)
~~~ ^~~~~~~~~~~
../include/linux/dev_printk.h:110:23: note: expanded from macro 'dev_printk_index_wrap'
_p_func(dev, fmt, ##__VA_ARGS__); \
~~~ ^~~~~~~~~~~
../include/uapi/sound/sof/abi.h:31:23: note: expanded from macro 'SOF_ABI_MINOR'
#define SOF_ABI_MINOR 22
^~
../sound/soc/sof/ipc3-topology.c:2343:34: error: format specifies type 'unsigned char' but the argument has type 'int' [-Werror,-Wformat]
SOF_ABI_MAJOR, SOF_ABI_MINOR, SOF_ABI_PATCH);
^~~~~~~~~~~~~
../include/linux/dev_printk.h:150:67: note: expanded from macro 'dev_info'
dev_printk_index_wrap(_dev_info, KERN_INFO, dev, dev_fmt(fmt), ##__VA_ARGS__)
~~~ ^~~~~~~~~~~
../include/linux/dev_printk.h:110:23: note: expanded from macro 'dev_printk_index_wrap'
_p_func(dev, fmt, ##__VA_ARGS__); \
~~~ ^~~~~~~~~~~
../include/uapi/sound/sof/abi.h:32:23: note: expanded from macro 'SOF_ABI_PATCH'
#define SOF_ABI_PATCH 0
^
3 errors generated.

Introduced by commit 323aa1f093e6 ("ASoC: SOF: Add a new IPC op for parsing
topology manifest") for little reason it seems?

diff --git a/sound/soc/sof/ipc3-topology.c b/sound/soc/sof/ipc3-topology.c
index b2cc046b9f60..65923e7a5976 100644
--- a/sound/soc/sof/ipc3-topology.c
+++ b/sound/soc/sof/ipc3-topology.c
@@ -2338,7 +2338,7 @@ static int sof_ipc3_parse_manifest(struct snd_soc_component *scomp, int index,
}

dev_info(scomp->dev,
- "Topology: ABI %d:%d:%d Kernel ABI %hhu:%hhu:%hhu\n",
+ "Topology: ABI %d:%d:%d Kernel ABI %d:%d:%d\n",
man->priv.data[0], man->priv.data[1], man->priv.data[2],
SOF_ABI_MAJOR, SOF_ABI_MINOR, SOF_ABI_PATCH);


========================================================================

I would really like to see patches in flight for these before this patch
is accepted but it is really awesome to see how close we are :)

Cheers,
Nathan

2022-07-21 20:41:13

by Justin Stitt

[permalink] [raw]
Subject: Re: [PATCH] Makefile.extrawarn: re-enable -Wformat for clang

On Thu, Jul 21, 2022 at 12:37 PM Nathan Chancellor <[email protected]> wrote:
>
> On Thu, Jul 21, 2022 at 08:10:27AM -0700, Nathan Chancellor wrote:
> > On Thu, Jul 21, 2022 at 07:27:34AM -0700, Nick Desaulniers wrote:
> > > On Wed, Jul 20, 2022 at 4:23 PM Justin Stitt <[email protected]> wrote:
> > > >
> > > > There's been an ongoing mission to re-enable the -Wformat warning for
> > > > Clang. A previous attempt at enabling the warning showed that there were
> > > > many instances of this warning throughout the codebase. The sheer amount
> > > > of these warnings really polluted builds and thus -Wno-format was added
> > > > to _temporarily_ toggle them off.
> > > >
> > > > After many patches the warning has largely been eradicated for x86,
> > > > x86_64, arm, and arm64 on a variety of configs. The time to enable the
> > > > warning has never been better as it seems for the first time we are
> > > > ahead of them and can now solve them as they appear rather than tackling
> > > > from a backlog.
> > > >
> > > > As to the root cause of this large backlog of warnings, Clang seems to
> > > > pickup on some more nuanced cases of format warnings caused by implicit
> > > > integer conversion as well as default argument promotions from
> > > > printf-like functions.
> > > >
> > > >
> > > > Link: https://github.com/ClangBuiltLinux/linux/issues/378
> > > > Suggested-by: Nick Desaulniers <[email protected]>
> > > > Signed-off-by: Justin Stitt <[email protected]>
> > > > ---
> > > > Previous attempt: (https://patchwork.kernel.org/project/linux-kbuild/patch/[email protected]/)
> > > >
> > > > Note:
> > > > For this patch to land on its feet, the plethora of supporting patches that
> > > > fixed various -Wformat warnings need to be picked up. Thanfully, a lot
> > > > of them have!
> > > >
> > > > Here are the patches still waiting to be picked up:
> > > > * https://lore.kernel.org/all/[email protected]/
> > > > * https://lore.kernel.org/all/[email protected]/
> > >
> > > Hi Masahiro, Nathan, and Tom,
> > > What are your thoughts for _when_ in the release cycle this should be
> > > picked up? I worry that if we don't remove this soon, we will
> > > backslide, and more -Wformat issues will crop up making removing this
> > > in the future like digging in sand. Justin has chased down many
> > > instances of this warning, and I'm happy to help clean up fallout from
> > > landing this.
> >
> > Let me do a series of builds with the two patches above against
> > next-20220721 to see if there are any instances of this warning across
> > the less frequently tested architectures then I will review/ack this.
>
> Alright, against next-20220721, I applied:
>
> * https://lore.kernel.org/[email protected]/ (applied to net-next, just not in this -next release)
> * https://lore.kernel.org/[email protected]/ (not picked up)
> * https://lore.kernel.org/[email protected]/ (not picked up)
>
> I still see the following warnings. I have suggested fixes, which I am happy to
> send unless Justin wants to.

I can tackle these by EOD.

>
> ========================================================================
>
> ARCH=arm allmodconfig:
>
> ../drivers/iommu/msm_iommu.c:603:6: error: format specifies type 'unsigned short' but the argument has type 'int' [-Werror,-Wformat]
> sid);
> ^~~
> ../include/linux/dev_printk.h:146:70: note: expanded from macro 'dev_warn'
> dev_printk_index_wrap(_dev_warn, KERN_WARNING, dev, dev_fmt(fmt), ##__VA_ARGS__)
> ~~~ ^~~~~~~~~~~
> ../include/linux/dev_printk.h:110:23: note: expanded from macro 'dev_printk_index_wrap'
> _p_func(dev, fmt, ##__VA_ARGS__); \
> ~~~ ^~~~~~~~~~~
> 1 error generated.
>
> Introduced by commit f78ebca8ff3d ("iommu/msm: Add support for generic master
> bindings").
>
> diff --git a/drivers/iommu/msm_iommu.c b/drivers/iommu/msm_iommu.c
> index 428919a474c1..6a24aa804ea3 100644
> --- a/drivers/iommu/msm_iommu.c
> +++ b/drivers/iommu/msm_iommu.c
> @@ -599,7 +599,7 @@ static int insert_iommu_master(struct device *dev,
>
> for (sid = 0; sid < master->num_mids; sid++)
> if (master->mids[sid] == spec->args[0]) {
> - dev_warn(dev, "Stream ID 0x%hx repeated; ignoring\n",
> + dev_warn(dev, "Stream ID 0x%x repeated; ignoring\n",
> sid);
> return 0;
> }
>
> ========================================================================
>
> ARCH=hexagon allmodconfig + CONFIG_FRAME_WARN=0:
>
> ../drivers/misc/lkdtm/bugs.c:107:3: error: format specifies type 'unsigned long' but the argument has type 'int' [-Werror,-Wformat]
> REC_STACK_SIZE, recur_count);
> ^~~~~~~~~~~~~~
> ../include/linux/printk.h:537:34: note: expanded from macro 'pr_info'
> printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
> ~~~ ^~~~~~~~~~~
> ../include/linux/printk.h:464:60: note: expanded from macro 'printk'
> #define printk(fmt, ...) printk_index_wrap(_printk, fmt, ##__VA_ARGS__)
> ~~~ ^~~~~~~~~~~
> ../include/linux/printk.h:436:19: note: expanded from macro 'printk_index_wrap'
> _p_func(_fmt, ##__VA_ARGS__); \
> ~~~~ ^~~~~~~~~~~
> ../drivers/misc/lkdtm/bugs.c:32:24: note: expanded from macro 'REC_STACK_SIZE'
> #define REC_STACK_SIZE (THREAD_SIZE / 8)
> ^~~~~~~~~~~~~~~~~
> 1 error generated.
>
> Introduced by commit 24cccab42c41 ("lkdtm/bugs: Adjust recursion test to avoid
> elision").
>
> diff --git a/drivers/misc/lkdtm/bugs.c b/drivers/misc/lkdtm/bugs.c
> index 009239ad1d8a..6381255aaecc 100644
> --- a/drivers/misc/lkdtm/bugs.c
> +++ b/drivers/misc/lkdtm/bugs.c
> @@ -29,7 +29,7 @@ struct lkdtm_list {
> #if defined(CONFIG_FRAME_WARN) && (CONFIG_FRAME_WARN > 0)
> #define REC_STACK_SIZE (_AC(CONFIG_FRAME_WARN, UL) / 2)
> #else
> -#define REC_STACK_SIZE (THREAD_SIZE / 8)
> +#define REC_STACK_SIZE ((unsigned long)(THREAD_SIZE / 8))
> #endif
> #define REC_NUM_DEFAULT ((THREAD_SIZE / REC_STACK_SIZE) * 2)
>
>
> ========================================================================
>
> ARCH=arm allmodconfig:
>
> ../drivers/nvme/target/auth.c:492:18: error: format specifies type 'unsigned long' but the argument has type 'size_t' (aka 'unsigned int') [-Werror,-Wformat]
> ctrl->cntlid, ctrl->dh_keysize, buf_size);
> ^~~~~~~~~~~~~~~~
> ../include/linux/printk.h:517:37: note: expanded from macro 'pr_warn'
> printk(KERN_WARNING pr_fmt(fmt), ##__VA_ARGS__)
> ~~~ ^~~~~~~~~~~
> ../include/linux/printk.h:464:60: note: expanded from macro 'printk'
> #define printk(fmt, ...) printk_index_wrap(_printk, fmt, ##__VA_ARGS__)
> ~~~ ^~~~~~~~~~~
> ../include/linux/printk.h:436:19: note: expanded from macro 'printk_index_wrap'
> _p_func(_fmt, ##__VA_ARGS__); \
> ~~~~ ^~~~~~~~~~~
> 1 error generated.
>
> Introduced by commit 71ebe3842ebe ("nvmet-auth: Diffie-Hellman key exchange
> support").
>
> This one is not clang specific and already has a fix pending:
>
> https://lore.kernel.org/[email protected]/
>
> ========================================================================
>
> Pretty much every allmodconfig:
>
> ../sound/soc/sof/ipc3-topology.c:2343:4: error: format specifies type 'unsigned char' but the argument has type 'int' [-Werror,-Wformat]
> SOF_ABI_MAJOR, SOF_ABI_MINOR, SOF_ABI_PATCH);
> ^~~~~~~~~~~~~
> ../include/linux/dev_printk.h:150:67: note: expanded from macro 'dev_info'
> dev_printk_index_wrap(_dev_info, KERN_INFO, dev, dev_fmt(fmt), ##__VA_ARGS__)
> ~~~ ^~~~~~~~~~~
> ../include/linux/dev_printk.h:110:23: note: expanded from macro 'dev_printk_index_wrap'
> _p_func(dev, fmt, ##__VA_ARGS__); \
> ~~~ ^~~~~~~~~~~
> ../include/uapi/sound/sof/abi.h:30:23: note: expanded from macro 'SOF_ABI_MAJOR'
> #define SOF_ABI_MAJOR 3
> ^
> ../sound/soc/sof/ipc3-topology.c:2343:19: error: format specifies type 'unsigned char' but the argument has type 'int' [-Werror,-Wformat]
> SOF_ABI_MAJOR, SOF_ABI_MINOR, SOF_ABI_PATCH);
> ^~~~~~~~~~~~~
> ../include/linux/dev_printk.h:150:67: note: expanded from macro 'dev_info'
> dev_printk_index_wrap(_dev_info, KERN_INFO, dev, dev_fmt(fmt), ##__VA_ARGS__)
> ~~~ ^~~~~~~~~~~
> ../include/linux/dev_printk.h:110:23: note: expanded from macro 'dev_printk_index_wrap'
> _p_func(dev, fmt, ##__VA_ARGS__); \
> ~~~ ^~~~~~~~~~~
> ../include/uapi/sound/sof/abi.h:31:23: note: expanded from macro 'SOF_ABI_MINOR'
> #define SOF_ABI_MINOR 22
> ^~
> ../sound/soc/sof/ipc3-topology.c:2343:34: error: format specifies type 'unsigned char' but the argument has type 'int' [-Werror,-Wformat]
> SOF_ABI_MAJOR, SOF_ABI_MINOR, SOF_ABI_PATCH);
> ^~~~~~~~~~~~~
> ../include/linux/dev_printk.h:150:67: note: expanded from macro 'dev_info'
> dev_printk_index_wrap(_dev_info, KERN_INFO, dev, dev_fmt(fmt), ##__VA_ARGS__)
> ~~~ ^~~~~~~~~~~
> ../include/linux/dev_printk.h:110:23: note: expanded from macro 'dev_printk_index_wrap'
> _p_func(dev, fmt, ##__VA_ARGS__); \
> ~~~ ^~~~~~~~~~~
> ../include/uapi/sound/sof/abi.h:32:23: note: expanded from macro 'SOF_ABI_PATCH'
> #define SOF_ABI_PATCH 0
> ^
> 3 errors generated.
>
> Introduced by commit 323aa1f093e6 ("ASoC: SOF: Add a new IPC op for parsing
> topology manifest") for little reason it seems?
>
> diff --git a/sound/soc/sof/ipc3-topology.c b/sound/soc/sof/ipc3-topology.c
> index b2cc046b9f60..65923e7a5976 100644
> --- a/sound/soc/sof/ipc3-topology.c
> +++ b/sound/soc/sof/ipc3-topology.c
> @@ -2338,7 +2338,7 @@ static int sof_ipc3_parse_manifest(struct snd_soc_component *scomp, int index,
> }
>
> dev_info(scomp->dev,
> - "Topology: ABI %d:%d:%d Kernel ABI %hhu:%hhu:%hhu\n",
> + "Topology: ABI %d:%d:%d Kernel ABI %d:%d:%d\n",
> man->priv.data[0], man->priv.data[1], man->priv.data[2],
> SOF_ABI_MAJOR, SOF_ABI_MINOR, SOF_ABI_PATCH);
>
>
> ========================================================================
>
> I would really like to see patches in flight for these before this patch
> is accepted but it is really awesome to see how close we are :)
>
> Cheers,
> Nathan

-Justin

2022-07-21 22:14:17

by Justin Stitt

[permalink] [raw]
Subject: Re: [PATCH] Makefile.extrawarn: re-enable -Wformat for clang

On Thu, Jul 21, 2022 at 12:37 PM Nathan Chancellor <[email protected]> wrote:
>
> On Thu, Jul 21, 2022 at 08:10:27AM -0700, Nathan Chancellor wrote:
> > On Thu, Jul 21, 2022 at 07:27:34AM -0700, Nick Desaulniers wrote:
> > > On Wed, Jul 20, 2022 at 4:23 PM Justin Stitt <[email protected]> wrote:
> > > >
> > > > There's been an ongoing mission to re-enable the -Wformat warning for
> > > > Clang. A previous attempt at enabling the warning showed that there were
> > > > many instances of this warning throughout the codebase. The sheer amount
> > > > of these warnings really polluted builds and thus -Wno-format was added
> > > > to _temporarily_ toggle them off.
> > > >
> > > > After many patches the warning has largely been eradicated for x86,
> > > > x86_64, arm, and arm64 on a variety of configs. The time to enable the
> > > > warning has never been better as it seems for the first time we are
> > > > ahead of them and can now solve them as they appear rather than tackling
> > > > from a backlog.
> > > >
> > > > As to the root cause of this large backlog of warnings, Clang seems to
> > > > pickup on some more nuanced cases of format warnings caused by implicit
> > > > integer conversion as well as default argument promotions from
> > > > printf-like functions.
> > > >
> > > >
> > > > Link: https://github.com/ClangBuiltLinux/linux/issues/378
> > > > Suggested-by: Nick Desaulniers <[email protected]>
> > > > Signed-off-by: Justin Stitt <[email protected]>
> > > > ---
> > > > Previous attempt: (https://patchwork.kernel.org/project/linux-kbuild/patch/[email protected]/)
> > > >
> > > > Note:
> > > > For this patch to land on its feet, the plethora of supporting patches that
> > > > fixed various -Wformat warnings need to be picked up. Thanfully, a lot
> > > > of them have!
> > > >
> > > > Here are the patches still waiting to be picked up:
> > > > * https://lore.kernel.org/all/[email protected]/
> > > > * https://lore.kernel.org/all/[email protected]/
> > >
> > > Hi Masahiro, Nathan, and Tom,
> > > What are your thoughts for _when_ in the release cycle this should be
> > > picked up? I worry that if we don't remove this soon, we will
> > > backslide, and more -Wformat issues will crop up making removing this
> > > in the future like digging in sand. Justin has chased down many
> > > instances of this warning, and I'm happy to help clean up fallout from
> > > landing this.
> >
> > Let me do a series of builds with the two patches above against
> > next-20220721 to see if there are any instances of this warning across
> > the less frequently tested architectures then I will review/ack this.
>
> Alright, against next-20220721, I applied:
>
> * https://lore.kernel.org/[email protected]/ (applied to net-next, just not in this -next release)
> * https://lore.kernel.org/[email protected]/ (not picked up)
> * https://lore.kernel.org/[email protected]/ (not picked up)
>
> I still see the following warnings. I have suggested fixes, which I am happy to
> send unless Justin wants to.

Thanks for reporting these. I got the patches sent out!

I added bookkeeping below each warning.

>
> ========================================================================
>
> ARCH=arm allmodconfig:
>
> ../drivers/iommu/msm_iommu.c:603:6: error: format specifies type 'unsigned short' but the argument has type 'int' [-Werror,-Wformat]
> sid);
> ^~~
> ../include/linux/dev_printk.h:146:70: note: expanded from macro 'dev_warn'
> dev_printk_index_wrap(_dev_warn, KERN_WARNING, dev, dev_fmt(fmt), ##__VA_ARGS__)
> ~~~ ^~~~~~~~~~~
> ../include/linux/dev_printk.h:110:23: note: expanded from macro 'dev_printk_index_wrap'
> _p_func(dev, fmt, ##__VA_ARGS__); \
> ~~~ ^~~~~~~~~~~
> 1 error generated.
>
> Introduced by commit f78ebca8ff3d ("iommu/msm: Add support for generic master
> bindings").
>
> diff --git a/drivers/iommu/msm_iommu.c b/drivers/iommu/msm_iommu.c
> index 428919a474c1..6a24aa804ea3 100644
> --- a/drivers/iommu/msm_iommu.c
> +++ b/drivers/iommu/msm_iommu.c
> @@ -599,7 +599,7 @@ static int insert_iommu_master(struct device *dev,
>
> for (sid = 0; sid < master->num_mids; sid++)
> if (master->mids[sid] == spec->args[0]) {
> - dev_warn(dev, "Stream ID 0x%hx repeated; ignoring\n",
> + dev_warn(dev, "Stream ID 0x%x repeated; ignoring\n",
> sid);
> return 0;
> }
>

Patch: https://lore.kernel.org/all/[email protected]/

> ========================================================================
>
> ARCH=hexagon allmodconfig + CONFIG_FRAME_WARN=0:
>
> ../drivers/misc/lkdtm/bugs.c:107:3: error: format specifies type 'unsigned long' but the argument has type 'int' [-Werror,-Wformat]
> REC_STACK_SIZE, recur_count);
> ^~~~~~~~~~~~~~
> ../include/linux/printk.h:537:34: note: expanded from macro 'pr_info'
> printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
> ~~~ ^~~~~~~~~~~
> ../include/linux/printk.h:464:60: note: expanded from macro 'printk'
> #define printk(fmt, ...) printk_index_wrap(_printk, fmt, ##__VA_ARGS__)
> ~~~ ^~~~~~~~~~~
> ../include/linux/printk.h:436:19: note: expanded from macro 'printk_index_wrap'
> _p_func(_fmt, ##__VA_ARGS__); \
> ~~~~ ^~~~~~~~~~~
> ../drivers/misc/lkdtm/bugs.c:32:24: note: expanded from macro 'REC_STACK_SIZE'
> #define REC_STACK_SIZE (THREAD_SIZE / 8)
> ^~~~~~~~~~~~~~~~~
> 1 error generated.
>
> Introduced by commit 24cccab42c41 ("lkdtm/bugs: Adjust recursion test to avoid
> elision").
>
> diff --git a/drivers/misc/lkdtm/bugs.c b/drivers/misc/lkdtm/bugs.c
> index 009239ad1d8a..6381255aaecc 100644
> --- a/drivers/misc/lkdtm/bugs.c
> +++ b/drivers/misc/lkdtm/bugs.c
> @@ -29,7 +29,7 @@ struct lkdtm_list {
> #if defined(CONFIG_FRAME_WARN) && (CONFIG_FRAME_WARN > 0)
> #define REC_STACK_SIZE (_AC(CONFIG_FRAME_WARN, UL) / 2)
> #else
> -#define REC_STACK_SIZE (THREAD_SIZE / 8)
> +#define REC_STACK_SIZE ((unsigned long)(THREAD_SIZE / 8))
> #endif
> #define REC_NUM_DEFAULT ((THREAD_SIZE / REC_STACK_SIZE) * 2)
>
>

Patch: https://lore.kernel.org/all/[email protected]/

> ========================================================================
>
> ARCH=arm allmodconfig:
>
> ../drivers/nvme/target/auth.c:492:18: error: format specifies type 'unsigned long' but the argument has type 'size_t' (aka 'unsigned int') [-Werror,-Wformat]
> ctrl->cntlid, ctrl->dh_keysize, buf_size);
> ^~~~~~~~~~~~~~~~
> ../include/linux/printk.h:517:37: note: expanded from macro 'pr_warn'
> printk(KERN_WARNING pr_fmt(fmt), ##__VA_ARGS__)
> ~~~ ^~~~~~~~~~~
> ../include/linux/printk.h:464:60: note: expanded from macro 'printk'
> #define printk(fmt, ...) printk_index_wrap(_printk, fmt, ##__VA_ARGS__)
> ~~~ ^~~~~~~~~~~
> ../include/linux/printk.h:436:19: note: expanded from macro 'printk_index_wrap'
> _p_func(_fmt, ##__VA_ARGS__); \
> ~~~~ ^~~~~~~~~~~
> 1 error generated.
>
> Introduced by commit 71ebe3842ebe ("nvmet-auth: Diffie-Hellman key exchange
> support").
>
> This one is not clang specific and already has a fix pending:
>
> https://lore.kernel.org/[email protected]/
Patch: ^^^^^^^^^
>
> ========================================================================
>
> Pretty much every allmodconfig:
>
> ../sound/soc/sof/ipc3-topology.c:2343:4: error: format specifies type 'unsigned char' but the argument has type 'int' [-Werror,-Wformat]
> SOF_ABI_MAJOR, SOF_ABI_MINOR, SOF_ABI_PATCH);
> ^~~~~~~~~~~~~
> ../include/linux/dev_printk.h:150:67: note: expanded from macro 'dev_info'
> dev_printk_index_wrap(_dev_info, KERN_INFO, dev, dev_fmt(fmt), ##__VA_ARGS__)
> ~~~ ^~~~~~~~~~~
> ../include/linux/dev_printk.h:110:23: note: expanded from macro 'dev_printk_index_wrap'
> _p_func(dev, fmt, ##__VA_ARGS__); \
> ~~~ ^~~~~~~~~~~
> ../include/uapi/sound/sof/abi.h:30:23: note: expanded from macro 'SOF_ABI_MAJOR'
> #define SOF_ABI_MAJOR 3
> ^
> ../sound/soc/sof/ipc3-topology.c:2343:19: error: format specifies type 'unsigned char' but the argument has type 'int' [-Werror,-Wformat]
> SOF_ABI_MAJOR, SOF_ABI_MINOR, SOF_ABI_PATCH);
> ^~~~~~~~~~~~~
> ../include/linux/dev_printk.h:150:67: note: expanded from macro 'dev_info'
> dev_printk_index_wrap(_dev_info, KERN_INFO, dev, dev_fmt(fmt), ##__VA_ARGS__)
> ~~~ ^~~~~~~~~~~
> ../include/linux/dev_printk.h:110:23: note: expanded from macro 'dev_printk_index_wrap'
> _p_func(dev, fmt, ##__VA_ARGS__); \
> ~~~ ^~~~~~~~~~~
> ../include/uapi/sound/sof/abi.h:31:23: note: expanded from macro 'SOF_ABI_MINOR'
> #define SOF_ABI_MINOR 22
> ^~
> ../sound/soc/sof/ipc3-topology.c:2343:34: error: format specifies type 'unsigned char' but the argument has type 'int' [-Werror,-Wformat]
> SOF_ABI_MAJOR, SOF_ABI_MINOR, SOF_ABI_PATCH);
> ^~~~~~~~~~~~~
> ../include/linux/dev_printk.h:150:67: note: expanded from macro 'dev_info'
> dev_printk_index_wrap(_dev_info, KERN_INFO, dev, dev_fmt(fmt), ##__VA_ARGS__)
> ~~~ ^~~~~~~~~~~
> ../include/linux/dev_printk.h:110:23: note: expanded from macro 'dev_printk_index_wrap'
> _p_func(dev, fmt, ##__VA_ARGS__); \
> ~~~ ^~~~~~~~~~~
> ../include/uapi/sound/sof/abi.h:32:23: note: expanded from macro 'SOF_ABI_PATCH'
> #define SOF_ABI_PATCH 0
> ^
> 3 errors generated.
>
> Introduced by commit 323aa1f093e6 ("ASoC: SOF: Add a new IPC op for parsing
> topology manifest") for little reason it seems?
>
> diff --git a/sound/soc/sof/ipc3-topology.c b/sound/soc/sof/ipc3-topology.c
> index b2cc046b9f60..65923e7a5976 100644
> --- a/sound/soc/sof/ipc3-topology.c
> +++ b/sound/soc/sof/ipc3-topology.c
> @@ -2338,7 +2338,7 @@ static int sof_ipc3_parse_manifest(struct snd_soc_component *scomp, int index,
> }
>
> dev_info(scomp->dev,
> - "Topology: ABI %d:%d:%d Kernel ABI %hhu:%hhu:%hhu\n",
> + "Topology: ABI %d:%d:%d Kernel ABI %d:%d:%d\n",
> man->priv.data[0], man->priv.data[1], man->priv.data[2],
> SOF_ABI_MAJOR, SOF_ABI_MINOR, SOF_ABI_PATCH);
>
>

Patch: https://lore.kernel.org/all/[email protected]/

> ========================================================================
>
> I would really like to see patches in flight for these before this patch
> is accepted but it is really awesome to see how close we are :)
>
> Cheers,
> Nathan

-Justin

2022-07-22 04:26:50

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH] Makefile.extrawarn: re-enable -Wformat for clang

On Fri, Jul 22, 2022 at 12:10 AM Nathan Chancellor <[email protected]> wrote:
>
> On Thu, Jul 21, 2022 at 07:27:34AM -0700, Nick Desaulniers wrote:
> > On Wed, Jul 20, 2022 at 4:23 PM Justin Stitt <[email protected]> wrote:
> > >
> > > There's been an ongoing mission to re-enable the -Wformat warning for
> > > Clang. A previous attempt at enabling the warning showed that there were
> > > many instances of this warning throughout the codebase. The sheer amount
> > > of these warnings really polluted builds and thus -Wno-format was added
> > > to _temporarily_ toggle them off.
> > >
> > > After many patches the warning has largely been eradicated for x86,
> > > x86_64, arm, and arm64 on a variety of configs. The time to enable the
> > > warning has never been better as it seems for the first time we are
> > > ahead of them and can now solve them as they appear rather than tackling
> > > from a backlog.
> > >
> > > As to the root cause of this large backlog of warnings, Clang seems to
> > > pickup on some more nuanced cases of format warnings caused by implicit
> > > integer conversion as well as default argument promotions from
> > > printf-like functions.
> > >
> > >
> > > Link: https://github.com/ClangBuiltLinux/linux/issues/378
> > > Suggested-by: Nick Desaulniers <[email protected]>
> > > Signed-off-by: Justin Stitt <[email protected]>
> > > ---
> > > Previous attempt: (https://patchwork.kernel.org/project/linux-kbuild/patch/[email protected]/)
> > >
> > > Note:
> > > For this patch to land on its feet, the plethora of supporting patches that
> > > fixed various -Wformat warnings need to be picked up. Thanfully, a lot
> > > of them have!
> > >
> > > Here are the patches still waiting to be picked up:
> > > * https://lore.kernel.org/all/[email protected]/
> > > * https://lore.kernel.org/all/[email protected]/
> >
> > Hi Masahiro, Nathan, and Tom,
> > What are your thoughts for _when_ in the release cycle this should be
> > picked up? I worry that if we don't remove this soon, we will
> > backslide, and more -Wformat issues will crop up making removing this
> > in the future like digging in sand. Justin has chased down many
> > instances of this warning, and I'm happy to help clean up fallout from
> > landing this.
>
> Let me do a series of builds with the two patches above against
> next-20220721 to see if there are any instances of this warning across
> the less frequently tested architectures then I will review/ack this.
>
> I don't think we need to worry much about backslide, as -Wformat is
> enabled with W=1, which the 0day folks already test with, so new
> instances of this warning should get reported to the authors when they
> are introduced so they can be fixed immediately. However, I would still
> like to see this applied sooner rather than later, although I would also
> like us to be completely warning clean before doing so, especially with
> -Werror now being selected with all{mod,yes}config. -rc8 is this Sunday
> and final should be July 31st so I think this could be applied at some
> point between those two dates then maybe sent to Linus for a late pull
> request once all other trees have been merged but that is ultimately up
> to Masahiro.

OK, I think that will be good timing.
Please ping me if I forget to pick it up.

I still worry about my pull request being rejected.





>
> Cheers,
> Nathan
>
> > >
> > > scripts/Makefile.extrawarn | 1 -
> > > 1 file changed, 1 deletion(-)
> > >
> > > diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn
> > > index f5f0d6f09053..9bbaf7112a9b 100644
> > > --- a/scripts/Makefile.extrawarn
> > > +++ b/scripts/Makefile.extrawarn
> > > @@ -47,7 +47,6 @@ else
> > >
> > > ifdef CONFIG_CC_IS_CLANG
> > > KBUILD_CFLAGS += -Wno-initializer-overrides
> > > -KBUILD_CFLAGS += -Wno-format
> > > KBUILD_CFLAGS += -Wno-sign-compare
> > > KBUILD_CFLAGS += -Wno-format-zero-length
> > > KBUILD_CFLAGS += $(call cc-disable-warning, pointer-to-enum-cast)
> > > --
> > > 2.37.0.170.g444d1eabd0-goog
> > >
> >
> >
> > --
> > Thanks,
> > ~Nick Desaulniers



--
Best Regards
Masahiro Yamada

2022-08-01 17:50:00

by Justin Stitt

[permalink] [raw]
Subject: Re: [PATCH] Makefile.extrawarn: re-enable -Wformat for clang

> OK, I think that will be good timing.
> Please ping me if I forget to pick it up.
>
Hey Masahiro, just pinging to see the state of this PR.

I think we are on pace to re-enable this warning.

I believe there exists only _two_ patches left still needing to go
through along with this patch:
1) https://lore.kernel.org/all/[email protected]/
2) https://lore.kernel.org/all/[email protected]/
> Best Regards
> Masahiro Yamada

-Justin

2022-08-01 18:43:42

by Nathan Chancellor

[permalink] [raw]
Subject: Re: [PATCH] Makefile.extrawarn: re-enable -Wformat for clang

On Mon, Aug 01, 2022 at 10:40:29AM -0700, Justin Stitt wrote:
> > OK, I think that will be good timing.
> > Please ping me if I forget to pick it up.
> >
> Hey Masahiro, just pinging to see the state of this PR.
>
> I think we are on pace to re-enable this warning.
>
> I believe there exists only _two_ patches left still needing to go
> through along with this patch:
> 1) https://lore.kernel.org/all/[email protected]/

This is now in the block tree, so it should be squared away:

https://lore.kernel.org/[email protected]/

Stephen is on vacation so -next hasn't updated for a few days but it
sounds like Mark is going to provide some coverage:

https://lore.kernel.org/[email protected]/

> 2) https://lore.kernel.org/all/[email protected]/

We need to chase this, as I haven't seen an "applied" email. We have two
options:

1. Ask the maintainers to apply the change to their branch directly.
2. Ask them for an "Ack" so that we can apply that change along with
this one.

It is worth a ping asking which they prefer. The first option is
simpler, as the change that introduced the warning is only in -next so
it can just be applied to the same branch; the only concern is making
sure that change makes -rc1. The second option gives us more flexibility
with enabling the warning in the event that the change missed being
added to the main pull request but it will require basing the change on
a non-rc base, which most maintainers don't really like.

It is ultimately up to Masahiro but my vision is:

1. Ping the patch, asking how to proceed.
2. If the maintainers can pick it up and it will make the merge window,
let them apply it then apply this patch to the Kbuild tree for -next.
3. If they prefer the "Ack" route, wait until mainline contains the
problematic patch then apply the warning fix patch and this patch to
the Kbuild tree on top of the problematic merge.
4. Wait until all other patches are in mainline (I can watch mainline
and build it continuously) then pull request the branch containing
whatever changes we need.

Masahiro, does that sound reasonable?

Cheers,
Nathan

2022-08-02 17:32:48

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH] Makefile.extrawarn: re-enable -Wformat for clang

On Tue, Aug 2, 2022 at 3:16 AM Nathan Chancellor <[email protected]> wrote:
>
> On Mon, Aug 01, 2022 at 10:40:29AM -0700, Justin Stitt wrote:
> > > OK, I think that will be good timing.
> > > Please ping me if I forget to pick it up.
> > >
> > Hey Masahiro, just pinging to see the state of this PR.
> >
> > I think we are on pace to re-enable this warning.
> >
> > I believe there exists only _two_ patches left still needing to go
> > through along with this patch:
> > 1) https://lore.kernel.org/all/[email protected]/
>
> This is now in the block tree, so it should be squared away:
>
> https://lore.kernel.org/[email protected]/
>
> Stephen is on vacation so -next hasn't updated for a few days but it
> sounds like Mark is going to provide some coverage:
>
> https://lore.kernel.org/[email protected]/
>
> > 2) https://lore.kernel.org/all/[email protected]/
>
> We need to chase this, as I haven't seen an "applied" email. We have two
> options:
>
> 1. Ask the maintainers to apply the change to their branch directly.
> 2. Ask them for an "Ack" so that we can apply that change along with
> this one.
>
> It is worth a ping asking which they prefer. The first option is
> simpler, as the change that introduced the warning is only in -next so
> it can just be applied to the same branch; the only concern is making
> sure that change makes -rc1. The second option gives us more flexibility
> with enabling the warning in the event that the change missed being
> added to the main pull request but it will require basing the change on
> a non-rc base, which most maintainers don't really like.
>
> It is ultimately up to Masahiro but my vision is:
>
> 1. Ping the patch, asking how to proceed.
> 2. If the maintainers can pick it up and it will make the merge window,
> let them apply it then apply this patch to the Kbuild tree for -next.
> 3. If they prefer the "Ack" route, wait until mainline contains the
> problematic patch then apply the warning fix patch and this patch to
> the Kbuild tree on top of the problematic merge.
> 4. Wait until all other patches are in mainline (I can watch mainline
> and build it continuously) then pull request the branch containing
> whatever changes we need.
>
> Masahiro, does that sound reasonable?
>
> Cheers,
> Nathan


Now applied to linux-kbuild.


If my pull request is rejected because of some warnings,
I may end up with dropping this, though.


--
Best Regards
Masahiro Yamada