More user-friendly libFuzzer flag

[george.karpenkov]

Summary

Currently, adding libfuzzer support is quite involved:

• User needs to manually link libFuzzer.a (which previously they needed to build themselves)
• User needs to specify coverage flags (-fsanitize-coverage-trace-pc-guard and friends), which are very long, and from my understanding subject to change.
• When compiling C files, user also would need to link libcxx.

With this patch we aim to simplify the procedure, requiring a single -fsanitize=fuzzer flag instead, which automatically inserts the required options.

[kcc]

It makes sense to have some shorter flag combination, I've been asked for it a couple of times.
One more reason for such flag is that I hope to have more/better/other coverage instrumentation and I don't want to make all users change their build rules.

It's not totally clear what such flag combination should be.

-fsanitize=fuzzer is a bit confusing, because it actually behaves differently compared to all other -fsanitize=foo (e.g. it provides main())

Another problem (not on OSX) is with msan. What if the user does -fsanitize=memory,fuzzer?
Right now this needs the libFuzzer itself to be msan-instrumented. :(

But ok, let's start from here

And yes, I'd really appreciate a linux implementation and documentation update.

[george.karpenkov]

it provides main()

We actually had a counterintuitive behavior while linking libfuzzer with something which has main already:

• if the target file already has main, it is executed. Lack of LLVMFuzzerTestOneInput is ignored.
• otherwise, fuzzing is done using LLVMFuzzerTestOneInput.
• If neither LLVMFuzzerTestOneInputnor main are present, linker complains about the lack of LLVMFuzzerTestOneInput.

We were not able to figure out what causes such behavior, hints will be appreciated.

And yes, I will upload docs and linux support shortly.

[kcc]

We were not able to figure out what causes such behavior, hints will be appreciated.

This is a linker-dependent behavior, which is hard to predict (a form of ODR violation).
Also depends on the order of the arguments to the linker.
E.g. on Linux:
% cat main.cc
int main() {}
% clang++ main.cc libFuzzer.a && ./a.out

produces a.out with empty main()

% clang++ libFuzzer.a main.cc
main.cc:(.text+0x0): multiple definition of `main'

[george.karpenkov]

I've added and tested linux support, documentation coming in a separate patch (a different repo).

[george.karpenkov]

Updated documentation in https://reviews.llvm.org/D32257

[kcc]

Please also add one full test with -fsanitize=fuzzer in lib/Fuzzer/test (probably, will need to create a subdir). Ok to have it in a separate change.

[kcc]

LGTM

[george.karpenkov]

Committed in 301212

[george.karpenkov]

Please also add one full test

I assume you don't want to simply change the flag in test/CMakeLists.txt to -fsanitize=fuzzer, and you would prefer to specify the coverage explicitly for tests?

As for the test, would you prefer to simply duplicate an existing test, or write a similar new one?

[kcc]

There is a problem with -fsanitize=fuzzer

It's both a compiler flag and a linker flags (just as -fsanitize=address), which is convenient since the users don't need to specify two flags.
But it's also a trouble because now if you set CFLAGS="-fsanitize=fuzzer" in some build systems (I've hit it in libpng) this flag will also be used for linking during the configure step and will cause failures,
because -fsanitize=fuzzer is supposed to work only with fuzz targets (a library w/o main and with LLVFuzzerTestOneInput).

Thoughts?

[george.karpenkov]

@kcc hmm that's strange, I did check logic *not* to link libLLVMFuzzer if output is a shared object. What is the output in your case? Is Clang recent?

[george.karpenkov]

@kcc otherwise even libFuzzer tests would fail, as they produce shared objects.

[kcc]

I am using TotT clang to build libpng with fsanitize=fuzzer and libpng's configure script fails
configure: error: C compiler cannot create executables

And of course it does!
It creates a tiny .c file with main() and tries to build it:
configure:3306: clang -O2 -fno-omit-frame-pointer -g -fsanitize=address,fuzzer conftest.c >&5
/tmp/conftest-e56ae6.o: In function `main':
...BUILD/conftest.c:14: multiple definition of `main'

[kubamracek]

If adding -fsanitize=fuzzer into CFLAGS enables it for linking as well and if that's undesired, how about adding -fno-sanitize=fuzzer into LDFLAGS?

[kcc]

OMG no.

I was thinking maybe to have a separate linker flag (-lfuzzer? -llibFuzzer? -libFuzzer?).
Yes, annoying to pass another flag, but I don't see a better solution for this. At least not yet

[george.karpenkov]

@kcc I guess I didn't have this problem yet as somehow on macos the symbol collision seems to be resolved "properly" in most cases.
Honestly the way I would do it is by providing a function run_fuzzer which would take a function pointer with an explicit callback.

But that's a large change, and I assume you would not want that?

For the libpng case, you can not link off-the-shelf library with libFuzzer anyway, right? As it does not have LLVMFuzzerTestOneInput symbol.
So the solution seems to be not to compile it with -fsanitize=fuzzer, but only with coverage flags, and then compile and link the fuzzing driver itself
with -fsanitize=fuzzer.
If there are too many coverage flags (which also keep changing) maybe we could also have -fsanitize=coverage or similar? In the driver then,
-fsanitize=fuzzer would link libc++ and imply -fsanitize=coverage.

[kcc]

In D32193#838574, @george.karpenkov wrote:

@kcc I guess I didn't have this problem yet as somehow on macos the symbol collision seems to be resolved "properly" in most cases.
Honestly the way I would do it is by providing a function run_fuzzer which would take a function pointer with an explicit callback.

And then all fuzzing engines will have a different API, and we won't have engine interoperability.

But that's a large change, and I assume you would not want that?

For the libpng case, you can not link off-the-shelf library with libFuzzer anyway, right?

It's not the libarary, it's the configure tests that the build system uses (most GNU libraries use it)

As it does not have LLVMFuzzerTestOneInput symbol.
So the solution seems to be not to compile it with -fsanitize=fuzzer, but only with coverage flags, and then compile and link the fuzzing driver itself
with -fsanitize=fuzzer.

Yes.

If there are too many coverage flags (which also keep changing) maybe we could also have -fsanitize=coverage or similar? In the driver then,
-fsanitize=fuzzer would link libc++ and imply -fsanitize=coverage.

Yes, that will probably be ok (except the name. maybe -fsanitize=fuzzer_coverage?)

[george.karpenkov]

@kcc I'm fine with either name.

[kcc]

Naming is hard. maybe -fsanitize=fuzzer-no-link?

[george.karpenkov]

@kcc yeah better than having underscores.

[kcc]

Than -fsanitize=fuzzer-no-link :)
Will you have time to make this change?

[george.karpenkov]

@kcc OK I think we need it anyway, since relying on correct "main" resolution is fragile.

include/clang/Basic/Sanitizers.def

 SANITIZER("address", Address)
 
 // Kernel AddressSanitizer (KASan)
 SANITIZER("kernel-address", KernelAddress)
 
 // MemorySanitizer
 SANITIZER("memory", Memory)
 
+// libFuzzer
+SANITIZER("fuzzer", Fuzzer)
+
 // ThreadSanitizer
 SANITIZER("thread", Thread)
 
 // LeakSanitizer
 SANITIZER("leak", Leak)
 
 // UndefinedBehaviorSanitizer
 SANITIZER("alignment", Alignment)

include/clang/Driver/SanitizerArgs.h

  public:
   /// Parses the sanitizer arguments from an argument list.
   SanitizerArgs(const ToolChain &TC, const llvm::opt::ArgList &Args);
 
   bool needsAsanRt() const { return Sanitizers.has(SanitizerKind::Address); }
   bool needsSharedAsanRt() const { return AsanSharedRuntime; }
   bool needsTsanRt() const { return Sanitizers.has(SanitizerKind::Thread); }
   bool needsMsanRt() const { return Sanitizers.has(SanitizerKind::Memory); }
+  bool needsFuzzer() const { return Sanitizers.has(SanitizerKind::Fuzzer); }
   bool needsLsanRt() const {
     return Sanitizers.has(SanitizerKind::Leak) &&
            !Sanitizers.has(SanitizerKind::Address);
   }
   bool needsUbsanRt() const;
   bool needsDfsanRt() const { return Sanitizers.has(SanitizerKind::DataFlow); }
   bool needsSafeStackRt() const {
     return Sanitizers.has(SanitizerKind::SafeStack);

lib/Driver/SanitizerArgs.cpp

if (Arg->getOption().matches(options::OPT_fsanitize_EQ)) {
       Add = expandSanitizerGroups(Add);
       // Group expansion may have enabled a sanitizer which is disabled later.
       Add &= ~AllRemove;
       // Silently discard any unsupported sanitizers implicitly enabled through
       // group expansion.
       Add &= ~InvalidTrappingKinds;
       Add &= Supported;
 
+      // Enable coverage if the fuzzing flag is set.
+      if (Add & Fuzzer)
+        CoverageFeatures |= CoverageTracePCGuard | CoverageIndirCall | CoverageTraceCmp;
+
       Kinds |= Add;
     } else if (Arg->getOption().matches(options::OPT_fno_sanitize_EQ)) {
       Arg->claim();
       SanitizerMask Remove = parseArgValues(D, Arg, true);
       AllRemove |= expandSanitizerGroups(Remove);
     }
   }
 

lib/Driver/ToolChains/Darwin.h

public:
 
   /// Add the linker arguments to link the ARC runtime library.
   virtual void AddLinkARCArgs(const llvm::opt::ArgList &Args,
                               llvm::opt::ArgStringList &CmdArgs) const {}
 
   /// Add the linker arguments to link the compiler runtime library.
   virtual void AddLinkRuntimeLibArgs(const llvm::opt::ArgList &Args,
                                      llvm::opt::ArgStringList &CmdArgs) const;
+  void AddLinkToFuzzer(const llvm::opt::ArgList &Args,
+                       llvm::opt::ArgStringList &CmdArgs) const;
 
   virtual void addStartObjectFileArgs(const llvm::opt::ArgList &Args,
                                       llvm::opt::ArgStringList &CmdArgs) const {
   }
 
   virtual void addMinVersionArgs(const llvm::opt::ArgList &Args,
                                  llvm::opt::ArgStringList &CmdArgs) const {}
 

lib/Driver/ToolChains/Darwin.cpp

if (AddRPath) {
 
     // Add the path to the resource dir to rpath to support using the dylib
     // from the default location without copying.
     CmdArgs.push_back("-rpath");
     CmdArgs.push_back(Args.MakeArgString(Dir));
   }
 }
 
+void MachO::AddFuzzerLinkArgs(const ArgList &Args, ArgStringList &CmdArgs) const {
+
+  // Go up one directory from Clang to find the libfuzzer archive file.
+  StringRef ParentDir = llvm::sys::path::parent_path(getDriver().InstalledDir);
+  SmallString<128> P(ParentDir);
+  llvm::sys::path::append(P, "lib", "libLLVMFuzzer.a");
+  CmdArgs.push_back(Args.MakeArgString(P));
+
+  // Libfuzzer is written in C++ and requires libcxx.
+  AddCXXStdlibLibArgs(Args, CmdArgs);
+}
+
 StringRef Darwin::getPlatformFamily() const {
   switch (TargetPlatform) {
     case DarwinPlatformKind::MacOS:
       return "MacOSX";
     case DarwinPlatformKind::IPhoneOS:
     case DarwinPlatformKind::IPhoneOSSimulator:
       return "iPhone";
     case DarwinPlatformKind::TvOS:
void DarwinClang::AddLinkRuntimeLibArgs(const ArgList &Args,
 
   const SanitizerArgs &Sanitize = getSanitizerArgs();
   if (Sanitize.needsAsanRt())
     AddLinkSanitizerLibArgs(Args, CmdArgs, "asan");
   if (Sanitize.needsUbsanRt())
     AddLinkSanitizerLibArgs(Args, CmdArgs, "ubsan");
   if (Sanitize.needsTsanRt())
     AddLinkSanitizerLibArgs(Args, CmdArgs, "tsan");
+  if (Sanitize.needsFuzzer())
+    AddFuzzerLinkArgs(Args, CmdArgs);
   if (Sanitize.needsStatsRt()) {
     StringRef OS = isTargetMacOS() ? "osx" : "iossim";
     AddLinkRuntimeLib(Args, CmdArgs,
                       (Twine("libclang_rt.stats_client_") + OS + ".a").str(),
                       /*AlwaysLink=*/true);
     AddLinkSanitizerLibArgs(Args, CmdArgs, "stats");
   }
   if (Sanitize.needsEsanRt())
if (isTargetIOSBased() || isTargetWatchOSBased() ||
     return;
   getDriver().Diag(diag::err_arc_unsupported_on_toolchain);
 }
 
 SanitizerMask Darwin::getSupportedSanitizers() const {
   const bool IsX86_64 = getTriple().getArch() == llvm::Triple::x86_64;
   SanitizerMask Res = ToolChain::getSupportedSanitizers();
   Res |= SanitizerKind::Address;
+  Res |= SanitizerKind::Fuzzer;
   if (isTargetMacOS()) {
     if (!isMacosxVersionLT(10, 9))
       Res |= SanitizerKind::Vptr;
     Res |= SanitizerKind::SafeStack;
     if (IsX86_64)
       Res |= SanitizerKind::Thread;
   } else if (isTargetIOSSimulator() || isTargetTvOSSimulator()) {
     if (IsX86_64)
       Res |= SanitizerKind::Thread;
   }
   return Res;
 }
 
 void Darwin::printVerboseInfo(raw_ostream &OS) const {
   CudaInstallation.print(OS);
 }

lib/Driver/ToolChains/Linux.cpp

const bool IsPowerPC64 = getTriple().getArch() == llvm::Triple::ppc64 ||
                            getTriple().getArch() == llvm::Triple::ppc64le;
   const bool IsAArch64 = getTriple().getArch() == llvm::Triple::aarch64 ||
                          getTriple().getArch() == llvm::Triple::aarch64_be;
   const bool IsArmArch = getTriple().getArch() == llvm::Triple::arm ||
                          llvm::Triple::thumb || llvm::Triple::armeb ||
                          llvm::Triple::thumbeb;
   SanitizerMask Res = ToolChain::getSupportedSanitizers();
   Res |= SanitizerKind::Address;
+  Res |= SanitizerKind::Fuzzer;
   Res |= SanitizerKind::KernelAddress;
   Res |= SanitizerKind::Vptr;
   Res |= SanitizerKind::SafeStack;
   if (IsX86_64 || IsMIPS64 || IsAArch64)
     Res |= SanitizerKind::DataFlow;
   if (IsX86_64 || IsMIPS64 || IsAArch64 || IsX86 || IsArmArch)
     Res |= SanitizerKind::Leak;
   if (IsX86_64 || IsMIPS64 || IsAArch64 || IsPowerPC64)

test/Driver/fuzzer.c

+// Test flags inserted by -fsanitize=fuzzer.
+
+// RUN: %clang -fsanitize=fuzzer %s -### 2>&1 | FileCheck --check-prefixes=CHECK-FUZZER-LIB,CHECK-LIBCXX,CHECK-COVERAGE-FLAGS %s
+//
+// CHECK-FUZZER-LIB: libLLVMFuzzer.a
+// CHECK-LIBCXX: -lc++
+// CHECK-COVERAGE: -fsanitize-coverage-trace-pc-guard
+// CHECK-COVERAGE-SAME: -fsanitize-coverage-indirect-calls
+// CHECK-COVERAGE-SAME: -fsanitize-coverage-trace-cmp
+
+// RUN: %clang -fsanitize=fuzzer %s
+
+int LLVMFuzzerTestOneInput(const char *Data, long Size) {
+  return 0;
+}