Xen EFI build system and gcov

Xen EFI build system and gcov

[Wei Liu]

Hi Jan

Today I had some free cycles so I spent some time looking at gcov
support in the hypervisor and tried to write a patch to fix the
currently broken gcov build. But my patch alone is not enough to fix
that.

There seems to be a problem with the EFI Makefile. With my patch
applied, efi/boot.init.o still gets all gcov options _and_
-DINIT_SECTIONS_ONLY. See output and patch for more context.

If I force efi to be disabled by putting in a hack into efi/Makefile,
Xen builds fine. It suggests for all other files my patch works.

I am confused why efi/boot.init.o gets both set of options and I'm not
sure what is the best approach to fix it.

Do you have any suggestion on fixing that?

Thanks
Wei.

gcc ... -fprofile-arcs -ftest-coverage -DTEST_COVERAGE -DINIT_SECTIONS_ONLY -c boot.c -o boot.o
objdump -h boot.o | sed -n '/[0-9]/{s,00*,0,g;p;}' | while read idx name sz rest; do \
	case "$name" in \
	.*.local) ;; \
	.text|.text.*|.data|.data.*|.bss) \
		test $sz != 0 || continue; \
		echo "Error: size of boot.o:$name is 0x$sz" >&2; \
		exit $(expr $idx + 1);; \
	esac; \
done
Error: size of boot.o:.text is 0x012
/local/work/xen.git/xen/Rules.mk:184: recipe for target 'boot.init.o' failed

---8<---
From 36c9fe8274187c6f8a4caa6f6b9c354ca9db2866 Mon Sep 17 00:00:00 2001
From: Wei Liu <wei.liu2@citrix.com>
Date: Mon, 29 Aug 2016 19:11:59 +0100
Subject: [PATCH] xen: fix gcov compilation

Currently enabling gcov in hypervisor won't build because although
26c9d03d ("gcov: Adding support for coverage information") claimed that
%.init.o files were excluded from applying compilation options, it was
in fact not true.

Fix that by filtering out the options correctly.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 xen/Rules.mk | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/xen/Rules.mk b/xen/Rules.mk
index a190ff0..e49566c 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -115,7 +115,9 @@ subdir-all := $(subdir-y) $(subdir-n)
 
 $(filter %.init.o,$(obj-y) $(obj-bin-y) $(extra-y)): CFLAGS += -DINIT_SECTIONS_ONLY
 
-$(obj-$(coverage)): CFLAGS += -fprofile-arcs -ftest-coverage -DTEST_COVERAGE
+ifeq ($(coverage),y)
+$(filter-out %.init.o,$(obj-y) $(obj-bin-y) $(extra-y)): CFLAGS += -fprofile-arcs -ftest-coverage -DTEST_COVERAGE
+endif
 
 ifeq ($(lto),y)
 # Would like to handle all object files as bitcode, but objects made from
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: Xen EFI build system and gcov

[Jan Beulich]

>>> On 29.08.16 at 20:41, <wei.liu2@citrix.com> wrote:
> Hi Jan
> 
> Today I had some free cycles so I spent some time looking at gcov
> support in the hypervisor and tried to write a patch to fix the
> currently broken gcov build. But my patch alone is not enough to fix
> that.
> 
> There seems to be a problem with the EFI Makefile. With my patch
> applied, efi/boot.init.o still gets all gcov options _and_
> -DINIT_SECTIONS_ONLY. See output and patch for more context.
> 
> If I force efi to be disabled by putting in a hack into efi/Makefile,
> Xen builds fine. It suggests for all other files my patch works.
> 
> I am confused why efi/boot.init.o gets both set of options and I'm not
> sure what is the best approach to fix it.

That's a result of

stub.o: $(extra-y)

since ...

> --- a/xen/Rules.mk
> +++ b/xen/Rules.mk
> @@ -115,7 +115,9 @@ subdir-all := $(subdir-y) $(subdir-n)
>  
>  $(filter %.init.o,$(obj-y) $(obj-bin-y) $(extra-y)): CFLAGS += 
> -DINIT_SECTIONS_ONLY
>  
> -$(obj-$(coverage)): CFLAGS += -fprofile-arcs -ftest-coverage -DTEST_COVERAGE
> +ifeq ($(coverage),y)
> +$(filter-out %.init.o,$(obj-y) $(obj-bin-y) $(extra-y)): CFLAGS += -fprofile-arcs -ftest-coverage -DTEST_COVERAGE
> +endif

... such constructs apply to the target as well as all of its prereqs.
Since the stub.o. dependency can't be eliminated easily (or else all
the objects listed in extra-y won't get built anymore), perhaps you
simply need to exclude stub.o from being compiler with coverage
options as well. Since it might well be that other exceptions become
necessary going forward, how about you introduce a nogcov-y
variable (subject to name improvement of course) and include that
in the $(filter-out ) above? This

--- unstable.orig/xen/Rules.mk
+++ unstable/xen/Rules.mk
@@ -115,7 +115,9 @@ subdir-all := $(subdir-y) $(subdir-n)
 
 $(filter %.init.o,$(obj-y) $(obj-bin-y) $(extra-y)): CFLAGS += -DINIT_SECTIONS_ONLY
 
