bazelbuild / rules_fuzzing

Bazel Starlark extensions for defining fuzz tests in Bazel projects
Apache License 2.0
82 stars 19 forks source link

cc_fuzz_test() breaks non-Clang builds #176

Closed haberman closed 2 years ago

haberman commented 2 years ago

Actual Behavior

The docs for cc_fuzz_target require you to add the following lines to your .bazelrc:

# Force the use of Clang for C++ builds.
build --action_env=CC=clang
build --action_env=CXX=clang++

This makes it impossible to build the repo with any compiler other than Clang. But I want my repo to be compatible with GCC, MSVC, and other compilers, for testing my code across compilers.

I tried specifying these actions for the libfuzzer configurations only:

# Force the use of Clang for fuzzer builds only.
build:asan-libfuzzer --action_env=CC=clang
build:asan-libfuzzer --action_env=CXX=clang++

However, I discovered that the cc_fuzz_target() would then fail to build with other compilers. For example, compiling with GCC I got the following error:

bazel-out/k8-fastbuild-ST-6de91758680c/bin/external/rules_fuzzing/fuzzing/replay/_objs/replay_main/replay_main.pic.o:replay_main.cc:function main: error: undefined reference to 'LLVMFuzzerInitialize'

This is unfortunate; even if nobody tries to build the fuzzing target directly, a broken target like this will make bazel test ... fail.

Expected Behavior

Ideally, cc_fuzz_target() would automatically opt itself out of non-Clang builds using target_compatible_with.

This would cause the fuzz targets to be automatically skipped by Bazel on non-Clang builds. Users could get the best of both worlds: the repo could continue to be compatible with any compiler, but Bazel would only attempt to build the fuzzing rules if the build is using Clang.

I tried doing this myself by adding target_compatible_with to my cc_fuzz_target() rule:

cc_fuzz_test(
    name = "file_descriptor_parsenew_fuzzer",
    srcs = ["file_descriptor_parsenew_fuzzer.cc"],
    deps = [
        "//:descriptor_upb_proto",
        "//:upb",
    ],
    target_compatible_with = [
        "@bazel_tools//tools/cpp:clang",
    ],
)

However this didn't work; Bazel seemed to skip the test no matter what I did. I tried both of these commands, and both of them skipped the fuzz test:

CC=clang CXX=clang++ bazel test ...

and

bazel test --compiler=clang ...

I'm probably doing something wrong at the Bazel level. If cc_fuzz_target() has this built in I wouldn't need to figure out the right incantation to get this working. :)

fmeum commented 2 years ago

This is a very good suggestion and I don't understand why it doesn't work. I filed https://github.com/bazelbuild/bazel/issues/13841 and will investigate further.

haberman commented 2 years ago

Awesome, thanks for looking into it!

fmeum commented 2 years ago

I found a design doc explaining why target or execution platforms are not the right place for compiler constraints. In fact, I believe that the @bazel_tools//tools/cpp:... constraint_values are only used to select toolchains (via exec_compatible_with), not targets. There is nice syntax for your use case described in the Incompatible Target Skipping design doc, but it hasn't been implemented yet.

What does work today is the following:

config_setting(
    name = "is_clang",
    flag_values = {"@bazel_tools//tools/cpp:compiler": "clang"},
)

cc_test(
    name = "clang",
    srcs = ["empty.cc"],
    target_compatible_with = select({
        ":is_clang": [],
        "//conditions:default": ["@platforms//:incompatible"]
    }),
)

@haberman If you find that this works for you, I would look into whether rules_fuzzing fuzz tests could have this behavior out of the box.

haberman commented 2 years ago

That seems to work!

I'm slightly surprised that a default blaze test uses gcc instead of cc (on my system cc symlinks to clang). But I know now that I need to explicitly specify clang to get clang.

fmeum commented 2 years ago

@stefanbucur Would you like to preserve compatibility with Bazel 3.x? If that is not a priority, the fuzz tests macros could use target_compatible_with to get graceful skipping with compiler other than clang by default.

stefanbucur commented 2 years ago

Technically, clang is only required for the libfuzzer configuration. So disabling the targets in non-clang configurations for all fuzzing engines seems a bit too broad.

The replay fuzzing engine should actually work with all major compilers, and can be used to turn all fuzz tests into regression tests.

But I see from @haberman's report that this doesn't happen:

However, I discovered that the cc_fuzz_target() would then fail to build with other compilers. For example, compiling with GCC I got the following error:

bazel-out/k8-fastbuild-ST-6de91758680c/bin/external/rules_fuzzing/fuzzing/replay/_objs/replay_main/replay_main.pic.o:replay_main.cc:function main: error: undefined reference to 'LLVMFuzzerInitialize'

I think this part is problematic, since the symbol was supposed to be weakly defined: https://github.com/bazelbuild/rules_fuzzing/blob/b2b3efdcd70d319c6e0e9b687baff3b54e5a5711/fuzzing/replay/replay_main.cc#L25

Maybe #139 introduced an incompatibility with GCC? @fmeum would you like to look into this?

fmeum commented 2 years ago

I will try to restore GCC compatibility and verify it in the CI.