[libFuzzer] Export external functions.

[mpividori]

Summary

We need to export external functions so they are found when calling GetProcAddress() on Windows.

[kcc]

I understand the problem, but I don't like the solution, I will need to think more.

[mpividori]

@kcc These tests pass on Darwin? I wonder if they pass, because we use dlsym on Darwin, so these functions should be exported, with visibility default.

[mpividori]

@kcc do you agree with this diff? I include the header in InitializeTest and BogusInitializeTest, the same that we do for CustomCrossOverTest and CustomMutatorTest.

[kcc]

No, I don't agree with this change.
First, FuzzerInterface.h must not depend on any other header.
Second, I don't want the tests depend on "FuzzerInterface.h"

I realize this is not in accordance to C++ best practices and I don't know how long
we'll be able to have fuzz targets not include any headers like "FuzzerInterface.h".
But I want to keep things as they are for now.

The main rationale here: the fuzz targest must be completely independent from the fuzz engines and the interface must be very thin.

Feel free to disable the tests on windows with a comment describing the problem and pointing to this review.

[mpividori]

@kcc Ok, this final diff is the best I can do.
I tried to use an "export.def" file, but that is not possible because these symbols are optional. I mean, they are not always present.
In the "export.def" I can not specify a symbol as optional. If it is not defined the linker fails.

So, I think these changes are ok, would you agree? I only add dllexports for the tests on Windows.
Otherwise, I should disable the tests, but I really think we should consider that tests on Windows.

[mpividori]

@kcc @zturner Now, I only modify the cmake file.

[kcc]

LGTM
I dislike this much less than the previous variants. Thanks!

[zturner]

How about this:

set(Tests
  AbsNegAndConstantTest
  AbsNegAndConstant64Test
  AccumulateAllocationsTest
  BufferOverflowOnInput
  CallerCalleeTest
  CounterTest
  CxxStringEqTest
  DivTest
  EmptyTest
  EquivalenceATest
  EquivalenceBTest
  FourIndependentBranchesTest
  FullCoverageSetTest
  MemcmpTest
  LeakTest
  LeakTimeoutTest
  LoadTest
  NullDerefTest
  NullDerefOnEmptyTest
  NthRunCrashTest
  OneHugeAllocTest
  OutOfMemoryTest
  OutOfMemorySingleLargeMallocTest
  RepeatedMemcmp
  RepeatedBytesTest
  SimpleCmpTest
  SimpleDictionaryTest
  SimpleHashTest
  SimpleTest
  SimpleThreadedTest
  SingleByteInputTest
  SingleMemcmpTest
  SingleStrcmpTest
  SingleStrncmpTest
  SpamyTest
  ShrinkControlFlowTest
  ShrinkValueProfileTest
  StrcmpTest
  StrncmpOOBTest
  StrncmpTest
  StrstrTest
  SwapCmpTest
  SwitchTest
  Switch2Test
  ThreadedLeakTest
  ThreadedTest
  TimeoutTest
  TimeoutEmptyTest
  TraceMallocTest
  )

set(ExportingTests
  BogusInitializeTest
  CustomCrossOverTest
  InitializeTest
  CustomMutatorTest
  )

foreach(Test ${ExportingTests})
  add_libfuzzer_test(${Test} SOURCES ${Test}.cpp EXPORT ${ExportSymbolName})
endforeach()

Then change add_libfuzzer_test() so that it has some code like this:

if(MSVC)
  string(CONCAT ExportSymbolName "LLVM" ${Test})
  set_target_properties(LLVMFuzzer-${target} PROPERTIES LINK_FLAGS
      "-export:${symbol}")
endif()

