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

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

[Justin Stitt]

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 <ndesaulniers@google.com>
Signed-off-by: Justin Stitt <justinstitt@google.com>
---
Previous attempt: (https://patchwork.kernel.org/project/linux-kbuild/patch/20190201210853.244043-1-jflat@chromium.org/)

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/20220718230626.1029318-1-justinstitt@google.com/
* https://lore.kernel.org/all/20220711222919.2043613-1-justinstitt@google.com/

 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

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

[Nick Desaulniers]

On Wed, Jul 20, 2022 at 4:23 PM Justin Stitt <justinstitt@google.com> 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 <ndesaulniers@google.com>
> Signed-off-by: Justin Stitt <justinstitt@google.com>
> ---
> Previous attempt: (https://patchwork.kernel.org/project/linux-kbuild/patch/20190201210853.244043-1-jflat@chromium.org/)
>
> 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/20220718230626.1029318-1-justinstitt@google.com/
> * https://lore.kernel.org/all/20220711222919.2043613-1-justinstitt@google.com/

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

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

[Nick Desaulniers]

On Thu, Jul 21, 2022 at 7:27 AM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> On Wed, Jul 20, 2022 at 4:23 PM Justin Stitt <justinstitt@google.com> 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 <ndesaulniers@google.com>
> > Signed-off-by: Justin Stitt <justinstitt@google.com>
> > ---
> > Previous attempt: (https://patchwork.kernel.org/project/linux-kbuild/patch/20190201210853.244043-1-jflat@chromium.org/)
> >
> > 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/20220718230626.1029318-1-justinstitt@google.com/
> > * https://lore.kernel.org/all/20220711222919.2043613-1-justinstitt@google.com/
>
> 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/20220716084532.2324050-1-youngmin.nam@samsung.com/

(This thread, for context for Youngmin:
https://lore.kernel.org/llvm/20220720232332.2720091-1-justinstitt@google.com/)

>
> >
> >  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

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

[Nathan Chancellor]

On Thu, Jul 21, 2022 at 07:27:34AM -0700, Nick Desaulniers wrote:
> On Wed, Jul 20, 2022 at 4:23 PM Justin Stitt <justinstitt@google.com> 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 <ndesaulniers@google.com>
> > Signed-off-by: Justin Stitt <justinstitt@google.com>
> > ---
> > Previous attempt: (https://patchwork.kernel.org/project/linux-kbuild/patch/20190201210853.244043-1-jflat@chromium.org/)
> >
> > 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/20220718230626.1029318-1-justinstitt@google.com/
> > * https://lore.kernel.org/all/20220711222919.2043613-1-justinstitt@google.com/
> 
> 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

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

[Nathan Chancellor]

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 <justinstitt@google.com> 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 <ndesaulniers@google.com>
> > > Signed-off-by: Justin Stitt <justinstitt@google.com>
> > > ---
> > > Previous attempt: (https://patchwork.kernel.org/project/linux-kbuild/patch/20190201210853.244043-1-jflat@chromium.org/)
> > >
> > > 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/20220718230626.1029318-1-justinstitt@google.com/
> > > * https://lore.kernel.org/all/20220711222919.2043613-1-justinstitt@google.com/
> > 
> > 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/20220712204900.660569-1-justinstitt@google.com/ (applied to net-next, just not in this -next release)
* https://lore.kernel.org/20220718230626.1029318-1-justinstitt@google.com/ (not picked up)
* https://lore.kernel.org/20220711222919.2043613-1-justinstitt@google.com/ (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/20220718050356.227647-1-hch@lst.de/

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

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

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

[Justin Stitt]

On Thu, Jul 21, 2022 at 12:37 PM Nathan Chancellor <nathan@kernel.org> 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 <justinstitt@google.com> 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 <ndesaulniers@google.com>
> > > > Signed-off-by: Justin Stitt <justinstitt@google.com>
> > > > ---
> > > > Previous attempt: (https://patchwork.kernel.org/project/linux-kbuild/patch/20190201210853.244043-1-jflat@chromium.org/)
> > > >
> > > > 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/20220718230626.1029318-1-justinstitt@google.com/
> > > > * https://lore.kernel.org/all/20220711222919.2043613-1-justinstitt@google.com/
> > >
> > > 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/20220712204900.660569-1-justinstitt@google.com/ (applied to net-next, just not in this -next release)
> * https://lore.kernel.org/20220718230626.1029318-1-justinstitt@google.com/ (not picked up)
> * https://lore.kernel.org/20220711222919.2043613-1-justinstitt@google.com/ (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/20220718050356.227647-1-hch@lst.de/
>
> ========================================================================
>
> 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

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

[Justin Stitt]

On Thu, Jul 21, 2022 at 12:37 PM Nathan Chancellor <nathan@kernel.org> 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 <justinstitt@google.com> 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 <ndesaulniers@google.com>
> > > > Signed-off-by: Justin Stitt <justinstitt@google.com>
> > > > ---
> > > > Previous attempt: (https://patchwork.kernel.org/project/linux-kbuild/patch/20190201210853.244043-1-jflat@chromium.org/)
> > > >
> > > > 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/20220718230626.1029318-1-justinstitt@google.com/
> > > > * https://lore.kernel.org/all/20220711222919.2043613-1-justinstitt@google.com/
> > >
> > > 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/20220712204900.660569-1-justinstitt@google.com/ (applied to net-next, just not in this -next release)
> * https://lore.kernel.org/20220718230626.1029318-1-justinstitt@google.com/ (not picked up)
> * https://lore.kernel.org/20220711222919.2043613-1-justinstitt@google.com/ (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/20220721210331.4012015-1-justinstitt@google.com/

> ========================================================================
>
> 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/20220721212012.4060328-1-justinstitt@google.com/

> ========================================================================
>
> 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/20220718050356.227647-1-hch@lst.de/
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/20220721211218.4039288-1-justinstitt@google.com/

> ========================================================================
>
> 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

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

[Masahiro Yamada]

On Fri, Jul 22, 2022 at 12:10 AM Nathan Chancellor <nathan@kernel.org> 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 <justinstitt@google.com> 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 <ndesaulniers@google.com>
> > > Signed-off-by: Justin Stitt <justinstitt@google.com>
> > > ---
> > > Previous attempt: (https://patchwork.kernel.org/project/linux-kbuild/patch/20190201210853.244043-1-jflat@chromium.org/)
> > >
> > > 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/20220718230626.1029318-1-justinstitt@google.com/
> > > * https://lore.kernel.org/all/20220711222919.2043613-1-justinstitt@google.com/
> >
> > 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

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

[Justin Stitt]

> 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/20220718050356.227647-1-hch@lst.de/
2) https://lore.kernel.org/all/YtnDltqEVeJQQkbW@dev-arch.thelio-3990X/
> Best Regards
> Masahiro Yamada

-Justin

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

[Nathan Chancellor]

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/20220718050356.227647-1-hch@lst.de/

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

https://lore.kernel.org/YuFhR9OoPvM9VsdT@infradead.org/

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/YugAzWWl++ArhhPS@sirena.org.uk/

> 2) https://lore.kernel.org/all/YtnDltqEVeJQQkbW@dev-arch.thelio-3990X/

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

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

[Masahiro Yamada]

On Tue, Aug 2, 2022 at 3:16 AM Nathan Chancellor <nathan@kernel.org> 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/20220718050356.227647-1-hch@lst.de/
>
> This is now in the block tree, so it should be squared away:
>
> https://lore.kernel.org/YuFhR9OoPvM9VsdT@infradead.org/
>
> 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/YugAzWWl++ArhhPS@sirena.org.uk/
>
> > 2) https://lore.kernel.org/all/YtnDltqEVeJQQkbW@dev-arch.thelio-3990X/
>
> 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