At the moment, trying to run xencov read/reset (calling SYSCTL_coverage_op
under the hood) results in a crash. This is due to a profiler trying to
access data in the .init.* sections (libfdt for Arm and libelf for x86)
that are stripped after boot. Normally, the build system compiles any
*.init.o file without COV_FLAGS. However, these two libraries are
handled differently as sections will be renamed to init after linking.

To override COV_FLAGS to empty for these libraries, lib{fdt,elf}.o were
added to nocov-y. This worked until e321576f4047 ("xen/build: start using
if_changed") that added lib{fdt,elf}-temp.o and their deps to extra-y.
This way, even though these objects appear as prerequisites of
lib{fdt,elf}.o and the settings should propagate to them, make can also
build them as a prerequisite of __build, in which case COV_FLAGS would
still have the unwanted flags. Fix it by switching to $(targets) instead.

Also, for libfdt, append libfdt.o to nocov-y only if CONFIG_OVERLAY_DTB
is not set. Otherwise, there is no section renaming and we should be able
to run the coverage.

Fixes: e321576f4047 ("xen/build: start using if_changed")
Signed-off-by: Michal Orzel <michal.orzel@amd.com>
---
Changes in v2:
 - was "coverage: filter out lib{fdt,elf}-temp.o"
 - switch to $(targets), update rationale
---
 xen/common/libelf/Makefile | 2 +-
 xen/common/libfdt/Makefile | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/xen/common/libelf/Makefile b/xen/common/libelf/Makefile
index 8a4522e4e141..917d12b006f7 100644
--- a/xen/common/libelf/Makefile
+++ b/xen/common/libelf/Makefile
@@ -13,4 +13,4 @@ $(obj)/libelf.o: $(obj)/libelf-temp.o FORCE
 $(obj)/libelf-temp.o: $(addprefix $(obj)/,$(libelf-objs)) FORCE
 	$(call if_changed,ld)
 
-extra-y += libelf-temp.o $(libelf-objs)
+targets += libelf-temp.o $(libelf-objs)
diff --git a/xen/common/libfdt/Makefile b/xen/common/libfdt/Makefile
index d50487aa6e32..6ce679f98f47 100644
--- a/xen/common/libfdt/Makefile
+++ b/xen/common/libfdt/Makefile
@@ -5,10 +5,10 @@ SECTIONS := text data $(SPECIAL_DATA_SECTIONS)
 # For CONFIG_OVERLAY_DTB, libfdt functionalities will be needed during runtime.
 ifneq ($(CONFIG_OVERLAY_DTB),y)
 OBJCOPYFLAGS := $(foreach s,$(SECTIONS),--rename-section .$(s)=.init.$(s))
+nocov-y += libfdt.o
 endif
 
 obj-y += libfdt.o
-nocov-y += libfdt.o
 
 CFLAGS-y += -I$(srctree)/include/xen/libfdt/
 
@@ -18,4 +18,4 @@ $(obj)/libfdt.o: $(obj)/libfdt-temp.o FORCE
 $(obj)/libfdt-temp.o: $(addprefix $(obj)/,$(LIBFDT_OBJS)) FORCE
 	$(call if_changed,ld)
 
-extra-y += libfdt-temp.o $(LIBFDT_OBJS)
+targets += libfdt-temp.o $(LIBFDT_OBJS)
-- 
2.25.1

On Mon, Jan 22, 2024 at 12:39:55PM +0100, Michal Orzel wrote:
> At the moment, trying to run xencov read/reset (calling SYSCTL_coverage_op
> under the hood) results in a crash. This is due to a profiler trying to
> access data in the .init.* sections (libfdt for Arm and libelf for x86)
> that are stripped after boot. Normally, the build system compiles any
> *.init.o file without COV_FLAGS. However, these two libraries are
> handled differently as sections will be renamed to init after linking.
> 
> To override COV_FLAGS to empty for these libraries, lib{fdt,elf}.o were
> added to nocov-y. This worked until e321576f4047 ("xen/build: start using
> if_changed") that added lib{fdt,elf}-temp.o and their deps to extra-y.
> This way, even though these objects appear as prerequisites of
> lib{fdt,elf}.o and the settings should propagate to them, make can also
> build them as a prerequisite of __build, in which case COV_FLAGS would
> still have the unwanted flags. Fix it by switching to $(targets) instead.
> 
> Also, for libfdt, append libfdt.o to nocov-y only if CONFIG_OVERLAY_DTB
> is not set. Otherwise, there is no section renaming and we should be able
> to run the coverage.
> 
> Fixes: e321576f4047 ("xen/build: start using if_changed")
> Signed-off-by: Michal Orzel <michal.orzel@amd.com>

Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>

Thanks,

-- 
Anthony PERARD

On 22.01.2024 15:46, Anthony PERARD wrote:
> On Mon, Jan 22, 2024 at 12:39:55PM +0100, Michal Orzel wrote:
>> At the moment, trying to run xencov read/reset (calling SYSCTL_coverage_op
>> under the hood) results in a crash. This is due to a profiler trying to
>> access data in the .init.* sections (libfdt for Arm and libelf for x86)
>> that are stripped after boot. Normally, the build system compiles any
>> *.init.o file without COV_FLAGS. However, these two libraries are
>> handled differently as sections will be renamed to init after linking.
>>
>> To override COV_FLAGS to empty for these libraries, lib{fdt,elf}.o were
>> added to nocov-y. This worked until e321576f4047 ("xen/build: start using
>> if_changed") that added lib{fdt,elf}-temp.o and their deps to extra-y.
>> This way, even though these objects appear as prerequisites of
>> lib{fdt,elf}.o and the settings should propagate to them, make can also
>> build them as a prerequisite of __build, in which case COV_FLAGS would
>> still have the unwanted flags. Fix it by switching to $(targets) instead.
>>
>> Also, for libfdt, append libfdt.o to nocov-y only if CONFIG_OVERLAY_DTB
>> is not set. Otherwise, there is no section renaming and we should be able
>> to run the coverage.
>>
>> Fixes: e321576f4047 ("xen/build: start using if_changed")
>> Signed-off-by: Michal Orzel <michal.orzel@amd.com>
> 
> Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>

Acked-by: Jan Beulich <jbeulich@suse.com>