| Message ID | 20220624160422.53457-7-anthony.perard@citrix.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Toolstack build system improvement, toward non-recursive makefiles | expand |
> On 24 Jun 2022, at 17:04, Anthony PERARD <anthony.perard@citrix.com> wrote: > > Rework dependencies of all objects. We don't need to add dependencies > for headers that $(CC) is capable of generating, we only need to > include $(DEPS_INCLUDE). Some dependencies are still needed so make > knows to generate symlinks for them. > > We remove the use of "vpath" for cpuid.c. While it works fine for now, > when we will convert this makefile to subdirmk, vpath will not be > usable. Also, "-iquote" is now needed to build "cpuid.o". > > Replace "-I." by "-iquote .", so it applies to double-quote includes > only. > > Rather than checking if a symlink exist, always regenerate the > symlink. So if the source tree changed location, the symlink is > updated. > > Since we are creating a new .gitignore for the symlink, also move the > entry to it. > > Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> > — Hi Antony, Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>
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 $< $@ ? > -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.) > @@ -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, 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 ? Jan
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,
On 02.08.2022 19:09, Anthony PERARD wrote: > On Mon, Jul 11, 2022 at 04:08:55PM +0200, Jan Beulich wrote: >> 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). Oh, I wasn't even aware of this as a general feature in tools/. Is this going to survive your rework to use non-recursive makefiles? Jan
On Wed, Aug 03, 2022 at 07:56:34AM +0200, Jan Beulich wrote: > On 02.08.2022 19:09, Anthony PERARD wrote: > > On Mon, Jul 11, 2022 at 04:08:55PM +0200, Jan Beulich wrote: > >> 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). > > Oh, I wasn't even aware of this as a general feature in tools/. Is > this going to survive your rework to use non-recursive makefiles? Yes. Executing `make` in any subdir will still works. It should be even better than the current situation, even in a fresh clone, we could simply run `./configure && make -C subdir` and make will make everything needed, that is probably the "include/" dir and the "libs/" if they are needed by anything in that subdir. At the moment, one can run ./configure then `make -C tools/include` and `make -C tools/libs` and probably run make in most subdir without having to wait for a long run of `make build-tools`. Cheers,
diff --git a/tools/fuzz/x86_instruction_emulator/Makefile b/tools/fuzz/x86_instruction_emulator/Makefile index 1a6dbf94e1..f11437e6a2 100644 --- 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 $< $@ -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 $< $@ -CFLAGS += $(CFLAGS_xeninclude) -D__XEN_TOOLS__ -I. +CFLAGS += $(CFLAGS_xeninclude) -D__XEN_TOOLS__ -iquote . +cpuid.o: CFLAGS += -iquote $(XEN_ROOT)/xen/lib/x86 GCOV_FLAGS := --coverage %-cov.o: %.c $(CC) -c $(CFLAGS) $(GCOV_FLAGS) $< -o $@ -x86.h := $(addprefix $(XEN_ROOT)/tools/include/xen/asm/,\ - x86-vendors.h x86-defns.h msr-index.h) \ - $(addprefix $(XEN_ROOT)/tools/include/xen/lib/x86/, \ - cpuid.h cpuid-autogen.h) -x86_emulate.h := x86-emulate.h x86_emulate/x86_emulate.h $(x86.h) - -# x86-emulate.c will be implicit for both -x86-emulate.o x86-emulate-cov.o: x86_emulate/x86_emulate.c $(x86_emulate.h) - -fuzz-emul.o fuzz-emulate-cov.o cpuid.o wrappers.o: $(x86_emulate.h) +x86-emulate.h: x86_emulate/x86_emulate.h +x86-emulate.o x86-emulate-cov.o: x86-emulate.h x86_emulate/x86_emulate.c +fuzz-emul.o fuzz-emul-cov.o wrappers.o: x86-emulate.h x86-insn-fuzzer.a: fuzz-emul.o x86-emulate.o cpuid.o $(AR) rc $@ $^ @@ -51,7 +45,7 @@ all: x86-insn-fuzz-all .PHONY: distclean distclean: clean - rm -f x86_emulate x86-emulate.c x86-emulate.h + rm -f x86_emulate x86-emulate.c x86-emulate.h wrappers.c cpuid.c .PHONY: clean clean: @@ -67,3 +61,5 @@ afl: afl-harness .PHONY: afl-cov afl-cov: afl-harness-cov + +-include $(DEPS_INCLUDE) diff --git a/.gitignore b/.gitignore index 6410dfbc72..8b6886f3fd 100644 --- a/.gitignore +++ b/.gitignore @@ -195,12 +195,6 @@ tools/flask/utils/flask-loadpolicy tools/flask/utils/flask-setenforce tools/flask/utils/flask-set-bool tools/flask/utils/flask-label-pci -tools/fuzz/x86_instruction_emulator/asm -tools/fuzz/x86_instruction_emulator/afl-harness -tools/fuzz/x86_instruction_emulator/afl-harness-cov -tools/fuzz/x86_instruction_emulator/wrappers.c -tools/fuzz/x86_instruction_emulator/x86_emulate -tools/fuzz/x86_instruction_emulator/x86-emulate.[ch] tools/helpers/init-xenstore-domain tools/helpers/xen-init-dom0 tools/hotplug/common/hotplugpath.sh diff --git a/tools/fuzz/x86_instruction_emulator/.gitignore b/tools/fuzz/x86_instruction_emulator/.gitignore new file mode 100644 index 0000000000..65c3cf9702 --- /dev/null +++ b/tools/fuzz/x86_instruction_emulator/.gitignore @@ -0,0 +1,7 @@ +/asm +/afl-harness +/afl-harness-cov +/cpuid.c +/wrappers.c +/x86_emulate +/x86-emulate.[ch]
Rework dependencies of all objects. We don't need to add dependencies for headers that $(CC) is capable of generating, we only need to include $(DEPS_INCLUDE). Some dependencies are still needed so make knows to generate symlinks for them. We remove the use of "vpath" for cpuid.c. While it works fine for now, when we will convert this makefile to subdirmk, vpath will not be usable. Also, "-iquote" is now needed to build "cpuid.o". Replace "-I." by "-iquote .", so it applies to double-quote includes only. Rather than checking if a symlink exist, always regenerate the symlink. So if the source tree changed location, the symlink is updated. Since we are creating a new .gitignore for the symlink, also move the entry to it. Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> --- Notes: v2: - create a new per-directory .gitignore to add the new entry and existing ones tools/fuzz/x86_instruction_emulator/Makefile | 32 ++++++++----------- .gitignore | 6 ---- .../fuzz/x86_instruction_emulator/.gitignore | 7 ++++ 3 files changed, 21 insertions(+), 24 deletions(-) create mode 100644 tools/fuzz/x86_instruction_emulator/.gitignore