Re: [XEN PATCH v3 06/25] tools/fuzz/x86_instruction_emulator: rework makefile

[Anthony PERARD <anthony.perard@citrix.com>]

On Mon, Jul 11, 2022 at 04:08:55PM +0200, Jan Beulich wrote:
> On 24.06.2022 18:04, Anthony PERARD wrote:
> > --- a/tools/fuzz/x86_instruction_emulator/Makefile
> > +++ b/tools/fuzz/x86_instruction_emulator/Makefile
> > @@ -8,33 +8,27 @@ else
> >  x86-insn-fuzz-all:
> >  endif
> >  
> > -# Add libx86 to the build
> > -vpath %.c $(XEN_ROOT)/xen/lib/x86
> > +cpuid.c: %: $(XEN_ROOT)/xen/lib/x86/% FORCE
> > +	ln -nsf $< $@
> 
> I guess the idea with the original construct was to allow using further
> source files from libx86 with as little code churn as possible. Your
> change now requires two more lines to be touched. As long as we avoid
> name collisions in the various directories (wrapper.c and a few more
> files come from yet somewhere else), couldn't this rule simply be
> 
> %.c: $(XEN_ROOT)/xen/lib/x86/%.c FORCE
> 	ln -nsf $< $@
> 
> ?

Sounds good.

> > -x86_emulate:
> > -	[ -L $@ ] || ln -sf $(XEN_ROOT)/xen/arch/x86/$@
> > +x86_emulate: FORCE
> > +	ln -nsf $(XEN_ROOT)/xen/arch/x86/$@
> >  
> >  x86_emulate/%: x86_emulate ;
> >  
> > -x86-emulate.c x86-emulate.h wrappers.c: %:
> > -	[ -L $* ] || ln -sf $(XEN_ROOT)/tools/tests/x86_emulator/$*
> > +x86-emulate.c x86-emulate.h wrappers.c: %: $(XEN_ROOT)/tools/tests/x86_emulator/% FORCE
> > +	ln -nsf $< $@
> 
> And similarly
> 
> %.c: $(XEN_ROOT)/tools/tests/x86_emulator/%.c FORCE
> 	ln -nsf $< $@
> 
> %.h: $(XEN_ROOT)/tools/tests/x86_emulator/%.h FORCE
> 	ln -nsf $< $@
> 
> here? (I'm hesitant to suggest plain %, i.e. without the filename
> suffixes, as that would likely be at least confusing for Makefile.)

Will do.

> > @@ -67,3 +61,5 @@ afl: afl-harness
> >  
> >  .PHONY: afl-cov
> >  afl-cov: afl-harness-cov
> > +
> > +-include $(DEPS_INCLUDE)
> 
> I would expect doing so was avoided for some reason. Albeit it may
> well be that too much cloning of tests/x86_emulator was done here,

There's quite a few places in tools/ where "-include $(DEPS_INCLUDE)" is
missing, so I kind of expect it to be forgotten rather than avoided on
purpose.

> and it's all fine this way. Can you confirm things to work when
> building locally in just this subdir, e.g. via
> 
> make -sC .../tools/fuzz/x86_instruction_emulator CC=/build/afl/2.52b-base/afl-gcc
> 
> ?

Yes, that still works. But I'm not sure why you would ask since the
tools/ build system works this way, execution of make in a subdir
doesn't depends on the execution from the parent dir (it doesn't
depends on anything in the envvar).

Thanks,

-- 
Anthony PERARD