Re: [PATCH v5 2/3] fuzz: add an instrumentation filter

[Darren Kenny]

Hi Alex,

Sorry, missed this one, apologies for the delay in responding.

On Tuesday, 2021-06-29 at 23:41:23 -04, Alexander Bulekov wrote:
> By default, -fsanitize=fuzzer instruments all code with coverage
> information. However, this means that libfuzzer will track coverage over
> hundreds of source files that are unrelated to virtual-devices. This
> means that libfuzzer will optimize inputs for coverage observed in timer
> code, memory APIs etc. This slows down the fuzzer and stores many inputs
> that are not relevant to the actual virtual-devices.
>
> With this change, clang versions that support the
> "-fsanitize-coverage-allowlist" will only instrument a subset of the
> compiled code, that is directly related to virtual-devices.
>
> Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
> ---
>  configure                                        | 13 +++++++++++++
>  scripts/oss-fuzz/instrumentation-filter-template | 14 ++++++++++++++
>  2 files changed, 27 insertions(+)
>  create mode 100644 scripts/oss-fuzz/instrumentation-filter-template
>
> diff --git a/configure b/configure
> index 38704b4e11..3b6ca054b9 100755
> --- a/configure
> +++ b/configure
> @@ -5189,6 +5189,11 @@ if test "$fuzzing" = "yes" && test -z 
> "${LIB_FUZZING_ENGINE+xxx}"; then
>      error_exit "Your compiler doesn't support -fsanitize=fuzzer"
>      exit 1
>    fi
> +  have_clang_coverage_filter=no
> +  echo > $TMPTXT
> +  if compile_prog "$CPU_CFLAGS -Werror -fsanitize=fuzzer 
> -fsanitize-coverage-allowlist=$TMPTXT" ""; then
> +      have_clang_coverage_filter=yes
> +  fi
>  fi
>  
>  # Thread sanitizer is, for now, much noisier than the other sanitizers;
> @@ -6120,6 +6125,14 @@ if test "$fuzzing" = "yes" ; then
>      # rule for the fuzzer adds these to the link_args. They need to be
>      # configurable, to support OSS-Fuzz
>      FUZZ_EXE_LDFLAGS="-fsanitize=fuzzer"
> +
> +    # Specify a filter to only instrument code that is directly related to
> +    # virtual-devices.
> +    if test "$have_clang_coverage_filter" = "yes" ; then
> +        cp "$source_path/scripts/oss-fuzz/instrumentation-filter-template" \
> +            instrumentation-filter
> +        QEMU_CFLAGS="$QEMU_CFLAGS 
> -fsanitize-coverage-allowlist=instrumentation-filter"
> 

The only concern that I would have here is that the file
instrumentaion-filter is being sepcified without a full path, and
whether that is acceptable as a generic QEMU_CFLAGS element.

I couldn't find anything that suggests it needs the be a full-path, and
all examples seem to be a simple filename, so maybe it searches up the
directory tree, but I can't find anything to say that off-hand.

If that is acceptable, and is working, then I'm ok with it, just wanted
to be sure:

Reviewed-by: Darren Kenny <darren.kenny@oracle.com>

Thanks,

Darren.

> +    fi
>    else
>      FUZZ_EXE_LDFLAGS="$LIB_FUZZING_ENGINE"
>    fi
> diff --git a/scripts/oss-fuzz/instrumentation-filter-template 
> b/scripts/oss-fuzz/instrumentation-filter-template
> new file mode 100644
> index 0000000000..44e853159c
> --- /dev/null
> +++ b/scripts/oss-fuzz/instrumentation-filter-template
> @@ -0,0 +1,14 @@
> +# Code that we actually want the fuzzer to target
> +# See: 
> https://clang.llvm.org/docs/SanitizerCoverage.html#disabling-instrumentation-without-source-modification
> +#
> +src:*/hw/*
> +src:*/include/hw/*
> +src:*/slirp/*
> +
> +# We don't care about coverage over fuzzer-specific code, however we should
> +# instrument the fuzzer entry-point so libFuzzer always sees at least some
> +# coverage - otherwise it will exit after the first input
> +src:*/tests/qtest/fuzz/fuzz.c
> +
> +# Enable instrumentation for all functions in those files
> +fun:*
> -- 
> 2.28.0