diff mbox series

[kvm-unit-tests,1/6] arm: keep efi debug information in a separate file

Message ID 20230617014930.2070-2-namit@vmware.com (mailing list archive)
State New, archived
Headers show
Series arm64: improve debuggability | expand

Commit Message

Nadav Amit June 17, 2023, 1:49 a.m. UTC 
From: Nadav Amit <namit@vmware.com>

Debugging tests that run on EFI is hard because the debug information is
not included in the EFI file. Dump it into a separeate .debug file to
allow the use of gdb or pretty_print_stacks script.

Signed-off-by: Nadav Amit <namit@vmware.com>
---
 arm/Makefile.common | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Comments

Andrew Jones June 24, 2023, 10:12 a.m. UTC | #1 
On Sat, Jun 17, 2023 at 01:49:25AM +0000, Nadav Amit wrote:
> From: Nadav Amit <namit@vmware.com>
> 
> Debugging tests that run on EFI is hard because the debug information is
> not included in the EFI file. Dump it into a separeate .debug file to
> allow the use of gdb or pretty_print_stacks script.
> 
> Signed-off-by: Nadav Amit <namit@vmware.com>
> ---
>  arm/Makefile.common | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/arm/Makefile.common b/arm/Makefile.common
> index d60cf8c..f904702 100644
> --- a/arm/Makefile.common
> +++ b/arm/Makefile.common
> @@ -69,7 +69,7 @@ FLATLIBS = $(libcflat) $(LIBFDT_archive) $(libeabi)
>  ifeq ($(CONFIG_EFI),y)
>  %.so: EFI_LDFLAGS += -defsym=EFI_SUBSYSTEM=0xa --no-undefined
>  %.so: %.o $(FLATLIBS) $(SRCDIR)/arm/efi/elf_aarch64_efi.lds $(cstart.o)
> -	$(CC) $(CFLAGS) -c -o $(@:.so=.aux.o) $(SRCDIR)/lib/auxinfo.c \
> +	$(CC) $(CFLAGS) -c -g -o $(@:.so=.aux.o) $(SRCDIR)/lib/auxinfo.c \
>  		-DPROGNAME=\"$(@:.so=.efi)\" -DAUXFLAGS=$(AUXFLAGS)
>  	$(LD) $(EFI_LDFLAGS) -o $@ -T $(SRCDIR)/arm/efi/elf_aarch64_efi.lds \
>  		$(filter %.o, $^) $(FLATLIBS) $(@:.so=.aux.o) \
> @@ -78,6 +78,9 @@ ifeq ($(CONFIG_EFI),y)
>  
>  %.efi: %.so
>  	$(call arch_elf_check, $^)
> +	$(OBJCOPY) --only-keep-debug $^ $@.debug
> +	$(OBJCOPY) --strip-debug $^
> +	$(OBJCOPY) --add-gnu-debuglink=$@.debug $^
>  	$(OBJCOPY) \
>  		-j .text -j .sdata -j .data -j .dynamic -j .dynsym \
>  		-j .rel -j .rela -j .rel.* -j .rela.* -j .rel* -j .rela* \
> -- 
> 2.34.1
>

Any reason to not also do this for x86?

One reason I ask, is that in order for pretty_print_stacks.py to make
use of this from run_tests.sh we need a patch like

