This patch set removes some unused variables in x86/boot, and add the
"-Wall" flag to Makefile, which is the old problem of x86 not sharing
makefiles.
Changes since v1:
- Add "-Wall" flag to x86/boot/compressed/Makefile
- Remove unused variables "et" in efi_get_system_table() and "ret" in
efi_get_conf_table()
- Remove unused variables "ret" in __efi_get_rsdp_addr() and
"nr_tables" in efi_get_rsdp_addr()
v1 at:
https://lore.kernel.org/all/[email protected]/
Li Zetao (2):
x86/boot/compressed: Add "-Wall" flag to Makefile
x86/boot: Remove unused variables
arch/x86/boot/compressed/Makefile | 2 +-
arch/x86/boot/compressed/acpi.c | 2 --
arch/x86/boot/compressed/efi.c | 2 --
arch/x86/boot/compressed/sev.c | 1 -
4 files changed, 1 insertion(+), 6 deletions(-)
--
2.34.1
Hi Li,
On Tue, Sep 27, 2022 at 08:15:10AM +0000, Li Zetao wrote:
> This patch set removes some unused variables in x86/boot, and add the
> "-Wall" flag to Makefile, which is the old problem of x86 not sharing
> makefiles.
>
> Changes since v1:
> - Add "-Wall" flag to x86/boot/compressed/Makefile
> - Remove unused variables "et" in efi_get_system_table() and "ret" in
> efi_get_conf_table()
> - Remove unused variables "ret" in __efi_get_rsdp_addr() and
> "nr_tables" in efi_get_rsdp_addr()
>
> v1 at:
> https://lore.kernel.org/all/[email protected]/
>
> Li Zetao (2):
> x86/boot/compressed: Add "-Wall" flag to Makefile
> x86/boot: Remove unused variables
>
> arch/x86/boot/compressed/Makefile | 2 +-
> arch/x86/boot/compressed/acpi.c | 2 --
> arch/x86/boot/compressed/efi.c | 2 --
> arch/x86/boot/compressed/sev.c | 1 -
> 4 files changed, 1 insertion(+), 6 deletions(-)
I took this series for a spin with clang and found a few extra warnings.
1.
In file included from arch/x86/boot/compressed/misc.c:15:
In file included from arch/x86/boot/compressed/misc.h:24:
In file included from ./include/linux/elf.h:6:
In file included from ./arch/x86/include/asm/elf.h:8:
In file included from ./include/linux/thread_info.h:60:
./arch/x86/include/asm/thread_info.h:175:13: warning: calling '__builtin_frame_address' with a nonzero argument is unsafe [-Wframe-address]
oldframe = __builtin_frame_address(1);
^~~~~~~~~~~~~~~~~~~~~~~~~~
./arch/x86/include/asm/thread_info.h:177:11: warning: calling '__builtin_frame_address' with a nonzero argument is unsafe [-Wframe-address]
frame = __builtin_frame_address(2);
^~~~~~~~~~~~~~~~~~~~~~~~~~
This warning is disabled in the main Makefile for this reason so we
should just be able to disable it:
diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
index 10abb7c45d04..3f004567f3d5 100644
--- a/arch/x86/boot/compressed/Makefile
+++ b/arch/x86/boot/compressed/Makefile
@@ -43,6 +43,7 @@ KBUILD_CFLAGS += -mno-mmx -mno-sse
KBUILD_CFLAGS += -ffreestanding -fshort-wchar
KBUILD_CFLAGS += -fno-stack-protector
KBUILD_CFLAGS += $(call cc-disable-warning, address-of-packed-member)
+KBUILD_CFLAGS += $(call cc-disable-warning, frame-address)
KBUILD_CFLAGS += $(call cc-disable-warning, gnu)
KBUILD_CFLAGS += -Wno-pointer-sign
KBUILD_CFLAGS += $(call cc-option,-fmacro-prefix-map=$(srctree)/=)
2.
arch/x86/boot/compressed/kaslr.c:627:6: warning: unused variable 'i' [-Wunused-variable]
int i;
^
This happens when CONFIG_MEMORY_HOTREMOVE or CONFIG_ACPI are 'n'. I
think it can just be fixed by aligning arch/x86/boot/compressed with the
rest of the kernel and explicitly compiling with '-std=gnu11', which
will allow us to declare the variable within the for loop, like so.
diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
index 3f004567f3d5..6c7e366a437b 100644
--- a/arch/x86/boot/compressed/Makefile
+++ b/arch/x86/boot/compressed/Makefile
@@ -34,7 +34,7 @@ targets := vmlinux vmlinux.bin vmlinux.bin.gz vmlinux.bin.bz2 vmlinux.bin.lzma \
# be valid.
KBUILD_CFLAGS := -m$(BITS) -O2 $(CLANG_FLAGS)
KBUILD_CFLAGS += -fno-strict-aliasing -fPIE
-KBUILD_CFLAGS += -Wundef -Wall
+KBUILD_CFLAGS += -Wundef -Wall -std=gnu11
KBUILD_CFLAGS += -DDISABLE_BRANCH_PROFILING
cflags-$(CONFIG_X86_32) := -march=i386
cflags-$(CONFIG_X86_64) := -mcmodel=small -mno-red-zone
diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
index 4a3f223973f4..be859c7e7f6b 100644
--- a/arch/x86/boot/compressed/kaslr.c
+++ b/arch/x86/boot/compressed/kaslr.c
@@ -624,7 +624,6 @@ static bool process_mem_region(struct mem_vector *region,
unsigned long minimum,
unsigned long image_size)
{
- int i;
/*
* If no immovable memory found, or MEMORY_HOTREMOVE disabled,
* use @region directly.
@@ -644,7 +643,7 @@ static bool process_mem_region(struct mem_vector *region,
* If immovable memory found, filter the intersection between
* immovable memory and @region.
*/
- for (i = 0; i < num_immovable_mem; i++) {
+ for (int i = 0; i < num_immovable_mem; i++) {
u64 start, end, entry_end, region_end;
struct mem_vector entry;
Additionally, I think these two patches should be reordered so that the
warnings are fixed before they are enabled.
With those comments addressed, consider the series:
Reviewed-by: Nathan Chancellor <[email protected]>
Cheers,
Nathan
On Tue, Sep 27, 2022 at 02:50:19PM -0700, Nathan Chancellor wrote:
> Hi Li,
>
> On Tue, Sep 27, 2022 at 08:15:10AM +0000, Li Zetao wrote:
> > This patch set removes some unused variables in x86/boot, and add the
> > "-Wall" flag to Makefile, which is the old problem of x86 not sharing
> > makefiles.
> >
> > Changes since v1:
> > - Add "-Wall" flag to x86/boot/compressed/Makefile
> > - Remove unused variables "et" in efi_get_system_table() and "ret" in
> > efi_get_conf_table()
> > - Remove unused variables "ret" in __efi_get_rsdp_addr() and
> > "nr_tables" in efi_get_rsdp_addr()
> >
> > v1 at:
> > https://lore.kernel.org/all/[email protected]/
> >
> > Li Zetao (2):
> > x86/boot/compressed: Add "-Wall" flag to Makefile
> > x86/boot: Remove unused variables
> >
> > arch/x86/boot/compressed/Makefile | 2 +-
> > arch/x86/boot/compressed/acpi.c | 2 --
> > arch/x86/boot/compressed/efi.c | 2 --
> > arch/x86/boot/compressed/sev.c | 1 -
> > 4 files changed, 1 insertion(+), 6 deletions(-)
>
> I took this series for a spin with clang and found a few extra warnings.
>
> 1.
>
> In file included from arch/x86/boot/compressed/misc.c:15:
> In file included from arch/x86/boot/compressed/misc.h:24:
> In file included from ./include/linux/elf.h:6:
> In file included from ./arch/x86/include/asm/elf.h:8:
> In file included from ./include/linux/thread_info.h:60:
> ./arch/x86/include/asm/thread_info.h:175:13: warning: calling '__builtin_frame_address' with a nonzero argument is unsafe [-Wframe-address]
> oldframe = __builtin_frame_address(1);
> ^~~~~~~~~~~~~~~~~~~~~~~~~~
> ./arch/x86/include/asm/thread_info.h:177:11: warning: calling '__builtin_frame_address' with a nonzero argument is unsafe [-Wframe-address]
> frame = __builtin_frame_address(2);
> ^~~~~~~~~~~~~~~~~~~~~~~~~~
>
> This warning is disabled in the main Makefile for this reason so we
> should just be able to disable it:
>
> diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
> index 10abb7c45d04..3f004567f3d5 100644
> --- a/arch/x86/boot/compressed/Makefile
> +++ b/arch/x86/boot/compressed/Makefile
> @@ -43,6 +43,7 @@ KBUILD_CFLAGS += -mno-mmx -mno-sse
> KBUILD_CFLAGS += -ffreestanding -fshort-wchar
> KBUILD_CFLAGS += -fno-stack-protector
> KBUILD_CFLAGS += $(call cc-disable-warning, address-of-packed-member)
> +KBUILD_CFLAGS += $(call cc-disable-warning, frame-address)
> KBUILD_CFLAGS += $(call cc-disable-warning, gnu)
> KBUILD_CFLAGS += -Wno-pointer-sign
> KBUILD_CFLAGS += $(call cc-option,-fmacro-prefix-map=$(srctree)/=)
>
> 2.
>
> arch/x86/boot/compressed/kaslr.c:627:6: warning: unused variable 'i' [-Wunused-variable]
> int i;
> ^
>
> This happens when CONFIG_MEMORY_HOTREMOVE or CONFIG_ACPI are 'n'. I
> think it can just be fixed by aligning arch/x86/boot/compressed with the
> rest of the kernel and explicitly compiling with '-std=gnu11', which
> will allow us to declare the variable within the for loop, like so.
>
> diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
> index 3f004567f3d5..6c7e366a437b 100644
> --- a/arch/x86/boot/compressed/Makefile
> +++ b/arch/x86/boot/compressed/Makefile
> @@ -34,7 +34,7 @@ targets := vmlinux vmlinux.bin vmlinux.bin.gz vmlinux.bin.bz2 vmlinux.bin.lzma \
> # be valid.
> KBUILD_CFLAGS := -m$(BITS) -O2 $(CLANG_FLAGS)
> KBUILD_CFLAGS += -fno-strict-aliasing -fPIE
> -KBUILD_CFLAGS += -Wundef -Wall
> +KBUILD_CFLAGS += -Wundef -Wall -std=gnu11
> KBUILD_CFLAGS += -DDISABLE_BRANCH_PROFILING
> cflags-$(CONFIG_X86_32) := -march=i386
> cflags-$(CONFIG_X86_64) := -mcmodel=small -mno-red-zone
> diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
> index 4a3f223973f4..be859c7e7f6b 100644
> --- a/arch/x86/boot/compressed/kaslr.c
> +++ b/arch/x86/boot/compressed/kaslr.c
> @@ -624,7 +624,6 @@ static bool process_mem_region(struct mem_vector *region,
> unsigned long minimum,
> unsigned long image_size)
> {
> - int i;
> /*
> * If no immovable memory found, or MEMORY_HOTREMOVE disabled,
> * use @region directly.
> @@ -644,7 +643,7 @@ static bool process_mem_region(struct mem_vector *region,
> * If immovable memory found, filter the intersection between
> * immovable memory and @region.
> */
> - for (i = 0; i < num_immovable_mem; i++) {
> + for (int i = 0; i < num_immovable_mem; i++) {
> u64 start, end, entry_end, region_end;
> struct mem_vector entry;
>
>
> Additionally, I think these two patches should be reordered so that the
> warnings are fixed before they are enabled.
>
> With those comments addressed, consider the series:
>
> Reviewed-by: Nathan Chancellor <[email protected]>
Oops, missed another one on second glance of my build logs:
3.
arch/x86/boot/compressed/acpi.c:23:1: warning: unused function '__efi_get_rsdp_addr' [-Wunused-function]
__efi_get_rsdp_addr(unsigned long cfg_tbl_pa, unsigned int cfg_tbl_len)
^
1 warning generated.
This happens when CONFIG_EFI is disabled. This will resolve it:
diff --git a/arch/x86/boot/compressed/acpi.c b/arch/x86/boot/compressed/acpi.c
index 21febd9f21ab..5f2b8966e723 100644
--- a/arch/x86/boot/compressed/acpi.c
+++ b/arch/x86/boot/compressed/acpi.c
@@ -19,10 +19,10 @@
*/
struct mem_vector immovable_mem[MAX_NUMNODES*2];
+#ifdef CONFIG_EFI
static acpi_physical_address
__efi_get_rsdp_addr(unsigned long cfg_tbl_pa, unsigned int cfg_tbl_len)
{
-#ifdef CONFIG_EFI
unsigned long rsdp_addr;
/*
@@ -41,9 +41,9 @@ __efi_get_rsdp_addr(unsigned long cfg_tbl_pa, unsigned int cfg_tbl_len)
return (acpi_physical_address)rsdp_addr;
debug_putstr("Error getting RSDP address.\n");
-#endif
return 0;
}
+#endif
static acpi_physical_address efi_get_rsdp_addr(void)
{
Hi Nathan,
On 2022/9/28 5:50, Nathan Chancellor wrote:
> Hi Li,
>
> On Tue, Sep 27, 2022 at 08:15:10AM +0000, Li Zetao wrote:
>> This patch set removes some unused variables in x86/boot, and add the
>> "-Wall" flag to Makefile, which is the old problem of x86 not sharing
>> makefiles.
>>
>> Changes since v1:
>> - Add "-Wall" flag to x86/boot/compressed/Makefile
>> - Remove unused variables "et" in efi_get_system_table() and "ret" in
>> efi_get_conf_table()
>> - Remove unused variables "ret" in __efi_get_rsdp_addr() and
>> "nr_tables" in efi_get_rsdp_addr()
>>
>> v1 at:
>> https://lore.kernel.org/all/[email protected]/
>>
>> Li Zetao (2):
>> x86/boot/compressed: Add "-Wall" flag to Makefile
>> x86/boot: Remove unused variables
>>
>> arch/x86/boot/compressed/Makefile | 2 +-
>> arch/x86/boot/compressed/acpi.c | 2 --
>> arch/x86/boot/compressed/efi.c | 2 --
>> arch/x86/boot/compressed/sev.c | 1 -
>> 4 files changed, 1 insertion(+), 6 deletions(-)
> I took this series for a spin with clang and found a few extra warnings.
>
> 1.
>
> In file included from arch/x86/boot/compressed/misc.c:15:
> In file included from arch/x86/boot/compressed/misc.h:24:
> In file included from ./include/linux/elf.h:6:
> In file included from ./arch/x86/include/asm/elf.h:8:
> In file included from ./include/linux/thread_info.h:60:
> ./arch/x86/include/asm/thread_info.h:175:13: warning: calling '__builtin_frame_address' with a nonzero argument is unsafe [-Wframe-address]
> oldframe = __builtin_frame_address(1);
> ^~~~~~~~~~~~~~~~~~~~~~~~~~
> ./arch/x86/include/asm/thread_info.h:177:11: warning: calling '__builtin_frame_address' with a nonzero argument is unsafe [-Wframe-address]
> frame = __builtin_frame_address(2);
> ^~~~~~~~~~~~~~~~~~~~~~~~~~
>
> This warning is disabled in the main Makefile for this reason so we
> should just be able to disable it:
>
> diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
> index 10abb7c45d04..3f004567f3d5 100644
> --- a/arch/x86/boot/compressed/Makefile
> +++ b/arch/x86/boot/compressed/Makefile
> @@ -43,6 +43,7 @@ KBUILD_CFLAGS += -mno-mmx -mno-sse
> KBUILD_CFLAGS += -ffreestanding -fshort-wchar
> KBUILD_CFLAGS += -fno-stack-protector
> KBUILD_CFLAGS += $(call cc-disable-warning, address-of-packed-member)
> +KBUILD_CFLAGS += $(call cc-disable-warning, frame-address)
> KBUILD_CFLAGS += $(call cc-disable-warning, gnu)
> KBUILD_CFLAGS += -Wno-pointer-sign
> KBUILD_CFLAGS += $(call cc-option,-fmacro-prefix-map=$(srctree)/=)
>
> 2.
>
> arch/x86/boot/compressed/kaslr.c:627:6: warning: unused variable 'i' [-Wunused-variable]
> int i;
> ^
>
> This happens when CONFIG_MEMORY_HOTREMOVE or CONFIG_ACPI are 'n'. I
> think it can just be fixed by aligning arch/x86/boot/compressed with the
> rest of the kernel and explicitly compiling with '-std=gnu11', which
> will allow us to declare the variable within the for loop, like so.
>
> diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
> index 3f004567f3d5..6c7e366a437b 100644
> --- a/arch/x86/boot/compressed/Makefile
> +++ b/arch/x86/boot/compressed/Makefile
> @@ -34,7 +34,7 @@ targets := vmlinux vmlinux.bin vmlinux.bin.gz vmlinux.bin.bz2 vmlinux.bin.lzma \
> # be valid.
> KBUILD_CFLAGS := -m$(BITS) -O2 $(CLANG_FLAGS)
> KBUILD_CFLAGS += -fno-strict-aliasing -fPIE
> -KBUILD_CFLAGS += -Wundef -Wall
> +KBUILD_CFLAGS += -Wundef -Wall -std=gnu11
> KBUILD_CFLAGS += -DDISABLE_BRANCH_PROFILING
> cflags-$(CONFIG_X86_32) := -march=i386
> cflags-$(CONFIG_X86_64) := -mcmodel=small -mno-red-zone
> diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
> index 4a3f223973f4..be859c7e7f6b 100644
> --- a/arch/x86/boot/compressed/kaslr.c
> +++ b/arch/x86/boot/compressed/kaslr.c
> @@ -624,7 +624,6 @@ static bool process_mem_region(struct mem_vector *region,
> unsigned long minimum,
> unsigned long image_size)
> {
> - int i;
> /*
> * If no immovable memory found, or MEMORY_HOTREMOVE disabled,
> * use @region directly.
> @@ -644,7 +643,7 @@ static bool process_mem_region(struct mem_vector *region,
> * If immovable memory found, filter the intersection between
> * immovable memory and @region.
> */
> - for (i = 0; i < num_immovable_mem; i++) {
> + for (int i = 0; i < num_immovable_mem; i++) {
> u64 start, end, entry_end, region_end;
> struct mem_vector entry;
>
>
> Additionally, I think these two patches should be reordered so that the
> warnings are fixed before they are enabled.
>
> With those comments addressed, consider the series:
>
> Reviewed-by: Nathan Chancellor <[email protected]>
>
> Cheers,
> Nathan
What is your clang compiler version and .config? My clang compiler is
14.0.0-1ubuntu1, and I can't reproduce
problem 1("calling '__builtin_frame_address' with a nonzero argument is
unsafe"). Can you provide more
information.
Thx.
Best regards,
Li Zetao
On Thu, Sep 29, 2022 at 10:16:11AM +0800, Li Zetao wrote:
> Hi Nathan,
>
> On 2022/9/28 5:50, Nathan Chancellor wrote:
> > Hi Li,
> >
> > On Tue, Sep 27, 2022 at 08:15:10AM +0000, Li Zetao wrote:
> > > This patch set removes some unused variables in x86/boot, and add the
> > > "-Wall" flag to Makefile, which is the old problem of x86 not sharing
> > > makefiles.
> > >
> > > Changes since v1:
> > > - Add "-Wall" flag to x86/boot/compressed/Makefile
> > > - Remove unused variables "et" in efi_get_system_table() and "ret" in
> > > efi_get_conf_table()
> > > - Remove unused variables "ret" in __efi_get_rsdp_addr() and
> > > "nr_tables" in efi_get_rsdp_addr()
> > >
> > > v1 at:
> > > https://lore.kernel.org/all/[email protected]/
> > >
> > > Li Zetao (2):
> > > x86/boot/compressed: Add "-Wall" flag to Makefile
> > > x86/boot: Remove unused variables
> > >
> > > arch/x86/boot/compressed/Makefile | 2 +-
> > > arch/x86/boot/compressed/acpi.c | 2 --
> > > arch/x86/boot/compressed/efi.c | 2 --
> > > arch/x86/boot/compressed/sev.c | 1 -
> > > 4 files changed, 1 insertion(+), 6 deletions(-)
> > I took this series for a spin with clang and found a few extra warnings.
> >
> > 1.
> >
> > In file included from arch/x86/boot/compressed/misc.c:15:
> > In file included from arch/x86/boot/compressed/misc.h:24:
> > In file included from ./include/linux/elf.h:6:
> > In file included from ./arch/x86/include/asm/elf.h:8:
> > In file included from ./include/linux/thread_info.h:60:
> > ./arch/x86/include/asm/thread_info.h:175:13: warning: calling '__builtin_frame_address' with a nonzero argument is unsafe [-Wframe-address]
> > oldframe = __builtin_frame_address(1);
> > ^~~~~~~~~~~~~~~~~~~~~~~~~~
> > ./arch/x86/include/asm/thread_info.h:177:11: warning: calling '__builtin_frame_address' with a nonzero argument is unsafe [-Wframe-address]
> > frame = __builtin_frame_address(2);
> > ^~~~~~~~~~~~~~~~~~~~~~~~~~
> >
> > This warning is disabled in the main Makefile for this reason so we
> > should just be able to disable it:
> >
> > diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
> > index 10abb7c45d04..3f004567f3d5 100644
> > --- a/arch/x86/boot/compressed/Makefile
> > +++ b/arch/x86/boot/compressed/Makefile
> > @@ -43,6 +43,7 @@ KBUILD_CFLAGS += -mno-mmx -mno-sse
> > KBUILD_CFLAGS += -ffreestanding -fshort-wchar
> > KBUILD_CFLAGS += -fno-stack-protector
> > KBUILD_CFLAGS += $(call cc-disable-warning, address-of-packed-member)
> > +KBUILD_CFLAGS += $(call cc-disable-warning, frame-address)
> > KBUILD_CFLAGS += $(call cc-disable-warning, gnu)
> > KBUILD_CFLAGS += -Wno-pointer-sign
> > KBUILD_CFLAGS += $(call cc-option,-fmacro-prefix-map=$(srctree)/=)
> >
> > 2.
> >
> > arch/x86/boot/compressed/kaslr.c:627:6: warning: unused variable 'i' [-Wunused-variable]
> > int i;
> > ^
> >
> > This happens when CONFIG_MEMORY_HOTREMOVE or CONFIG_ACPI are 'n'. I
> > think it can just be fixed by aligning arch/x86/boot/compressed with the
> > rest of the kernel and explicitly compiling with '-std=gnu11', which
> > will allow us to declare the variable within the for loop, like so.
> >
> > diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
> > index 3f004567f3d5..6c7e366a437b 100644
> > --- a/arch/x86/boot/compressed/Makefile
> > +++ b/arch/x86/boot/compressed/Makefile
> > @@ -34,7 +34,7 @@ targets := vmlinux vmlinux.bin vmlinux.bin.gz vmlinux.bin.bz2 vmlinux.bin.lzma \
> > # be valid.
> > KBUILD_CFLAGS := -m$(BITS) -O2 $(CLANG_FLAGS)
> > KBUILD_CFLAGS += -fno-strict-aliasing -fPIE
> > -KBUILD_CFLAGS += -Wundef -Wall
> > +KBUILD_CFLAGS += -Wundef -Wall -std=gnu11
> > KBUILD_CFLAGS += -DDISABLE_BRANCH_PROFILING
> > cflags-$(CONFIG_X86_32) := -march=i386
> > cflags-$(CONFIG_X86_64) := -mcmodel=small -mno-red-zone
> > diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
> > index 4a3f223973f4..be859c7e7f6b 100644
> > --- a/arch/x86/boot/compressed/kaslr.c
> > +++ b/arch/x86/boot/compressed/kaslr.c
> > @@ -624,7 +624,6 @@ static bool process_mem_region(struct mem_vector *region,
> > unsigned long minimum,
> > unsigned long image_size)
> > {
> > - int i;
> > /*
> > * If no immovable memory found, or MEMORY_HOTREMOVE disabled,
> > * use @region directly.
> > @@ -644,7 +643,7 @@ static bool process_mem_region(struct mem_vector *region,
> > * If immovable memory found, filter the intersection between
> > * immovable memory and @region.
> > */
> > - for (i = 0; i < num_immovable_mem; i++) {
> > + for (int i = 0; i < num_immovable_mem; i++) {
> > u64 start, end, entry_end, region_end;
> > struct mem_vector entry;
> >
> > Additionally, I think these two patches should be reordered so that the
> > warnings are fixed before they are enabled.
> >
> > With those comments addressed, consider the series:
> >
> > Reviewed-by: Nathan Chancellor <[email protected]>
> >
> > Cheers,
> > Nathan
>
> What is your clang compiler version and .config? My clang compiler is
> 14.0.0-1ubuntu1, and I can't reproduce
>
> problem 1("calling '__builtin_frame_address' with a nonzero argument is
> unsafe"). Can you provide more
>
> information.
Sure thing! My compiler is tip of tree LLVM, so clang 16.0.0. That
should not matter for this particular issue though. I can reproduce it
with my distribution's clang (14.0.6) with a configuration that has
CONFIG_FRAME_POINTER enabled; allnoconfig works for me here.
$ clang --version
clang version 14.0.6
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/bin
$ make -skj"$(nproc)" ARCH=i386 LLVM=1 clean allnoconfig all
...
In file included from arch/x86/boot/compressed/misc.c:15:
In file included from arch/x86/boot/compressed/misc.h:24:
In file included from ./include/linux/elf.h:6:
In file included from ./arch/x86/include/asm/elf.h:8:
In file included from ./include/linux/thread_info.h:60:
./arch/x86/include/asm/thread_info.h:175:13: warning: calling '__builtin_frame_address' with a nonzero argument is unsafe [-Wframe-address]
oldframe = __builtin_frame_address(1);
^~~~~~~~~~~~~~~~~~~~~~~~~~
./arch/x86/include/asm/thread_info.h:177:11: warning: calling '__builtin_frame_address' with a nonzero argument is unsafe [-Wframe-address]
frame = __builtin_frame_address(2);
^~~~~~~~~~~~~~~~~~~~~~~~~~
...
$ grep CONFIG_FRAME_POINTER .config
CONFIG_FRAME_POINTER=y
If there is any other information I can provide, please let me know!
Cheers,
Nathan
This series removes some unused variables in x86/boot, and add the
"-Wall" flag to Makefile, which is the old problem of x86 not sharing
makefiles.
Changes since v2:
- Add "frame-address" flag and "-std=gnu11" to
x86/boot/compressed/Makefile to fix warnings when "-Wall" flag added.
- Declare the variable "i" within the for loop to reslove unused
variable warning.
- Delete __efi_get_rsdp_addr function when CONFIG_EFI is disabled to
resolve unused function warning.
v2 at:
https://lore.kernel.org/all/[email protected]/
Changes since v1:
- Add "-Wall" flag to x86/boot/compressed/Makefile
- Remove unused variables "et" in efi_get_system_table() and "ret" in
efi_get_conf_table()
- Remove unused variables "ret" in __efi_get_rsdp_addr() and
"nr_tables" in efi_get_rsdp_addr()
v1 at:
https://lore.kernel.org/all/[email protected]/
Li Zetao (2):
x86/boot/compressed: Add "-Wall" flag to Makefile
x86/boot: Remove unused variables
arch/x86/boot/compressed/Makefile | 3 ++-
arch/x86/boot/compressed/acpi.c | 7 +++----
arch/x86/boot/compressed/efi.c | 2 --
arch/x86/boot/compressed/kaslr.c | 3 +--
arch/x86/boot/compressed/sev.c | 1 -
5 files changed, 6 insertions(+), 10 deletions(-)
--
2.34.1
Compressed/Makefile does not have "-Wall" flag, this is the old problem of
x86 not sharing makefiles. Fix by adding "-Wall" flag to Makefile. But when
"-Wall" flag added to Makefile, a few extra warnings were found.
1.
In file included from arch/x86/boot/compressed/misc.c:15:
In file included from arch/x86/boot/compressed/misc.h:24:
In file included from ./include/linux/elf.h:6:
In file included from ./arch/x86/include/asm/elf.h:8:
In file included from ./include/linux/thread_info.h:60:
./arch/x86/include/asm/thread_info.h:175:13: warning: calling
"__builtin_frame_address" with a nonzero argument is unsafe
[-Wframe-address]
oldframe = __builtin_frame_address(1);
^~~~~~~~~~~~~~~~~~~~~~~~~~
./arch/x86/include/asm/thread_info.h:177:11: warning: calling
"__builtin_frame_address" with a nonzero argument is unsafe
[-Wframe-address]
frame = __builtin_frame_address(2);
^~~~~~~~~~~~~~~~~~~~~~~~~~
This warning is disabled in the main Makefile for this reason so we
should just be able to disable it, adding "frame-address" flag to
Makefile.
2.
arch/x86/boot/compressed/kaslr.c:627:6: warning: unused variable
"i" [-Wunused-variable]
int i;
^
This happens when CONFIG_MEMORY_HOTREMOVE or CONFIG_ACPI are "n".
Fix by adding "-std=gnu11" flag to Makefile, and we should put
the variable "i" within the for loop.
3.
arch/x86/boot/compressed/acpi.c:23:1: warning: unused function
"__efi_get_rsdp_addr" [-Wunused-function]
This happens when CONFIG_EFI is disabled for the reason that
function "__efi_get_rsdp_addr" is only called in efi_get_rsdp_addr
when CONFIG_EFI enable. So function "__efi_get_rsdp_addr" should
not be defined when CONFIG_EFI is disabled.
Signed-off-by: Li Zetao <[email protected]>
Reviewed-by: Nathan Chancellor <[email protected]>
---
v1 -> v2: patch is new
v2 -> v3: resolve extra warnings after "-Wall" flag added.
arch/x86/boot/compressed/Makefile | 3 ++-
arch/x86/boot/compressed/acpi.c | 5 +++--
arch/x86/boot/compressed/kaslr.c | 3 +--
3 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
index 3a261abb6d15..8918a8306dff 100644
--- a/arch/x86/boot/compressed/Makefile
+++ b/arch/x86/boot/compressed/Makefile
@@ -35,7 +35,7 @@ targets := vmlinux vmlinux.bin vmlinux.bin.gz vmlinux.bin.bz2 vmlinux.bin.lzma \
# be valid.
KBUILD_CFLAGS := -m$(BITS) -O2 $(CLANG_FLAGS)
KBUILD_CFLAGS += -fno-strict-aliasing -fPIE
-KBUILD_CFLAGS += -Wundef
+KBUILD_CFLAGS += -Wundef -Wall -std=gnu11
KBUILD_CFLAGS += -DDISABLE_BRANCH_PROFILING
cflags-$(CONFIG_X86_32) := -march=i386
cflags-$(CONFIG_X86_64) := -mcmodel=small -mno-red-zone
@@ -44,6 +44,7 @@ KBUILD_CFLAGS += -mno-mmx -mno-sse
KBUILD_CFLAGS += -ffreestanding -fshort-wchar
KBUILD_CFLAGS += -fno-stack-protector
KBUILD_CFLAGS += $(call cc-disable-warning, address-of-packed-member)
+KBUILD_CFLAGS += $(call cc-disable-warning, frame-address)
KBUILD_CFLAGS += $(call cc-disable-warning, gnu)
KBUILD_CFLAGS += -Wno-pointer-sign
KBUILD_CFLAGS += $(call cc-option,-fmacro-prefix-map=$(srctree)/=)
diff --git a/arch/x86/boot/compressed/acpi.c b/arch/x86/boot/compressed/acpi.c
index 9caf89063e77..79742ab34e3f 100644
--- a/arch/x86/boot/compressed/acpi.c
+++ b/arch/x86/boot/compressed/acpi.c
@@ -19,10 +19,10 @@
*/
struct mem_vector immovable_mem[MAX_NUMNODES*2];
+#ifdef CONFIG_EFI
static acpi_physical_address
__efi_get_rsdp_addr(unsigned long cfg_tbl_pa, unsigned int cfg_tbl_len)
{
-#ifdef CONFIG_EFI
unsigned long rsdp_addr;
int ret;
@@ -42,9 +42,10 @@ __efi_get_rsdp_addr(unsigned long cfg_tbl_pa, unsigned int cfg_tbl_len)
return (acpi_physical_address)rsdp_addr;
debug_putstr("Error getting RSDP address.\n");
-#endif
+
return 0;
}
+#endif
static acpi_physical_address efi_get_rsdp_addr(void)
{
diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
index e476bcbd9b42..4abc9c42cf4d 100644
--- a/arch/x86/boot/compressed/kaslr.c
+++ b/arch/x86/boot/compressed/kaslr.c
@@ -625,7 +625,6 @@ static bool process_mem_region(struct mem_vector *region,
unsigned long minimum,
unsigned long image_size)
{
- int i;
/*
* If no immovable memory found, or MEMORY_HOTREMOVE disabled,
* use @region directly.
@@ -645,7 +644,7 @@ static bool process_mem_region(struct mem_vector *region,
* If immovable memory found, filter the intersection between
* immovable memory and @region.
*/
- for (i = 0; i < num_immovable_mem; i++) {
+ for (int i = 0; i < num_immovable_mem; i++) {
u64 start, end, entry_end, region_end;
struct mem_vector entry;
--
2.34.1
Gcc report warning as follows:
arch/x86/boot/compressed/efi.c: In function ‘efi_get_system_table’:
arch/x86/boot/compressed/efi.c:62:23: warning: unused variable ‘et’
[-Wunused-variable]
arch/x86/boot/compressed/efi.c: In function ‘efi_get_conf_table’:
arch/x86/boot/compressed/efi.c:134:13: warning: unused variable
‘ret’ [-Wunused-variable]
arch/x86/boot/compressed/acpi.c: In function ‘__efi_get_rsdp_addr’:
arch/x86/boot/compressed/acpi.c:27:13: warning: unused variable
‘ret’ [-Wunused-variable]
arch/x86/boot/compressed/acpi.c: In function ‘efi_get_rsdp_addr’:
arch/x86/boot/compressed/acpi.c:55:22: warning: unused variable
‘nr_tables’ [-Wunused-variable]
arch/x86/boot/compressed/sev.c: In function ‘enforce_vmpl0’:
arch/x86/boot/compressed/sev.c:256:13: error: unused variable ‘err’
[-Werror=unused-variable]
Fix these warnings by removing unused variables.
Fixes: 58f3e6b71f42 ("x86/compressed/acpi: Move EFI system table lookup to helper")
Fixes: 61c14ceda840 ("x86/compressed/acpi: Move EFI config table lookup to helper")
Fixes: dee602dd5d14 ("x86/compressed/acpi: Move EFI vendor table lookup to helper")
Fixes: f9d230e893e8 ("x86/boot: Correct RSDP parsing with 32-bit EFI")
Fixes: 81cc3df9a90e ("x86/sev: Check the VMPL level")
Signed-off-by: Li Zetao <[email protected]>
Reviewed-by: Nathan Chancellor <[email protected]>
---
v1 -> v2: Remove unused variables "et" in efi_get_system_table(), "ret" in
efi_get_conf_table(), "ret" in __efi_get_rsdp_addr() and "nr_tables" in
efi_get_rsdp_addr().
v2 -> v3: none
arch/x86/boot/compressed/acpi.c | 2 --
arch/x86/boot/compressed/efi.c | 2 --
arch/x86/boot/compressed/sev.c | 1 -
3 files changed, 5 deletions(-)
diff --git a/arch/x86/boot/compressed/acpi.c b/arch/x86/boot/compressed/acpi.c
index 79742ab34e3f..ee7e689d4e0f 100644
--- a/arch/x86/boot/compressed/acpi.c
+++ b/arch/x86/boot/compressed/acpi.c
@@ -24,7 +24,6 @@ static acpi_physical_address
__efi_get_rsdp_addr(unsigned long cfg_tbl_pa, unsigned int cfg_tbl_len)
{
unsigned long rsdp_addr;
- int ret;
/*
* Search EFI system tables for RSDP. Preferred is ACPI_20_TABLE_GUID to
@@ -53,7 +52,6 @@ static acpi_physical_address efi_get_rsdp_addr(void)
unsigned long cfg_tbl_pa = 0;
unsigned int cfg_tbl_len;
unsigned long systab_pa;
- unsigned int nr_tables;
enum efi_type et;
int ret;
diff --git a/arch/x86/boot/compressed/efi.c b/arch/x86/boot/compressed/efi.c
index 6edd034b0b30..6ffd22710ed2 100644
--- a/arch/x86/boot/compressed/efi.c
+++ b/arch/x86/boot/compressed/efi.c
@@ -59,7 +59,6 @@ unsigned long efi_get_system_table(struct boot_params *bp)
{
unsigned long sys_tbl_pa;
struct efi_info *ei;
- enum efi_type et;
/* Get systab from boot params. */
ei = &bp->efi_info;
@@ -131,7 +130,6 @@ int efi_get_conf_table(struct boot_params *bp, unsigned long *cfg_tbl_pa,
{
unsigned long sys_tbl_pa;
enum efi_type et;
- int ret;
if (!cfg_tbl_pa || !cfg_tbl_len)
return -EINVAL;
diff --git a/arch/x86/boot/compressed/sev.c b/arch/x86/boot/compressed/sev.c
index c93930d5ccbd..b9451761a69a 100644
--- a/arch/x86/boot/compressed/sev.c
+++ b/arch/x86/boot/compressed/sev.c
@@ -253,7 +253,6 @@ void do_boot_stage2_vc(struct pt_regs *regs, unsigned long exit_code)
static void enforce_vmpl0(void)
{
u64 attrs;
- int err;
/*
* RMPADJUST modifies RMP permissions of a lesser-privileged (numerically
--
2.34.1
On 2022/9/30 11:27, Li Zetao wrote:
> This series removes some unused variables in x86/boot, and add the
> "-Wall" flag to Makefile, which is the old problem of x86 not sharing
> makefiles.
>
> Changes since v2:
> - Add "frame-address" flag and "-std=gnu11" to
> x86/boot/compressed/Makefile to fix warnings when "-Wall" flag added.
> - Declare the variable "i" within the for loop to reslove unused
> variable warning.
> - Delete __efi_get_rsdp_addr function when CONFIG_EFI is disabled to
> resolve unused function warning.
>
> v2 at:
> https://lore.kernel.org/all/[email protected]/
>
> Changes since v1:
> - Add "-Wall" flag to x86/boot/compressed/Makefile
> - Remove unused variables "et" in efi_get_system_table() and "ret" in
> efi_get_conf_table()
> - Remove unused variables "ret" in __efi_get_rsdp_addr() and
> "nr_tables" in efi_get_rsdp_addr()
>
> v1 at:
> https://lore.kernel.org/all/[email protected]/
>
> Li Zetao (2):
> x86/boot/compressed: Add "-Wall" flag to Makefile
> x86/boot: Remove unused variables
>
> arch/x86/boot/compressed/Makefile | 3 ++-
> arch/x86/boot/compressed/acpi.c | 7 +++----
> arch/x86/boot/compressed/efi.c | 2 --
> arch/x86/boot/compressed/kaslr.c | 3 +--
> arch/x86/boot/compressed/sev.c | 1 -
> 5 files changed, 6 insertions(+), 10 deletions(-)
PING.
Best regards,
Li Zetao
On Sat, Oct 08, 2022 at 09:41:59PM +0800, Li Zetao wrote:
> On 2022/9/30 11:27, Li Zetao wrote:
> > This series removes some unused variables in x86/boot, and add the
> > "-Wall" flag to Makefile, which is the old problem of x86 not sharing
> > makefiles.
> >
> > Changes since v2:
> > - Add "frame-address" flag and "-std=gnu11" to
> > x86/boot/compressed/Makefile to fix warnings when "-Wall" flag added.
> > - Declare the variable "i" within the for loop to reslove unused
> > variable warning.
> > - Delete __efi_get_rsdp_addr function when CONFIG_EFI is disabled to
> > resolve unused function warning.
Nathan suggested earlier (and I agree): please re-order these patches so
the fixes for the new warnings are first, and then turn on the compiler
flags in a final patch. This will keep the build "warning free" at all
steps.
--
Kees Cook
This series removes some unused variables in x86/boot, and add the
"-Wall" flag to Makefile, which is the old problem of x86 not sharing
makefiles.
Changes since v3:
- Re-order the patches, so the fixes for the new warnings are first,
and then turn on the compiler flags in a final patch
v3 at:
https://lore.kernel.org/all/[email protected]/
Changes since v2:
- Add "frame-address" flag and "-std=gnu11" to
x86/boot/compressed/Makefile to fix warnings when "-Wall" flag added.
- Declare the variable "i" within the for loop to reslove unused
variable warning.
- Delete __efi_get_rsdp_addr function when CONFIG_EFI is disabled to
resolve unused function warning.
v2 at:
https://lore.kernel.org/all/[email protected]/
Changes since v1:
- Add "-Wall" flag to x86/boot/compressed/Makefile
- Remove unused variables "et" in efi_get_system_table() and "ret" in
efi_get_conf_table()
- Remove unused variables "ret" in __efi_get_rsdp_addr() and
"nr_tables" in efi_get_rsdp_addr()
v1 at:
https://lore.kernel.org/all/[email protected]/
Li Zetao (2):
x86/boot: Remove unused variables
x86/boot/compressed: Add "-Wall" flag to Makefile
arch/x86/boot/compressed/Makefile | 3 ++-
arch/x86/boot/compressed/acpi.c | 7 +++----
arch/x86/boot/compressed/efi.c | 2 --
arch/x86/boot/compressed/kaslr.c | 3 +--
arch/x86/boot/compressed/sev.c | 1 -
5 files changed, 6 insertions(+), 10 deletions(-)
--
2.34.1