Probably some details need to be worked out, but the main idea here is that the *only* thing anyone has to do to get a test to be correct here is add it to the correct list. They just add one line to the set(ExportingTests and everything works.

One thing is that you would need to change the test .cpp file to change the name of of the fuzzer function so that it can be deduced from the name of the test. Currently the Bogus test wouldn't work like this.

All that said, I'm fine with the original change as it is proposed, but I think this might make it a little easier, so maybe kcc@ likes this more? Up to him :)

[zturner]

Nevermind, he already lgtm'ed. Ignore me!

[zturner]

At the very least though define and call the function always, and put the if (MSVC) inside the function.

[mpividori]

@zturner Ok, thanks for your feedback. Done.

[zturner]

Looks good, I can think of some improvements to make this even more automatic, but this is good for now.

lib/Fuzzer/FuzzerInterface.h

 // all you need is to define the following function in your file:
 // extern "C" int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size);
 
 // WARNING: keep the interface in C.
 
 #ifndef LLVM_FUZZER_INTERFACE_H
 #define LLVM_FUZZER_INTERFACE_H
 
+#include "FuzzerDefs.h"
 #include <stddef.h>
 #include <stdint.h>
 
 #ifdef __cplusplus
 extern "C" {
 #endif  // __cplusplus
 
 // Mandatory user-provided target function.
 // Executes the code under test with [Data, Data+Size) as the input.
 // libFuzzer will invoke this function *many* times with different inputs.
 // Must return 0.
 int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size);
 
 // Optional user-provided initialization function.
 // If provided, this function will be called by libFuzzer once at startup.
 // It may read and modify argc/argv.
 // Must return 0.
+ATTRIBUTE_INTERFACE
 int LLVMFuzzerInitialize(int *argc, char ***argv);
 
 // Optional user-provided custom mutator.
 // Mutates raw data in [Data, Data+Size) inplace.
 // Returns the new size, which is not greater than MaxSize.
 // Given the same Seed produces the same mutation.
+ATTRIBUTE_INTERFACE
 size_t LLVMFuzzerCustomMutator(uint8_t *Data, size_t Size, size_t MaxSize,
                                unsigned int Seed);
 
 // Optional user-provided custom cross-over function.
 // Combines pieces of Data1 & Data2 together into Out.
 // Returns the new size, which is not greater than MaxOutSize.
 // Should produce the same mutation given the same Seed.
+ATTRIBUTE_INTERFACE
 size_t LLVMFuzzerCustomCrossOver(const uint8_t *Data1, size_t Size1,
                                  const uint8_t *Data2, size_t Size2,
                                  uint8_t *Out, size_t MaxOutSize,
                                  unsigned int Seed);
 
 // Experimental, may go away in future.
 // libFuzzer-provided function to be used inside LLVMFuzzerCustomMutator.
 // Mutates raw data in [Data, Data+Size) inplace.
 // Returns the new size, which is not greater than MaxSize.
 size_t LLVMFuzzerMutate(uint8_t *Data, size_t Size, size_t MaxSize);
 
 #ifdef __cplusplus
 }  // extern "C"
 #endif  // __cplusplus
 
 #endif  // LLVM_FUZZER_INTERFACE_H

lib/Fuzzer/test/BogusInitializeTest.cpp

 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 
 // Make sure LLVMFuzzerInitialize does not change argv[0].
 #include <stddef.h>
 #include <stdint.h>
 
+#include "FuzzerInterface.h"
+
 extern "C" int LLVMFuzzerInitialize(int *argc, char ***argv) {
   ***argv = 'X';
   return 0;
 }
 
 extern "C" int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size) {
   return 0;
 }

lib/Fuzzer/test/InitializeTest.cpp

 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 
 // Make sure LLVMFuzzerInitialize is called.
 #include <assert.h>
 #include <stddef.h>
 #include <stdint.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
 
+#include "FuzzerInterface.h"
+
 static char *argv0;
 
 extern "C" int LLVMFuzzerInitialize(int *argc, char ***argv) {
   assert(*argc > 0);
   argv0 = **argv;
   fprintf(stderr, "LLVMFuzzerInitialize: %s\n", argv0);
   return 0;
 }