| Message ID | 20170201120239.13993-5-wei.liu2@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 01.02.17 at 13:02, <wei.liu2@citrix.com> wrote: > --- a/tools/fuzz/x86_instruction_emulator/Makefile > +++ b/tools/fuzz/x86_instruction_emulator/Makefile > @@ -11,10 +11,15 @@ endif > x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h: > [ -L x86_emulate ] || ln -sf $(XEN_ROOT)/xen/arch/x86/x86_emulate . > > +asm/x86-vendors.h: > + [ -L asm ] || ln -sf $(XEN_ROOT)/xen/include/asm-x86 asm Hmm, that's making all headers available. I don't really like it to be that broad. Andrew, what do you think > -CFLAGS += $(CFLAGS_xeninclude) -D__XEN_TOOLS__ > +CFLAGS += $(CFLAGS_xeninclude) -D__XEN_TOOLS__ -I. > + > +x86_emulate.h: asm/x86-vendors.h That's not a true dependency; I'd rather see this build on the suggested macroization (i.e. introduce it here too, and do the same for the non-fuzzing harness). Jan
On 01/02/17 13:09, Jan Beulich wrote: >>>> On 01.02.17 at 13:02, <wei.liu2@citrix.com> wrote: >> --- a/tools/fuzz/x86_instruction_emulator/Makefile >> +++ b/tools/fuzz/x86_instruction_emulator/Makefile >> @@ -11,10 +11,15 @@ endif >> x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h: >> [ -L x86_emulate ] || ln -sf $(XEN_ROOT)/xen/arch/x86/x86_emulate . >> >> +asm/x86-vendors.h: >> + [ -L asm ] || ln -sf $(XEN_ROOT)/xen/include/asm-x86 asm > Hmm, that's making all headers available. I don't really like it to > be that broad. Andrew, what do you think This is just a test utility. I think we can trust ourselves to only include appropriate headers. This is simpler and easier than keeping a whitelist of permitted headers, which doesn't require any modification when we add a new header. ~Andrew
On Wed, Feb 01, 2017 at 01:13:44PM +0000, Andrew Cooper wrote: > On 01/02/17 13:09, Jan Beulich wrote: > >>>> On 01.02.17 at 13:02, <wei.liu2@citrix.com> wrote: > >> --- a/tools/fuzz/x86_instruction_emulator/Makefile > >> +++ b/tools/fuzz/x86_instruction_emulator/Makefile > >> @@ -11,10 +11,15 @@ endif > >> x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h: > >> [ -L x86_emulate ] || ln -sf $(XEN_ROOT)/xen/arch/x86/x86_emulate . > >> > >> +asm/x86-vendors.h: > >> + [ -L asm ] || ln -sf $(XEN_ROOT)/xen/include/asm-x86 asm > > Hmm, that's making all headers available. I don't really like it to > > be that broad. Andrew, what do you think > > This is just a test utility. I think we can trust ourselves to only > include appropriate headers. > > This is simpler and easier than keeping a whitelist of permitted > headers, which doesn't require any modification when we add a new header. > Yes, this is exactly the reason why I did it like this. Wei. > ~Andrew
>>> On 01.02.17 at 14:13, <andrew.cooper3@citrix.com> wrote: > On 01/02/17 13:09, Jan Beulich wrote: >>>>> On 01.02.17 at 13:02, <wei.liu2@citrix.com> wrote: >>> --- a/tools/fuzz/x86_instruction_emulator/Makefile >>> +++ b/tools/fuzz/x86_instruction_emulator/Makefile >>> @@ -11,10 +11,15 @@ endif >>> x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h: >>> [ -L x86_emulate ] || ln -sf $(XEN_ROOT)/xen/arch/x86/x86_emulate . >>> >>> +asm/x86-vendors.h: >>> + [ -L asm ] || ln -sf $(XEN_ROOT)/xen/include/asm-x86 asm >> Hmm, that's making all headers available. I don't really like it to >> be that broad. Andrew, what do you think > > This is just a test utility. I think we can trust ourselves to only > include appropriate headers. > > This is simpler and easier than keeping a whitelist of permitted > headers, which doesn't require any modification when we add a new header. Well, okay then (albeit this "doesn't require any modification" doesn't hold with the dependency the patch also adds; perhaps midterm we should arrange for the compiler to generate dependencies for us). Jan
diff --git a/tools/fuzz/x86_instruction_emulator/Makefile b/tools/fuzz/x86_instruction_emulator/Makefile index f2bb12e871..d5c5bba0b0 100644 --- a/tools/fuzz/x86_instruction_emulator/Makefile +++ b/tools/fuzz/x86_instruction_emulator/Makefile @@ -11,10 +11,15 @@ endif x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h: [ -L x86_emulate ] || ln -sf $(XEN_ROOT)/xen/arch/x86/x86_emulate . +asm/x86-vendors.h: + [ -L asm ] || ln -sf $(XEN_ROOT)/xen/include/asm-x86 asm + x86_emulate.c x86_emulate.h: %: [ -L $* ] || ln -sf $(XEN_ROOT)/tools/tests/x86_emulator/$* -CFLAGS += $(CFLAGS_xeninclude) -D__XEN_TOOLS__ +CFLAGS += $(CFLAGS_xeninclude) -D__XEN_TOOLS__ -I. + +x86_emulate.h: asm/x86-vendors.h x86_emulate.o: x86_emulate.c x86_emulate.h x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h @@ -31,7 +36,7 @@ all: x86-instruction-emulator-fuzzer-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 asm .PHONY: clean clean: diff --git a/tools/tests/x86_emulator/Makefile b/tools/tests/x86_emulator/Makefile index 883daf30a3..bd1c4d664e 100644 --- a/tools/tests/x86_emulator/Makefile +++ b/tools/tests/x86_emulator/Makefile @@ -32,7 +32,7 @@ $(TARGET): x86_emulate.o test_x86_emulator.o .PHONY: clean clean: - rm -rf $(TARGET) *.o *~ core blowfish.h blowfish.bin x86_emulate + rm -rf $(TARGET) *.o *~ core blowfish.h blowfish.bin x86_emulate asm .PHONY: distclean distclean: clean @@ -43,7 +43,12 @@ install: x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h: [ -L x86_emulate ] || ln -sf $(XEN_ROOT)/xen/arch/x86/x86_emulate . -HOSTCFLAGS += $(CFLAGS_xeninclude) +asm/x86-vendors.h: + [ -L asm ] || ln -sf $(XEN_ROOT)/xen/include/asm-x86 asm + +x86_emulate.h: asm/x86-vendors.h + +HOSTCFLAGS += $(CFLAGS_xeninclude) -I. x86_emulate.o: x86_emulate.c x86_emulate.h x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h $(HOSTCC) $(HOSTCFLAGS) -D__XEN_TOOLS__ -c -g -o $@ $< diff --git a/tools/tests/x86_emulator/x86_emulate.h b/tools/tests/x86_emulator/x86_emulate.h index 3a6badee46..db287004e6 100644 --- a/tools/tests/x86_emulator/x86_emulate.h +++ b/tools/tests/x86_emulator/x86_emulate.h @@ -6,6 +6,8 @@ #include <string.h> #include <xen/xen.h> +#include <asm/x86-vendors.h> + #define BUG() abort() #define ASSERT assert #define ASSERT_UNREACHABLE() assert(!__LINE__) @@ -38,11 +40,6 @@ #define is_canonical_address(x) (((int64_t)(x) >> 47) == ((int64_t)(x) >> 63)) -/* There's no strict need for these to be in sync with processor.h. */ -#define X86_VENDOR_INTEL 0 -#define X86_VENDOR_AMD 2 -#define X86_VENDOR_UNKNOWN 0xff - #define MMAP_SZ 16384 bool emul_test_make_stack_executable(void);
No functional change. Signed-off-by: Wei Liu <wei.liu2@citrix.com> --- Cc: Jan Beulich <jbeulich@suse.com> Cc: Andrew Cooper <andrew.cooper3@citrix.com> V3: link asm-x86 to working directory --- tools/fuzz/x86_instruction_emulator/Makefile | 9 +++++++-- tools/tests/x86_emulator/Makefile | 9 +++++++-- tools/tests/x86_emulator/x86_emulate.h | 7 ++----- 3 files changed, 16 insertions(+), 9 deletions(-)