On Tue, Jun 23, 2020 at 10:55:00AM -0400, Alexander Bulekov wrote: > On 200623 1514, Stefan Hajnoczi wrote: > > On Thu, Jun 11, 2020 at 01:56:51AM -0400, Alexander Bulekov wrote: > > > Signed-off-by: Alexander Bulekov <[email protected]> > > > --- > > > exec.c | 17 ++++++++++++++++- > > > include/exec/memory.h | 8 ++++++++ > > > include/exec/memory_ldst_cached.inc.h | 9 +++++++++ > > > include/sysemu/dma.h | 5 ++++- > > > memory_ldst.inc.c | 12 ++++++++++++ > > > 5 files changed, 49 insertions(+), 2 deletions(-) > > > > Please rename dma_read_cb() to fuzz_dma_read_cb() so the purpose of the > > function is clear. > > > > The ifdefs can be avoided by defining an empty function when CONFIG_FUZZ > > is undefined. In a header file: > > > > #ifdef CONFIG_FUZZ > > void fuzz_dma_read_cb(size_t addr, size_t len); > > #else > > static inline void fuzz_dma_read_cb(size_t addr, size_t len) > > { > > /* Do nothing */ > > } > > #endif > > > > Now the compiler should eliminate the deadcode: > > > > #ifdef CONFIG_FUZZ > > if (as->root == get_system_memory()) { > > dma_read_cb(addr, len); > > } > > #endif > > > > becomes: > > > > if (as->root == get_system_memory()) { > > fuzz_dma_read_cb(addr, len); > > } > > > > Hopefully gcc and clang will eliminate this and emit no instructions > > when CONFIG_FUZZ is undefined. If not, you can simply pass in 'as' and > > 'is_write' too: > > > > void fuzz_dma_read_cb(AddressSpace *as, bool is_write, size_t addr, > > size_t len) > > > > This way the conditional is moved inside fuzz_dma_read_cb() and deadcode > > elimination becomes trivial for the compiler: > > > > fuzz_read_cb(as, is_write, addr, len); > > Do you think it would be better to have a "trace_dma_read" or > "trace_device_read_from_guest_memory"? I haven't looked under the hood > too much for the tracepoint api, but these would just be standard > tracepoints(disabled for the majority of builds). When we build the > fuzzer, we could compile with --wrap="trace_dma_read" and implement > a __wrap_trace_dma_read in the generic fuzzer. I looked at the symbols > for a qemu build and it looks like trace_* are actual functions, rather > than preprocessor magic, so maybe this could work?
I think plain old functions are fine for what you are doing. QEMU's tracing does not provide callbacks that are invoked when a trace event is emitted. The fuzzing code wouldn't be able to hook trace_device_read_from_guest_memory, you could only analyze a trace after the fact. Stefan
signature.asc
Description: PGP signature
