| Message ID | a67c2fa3-ba1c-3783-c3ee-250aff6903d5@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | x86emul: rework wrapping of libc functions in test and fuzzing harnesses | expand |
On 17/08/2023 12:47 pm, Jan Beulich wrote: > Our present approach is working fully behind the compiler's back. This > was found to not work with LTO. Employ ld's --wrap= option instead. Note > that while this makes the build work at least with new enough gcc (it > doesn't with gcc7, for example, due to tool chain side issues afaict), > according to my testing things still won't work when building the > fuzzing harness with afl-cc: While with the gcc7 tool chain I see afl-as > getting invoked, this does not happen with gcc13. Yet without using that > assembler wrapper the resulting binary will look uninstrumented to > afl-fuzz. > > While checking the resulting binaries I noticed that we've gained uses > of snprintf() and strstr(), which only just so happen to not cause any > problems. Add a wrappers for them as well. > > Since we don't have any actual uses of v{,sn}printf(), no definitions of > their wrappers appear (just yet). But I think we want > __wrap_{,sn}printf() to properly use __real_v{,sn}printf() right away, > which means we need delarations of the latter. > > Reported-by: Andrew Cooper <andrew.cooper3@citrix.com> > Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com> > Signed-off-by: Jan Beulich <jbeulich@suse.com> This does resolve the build issue. I do get a binary out of the end, so Tested-by: Andrew Cooper <andrew.cooper3@citrix.com>. I presume that you've smoke tested the resulting binary? However, I do see something else in the logs which is concerning. Likely unrelated. make[6]: Entering directory '/builddir/build/BUILD/xen-4.18.0/tools/tests/x86_emulator' gcc -m32 -march=i686 -DBUILD_ID -fno-strict-aliasing -std=gnu99 -Wall -Wstrict-prototypes -Wdeclaration-after-statement -Wno-unused-but-set-variable -Wno-unused-local-typedefs -g3 -Werror -Og -fno-omit-frame-pointer -D__XEN_INTERFACE_VERSION__=__XEN_LATEST_INTERFACE_VERSION__ -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -mno-tls-direct-seg-refs -fno-pie -fno-stack-protector -fno-exceptions -fno-asynchronous-unwind-tables -fno-builtin -g0 -D_64f2 -mavx512fp16 -ffixed-xmm0 -Os -DVEC_SIZE=64 -DFLOAT_SIZE=2 -c avx512fp16.c make[6]: Leaving directory '/builddir/build/BUILD/xen-4.18.0/tools/tests/x86_emulator' /tmp/ccrznrqa.s: Assembler messages: /tmp/ccrznrqa.s:98: Error: no such instruction: `vmovw .LC0,%xmm3' /tmp/ccrznrqa.s:99: Error: no such instruction: `vmovw %xmm3,58(%esp)' /tmp/ccrznrqa.s:105: Error: no such instruction: `vcvtsi2shl %eax,%xmm1,%xmm1' /tmp/ccrznrqa.s:106: Error: no such instruction: `vmovw %xmm3,382(%esp,%eax,2)' /tmp/ccrznrqa.s:107: Error: no such instruction: `vmovw %xmm1,-2(%edx,%eax,2)' /tmp/ccrznrqa.s:108: Error: no such instruction: `vcvtsi2shl %ecx,%xmm1,%xmm1' /tmp/ccrznrqa.s:109: Error: no such instruction: `vmovw %xmm1,318(%esp,%eax,2)' /tmp/ccrznrqa.s:113: Error: no such instruction: `vaddph 256(%esp),%zmm7,%zmm5' <snip many> simd-fma.c:208: Error: no such instruction: `vfmaddsub231ph 60(%esp){1to32},%zmm6,%zmm5' simd-fma.c:209: Error: no such instruction: `vfmaddsub231ph 60(%esp){1to32},%zmm6,%zmm1' GCC is 12.2.1, binutils is 2.37 AVX512_FP16 was added in bintuils 2.38 so I understand the simd-fma.c complains, but ccrznrqa.s suggest that there's a bad -m passed. I haven't figured out which source file it logically associated with. ~Andrew
On 17.08.2023 14:58, Andrew Cooper wrote: > On 17/08/2023 12:47 pm, Jan Beulich wrote: >> Our present approach is working fully behind the compiler's back. This >> was found to not work with LTO. Employ ld's --wrap= option instead. Note >> that while this makes the build work at least with new enough gcc (it >> doesn't with gcc7, for example, due to tool chain side issues afaict), >> according to my testing things still won't work when building the >> fuzzing harness with afl-cc: While with the gcc7 tool chain I see afl-as >> getting invoked, this does not happen with gcc13. Yet without using that >> assembler wrapper the resulting binary will look uninstrumented to >> afl-fuzz. >> >> While checking the resulting binaries I noticed that we've gained uses >> of snprintf() and strstr(), which only just so happen to not cause any >> problems. Add a wrappers for them as well. >> >> Since we don't have any actual uses of v{,sn}printf(), no definitions of >> their wrappers appear (just yet). But I think we want >> __wrap_{,sn}printf() to properly use __real_v{,sn}printf() right away, >> which means we need delarations of the latter. >> >> Reported-by: Andrew Cooper <andrew.cooper3@citrix.com> >> Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com> >> Signed-off-by: Jan Beulich <jbeulich@suse.com> > > This does resolve the build issue. I do get a binary out of the end, so > Tested-by: Andrew Cooper <andrew.cooper3@citrix.com>. Thanks. > I presume that you've smoke tested the resulting binary? The fuzzer one? No. I didn't think it is of any use when not driven by afl. I did a proper test of the test harness one, albeit not with LTO in use (I focused on the fuzzer one with the LTO issue). > However, I do see something else in the logs which is concerning. > Likely unrelated. > > make[6]: Entering directory > '/builddir/build/BUILD/xen-4.18.0/tools/tests/x86_emulator' > gcc -m32 -march=i686 -DBUILD_ID -fno-strict-aliasing -std=gnu99 -Wall > -Wstrict-prototypes -Wdeclaration-after-statement > -Wno-unused-but-set-variable -Wno-unused-local-typedefs -g3 -Werror -Og > -fno-omit-frame-pointer > -D__XEN_INTERFACE_VERSION__=__XEN_LATEST_INTERFACE_VERSION__ > -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -mno-tls-direct-seg-refs > -fno-pie -fno-stack-protector -fno-exceptions > -fno-asynchronous-unwind-tables -fno-builtin -g0 -D_64f2 -mavx512fp16 > -ffixed-xmm0 -Os -DVEC_SIZE=64 -DFLOAT_SIZE=2 -c avx512fp16.c > make[6]: Leaving directory > '/builddir/build/BUILD/xen-4.18.0/tools/tests/x86_emulator' > /tmp/ccrznrqa.s: Assembler messages: > /tmp/ccrznrqa.s:98: Error: no such instruction: `vmovw .LC0,%xmm3' > /tmp/ccrznrqa.s:99: Error: no such instruction: `vmovw %xmm3,58(%esp)' > /tmp/ccrznrqa.s:105: Error: no such instruction: `vcvtsi2shl > %eax,%xmm1,%xmm1' > /tmp/ccrznrqa.s:106: Error: no such instruction: `vmovw > %xmm3,382(%esp,%eax,2)' > /tmp/ccrznrqa.s:107: Error: no such instruction: `vmovw > %xmm1,-2(%edx,%eax,2)' > /tmp/ccrznrqa.s:108: Error: no such instruction: `vcvtsi2shl > %ecx,%xmm1,%xmm1' > /tmp/ccrznrqa.s:109: Error: no such instruction: `vmovw > %xmm1,318(%esp,%eax,2)' > /tmp/ccrznrqa.s:113: Error: no such instruction: `vaddph > 256(%esp),%zmm7,%zmm5' > <snip many> > simd-fma.c:208: Error: no such instruction: `vfmaddsub231ph > 60(%esp){1to32},%zmm6,%zmm5' > simd-fma.c:209: Error: no such instruction: `vfmaddsub231ph > 60(%esp){1to32},%zmm6,%zmm1' > > GCC is 12.2.1, binutils is 2.37 > > AVX512_FP16 was added in bintuils 2.38 so I understand the simd-fma.c > complains, Right. I assume the gcc is not the system one, or else I'd find it odd to have a compiler backed by a less capable assembler. > but ccrznrqa.s suggest that there's a bad -m passed. I > haven't figured out which source file it logically associated with. The source file is avx512fp16.c as per the compiler command line, which in turn is a symlink to simd.c. We don't pass any naming options from testcase.mk, so the compiler using a made up name is expected. Jan
On 17.08.2023 14:58, Andrew Cooper wrote: > On 17/08/2023 12:47 pm, Jan Beulich wrote: >> Our present approach is working fully behind the compiler's back. This >> was found to not work with LTO. Employ ld's --wrap= option instead. Note >> that while this makes the build work at least with new enough gcc (it >> doesn't with gcc7, for example, due to tool chain side issues afaict), >> according to my testing things still won't work when building the >> fuzzing harness with afl-cc: While with the gcc7 tool chain I see afl-as >> getting invoked, this does not happen with gcc13. Yet without using that >> assembler wrapper the resulting binary will look uninstrumented to >> afl-fuzz. >> >> While checking the resulting binaries I noticed that we've gained uses >> of snprintf() and strstr(), which only just so happen to not cause any >> problems. Add a wrappers for them as well. >> >> Since we don't have any actual uses of v{,sn}printf(), no definitions of >> their wrappers appear (just yet). But I think we want >> __wrap_{,sn}printf() to properly use __real_v{,sn}printf() right away, >> which means we need delarations of the latter. >> >> Reported-by: Andrew Cooper <andrew.cooper3@citrix.com> >> Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com> >> Signed-off-by: Jan Beulich <jbeulich@suse.com> > > This does resolve the build issue. I do get a binary out of the end, so > Tested-by: Andrew Cooper <andrew.cooper3@citrix.com>. I presume that > you've smoke tested the resulting binary? Oh, another question: Because of you asking it's not really clear whether the T-b is kind of implying an A-b as well. Could you confirm one way or the other, please? Jan
On 17/08/2023 2:15 pm, Jan Beulich wrote: > On 17.08.2023 14:58, Andrew Cooper wrote: >> On 17/08/2023 12:47 pm, Jan Beulich wrote: >>> Our present approach is working fully behind the compiler's back. This >>> was found to not work with LTO. Employ ld's --wrap= option instead. Note >>> that while this makes the build work at least with new enough gcc (it >>> doesn't with gcc7, for example, due to tool chain side issues afaict), >>> according to my testing things still won't work when building the >>> fuzzing harness with afl-cc: While with the gcc7 tool chain I see afl-as >>> getting invoked, this does not happen with gcc13. Yet without using that >>> assembler wrapper the resulting binary will look uninstrumented to >>> afl-fuzz. >>> >>> While checking the resulting binaries I noticed that we've gained uses >>> of snprintf() and strstr(), which only just so happen to not cause any >>> problems. Add a wrappers for them as well. >>> >>> Since we don't have any actual uses of v{,sn}printf(), no definitions of >>> their wrappers appear (just yet). But I think we want >>> __wrap_{,sn}printf() to properly use __real_v{,sn}printf() right away, >>> which means we need delarations of the latter. >>> >>> Reported-by: Andrew Cooper <andrew.cooper3@citrix.com> >>> Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com> >>> Signed-off-by: Jan Beulich <jbeulich@suse.com> >> This does resolve the build issue. I do get a binary out of the end, so >> Tested-by: Andrew Cooper <andrew.cooper3@citrix.com>. I presume that >> you've smoke tested the resulting binary? > Oh, another question: Because of you asking it's not really clear whether > the T-b is kind of implying an A-b as well. Could you confirm one way or > the other, please? Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
On 17/08/2023 2:13 pm, Jan Beulich wrote: > On 17.08.2023 14:58, Andrew Cooper wrote: >> On 17/08/2023 12:47 pm, Jan Beulich wrote: >>> Our present approach is working fully behind the compiler's back. This >>> was found to not work with LTO. Employ ld's --wrap= option instead. Note >>> that while this makes the build work at least with new enough gcc (it >>> doesn't with gcc7, for example, due to tool chain side issues afaict), >>> according to my testing things still won't work when building the >>> fuzzing harness with afl-cc: While with the gcc7 tool chain I see afl-as >>> getting invoked, this does not happen with gcc13. Yet without using that >>> assembler wrapper the resulting binary will look uninstrumented to >>> afl-fuzz. >>> >>> While checking the resulting binaries I noticed that we've gained uses >>> of snprintf() and strstr(), which only just so happen to not cause any >>> problems. Add a wrappers for them as well. >>> >>> Since we don't have any actual uses of v{,sn}printf(), no definitions of >>> their wrappers appear (just yet). But I think we want >>> __wrap_{,sn}printf() to properly use __real_v{,sn}printf() right away, >>> which means we need delarations of the latter. >>> >>> Reported-by: Andrew Cooper <andrew.cooper3@citrix.com> >>> Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com> >>> Signed-off-by: Jan Beulich <jbeulich@suse.com> >> This does resolve the build issue. I do get a binary out of the end, so >> Tested-by: Andrew Cooper <andrew.cooper3@citrix.com>. > Thanks. > >> I presume that you've smoke tested the resulting binary? > The fuzzer one? No. I didn't think it is of any use when not driven by afl. > I did a proper test of the test harness one, albeit not with LTO in use (I > focused on the fuzzer one with the LTO issue). The fuzzer is only built because it's active-by-default in tools/, not because it's used. The test harness is the one that gets used routinely in testing. > >> However, I do see something else in the logs which is concerning. >> Likely unrelated. >> >> make[6]: Entering directory >> '/builddir/build/BUILD/xen-4.18.0/tools/tests/x86_emulator' >> gcc -m32 -march=i686 -DBUILD_ID -fno-strict-aliasing -std=gnu99 -Wall >> -Wstrict-prototypes -Wdeclaration-after-statement >> -Wno-unused-but-set-variable -Wno-unused-local-typedefs -g3 -Werror -Og >> -fno-omit-frame-pointer >> -D__XEN_INTERFACE_VERSION__=__XEN_LATEST_INTERFACE_VERSION__ >> -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -mno-tls-direct-seg-refs >> -fno-pie -fno-stack-protector -fno-exceptions >> -fno-asynchronous-unwind-tables -fno-builtin -g0 -D_64f2 -mavx512fp16 >> -ffixed-xmm0 -Os -DVEC_SIZE=64 -DFLOAT_SIZE=2 -c avx512fp16.c >> make[6]: Leaving directory >> '/builddir/build/BUILD/xen-4.18.0/tools/tests/x86_emulator' >> /tmp/ccrznrqa.s: Assembler messages: >> /tmp/ccrznrqa.s:98: Error: no such instruction: `vmovw .LC0,%xmm3' >> /tmp/ccrznrqa.s:99: Error: no such instruction: `vmovw %xmm3,58(%esp)' >> /tmp/ccrznrqa.s:105: Error: no such instruction: `vcvtsi2shl >> %eax,%xmm1,%xmm1' >> /tmp/ccrznrqa.s:106: Error: no such instruction: `vmovw >> %xmm3,382(%esp,%eax,2)' >> /tmp/ccrznrqa.s:107: Error: no such instruction: `vmovw >> %xmm1,-2(%edx,%eax,2)' >> /tmp/ccrznrqa.s:108: Error: no such instruction: `vcvtsi2shl >> %ecx,%xmm1,%xmm1' >> /tmp/ccrznrqa.s:109: Error: no such instruction: `vmovw >> %xmm1,318(%esp,%eax,2)' >> /tmp/ccrznrqa.s:113: Error: no such instruction: `vaddph >> 256(%esp),%zmm7,%zmm5' >> <snip many> >> simd-fma.c:208: Error: no such instruction: `vfmaddsub231ph >> 60(%esp){1to32},%zmm6,%zmm5' >> simd-fma.c:209: Error: no such instruction: `vfmaddsub231ph >> 60(%esp){1to32},%zmm6,%zmm1' >> >> GCC is 12.2.1, binutils is 2.37 >> >> AVX512_FP16 was added in bintuils 2.38 so I understand the simd-fma.c >> complains, > Right. I assume the gcc is not the system one, or else I'd find it > odd to have a compiler backed by a less capable assembler. It is the system one, but it's entirely possible that there has been a bootstrapping error. We're trying something a bit new. There are tasks to update both. I'll keep an eye on the result. ~Andrew
--- a/tools/fuzz/x86_instruction_emulator/Makefile +++ b/tools/fuzz/x86_instruction_emulator/Makefile @@ -35,6 +35,8 @@ OBJS := fuzz-emul.o x86-emulate.o OBJS += x86_emulate/0f01.o x86_emulate/0fae.o x86_emulate/0fc7.o OBJS += x86_emulate/decode.o x86_emulate/fpu.o +WRAPPED = $(shell sed -n 's,^ *WRAP(\([[:alnum:]_]*\));,\1,p' x86-emulate.h) + private.h := x86-emulate.h x86_emulate/x86_emulate.h x86_emulate/private.h x86-emulate.h: x86_emulate/x86_emulate.h @@ -51,10 +53,10 @@ x86-insn-fuzzer.a: $(OBJS) cpuid.o $(AR) rc $@ $^ afl-harness: afl-harness.o $(OBJS) cpuid.o wrappers.o - $(CC) $(CFLAGS) $^ -o $@ + $(CC) $(CFLAGS) $(addprefix -Wl$(comma)--wrap=,$(WRAPPED)) $^ -o $@ afl-harness-cov: afl-harness-cov.o $(patsubst %.o,%-cov.o,$(OBJS)) cpuid.o wrappers.o - $(CC) $(CFLAGS) $(GCOV_FLAGS) $^ -o $@ + $(CC) $(CFLAGS) $(GCOV_FLAGS) $(addprefix -Wl$(comma)--wrap=,$(WRAPPED)) $^ -o $@ # Common targets .PHONY: all --- a/tools/tests/x86_emulator/Makefile +++ b/tools/tests/x86_emulator/Makefile @@ -259,8 +259,10 @@ OBJS := x86-emulate.o cpuid.o test_x86_e OBJS += x86_emulate/0f01.o x86_emulate/0fae.o x86_emulate/0fc7.o OBJS += x86_emulate/blk.o x86_emulate/decode.o x86_emulate/fpu.o x86_emulate/util.o +WRAPPED := $(shell sed -n 's,^ *WRAP(\([[:alnum:]_]*\));,\1,p' x86-emulate.h) + $(TARGET): $(OBJS) - $(HOSTCC) $(HOSTCFLAGS) -o $@ $^ + $(HOSTCC) $(HOSTCFLAGS) $(addprefix -Wl$(comma)--wrap=,$(WRAPPED)) -o $@ $^ .PHONY: clean clean: --- a/tools/tests/x86_emulator/wrappers.c +++ b/tools/tests/x86_emulator/wrappers.c @@ -1,78 +1,103 @@ #include <stdarg.h> -#define WRAP(x) typeof(x) emul_##x +#define WRAP(x) typeof(x) __wrap_ ## x, __real_ ## x #include "x86-emulate.h" -size_t emul_fwrite(const void *src, size_t sz, size_t n, FILE *f) +size_t __wrap_fwrite(const void *src, size_t sz, size_t n, FILE *f) { emul_save_fpu_state(); - sz = fwrite(src, sz, n, f); + sz = __real_fwrite(src, sz, n, f); emul_restore_fpu_state(); return sz; } -int emul_memcmp(const void *p1, const void *p2, size_t sz) +int __wrap_memcmp(const void *p1, const void *p2, size_t sz) { int rc; emul_save_fpu_state(); - rc = memcmp(p1, p2, sz); + rc = __real_memcmp(p1, p2, sz); emul_restore_fpu_state(); return rc; } -void *emul_memcpy(void *dst, const void *src, size_t sz) +void *__wrap_memcpy(void *dst, const void *src, size_t sz) { emul_save_fpu_state(); - memcpy(dst, src, sz); + __real_memcpy(dst, src, sz); emul_restore_fpu_state(); return dst; } -void *emul_memset(void *dst, int c, size_t sz) +void *__wrap_memset(void *dst, int c, size_t sz) { emul_save_fpu_state(); - memset(dst, c, sz); + __real_memset(dst, c, sz); emul_restore_fpu_state(); return dst; } -int emul_printf(const char *fmt, ...) +int __wrap_printf(const char *fmt, ...) { va_list varg; int rc; emul_save_fpu_state(); va_start(varg, fmt); - rc = vprintf(fmt, varg); + rc = __real_vprintf(fmt, varg); va_end(varg); emul_restore_fpu_state(); return rc; } -int emul_putchar(int c) +int __wrap_putchar(int c) { int rc; emul_save_fpu_state(); - rc = putchar(c); + rc = __real_putchar(c); emul_restore_fpu_state(); return rc; } -int emul_puts(const char *str) +int __wrap_puts(const char *str) { int rc; emul_save_fpu_state(); - rc = puts(str); + rc = __real_puts(str); emul_restore_fpu_state(); return rc; } + +int __wrap_snprintf(char *buf, size_t n, const char *fmt, ...) +{ + va_list varg; + int rc; + + emul_save_fpu_state(); + va_start(varg, fmt); + rc = __real_vsnprintf(buf, n, fmt, varg); + va_end(varg); + emul_restore_fpu_state(); + + return rc; +} + +char *__wrap_strstr(const char *s1, const char *s2) +{ + char *s; + + emul_save_fpu_state(); + s = __real_strstr(s1, s2); + emul_restore_fpu_state(); + + return s; +} --- a/tools/tests/x86_emulator/x86-emulate.h +++ b/tools/tests/x86_emulator/x86-emulate.h @@ -32,9 +32,7 @@ #ifdef EOF # error "Must not include <stdio.h> before x86-emulate.h" #endif -#ifdef WRAP -# include <stdio.h> -#endif +#include <stdio.h> #include <xen/xen.h> @@ -88,11 +86,7 @@ struct x86_fxsr *get_fpu_save_area(void) * around the actual function. */ #ifndef WRAP -# if 0 /* This only works for explicit calls, not for compiler generated ones. */ -# define WRAP(x) typeof(x) x asm("emul_" #x) -# else -# define WRAP(x) asm(".equ " #x ", emul_" #x) -# endif +# define WRAP(x) typeof(x) __wrap_ ## x #endif WRAP(fwrite); @@ -102,6 +96,10 @@ WRAP(memset); WRAP(printf); WRAP(putchar); WRAP(puts); +WRAP(snprintf); +WRAP(strstr); +WRAP(vprintf); +WRAP(vsnprintf); #undef WRAP
Our present approach is working fully behind the compiler's back. This was found to not work with LTO. Employ ld's --wrap= option instead. Note that while this makes the build work at least with new enough gcc (it doesn't with gcc7, for example, due to tool chain side issues afaict), according to my testing things still won't work when building the fuzzing harness with afl-cc: While with the gcc7 tool chain I see afl-as getting invoked, this does not happen with gcc13. Yet without using that assembler wrapper the resulting binary will look uninstrumented to afl-fuzz. While checking the resulting binaries I noticed that we've gained uses of snprintf() and strstr(), which only just so happen to not cause any problems. Add a wrappers for them as well. Since we don't have any actual uses of v{,sn}printf(), no definitions of their wrappers appear (just yet). But I think we want __wrap_{,sn}printf() to properly use __real_v{,sn}printf() right away, which means we need delarations of the latter. Reported-by: Andrew Cooper <andrew.cooper3@citrix.com> Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com> Signed-off-by: Jan Beulich <jbeulich@suse.com>