diff --git a/run_tests.sh b/run_tests.sh
index f61e0057b537..67b239f1adc7 100755
--- a/run_tests.sh
+++ b/run_tests.sh
@@ -125,8 +125,14 @@ fi
 RUNTIME_log_stderr () { process_test_output "$1"; }
 RUNTIME_log_stdout () {
     local testname="$1"
+    local kernel
+
     if [ "$PRETTY_PRINT_STACKS" = "yes" ]; then
-        local kernel="$2"
+        if [ "$CONFIG_EFI" = "y" ]; then
+            kernel="${TEST_DIR}/${2}.efi.debug"
+        else
+            kernel="$2"
+        fi
         ./scripts/pretty_print_stacks.py "$kernel" | process_test_output "$testname"
     else
         process_test_output "$testname"


We'd have to special-case that CONFIG_EFI condition for arm64 if we don't
also create .efi.debug files for x86.

Thanks,
drew
Andrew Jones June 24, 2023, 10:31 a.m. UTC | #2 
On Sat, Jun 17, 2023 at 01:49:25AM +0000, Nadav Amit wrote:
> From: Nadav Amit <namit@vmware.com>
> 
> Debugging tests that run on EFI is hard because the debug information is
> not included in the EFI file. Dump it into a separeate .debug file to
> allow the use of gdb or pretty_print_stacks script.
> 
> Signed-off-by: Nadav Amit <namit@vmware.com>
> ---
>  arm/Makefile.common | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/arm/Makefile.common b/arm/Makefile.common
> index d60cf8c..f904702 100644
> --- a/arm/Makefile.common
> +++ b/arm/Makefile.common
> @@ -69,7 +69,7 @@ FLATLIBS = $(libcflat) $(LIBFDT_archive) $(libeabi)
>  ifeq ($(CONFIG_EFI),y)
>  %.so: EFI_LDFLAGS += -defsym=EFI_SUBSYSTEM=0xa --no-undefined
>  %.so: %.o $(FLATLIBS) $(SRCDIR)/arm/efi/elf_aarch64_efi.lds $(cstart.o)
> -	$(CC) $(CFLAGS) -c -o $(@:.so=.aux.o) $(SRCDIR)/lib/auxinfo.c \
> +	$(CC) $(CFLAGS) -c -g -o $(@:.so=.aux.o) $(SRCDIR)/lib/auxinfo.c \
>  		-DPROGNAME=\"$(@:.so=.efi)\" -DAUXFLAGS=$(AUXFLAGS)
>  	$(LD) $(EFI_LDFLAGS) -o $@ -T $(SRCDIR)/arm/efi/elf_aarch64_efi.lds \
>  		$(filter %.o, $^) $(FLATLIBS) $(@:.so=.aux.o) \
> @@ -78,6 +78,9 @@ ifeq ($(CONFIG_EFI),y)
>  
>  %.efi: %.so
>  	$(call arch_elf_check, $^)
> +	$(OBJCOPY) --only-keep-debug $^ $@.debug
> +	$(OBJCOPY) --strip-debug $^
> +	$(OBJCOPY) --add-gnu-debuglink=$@.debug $^
>  	$(OBJCOPY) \
>  		-j .text -j .sdata -j .data -j .dynamic -j .dynsym \
>  		-j .rel -j .rela -j .rel.* -j .rela.* -j .rel* -j .rela* \
> -- 
> 2.34.1
>

We also need a change to arm_clean to clean these new files up. Or, since
we probably want them for x86 as well and we already have other efi
cleanup to do, then maybe we should have a common efi_clean in the top-
level Makefile which x86's and arm's clean use to clean up all efi related
files.

Thanks,
drew
Nadav Amit June 25, 2023, 7:21 p.m. UTC | #3 
> On Jun 24, 2023, at 3:31 AM, Andrew Jones <andrew.jones@linux.dev> wrote:
> 
> !! External Email
> 
> On Sat, Jun 17, 2023 at 01:49:25AM +0000, Nadav Amit wrote:
>> From: Nadav Amit <namit@vmware.com>
>> 
>> Debugging tests that run on EFI is hard because the debug information is
>> not included in the EFI file. Dump it into a separeate .debug file to
>> allow the use of gdb or pretty_print_stacks script.
>> 
>> Signed-off-by: Nadav Amit <namit@vmware.com>
>> ---
>> arm/Makefile.common | 5 ++++-
>> 1 file changed, 4 insertions(+), 1 deletion(-)
>> 
>> diff --git a/arm/Makefile.common b/arm/Makefile.common
>> index d60cf8c..f904702 100644
>> --- a/arm/Makefile.common
>> +++ b/arm/Makefile.common
>> @@ -69,7 +69,7 @@ FLATLIBS = $(libcflat) $(LIBFDT_archive) $(libeabi)
>> ifeq ($(CONFIG_EFI),y)
>> %.so: EFI_LDFLAGS += -defsym=EFI_SUBSYSTEM=0xa --no-undefined
>> %.so: %.o $(FLATLIBS) $(SRCDIR)/arm/efi/elf_aarch64_efi.lds $(cstart.o)
>> -     $(CC) $(CFLAGS) -c -o $(@:.so=.aux.o) $(SRCDIR)/lib/auxinfo.c \
>> +     $(CC) $(CFLAGS) -c -g -o $(@:.so=.aux.o) $(SRCDIR)/lib/auxinfo.c \
>>              -DPROGNAME=\"$(@:.so=.efi)\" -DAUXFLAGS=$(AUXFLAGS)
>>      $(LD) $(EFI_LDFLAGS) -o $@ -T $(SRCDIR)/arm/efi/elf_aarch64_efi.lds \
>>              $(filter %.o, $^) $(FLATLIBS) $(@:.so=.aux.o) \
>> @@ -78,6 +78,9 @@ ifeq ($(CONFIG_EFI),y)
>> 
>> %.efi: %.so
>>      $(call arch_elf_check, $^)
>> +     $(OBJCOPY) --only-keep-debug $^ $@.debug
>> +     $(OBJCOPY) --strip-debug $^
>> +     $(OBJCOPY) --add-gnu-debuglink=$@.debug $^
>>      $(OBJCOPY) \
>>              -j .text -j .sdata -j .data -j .dynamic -j .dynsym \
>>              -j .rel -j .rela -j .rel.* -j .rela.* -j .rel* -j .rela* \
>> --
>> 2.34.1
>> 
> 
> We also need a change to arm_clean to clean these new files up. Or, since
> we probably want them for x86 as well and we already have other efi
> cleanup to do, then maybe we should have a common efi_clean in the top-
> level Makefile which x86's and arm's clean use to clean up all efi related
> files.

[ I aggregate my response to both of your emails ]

Thanks!

I will create .debug files for x86, but I am going to remove the ‘-g’ option.
If anyone wants it, he would need to provide CFLAGS during configure.

I will cleanup the .debug files for each arch separately since other files
are cleanup’d that way.
diff mbox series

Patch

diff --git a/arm/Makefile.common b/arm/Makefile.common
index d60cf8c..f904702 100644
--- a/arm/Makefile.common
+++ b/arm/Makefile.common
@@ -69,7 +69,7 @@  FLATLIBS = $(libcflat) $(LIBFDT_archive) $(libeabi)
 ifeq ($(CONFIG_EFI),y)
 %.so: EFI_LDFLAGS += -defsym=EFI_SUBSYSTEM=0xa --no-undefined
 %.so: %.o $(FLATLIBS) $(SRCDIR)/arm/efi/elf_aarch64_efi.lds $(cstart.o)
-	$(CC) $(CFLAGS) -c -o $(@:.so=.aux.o) $(SRCDIR)/lib/auxinfo.c \
+	$(CC) $(CFLAGS) -c -g -o $(@:.so=.aux.o) $(SRCDIR)/lib/auxinfo.c \
 		-DPROGNAME=\"$(@:.so=.efi)\" -DAUXFLAGS=$(AUXFLAGS)
 	$(LD) $(EFI_LDFLAGS) -o $@ -T $(SRCDIR)/arm/efi/elf_aarch64_efi.lds \
 		$(filter %.o, $^) $(FLATLIBS) $(@:.so=.aux.o) \
@@ -78,6 +78,9 @@  ifeq ($(CONFIG_EFI),y)
 
 %.efi: %.so
 	$(call arch_elf_check, $^)
+	$(OBJCOPY) --only-keep-debug $^ $@.debug
+	$(OBJCOPY) --strip-debug $^
+	$(OBJCOPY) --add-gnu-debuglink=$@.debug $^
 	$(OBJCOPY) \
 		-j .text -j .sdata -j .data -j .dynamic -j .dynsym \
 		-j .rel -j .rela -j .rel.* -j .rela.* -j .rel* -j .rela* \