Re: [PATCH -next] Makefile: add implicit enum-conversion check for compile build

Nick Desaulniers
2022-10-11T21:10:43Z

On Mon, Oct 10, 2022 at 8:24 PM Zeng Heng <zengheng4@huawei.com> wrote:
>
> Enable implicit enum-conversion warning option in kernel gcc build.
> When it set enabled, it can detect implicit enum type conversion
> and locate conversion errors like below (use "allmodconfig"):
>
> drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn20/display_mode_vba_20.c:3904:46:
> error: implicit conversion from ‘enum <anonymous>’ to ‘enum odm_combine_mode’
> [-Werror=enum-conversion]
> 3904 | locals->ODMCombineEnablePerState[i][k] = true;
> | ^
>
> The '-Wenum-conversion' could be regarded as effective check in
> compile runtime and call attention on potential mistakes, which is
> firstly introduced from GNU gcc-10.
>
> Although "-Wenum-conversion" could be enabled by "-Wextra"
> when compiling with 'W=[123]' option, there are many warnings
> generated by '-Wextra' that cause too much noise in the build.
>
> Seeing the more details from the following link:
> https://gcc.gnu.org/onlinedocs/gcc-11.3.0/gcc/Warning-Options.html
>
> Therefore, "-Wenum-conversion" warning option needs to be
> explicitly requested for GCC when compilation process is only
> companied with '-Wall'.
>
> With clang, -Wenum-conversion is just default enabled, not even
> behind -Wall.

Probably didn't need the examples below since you already provide an
example above, too, but it doesn't matter. Thanks for the patch.

Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

>
> Provide a couple examples for reference as below:
>
> $ cat test.c
> enum enum1 { A = 1 };
> enum enum2 { B = 2 };
>
> enum enum1 foo(enum enum2 bar)
> {
> return bar;
> }
>
> $ gcc -Wall -fsyntax-only test.c
>
> $ gcc -Wall -Wenum-conversion -fsyntax-only test.c
> test.c: In function ‘foo’:
> test.c:6:9: warning: implicit conversion from ‘enum enum2’ to ‘enum enum1’ [-Wenum-conversion]
> 6 | return bar;
> | ^~~
>
> $ gcc -Wextra -fsyntax-only test.c
> test.c: In function ‘foo’:
> test.c:6:9: warning: implicit conversion from ‘enum enum2’ to ‘enum enum1’ [-Wenum-conversion]
> 6 | return bar;
> | ^~~
>
> $ clang -fsyntax-only test.c
> test.c:6:9: warning: implicit conversion from enumeration type 'enum enum2' to different enumeration type 'enum enum1' [-Wenum-conversion]
> return bar;
> ~~~~~~ ^~~
> 1 warning generated.
>
> Signed-off-by: Zeng Heng <zengheng4@huawei.com>
> Suggested-by: Nick Desaulniers <ndesaulniers@google.com>
> Suggested-by: Nathan Chancellor <nathan@kernel.org>
> ---
> Makefile | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/Makefile b/Makefile
> index 8f6ed52fa08f..72103d22df23 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -880,6 +880,10 @@ endif
> KBUILD_CFLAGS += $(call cc-disable-warning, unused-but-set-variable)
> KBUILD_CFLAGS += $(call cc-disable-warning, unused-const-variable)
>
> +# implicit enum conversion checking is supported since from gcc-10
> +# this warning option has to be explicitly requested for GCC
> +KBUILD_CFLAGS += $(call cc-option, -Wenum-conversion)
> +
> # These result in bogus false positives
> KBUILD_CFLAGS += $(call cc-disable-warning, dangling-pointer)
>
> --
> 2.25.1
>

--
Thanks,
~Nick Desaulniers