| Message ID | 20230829-missingvardecl-efi-v1-1-13d055a55176@google.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere, archived |
| Headers | show |
| Series | efi: fix -Wmissing-variable-declarations warning | expand |
Hi Justin, On Wed, 30 Aug 2023 at 00:54, Justin Stitt <justinstitt@google.com> wrote: > > 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) They are: drivers/firmware/efi/efi.c:extern __weak struct kobj_attribute efi_attr_fw_vendor; drivers/firmware/efi/efi.c:extern __weak struct kobj_attribute efi_attr_runtime; drivers/firmware/efi/efi.c:extern __weak struct kobj_attribute efi_attr_config_table; > so let's > add the static keyword and ensure we follow the ODR. > One Definition Rule, right? Better to spell that out. > Link: https://github.com/ClangBuiltLinux/linux/issues/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); This is duplicated. Is this a b4 fail? > --- > Note: build-tested. > --- > arch/x86/platform/efi/efi.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c > index e9f99c56f3ce..30c354c52ad4 100644 > --- a/arch/x86/platform/efi/efi.c > +++ b/arch/x86/platform/efi/efi.c > @@ -931,9 +931,9 @@ EFI_ATTR_SHOW(fw_vendor); > EFI_ATTR_SHOW(runtime); > EFI_ATTR_SHOW(config_table); > > -struct kobj_attribute efi_attr_fw_vendor = __ATTR_RO(fw_vendor); > -struct kobj_attribute efi_attr_runtime = __ATTR_RO(runtime); > -struct kobj_attribute efi_attr_config_table = __ATTR_RO(config_table); > +static struct kobj_attribute efi_attr_fw_vendor = __ATTR_RO(fw_vendor); > +static struct kobj_attribute efi_attr_runtime = __ATTR_RO(runtime); > +static struct kobj_attribute efi_attr_config_table = __ATTR_RO(config_table); > > umode_t efi_attr_is_visible(struct kobject *kobj, struct attribute *attr, int n) > { > This won't work. Those variables are referenced via weak references in generic code. The idea is that the weak references resolve to NULL pointers on architectures other than x86, terminating the array early and hiding the non-existent variables. Making them static in arch/x86/platform/efi/efi.c means that these references will remain unsatisfied, and so the variables will no longer be exposed on x86 either.
On Wed, Aug 30, 2023 at 2:04 AM Ard Biesheuvel <ardb@kernel.org> wrote: > On Wed, 30 Aug 2023 at 00:54, Justin Stitt <justinstitt@google.com> wrote: ... > > 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) > > They are: > > drivers/firmware/efi/efi.c:extern __weak struct kobj_attribute > efi_attr_fw_vendor; > drivers/firmware/efi/efi.c:extern __weak struct kobj_attribute efi_attr_runtime; > drivers/firmware/efi/efi.c:extern __weak struct kobj_attribute > efi_attr_config_table; > > > so let's add the static keyword and ensure we follow the ODR. > This won't work. > > Those variables are referenced via weak references in generic code. > The idea is that the weak references resolve to NULL pointers on > architectures other than x86, terminating the array early and hiding > the non-existent variables. > > Making them static in arch/x86/platform/efi/efi.c means that these > references will remain unsatisfied, and so the variables will no > longer be exposed on x86 either. So it means that we have no definitions in the header for these, right?
On Wed, 30 Aug 2023 at 15:25, Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > > On Wed, Aug 30, 2023 at 2:04 AM Ard Biesheuvel <ardb@kernel.org> wrote: > > On Wed, 30 Aug 2023 at 00:54, Justin Stitt <justinstitt@google.com> wrote: > > ... > > > > 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) > > > > They are: > > > > drivers/firmware/efi/efi.c:extern __weak struct kobj_attribute > > efi_attr_fw_vendor; > > drivers/firmware/efi/efi.c:extern __weak struct kobj_attribute efi_attr_runtime; > > drivers/firmware/efi/efi.c:extern __weak struct kobj_attribute > > efi_attr_config_table; > > > > > so let's add the static keyword and ensure we follow the ODR. > > > This won't work. > > > > Those variables are referenced via weak references in generic code. > > The idea is that the weak references resolve to NULL pointers on > > architectures other than x86, terminating the array early and hiding > > the non-existent variables. > > > > Making them static in arch/x86/platform/efi/efi.c means that these > > references will remain unsatisfied, and so the variables will no > > longer be exposed on x86 either. > > So it means that we have no definitions in the header for these, right? > Indeed. If there are better ways of fixing this that don't involve weak references, I am also fine with that, but just moving the existing extern declarations into linux/efi.h is probably the easiest approach here.
diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c index e9f99c56f3ce..30c354c52ad4 100644 --- a/arch/x86/platform/efi/efi.c +++ b/arch/x86/platform/efi/efi.c @@ -931,9 +931,9 @@ EFI_ATTR_SHOW(fw_vendor); EFI_ATTR_SHOW(runtime); EFI_ATTR_SHOW(config_table); -struct kobj_attribute efi_attr_fw_vendor = __ATTR_RO(fw_vendor); -struct kobj_attribute efi_attr_runtime = __ATTR_RO(runtime); -struct kobj_attribute efi_attr_config_table = __ATTR_RO(config_table); +static struct kobj_attribute efi_attr_fw_vendor = __ATTR_RO(fw_vendor); +static struct kobj_attribute efi_attr_runtime = __ATTR_RO(runtime); +static struct kobj_attribute efi_attr_config_table = __ATTR_RO(config_table); umode_t efi_attr_is_visible(struct kobject *kobj, struct attribute *attr, int n) {
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: https://github.com/ClangBuiltLinux/linux/issues/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); --- Note: build-tested. --- arch/x86/platform/efi/efi.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) --- base-commit: 706a741595047797872e669b3101429ab8d378ef change-id: 20230829-missingvardecl-efi-59306aacfed4 Best regards, -- Justin Stitt <justinstitt@google.com>