diff mbox series

Makefile.extrawarn: enable -Wmissing-variable-declarations for W=1

Message ID 20230807-missing_proto-v1-1-7f566b7ba5ca@google.com (mailing list archive)
State New, archived
Headers show
Series Makefile.extrawarn: enable -Wmissing-variable-declarations for W=1 | expand

Commit Message

Nick Desaulniers Aug. 7, 2023, 4:50 p.m. UTC 
I noticed Tom was sending patches where smatch was recommending
annotating functions as static when no previous declaration existed.
Surely the compiler could make such recommendations as well, I thought.

Looks like -Wmissing-variable-declarations can make such
recommendations.

GCC just added support for this warning (gcc 14.1.0 will ship with
support), and all versions of clang relevant to building the kernel
support this flag.

Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
 scripts/Makefile.extrawarn | 1 +
 1 file changed, 1 insertion(+)


---
base-commit: 52a93d39b17dc7eb98b6aa3edb93943248e03b2f
change-id: 20230807-missing_proto-0cb90ec6454c

Best regards,

Comments

Nick Desaulniers Aug. 7, 2023, 4:54 p.m. UTC | #1 
On Mon, Aug 7, 2023 at 9:50 AM Nick Desaulniers <ndesaulniers@google.com> wrote:
>
> I noticed Tom was sending patches where smatch was recommending
> annotating functions as static when no previous declaration existed.
> Surely the compiler could make such recommendations as well, I thought.
>
> Looks like -Wmissing-variable-declarations can make such
> recommendations.
>
> GCC just added support for this warning (gcc 14.1.0 will ship with
> support), and all versions of clang relevant to building the kernel
> support this flag.
>
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> ---
>  scripts/Makefile.extrawarn | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn
> index 40cd13eca82e..617739eb84e2 100644
> --- a/scripts/Makefile.extrawarn
> +++ b/scripts/Makefile.extrawarn
> @@ -32,6 +32,7 @@ KBUILD_CFLAGS += $(call cc-option, -Wunused-but-set-variable)
>  KBUILD_CFLAGS += $(call cc-option, -Wunused-const-variable)
>  KBUILD_CFLAGS += $(call cc-option, -Wpacked-not-aligned)
>  KBUILD_CFLAGS += $(call cc-option, -Wstringop-truncation)
> +KBUILD_CFLAGS += $(call cc-option, -Wmissing-varibale-declarations)

^ sigh, misspelled

>  # The following turn off the warnings enabled by -Wextra
>  KBUILD_CFLAGS += -Wno-missing-field-initializers
>  KBUILD_CFLAGS += -Wno-sign-compare
>
> ---
> base-commit: 52a93d39b17dc7eb98b6aa3edb93943248e03b2f
> change-id: 20230807-missing_proto-0cb90ec6454c
>
> Best regards,
> --
> Nick Desaulniers <ndesaulniers@google.com>
>
Nathan Chancellor Aug. 7, 2023, 4:55 p.m. UTC | #2 
On Mon, Aug 07, 2023 at 09:50:32AM -0700, Nick Desaulniers wrote:
> I noticed Tom was sending patches where smatch was recommending
> annotating functions as static when no previous declaration existed.
> Surely the compiler could make such recommendations as well, I thought.
> 
> Looks like -Wmissing-variable-declarations can make such
> recommendations.
> 
> GCC just added support for this warning (gcc 14.1.0 will ship with
> support), and all versions of clang relevant to building the kernel
> support this flag.
> 
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>

Any idea how many instances of this there are? At least x86_64 defconfig
fails immediately with:

  In file included from scripts/mod/devicetable-offsets.c:3:
  In file included from include/linux/mod_devicetable.h:14:
  In file included from include/linux/uuid.h:11:
  In file included from include/linux/string.h:20:
  In file included from arch/x86/include/asm/string.h:5:
  In file included from arch/x86/include/asm/string_64.h:6:
  In file included from include/linux/jump_label.h:112:
  In file included from arch/x86/include/asm/jump_label.h:7:
  arch/x86/include/asm/asm.h:208:24: error: no previous extern declaration for non-static variable 'current_stack_pointer' [-Werror,-Wmissing-variable-declarations]
    208 | register unsigned long current_stack_pointer asm(_ASM_SP);
        |                        ^
  arch/x86/include/asm/asm.h:208:10: note: declare 'static' if the variable is not intended to be used outside of this translation unit
    208 | register unsigned long current_stack_pointer asm(_ASM_SP);
        |          ^
  1 error generated.

> ---
>  scripts/Makefile.extrawarn | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn
> index 40cd13eca82e..617739eb84e2 100644
> --- a/scripts/Makefile.extrawarn
> +++ b/scripts/Makefile.extrawarn
> @@ -32,6 +32,7 @@ KBUILD_CFLAGS += $(call cc-option, -Wunused-but-set-variable)
>  KBUILD_CFLAGS += $(call cc-option, -Wunused-const-variable)
>  KBUILD_CFLAGS += $(call cc-option, -Wpacked-not-aligned)
>  KBUILD_CFLAGS += $(call cc-option, -Wstringop-truncation)
> +KBUILD_CFLAGS += $(call cc-option, -Wmissing-varibale-declarations)
                                                ^ variable
