Reimplement fuzzing instrumentation using Bazel transitions. (#86)

* Reimplement fuzzing instrumentation using Bazel transitions.

This approach eliminates the need for inlining the instrumentation options in the bazelrc file and simplifies the adoption of the rules.

* Updated the documentation, too.

* Fix tagging issues with the new rule separation.

* Updated again the documentation.

* Fixed CI tests.

* Revamped the presubmit tests to include richer smoke testing behavior.

* Installing Honggfuzz deps in the smoke test workflow.

* Exclude some MSAN smoke tests.

* Added code documentation for the cc_engine_sanitizer values.

* Address reviewer comments.

* Make buildifier happy.

* Rename msan-repro sanitizer option to msan-origin-tracking.

.bazelrc

@@ -12,45 +12,31 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-# TODO(sbucur): Verify whether this can be specified directly in Starlark,
-# perhaps using a C++ toolchain customization.
+# Force the use of Clang for all builds.
+build --action_env=CC=clang
+build --action_env=CXX=clang++
 
-# Common flags for Clang.
-build:clang --action_env=CC=clang
-build:clang --action_env=CXX=clang++
+# LibFuzzer + ASAN
+build:asan-libfuzzer --//fuzzing:cc_engine=//fuzzing/engines:libfuzzer
+build:asan-libfuzzer --//fuzzing:cc_engine_instrumentation=libfuzzer
+build:asan-libfuzzer --//fuzzing:cc_engine_sanitizer=asan
 
-# Common fuzzing flags. To be only used by other configs.
-build:_fuzzer_common --config=clang
-build:_fuzzer_common --copt=-DFUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION
+# LibFuzzer + MSAN
+build:msan-libfuzzer --//fuzzing:cc_engine=//fuzzing/engines:libfuzzer
+build:msan-libfuzzer --//fuzzing:cc_engine_instrumentation=libfuzzer
+build:msan-libfuzzer --//fuzzing:cc_engine_sanitizer=msan
 
-# Flags for Clang with ASAN and libfuzzer.
-build:asan-libfuzzer --config=_fuzzer_common
-build:asan-libfuzzer --linkopt=-fsanitize=fuzzer,address
-build:asan-libfuzzer --copt=-fsanitize=fuzzer,address
-build:asan-libfuzzer --//fuzzing:cc_engine=//fuzzing/engines:libfuzzer
+# LibFuzzer + MSAN (reproduction mode)
+build:msan-libfuzzer-repro --//fuzzing:cc_engine=//fuzzing/engines:libfuzzer
+build:msan-libfuzzer-repro --//fuzzing:cc_engine_instrumentation=libfuzzer
+build:msan-libfuzzer-repro --//fuzzing:cc_engine_sanitizer=msan-origin-tracking
 
-# Flags for Clang with ASAN and Honggfuzz.
-# Reflects the set of options at
-# https://github.com/google/honggfuzz/blob/master/hfuzz_cc/hfuzz-cc.c
-build:asan-honggfuzz --config=_fuzzer_common
-build:asan-honggfuzz --dynamic_mode=off
-build:asan-honggfuzz --copt=-mllvm
-build:asan-honggfuzz --copt=-inline-threshold=2000
-build:asan-honggfuzz --copt=-fno-builtin
-build:asan-honggfuzz --copt=-fno-omit-frame-pointer
-build:asan-honggfuzz --copt=-D__NO_STRING_INLINES
-build:asan-honggfuzz --copt=-fsanitize=address
-build:asan-honggfuzz --copt=-fno-sanitize=fuzzer
-build:asan-honggfuzz --copt=-fsanitize-coverage=trace-pc-guard,trace-cmp,trace-div,indirect-calls
-build:asan-honggfuzz --linkopt=-fsanitize=address
+# Honggfuzz + ASAN
 build:asan-honggfuzz --//fuzzing:cc_engine=//fuzzing/engines:honggfuzz
+build:asan-honggfuzz --//fuzzing:cc_engine_instrumentation=honggfuzz
+build:asan-honggfuzz --//fuzzing:cc_engine_sanitizer=asan
 
-# Flags for Clang with MSAN and libfuzzer.
-build:msan-libfuzzer --config=_fuzzer_common
-build:msan-libfuzzer --linkopt=-fsanitize=memory,fuzzer
-build:msan-libfuzzer --copt=-fsanitize=memory,fuzzer
-build:msan-libfuzzer --//fuzzing:cc_engine=//fuzzing/engines:libfuzzer
-
-# Flags for Clang with MSAN and libfuzzer, outputting detailed report.
-build:msan-libfuzzer-repro --config=msan-libfuzzer
-build:msan-libfuzzer-repro --copt=-fsanitize-memory-track-origins=2
+# Honggfuzz + MSAN
+build:msan-honggfuzz --//fuzzing:cc_engine=//fuzzing/engines:honggfuzz
+build:msan-honggfuzz --//fuzzing:cc_engine_instrumentation=honggfuzz
+build:msan-honggfuzz --//fuzzing:cc_engine_sanitizer=msan

.github/workflows/bazel_test.yml

@@ -17,19 +17,38 @@ name: Bazel Test
 on: [push, pull_request]
 
 jobs:
-  bazel_test:
+  unit_tests:
+    name: All project tests
     runs-on: ubuntu-latest
+    timeout-minutes: 30
     steps:
       - name: Checkout repository
         uses: actions/checkout@v2
+      - name: Run unit tests
+        run: |
+          bazel test //... --test_tag_filters=-fuzz-test --build_tests_only
       - name: Build fuzz test examples
         # The -FDP is for filtering out the files which contains "#include <fuzzer/FuzzedDataProvider.h>",
         # The "<fuzzer/FDP...>" is only supported since clang-10 while the clang version in github runner is 9
         run: |
           bazel build --verbose_failures --build_tag_filters=fuzz-test,-FDP --config=asan-libfuzzer //examples/...
-      - name: Run common tests
-        run: |
-          bazel test --config=clang //... --test_tag_filters=-fuzz-test --build_tests_only
-      - name: Run dedicated tests
+  smoke_tests:
+    name: Smoke tests on fuzz targets
+    runs-on: ubuntu-latest
+    timeout-minutes: 30
+    strategy:
+      matrix:
+        config: ["asan-libfuzzer", "msan-libfuzzer", "asan-honggfuzz"]
+        target: ["//examples:empty_fuzz_test_run", "//examples:re2_fuzz_test_run"]
+        exclude:
+          # MSAN currently fails on the RE2 target.
+          - config: "msan-libfuzzer"
+            target: "//examples:re2_fuzz_test_run"
+    steps:
+      - name: Checkout repository
+        uses: actions/checkout@v2
+      - name: Install dependencies
+        run: sudo apt-get update && sudo apt-get install -yq libunwind-dev libblocksruntime-dev
+      - name: Run smoke test
         run: |
-          bazel run //examples:empty_fuzz_test_run --config=asan-libfuzzer -- --timeout_secs=5
+          bazel run ${{ matrix.target }} --config=${{ matrix.config }} -- --clean --timeout_secs=5

README.md

@@ -94,7 +94,7 @@ Check out the [`examples/`](examples/) directory, which showcases additional fea
 
 ## Using the rules in your project
 
-To use the fuzzing rules in your project, you will need to load and set them up in your workspace, along with creating the necessary `--config` commands in your `.bazelrc` file.
+To use the fuzzing rules in your project, you will need to load and set them up in your workspace. We also recommend creating `--config` commands in your `.bazelrc` file for the fuzzing engine + sanitizer configurations you wish to use in your project.
 
 ### Configuring the WORKSPACE
 
@@ -123,11 +123,15 @@ The project is still under active development, so you many need to change the `u
 
 ### Configuring the .bazelrc file
 
-To make sure the fuzz tests are built with the correct instrumentation flags for each engine / instrumentation supported, we recommend using the configurations defined in this repository's [`.bazelrc` file](/.bazelrc), which you can copy and paste in your own `.bazelrc` file.
+Each fuzz test is built with a fuzzing engine and instrumentation specified in three build settings, available as flags on the Bazel command line:
 
-Currently, the following configurations are available, based on the fuzzing engines defined in this repository:
+* `--@rules_fuzzing//fuzzing:cc_engine` points to the `cc_fuzzing_engine` target of the fuzzing engine to use.
+* `--@rules_fuzzing//fuzzing:cc_engine_instrumentation` specifies the compiler instrumentation to use (for example, libFuzzer or Honggfuzz).
+* `--@rules_fuzzing//fuzzing:cc_engine_sanitizer` specifies the sanitizer configuration used to detect bugs (for example, ASAN or MSAN).
 
-| Configuration             | Fuzzing engine | Instrumentation          |
+To simplify specifying these settings on the command line, we recommend combining them as `--config` settings in your project's [`.bazelrc` file](https://docs.bazel.build/versions/master/guide.html#bazelrc-the-bazel-configuration-file). You can copy and paste the [`.bazelrc` file of this repository](/.bazelrc) as a starting point, which defines the following configurations:
+
+| Configuration             | Fuzzing engine | Sanitizer                |
 |---------------------------|----------------|--------------------------|
 | `--config=asan-fuzzer`    | libFuzzer      | Address Sanitizer (ASAN) |
 | `--config=msan-fuzzer`    | libFuzzer      | Memory Sanitizer (MSAN)  |

docs/BUILD

@@ -45,9 +45,11 @@ bzl_library(
     name = "cc_fuzzing_rules",
     srcs = [
         "//fuzzing:cc_deps.bzl",
+        "//fuzzing:instrum_opts.bzl",
         "//fuzzing/private:common.bzl",
         "//fuzzing/private:engine.bzl",
         "//fuzzing/private:fuzz_test.bzl",
+        "//fuzzing/private:instrument.bzl",
     ],
     deps = [
         ":rules_cc",

docs/cc-fuzzing-rules.md

@@ -29,7 +29,7 @@ Specifies a fuzzing engine that can be used to run C++ fuzz targets.
 ## cc_fuzz_test
 
 <pre>
-cc_fuzz_test(<a href="#cc_fuzz_test-name">name</a>, <a href="#cc_fuzz_test-corpus">corpus</a>, <a href="#cc_fuzz_test-dicts">dicts</a>, <a href="#cc_fuzz_test-engine">engine</a>, <a href="#cc_fuzz_test-binary_kwargs">binary_kwargs</a>)
+cc_fuzz_test(<a href="#cc_fuzz_test-name">name</a>, <a href="#cc_fuzz_test-corpus">corpus</a>, <a href="#cc_fuzz_test-dicts">dicts</a>, <a href="#cc_fuzz_test-engine">engine</a>, <a href="#cc_fuzz_test-tags">tags</a>, <a href="#cc_fuzz_test-binary_kwargs">binary_kwargs</a>)
 </pre>
 
 Defines a fuzz test and a few associated tools and metadata.
@@ -47,6 +47,9 @@ For each fuzz test `<name>`, this macro expands into a number of targets:
 * `<name>_corpus_zip`: Generates a zip archive of the corpus directory.
 * `<name>_dict`: Validates the set of dictionary files provided and emits
   the result to a `<name>.dict` file.
+* `<name>_raw`: The raw, uninstrumented fuzz test executable. This should be
+  rarely needed and may be useful when debugging instrumentation-related
+  build failures or misbehavior.
 
 > TODO: Document here the command line interface of the `<name>_run`
 targets.
@@ -61,6 +64,7 @@ targets.
 | <a id="cc_fuzz_test-corpus"></a>corpus |  A list containing corpus files.   |  <code>None</code> |
 | <a id="cc_fuzz_test-dicts"></a>dicts |  A list containing dictionaries.   |  <code>None</code> |
 | <a id="cc_fuzz_test-engine"></a>engine |  A label pointing to the fuzzing engine to use.   |  <code>"@rules_fuzzing//fuzzing:cc_engine"</code> |
+| <a id="cc_fuzz_test-tags"></a>tags |  Tags set on the fuzz test executable.   |  <code>None</code> |
 | <a id="cc_fuzz_test-binary_kwargs"></a>binary_kwargs |  Keyword arguments directly forwarded to the fuzz test   binary rule.   |  none |
 
 

fuzzing/BUILD

@@ -12,12 +12,45 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+load("@bazel_skylib//rules:common_settings.bzl", "string_flag")
+
 label_flag(
     name = "cc_engine",
     build_setting_default = "//fuzzing/engines:libfuzzer",
     visibility = ["//visibility:public"],
 )
 
+string_flag(
+    name = "cc_engine_instrumentation",
+    build_setting_default = "none",
+    values = [
+        "none",
+        "libfuzzer",
+        "honggfuzz",
+    ],
+    visibility = ["//visibility:public"],
+)
+
+string_flag(
+    name = "cc_engine_sanitizer",
+    build_setting_default = "none",
+    values = [
+        # No sanitizer instrumentation.
+        "none",
+        # Address sanitizer (ASAN).
+        # See https://clang.llvm.org/docs/AddressSanitizer.html
+        "asan",
+        # Memory sanitizer (MSAN).
+        # See https://clang.llvm.org/docs/MemorySanitizer.html
+        "msan",
+        # MSAN + origin tracking enabled.
+        # Useful for debugging crash reproducers, 1.5-2x slower.
+        "msan-origin-tracking",
+    ],
+    visibility = ["//visibility:public"],
+)
+
 exports_files([
     "cc_deps.bzl",
+    "instrum_opts.bzl",
 ])

fuzzing/instrum_opts.bzl

@@ -0,0 +1,100 @@
+# Copyright 2020 Google LLC
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     https://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""Fuzz test instrumentation options.
+
+Each fuzzing engine or sanitizer instrumentation recognized by the
+//fuzzing:cc_engine_instrumentation and //fuzzing:cc_engine_sanitizer
+configuration flag should be defined here.
+"""
+
+def _is_string_list(value):
+    if type(value) != type([]):
+        return False
+    if any([type(element) != type("") for element in value]):
+        return False
+    return True
+
+def instrumentation_opts(copts = [], linkopts = []):
+    """Creates new instrumentation options.
+
+    The struct fields mirror the argument names of this function.
+
+    Args:
+      copts: A list of compilation options to pass as `--copt`
+        configuration flags.
+      linkopts: A list of linker options to pass as `--linkopt`
+        configuration flags.
+    Returns:
+      A struct with the given instrumentation options.
+    """
+    if not _is_string_list(copts):
+        fail("copts should be a list of strings")
+    if not _is_string_list(linkopts):
+        fail("linkopts should be a list of strings")
+    return struct(
+        copts = copts,
+        linkopts = linkopts,
+    )
+
+# Base instrumentation applied to all fuzz test executables.
+base_opts = instrumentation_opts(
+    copts = ["-DFUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION"],
+    linkopts = [],
+)
+
+# Engine-specific instrumentation.
+fuzzing_engine_opts = {
+    "none": instrumentation_opts(),
+    "libfuzzer": instrumentation_opts(
+        copts = ["-fsanitize=fuzzer"],
+        linkopts = ["-fsanitize=fuzzer"],
+    ),
+    # Reflects the set of options at
+    # https://github.com/google/honggfuzz/blob/master/hfuzz_cc/hfuzz-cc.c
+    "honggfuzz": instrumentation_opts(
+        copts = [
+            "-mllvm",
+            "-inline-threshold=2000",
+            "-fno-builtin",
+            "-fno-omit-frame-pointer",
+            "-D__NO_STRING_INLINES",
+            "-fsanitize-coverage=trace-pc-guard,trace-cmp,trace-div,indirect-calls",
+            "-fno-sanitize=fuzzer",
+        ],
+        linkopts = [
+            "-fno-sanitize=fuzzer",
+        ],
+    ),
+}
+
+# Sanitizer-specific instrumentation.
+sanitizer_opts = {
+    "none": instrumentation_opts(),
+    "asan": instrumentation_opts(
+        copts = ["-fsanitize=address"],
+        linkopts = ["-fsanitize=address"],
+    ),
+    "msan": instrumentation_opts(
+        copts = ["-fsanitize=memory"],
+        linkopts = ["-fsanitize=memory"],
+    ),
+    "msan-origin-tracking": instrumentation_opts(
+        copts = [
+            "-fsanitize=memory",
+            "-fsanitize-memory-track-origins=2",
+        ],
+        linkopts = ["-fsanitize=memory"],
+    ),
+}

fuzzing/private/BUILD

@@ -22,4 +22,5 @@ exports_files([
     "common.bzl",
     "engine.bzl",
     "fuzz_test.bzl",
+    "instrument.bzl",
 ])

fuzzing/private/fuzz_test.bzl

@@ -17,12 +17,14 @@
 load("@rules_cc//cc:defs.bzl", "cc_test")
 load("@rules_pkg//:pkg.bzl", "pkg_zip")
 load("//fuzzing/private:common.bzl", "fuzzing_corpus", "fuzzing_dictionary", "fuzzing_launcher")
+load("//fuzzing/private:instrument.bzl", "instrumented_fuzzing_binary")
 
 def cc_fuzz_test(
         name,
         corpus = None,
         dicts = None,
         engine = "@rules_fuzzing//fuzzing:cc_engine",
+        tags = None,
         **binary_kwargs):
     """Defines a fuzz test and a few associated tools and metadata.
 
@@ -39,6 +41,9 @@ def cc_fuzz_test(
     * `<name>_corpus_zip`: Generates a zip archive of the corpus directory.
     * `<name>_dict`: Validates the set of dictionary files provided and emits
       the result to a `<name>.dict` file.
+    * `<name>_raw`: The raw, uninstrumented fuzz test executable. This should be
+      rarely needed and may be useful when debugging instrumentation-related
+      build failures or misbehavior.
 
     > TODO: Document here the command line interface of the `<name>_run`
     targets.
@@ -48,17 +53,29 @@ def cc_fuzz_test(
         corpus: A list containing corpus files.
         dicts: A list containing dictionaries.
         engine: A label pointing to the fuzzing engine to use.
+        tags: Tags set on the fuzz test executable.
         **binary_kwargs: Keyword arguments directly forwarded to the fuzz test
           binary rule.
     """
 
-    binary_kwargs.setdefault("tags", []).append("fuzz-test")
     binary_kwargs.setdefault("deps", []).append(engine)
     cc_test(
-        name = name,
+        name = name + "_raw",
+        tags = [
+            "manual",
+        ],
         **binary_kwargs
     )
 
+    instrumented_fuzzing_binary(
+        name = name,
+        binary = name + "_raw",
+        tags = (tags or []) + [
+            "fuzz-test",
+        ],
+        testonly = True,
+    )
+
     if corpus:
         fuzzing_corpus(
             name = name + "_corpus",