On Mon, Jun 24, 2024 at 11:55 AM Jan Beulich <jbeul...@suse.com> wrote: > > On 21.06.2024 21:14, Tamas K Lengyel wrote: > > @@ -58,6 +58,9 @@ afl-harness: afl-harness.o $(OBJS) cpuid.o wrappers.o > > afl-harness-cov: afl-harness-cov.o $(patsubst %.o,%-cov.o,$(OBJS)) cpuid.o > > wrappers.o > > $(CC) $(CFLAGS) $(GCOV_FLAGS) $(addprefix > > -Wl$(comma)--wrap=,$(WRAPPED)) $^ -o $@ > > > > +libfuzzer-harness: $(OBJS) cpuid.o > > + $(CC) $(CFLAGS) $(LIB_FUZZING_ENGINE) -fsanitize=fuzzer $^ -o $@ > > What is LIB_FUZZING_ENGINE? I don't think we have any use of that in the > tree anywhere.
It's used by oss-fuzz, otherwise it's not doing anything. > > I'm further surprised you get away here without wrappers.o. Wrappers.o was actually breaking the build for oss-fuzz at the linking stage. It works just fine without it. > > Finally, despite its base name the lack of an extension suggest to me > this isn't actually a library. Can you help me bring both aspects together? Libfuzzer is the the name of the fuzzing engine, like afl: https://llvm.org/docs/LibFuzzer.html > > > @@ -67,7 +70,7 @@ distclean: clean > > > > .PHONY: clean > > clean: > > - rm -f *.a *.o $(DEPS_RM) afl-harness afl-harness-cov *.gcda *.gcno > > *.gcov > > + rm -f *.a *.o $(DEPS_RM) afl-harness afl-harness-cov *.gcda *.gcno > > *.gcov libfuzzer-harness > > I'm inclined to suggest it's time to split this line (e.g. keeping all the > wildcard patterns together and moving the rest to a new rm invocation). Sure. > > > --- a/tools/fuzz/x86_instruction_emulator/fuzz-emul.c > > +++ b/tools/fuzz/x86_instruction_emulator/fuzz-emul.c > > @@ -906,14 +906,12 @@ int LLVMFuzzerTestOneInput(const uint8_t *data_p, > > size_t size) > > > > if ( size <= DATA_OFFSET ) > > { > > - printf("Input too small\n"); > > - return 1; > > + return -1; > > } > > > > if ( size > FUZZ_CORPUS_SIZE ) > > { > > - printf("Input too large\n"); > > - return 1; > > + return -1; > > } > > > > memcpy(&input, data_p, size); > > This part of the change clearly needs explaining in the description. > It's not even clear to me in how far this is related to the purpose > of the patch here (iow it may want to be a separate change, depending > on why the change is needed). The printf simply produces a ton of unnecessary output while the fuzzer is running, slowing it down. It's also not useful at all, even for debugging. Switching the return -1 is necessary because beside 0/-1 values are reserved by libfuzzer for "future use". No issue about putting this info into the commit message. Tamas
