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

Masahiro Yamada
2022-10-11T21:36:35Z

On Wed, Oct 12, 2022 at 6:10 AM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> 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

Applied to linux-kbuild. Thanks.

--
Best Regards
Masahiro Yamada