Makefile.extrawarn: re-enable -Wformat for clang
Subsystems (9)
PATCH 1/1 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.
---
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(-)
scripts/Makefile.extrawarn +0 -1
@@ -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)
Signature · 1 line
2.37.0.170.g444d1eabd0-goog
Quoted 25 lines · 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.
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]/)
Quoted 8 lines
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.
Quoted 19 lines
scripts/Makefile.extrawarn | 1 - 1 file changed, 1 deletion(-)scripts/Makefile.extrawarn +0
-2@@ -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
Signature · 2 lines
Thanks, ~Nick Desaulniers
Quoted 36 lines · Nick Desaulniers
Quoted 25 lines · 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.
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]/)Quoted 8 lines
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]/
Quoted 7 lines
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]/)
Quoted 25 lines
Quoted 18 lines
scripts/Makefile.extrawarn | 1 - 1 file changed, 1 deletion(-)scripts/Makefile.extrawarn +0
-2@@ -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) --Quoted 2 lines
2.37.0.170.g444d1eabd0-goog
--
Thanks,
~Nick Desaulniers
Signature · 2 lines
Thanks, ~Nick Desaulniers
Nathan Chancellor
maintainerQuoted 35 lines · Nick Desaulniers
Quoted 25 lines · 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.
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]/)Quoted 8 lines
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]/
Quoted 7 lines
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
Quoted 24 lines
Quoted 19 lines
scripts/Makefile.extrawarn | 1 - 1 file changed, 1 deletion(-)scripts/Makefile.extrawarn +0
-2@@ -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
Signature · 2 lines
Thanks, ~Nick Desaulniers
Nathan Chancellor
maintainerQuoted 44 lines · Nathan Chancellor
Quoted 35 lines · Nick Desaulniers
Quoted 25 lines · 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.
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]/)Quoted 8 lines
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]/Quoted 7 lines
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.
Quoted 3 lines
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").
drivers/iommu/msm_iommu.c +1 -1
@@ -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").
drivers/misc/lkdtm/bugs.c +1 -1
@@ -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?
sound/soc/sof/ipc3-topology.c +1 -1
@@ -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
Quoted 50 lines · Nathan Chancellor
Quoted 44 lines · Nathan Chancellor
Quoted 35 lines · Nick Desaulniers
Quoted 25 lines · 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.
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]/)Quoted 8 lines
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]/Quoted 7 lines
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.Quoted 3 lines
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.
Quoted 8 lines
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.
Quoted 155 lines
========================================================================
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").drivers/iommu/msm_iommu.c +1
-1@@ -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").drivers/misc/lkdtm/bugs.c +1
-1@@ -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?sound/soc/sof/ipc3-topology.c +1
-1@@ -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 );
Quoted 7 lines
========================================================================
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
Quoted 50 lines · Nathan Chancellor
Quoted 44 lines · Nathan Chancellor
Quoted 35 lines · Nick Desaulniers
Quoted 25 lines · 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.
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]/)Quoted 8 lines
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]/Quoted 7 lines
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.Quoted 3 lines
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.
Quoted 8 lines
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.
Quoted 25 lines
========================================================================
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").drivers/iommu/msm_iommu.c +0
-0
drivers/iommu/msm_iommu.c static int insert_iommu_master(struct device *dev, · line 605
Quoted 8 lines
drivers/iommu/msm_iommu.c +1
-1@@ -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 ; }
Quoted 30 lines
========================================================================
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").drivers/misc/lkdtm/bugs.c +0
-0
drivers/misc/lkdtm/bugs.c struct lkdtm_list { · line 35
Quoted 8 lines
drivers/misc/lkdtm/bugs.c +1
-1@@ -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)
Quoted 16 lines
========================================================================
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__); \ ~~~~ ^~~~~~~~~~~
Quoted 8 lines
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:
Patch: ^^^^^^^^^
Quoted 52 lines
========================================================================
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?sound/soc/sof/ipc3-topology.c +0
-0
sound/soc/sof/ipc3-topology.c static int sof_ipc3_parse_manifest(struct snd_soc_component *scomp, int index, · line 2344
Quoted 8 lines
sound/soc/sof/ipc3-topology.c +1
-1@@ -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 );
Quoted 7 lines
========================================================================
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
Quoted 49 lines · Nathan Chancellor
Quoted 35 lines · Nick Desaulniers
Quoted 25 lines · 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.
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]/)Quoted 8 lines
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]/Quoted 7 lines
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.
Quoted 11 lines
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.
Quoted 28 lines
Cheers,
NathanQuoted 24 lines
Quoted 17 lines
scripts/Makefile.extrawarn | 1 - 1 file changed, 1 deletion(-)scripts/Makefile.extrawarn +0
-2@@ -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) --Quoted 2 lines
2.37.0.170.g444d1eabd0-goog
--
Thanks,
~Nick Desaulniers
Signature · 2 lines
Best Regards Masahiro Yamada
Quoted 3 lines
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]/
Quoted 2 lines
Best Regards
Masahiro Yamada
-Justin
Nathan Chancellor
maintainerQuoted 10 lines · Justin Stitt
Quoted 3 lines
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]/
Quoted 1 line
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
Quoted 51 lines · Nathan Chancellor
Quoted 10 lines · Justin Stitt
Quoted 3 lines
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]/
Quoted 1 line
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 theKbuildtree 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 theKbuildtree 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.
Quoted 4 lines
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.
Signature · 2 lines
Best Regards Masahiro Yamada