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

[Nick Desaulniers <ndesaulniers@google.com>]

On Tue, Aug 27, 2019 at 8:49 AM Qian Cai <cai@lca.pw> 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@lca.pw>
> ---
>  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)
> +

-- 
Thanks,
~Nick Desaulniers