Re: [PATCH] coverage: filter out lib{fdt,elf}-temp.o

[Michal Orzel <michal.orzel@xxxxxxx>]

Hi Anthony,

On 18/01/2024 18:37, Anthony PERARD wrote:
> 
> 
> On Thu, Jan 18, 2024 at 02:12:21PM +0100, Jan Beulich wrote:
>> On 18.01.2024 13:06, 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 an attempt to
>>> access code 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.
>>>
>>> This worked until e321576f4047 ("xen/build: start using if_changed")
>>> that added lib{fdt,elf}-temp.o to extra-y. Any file listed there without
>>> *.init.o suffix will be part of non-init-objects for which COV_FLAGS
>>> will be appended.
>>
>> While this is true, aiui COV_FLAGS would be empty for anything listed
>> in nocov-y and all of the prerequisites of those objects (iirc target-
>> specific variable settings propagate to prerequisites). Therefore ...
>>
>>> In such case, the solution is to add a file to nocov-y.
>>
>> ... libelf.o / libfdt.o already being listed there ought to suffice.
>> Alternatively listing only libelf-temp.o / libfdt-temp.o ought to
>> suffice as well.
>>
>> Since you apparently observed things not working, I must be missing
>> something.
> 
> Yes, $(extra-y) is like $(obj-y), but objects there will not be added
> "built_in.o". So, make is likely building "libelf-temp.o" and deps
> because it's in $(extra-y) rather than because it's a prerequisite of
> "libelf.o". We could ask make to process prerequisite in a reverse
> order, and suddenly, the command line to make all "libelf-*.o" is
> different: `make --shuffle=reverse V=2`.
That's helpful.

> 
> So, adding extra object to $(nocov-y) is a workaround, but I think a
> better fix would be to add those objects to $(targets) instead of
> $(extra-y). I think I've made a mistake by using $(extra-y) instead of
> $(targets) in that original commit.
I can confirm that by moving lib{fdt,elf}-temp.o and deps to targets, the issue 
is gone as well.

Is my understanding correct that by switching from extra-y to targets we are 
preventing these objects to
appear in non-init-objects (and thus having COV_FLAGS appended) while retaining 
the proper if_changed behavior?

According to docs/misc/xen-makefiles/makefiles.rst:
Any target that utilises if_changed must be listed in $(targets),
otherwise the command line check will fail, and the target will
always be built.

~Michal