Re: [PATCH] mm: silence -Woverride-init/initializer-overrides

From: Nick Desaulniers
Date: Tue Aug 27 2019 - 19:29:57 EST


On Tue, Aug 27, 2019 at 4:25 PM Nick Desaulniers
<ndesaulniers@xxxxxxxxxx> wrote:
>
> On Tue, Aug 27, 2019 at 8:49 AM Qian Cai <cai@xxxxxx> wrote:
> >
> > When compiling a kernel with W=1, there are several of those warnings
> > due to arm64 override a field by purpose. Just disable those warnings
> > for both GCC and Clang of this file, so it will help dig "gems" hidden
> > in the W=1 warnings by reducing some noises.
> >
> > mm/init-mm.c:39:2: warning: initializer overrides prior initialization
> > of this subobject [-Winitializer-overrides]
> > INIT_MM_CONTEXT(init_mm)
> > ^~~~~~~~~~~~~~~~~~~~~~~~
> > ./arch/arm64/include/asm/mmu.h:133:9: note: expanded from macro
> > 'INIT_MM_CONTEXT'
> > .pgd = init_pg_dir,
> > ^~~~~~~~~~~
> > mm/init-mm.c:30:10: note: previous initialization is here
> > .pgd = swapper_pg_dir,
> > ^~~~~~~~~~~~~~
> >
> > Note: there is a side project trying to support explicitly allowing
> > specific initializer overrides in Clang, but there is no guarantee it
> > will happen or not.
> >
> > https://github.com/ClangBuiltLinux/linux/issues/639
> >
> > Signed-off-by: Qian Cai <cai@xxxxxx>
> > ---
> > mm/Makefile | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/mm/Makefile b/mm/Makefile
> > index d0b295c3b764..5a30b8ecdc55 100644
> > --- a/mm/Makefile
> > +++ b/mm/Makefile
>
> Hi Qian, thanks for the patch.
> Rather than disable the warning outright, and bury the disabling in a
> directory specific Makefile, why not move it to W=2 in
> scripts/Makefile.extrawarn?
>
>
> I think even better would be to use pragma's to disable the warning in
> mm/init.c. Looks like __diag support was never ported for clang yet
> from include/linux/compiler-gcc.h to include/linux/compiler-clang.h.
>
> Then you could do:
>
> 28 struct mm_struct init_mm = {
> 29 .mm_rb = RB_ROOT,
> 30 .pgd = swapper_pg_dir,
> 31 .mm_users = ATOMIC_INIT(2),
> 32 .mm_count = ATOMIC_INIT(1),
> 33 .mmap_sem = __RWSEM_INITIALIZER(init_mm.mmap_sem),
> 34 .page_table_lock =
> __SPIN_LOCK_UNLOCKED(init_mm.page_table_lock),
> 35 .arg_lock = __SPIN_LOCK_UNLOCKED(init_mm.arg_lock),
> 36 .mmlist = LIST_HEAD_INIT(init_mm.mmlist),
> 37 .user_ns = &init_user_ns,
> 38 .cpu_bitmap = { [BITS_TO_LONGS(NR_CPUS)] = 0},
> __diag_push();
> __diag_ignore(CLANG, 4, "-Winitializer-overrides")
> 39 INIT_MM_CONTEXT(init_mm)
> __diag_pop();
> 40 };
>
>
> I mean, the arm64 case is not a bug, but I worry about turning this
> warning off. I'd expect it to only warn once during an arm64 build,
> so does the warning really detract from "W=1 gem finding?"
>
> > @@ -21,6 +21,9 @@ KCOV_INSTRUMENT_memcontrol.o := n
> > KCOV_INSTRUMENT_mmzone.o := n
> > KCOV_INSTRUMENT_vmstat.o := n
> >
> > +CFLAGS_init-mm.o += $(call cc-disable-warning, override-init)
>
> -Woverride-init isn't mentioned in the commit message, so not sure if
> it's meant to ride along?
>
> > +CFLAGS_init-mm.o += $(call cc-disable-warning, initializer-overrides)

That said, it's not too bad to disable it for one object file that
contains a single struct definition.

--
Thanks,
~Nick Desaulniers