-$(obj-$(coverage)): CFLAGS += -fprofile-arcs -ftest-coverage -DTEST_COVERAGE
+ifeq ($(coverage),y)
+$(filter-out %.init.o $(nogcov-y),$(obj-y) $(obj-bin-y) $(extra-y)): CFLAGS += -fprofile-arcs -ftest-coverage -DTEST_COVERAGE
+endif
 
 ifeq ($(lto),y)
 # Would like to handle all object files as bitcode, but objects made from
--- unstable.orig/xen/arch/x86/efi/Makefile
+++ unstable/xen/arch/x86/efi/Makefile
@@ -15,3 +15,4 @@ extra-$(efi) += boot.init.o relocs-dummy
 	$(OBJCOPY) -I ihex -O binary $< $@
 
 stub.o: $(extra-y)
+nogcov-$(efi) := stub.o


works for me (albeit there's then a significant number of duplicate
symbol names getting warned about, which may be a separate thing
in need of taking care of).

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: Xen EFI build system and gcov

[Wei Liu]

On Tue, Aug 30, 2016 at 02:38:44AM -0600, Jan Beulich wrote:
> >>> On 29.08.16 at 20:41, <wei.liu2@citrix.com> wrote:
> > Hi Jan
> > 
> > Today I had some free cycles so I spent some time looking at gcov
> > support in the hypervisor and tried to write a patch to fix the
> > currently broken gcov build. But my patch alone is not enough to fix
> > that.
> > 
> > There seems to be a problem with the EFI Makefile. With my patch
> > applied, efi/boot.init.o still gets all gcov options _and_
> > -DINIT_SECTIONS_ONLY. See output and patch for more context.
> > 
> > If I force efi to be disabled by putting in a hack into efi/Makefile,
> > Xen builds fine. It suggests for all other files my patch works.
> > 
> > I am confused why efi/boot.init.o gets both set of options and I'm not
> > sure what is the best approach to fix it.
> 
> That's a result of
> 
> stub.o: $(extra-y)
> 
> since ...
> 
> > --- a/xen/Rules.mk
> > +++ b/xen/Rules.mk
> > @@ -115,7 +115,9 @@ subdir-all := $(subdir-y) $(subdir-n)
> >  
> >  $(filter %.init.o,$(obj-y) $(obj-bin-y) $(extra-y)): CFLAGS += 
> > -DINIT_SECTIONS_ONLY
> >  
> > -$(obj-$(coverage)): CFLAGS += -fprofile-arcs -ftest-coverage -DTEST_COVERAGE
> > +ifeq ($(coverage),y)
> > +$(filter-out %.init.o,$(obj-y) $(obj-bin-y) $(extra-y)): CFLAGS += -fprofile-arcs -ftest-coverage -DTEST_COVERAGE
> > +endif
> 
> ... such constructs apply to the target as well as all of its prereqs.

Right. Thanks for the explanation.

> Since the stub.o. dependency can't be eliminated easily (or else all
> the objects listed in extra-y won't get built anymore), perhaps you
> simply need to exclude stub.o from being compiler with coverage
> options as well. Since it might well be that other exceptions become
> necessary going forward, how about you introduce a nogcov-y
> variable (subject to name improvement of course) and include that
> in the $(filter-out ) above? This
> 
> --- unstable.orig/xen/Rules.mk
> +++ unstable/xen/Rules.mk
> @@ -115,7 +115,9 @@ subdir-all := $(subdir-y) $(subdir-n)
>  
>  $(filter %.init.o,$(obj-y) $(obj-bin-y) $(extra-y)): CFLAGS += -DINIT_SECTIONS_ONLY
>  
> -$(obj-$(coverage)): CFLAGS += -fprofile-arcs -ftest-coverage -DTEST_COVERAGE
> +ifeq ($(coverage),y)
> +$(filter-out %.init.o $(nogcov-y),$(obj-y) $(obj-bin-y) $(extra-y)): CFLAGS += -fprofile-arcs -ftest-coverage -DTEST_COVERAGE
> +endif
>  
>  ifeq ($(lto),y)
>  # Would like to handle all object files as bitcode, but objects made from
> --- unstable.orig/xen/arch/x86/efi/Makefile
> +++ unstable/xen/arch/x86/efi/Makefile
> @@ -15,3 +15,4 @@ extra-$(efi) += boot.init.o relocs-dummy
>  	$(OBJCOPY) -I ihex -O binary $< $@
>  
>  stub.o: $(extra-y)
> +nogcov-$(efi) := stub.o
> 
> 
> works for me (albeit there's then a significant number of duplicate
> symbol names getting warned about, which may be a separate thing
> in need of taking care of).
> 

FWIW I also investigated how Linux does this. It has a rather
complex system for generating Makefile rules, but in the end it still
requires developers to manually mark which portion (either directory or
files) of the kernel can't be built with gcov.

So I think your approach is fine.  I will incorporate your change to my
patch and stick your SoB in too.

Wei.

> Jan
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel