-
Notifications
You must be signed in to change notification settings - Fork 14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
enable -Wmissing-variable-declarations for W=1 #1920
Comments
A good amount of these are caused by variables with register storage. llvm/llvm-project#64509 should solve some of these warnings with Clang, right? |
Can someone help me understand what to do in order to fix some of these warnings? Here's a case I found from an x86/defconfig build: arch/x86/ia32/audit.c:6:10: warning: no previous extern declaration for non-static variable 'ia32_dir_class' [-Wmissing-variable-declarations]
6 | unsigned ia32_dir_class[] = { so it says "no previous extern declaration" so it seems the fix would be to add arch/x86/ia32/audit.c:6:17: warning: unused variable 'ia32_dir_class' [-Wunused-variable]
6 | static unsigned ia32_dir_class[] = {
| ^~~~~~~~~~~~~~ Well now we get an unused variable warning and it seems there is an extern:
So what am I to make of this? Is the warning wrong? Is there a different solution that I should be going for here? I wonder if it's caused by the differing types edit: Ah, it looks like |
It seems like in that case, the declarations in |
@nathanchance should they be hiding: #ifdef CONFIG_IA32_EMULATION
extern __u32 ia32_dir_class[];
extern __u32 ia32_write_class[];
extern __u32 ia32_read_class[];
extern __u32 ia32_chattr_class[];
extern __u32 ia32_signal_class[];
audit_register_class(AUDIT_CLASS_WRITE_32, ia32_write_class);
audit_register_class(AUDIT_CLASS_READ_32, ia32_read_class);
audit_register_class(AUDIT_CLASS_DIR_WRITE_32, ia32_dir_class);
audit_register_class(AUDIT_CLASS_CHATTR_32, ia32_chattr_class);
audit_register_class(AUDIT_CLASS_SIGNAL_32, ia32_signal_class);
#endif behind this config or have this in a header like you said (without the ifdef) |
I don't think the declarations need to be put under an |
Yeah, that's what I'm wodering. Why are they in the ifdef at all. I'll test moving them out. |
…lass When building x86 defconfig with Clang-18 I get the following warnings: | arch/x86/ia32/audit.c:6:10: warning: no previous extern declaration for non-static variable 'ia32_dir_class' [-Wmissing-variable-declarations] | 6 | unsigned ia32_dir_class[] = { | arch/x86/ia32/audit.c:11:10: warning: no previous extern declaration for non-static variable 'ia32_chattr_class' [-Wmissing-variable-declarations] | 11 | unsigned ia32_chattr_class[] = { | arch/x86/ia32/audit.c:16:10: warning: no previous extern declaration for non-static variable 'ia32_write_class' [-Wmissing-variable-declarations] | 16 | unsigned ia32_write_class[] = { | arch/x86/ia32/audit.c:21:10: warning: no previous extern declaration for non-static variable 'ia32_read_class' [-Wmissing-variable-declarations] | 21 | unsigned ia32_read_class[] = { | arch/x86/ia32/audit.c:26:10: warning: no previous extern declaration for non-static variable 'ia32_signal_class' [-Wmissing-variable-declarations] | 26 | unsigned ia32_signal_class[] = { These warnings occur due to their respective extern declarations being scoped inside of audit_classes_init as well as only being enabled with `CONFIG_IA32_EMULATION=y`: | static int __init audit_classes_init(void) | { | #ifdef CONFIG_IA32_EMULATION | extern __u32 ia32_dir_class[]; | extern __u32 ia32_write_class[]; | extern __u32 ia32_read_class[]; | extern __u32 ia32_chattr_class[]; | audit_register_class(AUDIT_CLASS_WRITE_32, ia32_write_class); | audit_register_class(AUDIT_CLASS_READ_32, ia32_read_class); | audit_register_class(AUDIT_CLASS_DIR_WRITE_32, ia32_dir_class); | audit_register_class(AUDIT_CLASS_CHATTR_32, ia32_chattr_class); | #endif | audit_register_class(AUDIT_CLASS_WRITE, write_class); | audit_register_class(AUDIT_CLASS_READ, read_class); | audit_register_class(AUDIT_CLASS_DIR_WRITE, dir_class); | audit_register_class(AUDIT_CLASS_CHATTR, chattr_class); | return 0; | } Lift the extern declarations to their own header and resolve scoping issues (and thus fix the warnings). Moreover, change __u32 to unsigned so that we match the definitions: | unsigned ia32_dir_class[] = { | #include <asm-generic/audit_dir_write.h> | ~0U | }; | | unsigned ia32_chattr_class[] = { | #include <asm-generic/audit_change_attr.h> | ~0U | }; | ... This patch is similar to commit: 0e5e3d4 ("x86/audit: Fix a -Wmissing-prototypes warning for ia32_classify_syscall()") [1] Link: https://lore.kernel.org/all/20200516123816.2680-1-b.thiel@posteo.de/ [1] Link: ClangBuiltLinux#1920 Signed-off-by: Justin Stitt <justinstitt@google.com>
When building x86/defconfig with Clang-18 I encounter the following warnings: | arch/x86/platform/efi/efi.c:934:23: warning: no previous extern declaration for non-static variable 'efi_attr_fw_vendor' [-Wmissing-variable-declarations] | 934 | struct kobj_attribute efi_attr_fw_vendor = __ATTR_RO(fw_vendor); | arch/x86/platform/efi/efi.c:935:23: warning: no previous extern declaration for non-static variable 'efi_attr_runtime' [-Wmissing-variable-declarations] | 935 | struct kobj_attribute efi_attr_runtime = __ATTR_RO(runtime); | arch/x86/platform/efi/efi.c:936:23: warning: no previous extern declaration for non-static variable 'efi_attr_config_table' [-Wmissing-variable-declarations] | 936 | struct kobj_attribute efi_attr_config_table = __ATTR_RO(config_table); These variables are not externally declared anywhere (AFAIK) so let's add the static keyword and ensure we follow the ODR. Link: ClangBuiltLinux#1920 Signed-off-by: Justin Stitt <justinstitt@google.com>
Hi all, I was looking to get some help on solving this -Wmissing-variable-declarations warning as there is some hope to turn it on for W=1 soon [1]. When building x86/defconfig with Clang-18 I encounter the following warning: | init/main.c:189:13: warning: no previous extern declaration for non-static variable 'envp_init' [-Wmissing-variable-declarations] | 189 | const char *envp_init[MAX_INIT_ENVS+2] = { "HOME=/", "TERM=linux", NULL, }; | | ^ | init/main.c:189:7: note: declare 'static' if the variable is not intended to be used outside of this translation unit | 189 | const char *envp_init[MAX_INIT_ENVS+2] = { "HOME=/", "TERM=linux", NULL, }; | | ^ It seems like the obvious solution is to just add the `static` keyword and be done with it. I suspect, however, that it is not so simple for the following reasons: Firstly, `envp_init` is surrounded by two other variables that have been explicitly marked as `static` which leads me to believe that this one was intentionally _not_ marked as static for some reason: | static const char *argv_init[MAX_INIT_ARGS+2] = { "init", NULL, }; | static const char *envp_init[MAX_INIT_ENVS+2] = { "HOME=/", "TERM=linux", NULL, }; | static const char *panic_later, *panic_param; Secondly, there exists this `extern` declaration for `envp_init`: | init/do_mounts_initrd.c | 90: extern char *envp_init[]; This one is tricky because it seems like I can rename (effectively remove) this extern symbol entirely and still build a kernel image. If it truly is the case that this extern declaration works then why does Clang produce the warning at all? FWIW, I've tried moving `extern char *envp_init[];` to top-level scope inside of `do_mounts_initrd.c` which did _not_ work in fixing the warning. So all in all, it looks like just adding `static` fixes the warning (which it does) but I am not sure about the other ramifications of this patch especially considering the second point I brought up above regarding the extern declaration already existing (but seemingly not doing anything). Any help here would be appreciated! Link: ClangBuiltLinux#1920 [1] Signed-off-by: Justin Stitt <justinstitt@google.com>
When building x86/defconfig with Clang-18 I encounter the following warnings: | arch/x86/platform/efi/efi.c:934:23: warning: no previous extern declaration for non-static variable 'efi_attr_fw_vendor' [-Wmissing-variable-declarations] | 934 | struct kobj_attribute efi_attr_fw_vendor = __ATTR_RO(fw_vendor); | arch/x86/platform/efi/efi.c:935:23: warning: no previous extern declaration for non-static variable 'efi_attr_runtime' [-Wmissing-variable-declarations] | 935 | struct kobj_attribute efi_attr_runtime = __ATTR_RO(runtime); | arch/x86/platform/efi/efi.c:936:23: warning: no previous extern declaration for non-static variable 'efi_attr_config_table' [-Wmissing-variable-declarations] | 936 | struct kobj_attribute efi_attr_config_table = __ATTR_RO(config_table); These symbols are declared extern in drivers/firmware/efi/efi.c. Move the declarations to linux/efi.h to resolve these warnings. Also, trim a trailing space from efi_set_variable_t typedef. Link: ClangBuiltLinux#1920 Suggested-by: Ard Biesheuvel <ardb@kernel.org> Signed-off-by: Justin Stitt <justinstitt@google.com>
…lass When building x86 defconfig with Clang-18 I get the following warnings: | arch/x86/ia32/audit.c:6:10: warning: no previous extern declaration for non-static variable 'ia32_dir_class' [-Wmissing-variable-declarations] | 6 | unsigned ia32_dir_class[] = { | arch/x86/ia32/audit.c:11:10: warning: no previous extern declaration for non-static variable 'ia32_chattr_class' [-Wmissing-variable-declarations] | 11 | unsigned ia32_chattr_class[] = { | arch/x86/ia32/audit.c:16:10: warning: no previous extern declaration for non-static variable 'ia32_write_class' [-Wmissing-variable-declarations] | 16 | unsigned ia32_write_class[] = { | arch/x86/ia32/audit.c:21:10: warning: no previous extern declaration for non-static variable 'ia32_read_class' [-Wmissing-variable-declarations] | 21 | unsigned ia32_read_class[] = { | arch/x86/ia32/audit.c:26:10: warning: no previous extern declaration for non-static variable 'ia32_signal_class' [-Wmissing-variable-declarations] | 26 | unsigned ia32_signal_class[] = { These warnings occur due to their respective extern declarations being scoped inside of audit_classes_init as well as only being enabled with `CONFIG_IA32_EMULATION=y`: | static int __init audit_classes_init(void) | { | #ifdef CONFIG_IA32_EMULATION | extern __u32 ia32_dir_class[]; | extern __u32 ia32_write_class[]; | extern __u32 ia32_read_class[]; | extern __u32 ia32_chattr_class[]; | audit_register_class(AUDIT_CLASS_WRITE_32, ia32_write_class); | audit_register_class(AUDIT_CLASS_READ_32, ia32_read_class); | audit_register_class(AUDIT_CLASS_DIR_WRITE_32, ia32_dir_class); | audit_register_class(AUDIT_CLASS_CHATTR_32, ia32_chattr_class); | #endif | audit_register_class(AUDIT_CLASS_WRITE, write_class); | audit_register_class(AUDIT_CLASS_READ, read_class); | audit_register_class(AUDIT_CLASS_DIR_WRITE, dir_class); | audit_register_class(AUDIT_CLASS_CHATTR, chattr_class); | return 0; | } Lift the extern declarations to their own header and resolve scoping issues (and thus fix the warnings). Moreover, change __u32 to unsigned so that we match the definitions: | unsigned ia32_dir_class[] = { | #include <asm-generic/audit_dir_write.h> | ~0U | }; | | unsigned ia32_chattr_class[] = { | #include <asm-generic/audit_change_attr.h> | ~0U | }; | ... This patch is similar to commit: 0e5e3d4 ("x86/audit: Fix a -Wmissing-prototypes warning for ia32_classify_syscall()") [1] Signed-off-by: Justin Stitt <justinstitt@google.com> Signed-off-by: Ingo Molnar <mingo@kernel.org> Link: https://lore.kernel.org/all/20200516123816.2680-1-b.thiel@posteo.de/ [1] Link: ClangBuiltLinux#1920 Link: https://lore.kernel.org/r/20230829-missingvardecl-audit-v1-1-34efeb7f3539@google.com
Hi all, I was looking to get some help on solving this -Wmissing-variable-declarations warning as there is some hope to turn it on for W=1 soon [1]. When building x86/defconfig with Clang-18 I encounter the following warning: | init/main.c:189:13: warning: no previous extern declaration for non-static variable 'envp_init' [-Wmissing-variable-declarations] | 189 | const char *envp_init[MAX_INIT_ENVS+2] = { "HOME=/", "TERM=linux", NULL, }; | | ^ | init/main.c:189:7: note: declare 'static' if the variable is not intended to be used outside of this translation unit | 189 | const char *envp_init[MAX_INIT_ENVS+2] = { "HOME=/", "TERM=linux", NULL, }; | | ^ It seems like the obvious solution is to just add the `static` keyword and be done with it. I suspect, however, that it is not so simple for the following reasons: Firstly, `envp_init` is surrounded by two other variables that have been explicitly marked as `static` which leads me to believe that this one was intentionally _not_ marked as static for some reason: | static const char *argv_init[MAX_INIT_ARGS+2] = { "init", NULL, }; | static const char *envp_init[MAX_INIT_ENVS+2] = { "HOME=/", "TERM=linux", NULL, }; | static const char *panic_later, *panic_param; Secondly, there exists this `extern` declaration for `envp_init`: | init/do_mounts_initrd.c | 90: extern char *envp_init[]; This one is tricky because it seems like I can rename (effectively remove) this extern symbol entirely and still build a kernel image. If it truly is the case that this extern declaration works then why does Clang produce the warning at all? FWIW, I've tried moving `extern char *envp_init[];` to top-level scope inside of `do_mounts_initrd.c` which did _not_ work in fixing the warning. So all in all, it looks like just adding `static` fixes the warning (which it does) but I am not sure about the other ramifications of this patch especially considering the second point I brought up above regarding the extern declaration already existing (but seemingly not doing anything). Any help here would be appreciated! Link: ClangBuiltLinux#1920 [1] Signed-off-by: Justin Stitt <justinstitt@google.com>
When building x86/defconfig with Clang-18 I encounter the following warning: | init/main.c:189:13: warning: no previous extern declaration for non-static variable 'envp_init' [-Wmissing-variable-declarations] | 189 | const char *envp_init[MAX_INIT_ENVS+2] = { "HOME=/", "TERM=linux", NULL, }; | init/main.c:189:7: note: declare 'static' if the variable is not intended to be used outside of this translation unit | 189 | const char *envp_init[MAX_INIT_ENVS+2] = { "HOME=/", "TERM=linux", NULL, }; Move the extern declaration to a header file so that the compiler can find it when compiling init/main.c. Add `const` qualifier to extern declaration so that the types match and we follow the One-Definition Rule (ODR). Link: ClangBuiltLinux#1920 Signed-off-by: Justin Stitt <justinstitt@google.com>
When building x86/defconfig with Clang-18 I encounter the following warning: | init/main.c:189:13: warning: no previous extern declaration for non-static variable 'envp_init' [-Wmissing-variable-declarations] | 189 | const char *envp_init[MAX_INIT_ENVS+2] = { "HOME=/", "TERM=linux", NULL, }; | init/main.c:189:7: note: declare 'static' if the variable is not intended to be used outside of this translation unit | 189 | const char *envp_init[MAX_INIT_ENVS+2] = { "HOME=/", "TERM=linux", NULL, }; Make `envp_init` static and provide `handle_initrd` its own copy. This silences the warning and makes the code more readable as you no longer have to track down extern definitions inside of `handle_initrd`. It is now more self-contained. Link: ClangBuiltLinux#1920 Signed-off-by: Justin Stitt <justinstitt@google.com>
I had sent a patch for this, but didn't have time to follow through due to the srso patch set causing a fair amount of breakage for us that week.
https://lore.kernel.org/llvm/20230807-missing_proto-v2-1-3ae2e188bb0c@google.com/
We should work through cleaning up some of the observed issues first before resending the above.
The text was updated successfully, but these errors were encountered: