[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.

llvm/trunk/lib/Fuzzer/test/CMakeLists.txt

 else()
   set(HAS_LSAN 1)
 endif()
 
 foreach(Test ${Tests})
   add_libfuzzer_test(${Test} SOURCES ${Test}.cpp)
 endforeach()
 
+function(test_export_symbol target symbol)
+  if(MSVC)
+    set_target_properties(LLVMFuzzer-${target} PROPERTIES LINK_FLAGS
+        "-export:${symbol}")
+  endif()
+endfunction()
+
+test_export_symbol(InitializeTest "LLVMFuzzerInitialize")
+test_export_symbol(BogusInitializeTest "LLVMFuzzerInitialize")
+test_export_symbol(CustomCrossOverTest "LLVMFuzzerCustomCrossOver")
+test_export_symbol(CustomMutatorTest "LLVMFuzzerCustomMutator")
+
 ###############################################################################
 # Unit tests
 ###############################################################################
 
 add_executable(LLVMFuzzer-Unittest
   FuzzerUnittest.cpp
   )