>  # The following turn off the warnings enabled by -Wextra
>  KBUILD_CFLAGS += -Wno-missing-field-initializers
>  KBUILD_CFLAGS += -Wno-sign-compare
> 
> ---
> base-commit: 52a93d39b17dc7eb98b6aa3edb93943248e03b2f
> change-id: 20230807-missing_proto-0cb90ec6454c
> 
> Best regards,
> -- 
> Nick Desaulniers <ndesaulniers@google.com>
>
Nick Desaulniers Aug. 7, 2023, 5:02 p.m. UTC | #3 
On Mon, Aug 7, 2023 at 9:55 AM Nathan Chancellor <nathan@kernel.org> wrote:
>
> On Mon, Aug 07, 2023 at 09:50:32AM -0700, Nick Desaulniers wrote:
> > I noticed Tom was sending patches where smatch was recommending
> > annotating functions as static when no previous declaration existed.
> > Surely the compiler could make such recommendations as well, I thought.
> >
> > Looks like -Wmissing-variable-declarations can make such
> > recommendations.
> >
> > GCC just added support for this warning (gcc 14.1.0 will ship with
> > support), and all versions of clang relevant to building the kernel
> > support this flag.
> >
> > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
>
> Any idea how many instances of this there are? At least x86_64 defconfig
> fails immediately with:

$ make LLVM=1 -j128 W=1 allnoconfig 2>log.txt
$ grep -r Wmissing-variable-declarations log.txt | sort -u | wc -l
140
Though that's not perfectly filtered due to overlapping output due to -j 128.

>
>   In file included from scripts/mod/devicetable-offsets.c:3:
>   In file included from include/linux/mod_devicetable.h:14:
>   In file included from include/linux/uuid.h:11:
>   In file included from include/linux/string.h:20:
>   In file included from arch/x86/include/asm/string.h:5:
>   In file included from arch/x86/include/asm/string_64.h:6:
>   In file included from include/linux/jump_label.h:112:
>   In file included from arch/x86/include/asm/jump_label.h:7:
>   arch/x86/include/asm/asm.h:208:24: error: no previous extern declaration for non-static variable 'current_stack_pointer' [-Werror,-Wmissing-variable-declarations]
>     208 | register unsigned long current_stack_pointer asm(_ASM_SP);
>         |                        ^
>   arch/x86/include/asm/asm.h:208:10: note: declare 'static' if the variable is not intended to be used outside of this translation unit
>     208 | register unsigned long current_stack_pointer asm(_ASM_SP);
>         |          ^

Hmm...I wonder if clang and gcc should ignore variables with register
storage for this warning.

>   1 error generated.
>
> > ---
> >  scripts/Makefile.extrawarn | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn
> > index 40cd13eca82e..617739eb84e2 100644
> > --- a/scripts/Makefile.extrawarn
> > +++ b/scripts/Makefile.extrawarn
> > @@ -32,6 +32,7 @@ KBUILD_CFLAGS += $(call cc-option, -Wunused-but-set-variable)
> >  KBUILD_CFLAGS += $(call cc-option, -Wunused-const-variable)
> >  KBUILD_CFLAGS += $(call cc-option, -Wpacked-not-aligned)
> >  KBUILD_CFLAGS += $(call cc-option, -Wstringop-truncation)
> > +KBUILD_CFLAGS += $(call cc-option, -Wmissing-varibale-declarations)
>                                                 ^ variable
> >  # The following turn off the warnings enabled by -Wextra
> >  KBUILD_CFLAGS += -Wno-missing-field-initializers
> >  KBUILD_CFLAGS += -Wno-sign-compare
> >
> > ---
> > base-commit: 52a93d39b17dc7eb98b6aa3edb93943248e03b2f
> > change-id: 20230807-missing_proto-0cb90ec6454c
> >
> > Best regards,
> > --
> > Nick Desaulniers <ndesaulniers@google.com>
> >
diff mbox series

Patch

diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn
index 40cd13eca82e..617739eb84e2 100644
--- a/scripts/Makefile.extrawarn
+++ b/scripts/Makefile.extrawarn
@@ -32,6 +32,7 @@  KBUILD_CFLAGS += $(call cc-option, -Wunused-but-set-variable)
 KBUILD_CFLAGS += $(call cc-option, -Wunused-const-variable)
 KBUILD_CFLAGS += $(call cc-option, -Wpacked-not-aligned)
 KBUILD_CFLAGS += $(call cc-option, -Wstringop-truncation)
+KBUILD_CFLAGS += $(call cc-option, -Wmissing-varibale-declarations)
 # The following turn off the warnings enabled by -Wextra
 KBUILD_CFLAGS += -Wno-missing-field-initializers
 KBUILD_CFLAGS += -Wno-sign-compare