[libFuzzer] Diff 25 - Fix test with shared libraries on Windows.

[kcc]

lib/Fuzzer/test/CMakeLists.txt

218 ↗

(On Diff #81818)

That's bad. Please try to find a way to avoid this.

[zturner]

If

lib/Fuzzer/test/CMakeLists.txt

218 ↗

(On Diff #81818)

I'm not sure I follow the logic here. Without this patch, where do the .lib and .dll file end up, and is the problem occuring during link time or runtime?

[mpividori]

lib/Fuzzer/test/CMakeLists.txt

218 ↗

(On Diff #81818)

I tried to find a better solution, but didn't succeed.
Without this patch, the .lib ends in: "<CMAKE_BINARY_DIR>/bin" (because the .dll is created there with the rule: CMAKE_CXX_CREATE_SHARED_LIBRARY).
But cmake tries to find it in: "<CMAKE_BINARY_DIR>/lib"
So the problem is at link time.

With this patch, I let cmake knows that the .lib file is in "<CMAKE_BINARY_DIR>/bin".

[mpividori]

@zturner, The problem is that cmake will add "lib\LLVMFuzzer-DSO1.lib" to the linker, at it won't find it.

[zturner]

What's causing the .lib file to end up in the bin directory in the first place? We use DLLs in LLDB as well. we compile most of lldb into liblldb.dll, which produces liblldb.lib in the lib/ directory. Then when we link lldb.exe and specify the dependency to the dll's target, it automatically knows that it put the implib in the lib/ folder and links it from there.

Are we overriding that behavior in this CMake somewhere, because LLDB doesn't do anything special and by default the implib ends up in lib/. Is this perhaps a side effect of your earlier patch which overrides CMAKE_CXX_LINK_EXECUTABLE?

[mpividori]

@zturner I updated the code. I think this final solution is appropiate. Following cmake documentation:
+ I set ARCHIVE_OUTPUT_DIRECTORY as the destination of the import library.
+ I set RUNTIME_OUTPUT_DIRECTORY as the destination of the dll library.
Let me know if you agree.

[zturner]

But isn't the DLL now going into the lib directory too? I thought the DLL should go into the bin directory. This is how it works by default for clang and lldb. clang puts libclang.lib into lib/ and libclang.dll into bin/. Similarly for lldb. What are we doing differently that makes this not possible?

[mpividori]

@zturner thanks for your comment.
If I don't add this changes, both the dll and the import library will be created in the bin directory.
I think this is probably happening because of the rule: "CMAKE_CXX_CREATE_SHARED_LIBRARY" that we define.

As previous definitions move the shared library to lib/Fuzzer/lib , I decided it would be reasonable to do the same with the dll and import library.

[zturner]

beanz, do you know anything about how this works? On Windows when you build a shared library we get .lib files in the lib/ directory and .dll files in the bin/ directory, and when linking against a DLL CMake automatically knows to look in the lib/ directory for the .lib file but still put the resulting exe into the bin directory.

Is there some magic that we need to know to make this happen? I'd like for everything in libFuzzer to work the same way as it does in clang and lldb.

[beanz]

In most of LLVM & Clang this is done inside llvm_add_library. Buried in there we have a call to set_output_directory, which sets everything up.

libFuzzer could add a call to set_output_directory, or it could implement its library add to use llvm_add_library instead of calling add_library directly.

[mpividori]

@beanz Thanks for that information.

@zturner This libraries are only created for a specific test. So, I think that is the reason why Kostya outputs it to "${CMAKE_BINARY_DIR}/lib/Fuzzer/lib" instead of the main "${CMAKE_BINARY_DIR}/lib" directory.

I modified the cmake file, so now we create the dll in the same folder that the test: "${CMAKE_BINARY_DIR}/lib/Fuzzer/test".
And the import library in: "${CMAKE_BINARY_DIR}/lib/Fuzzer/lib". Would you agree?

Thanks.

[beanz]

BINARY_DIR is only used for the dynamic library on Windows, not on Unix. On Unix dynamic libraries go in the LIBRARY_DIR.

To make this patch work on Unix systems you would also need to add something like:

set_target_properties(LLVMFuzzer-DSO1 PROPERTIES
            BUILD_WITH_INSTALL_RPATH On
            INSTALL_RPATH "${CMAKE_BINARY_DIR}/lib/Fuzzer/lib")
set_target_properties(LLVMFuzzer-DSO2 PROPERTIES
            BUILD_WITH_INSTALL_RPATH On
            INSTALL_RPATH "${CMAKE_BINARY_DIR}/lib/Fuzzer/lib")

[mpividori]

@beanz thanks for your comments.
Yes, I output the shared library in LIBRARY_DIR for Unix, just like previous implementation does. (output the shared library in: "${CMAKE_BINARY_DIR}/lib/Fuzzer/lib")
Surprisingly, the tests works on linux, the loader finds the shared library...

[kcc]

LGTM, assuming this does not change the behavior on linux

[mpividori]

@kcc @zturner Again here. I was confused but the previous changes didn't fix this for Windows. This is the situation:

• We need to set BINARY_DIR to: ${CMAKE_BINARY_DIR}/lib/Fuzzer/test , so the dll is placed in the same directory than the test LLVMFuzzer-DSOTest, and is found when executing that test.

• As we are using CMAKE_CXX_CREATE_SHARED_LIBRARY to link the dll, we can't modify the output directory for the import library. It will be created in the same directory than the dll (in BINARY_DIR), no matter which value we set to LIBRARY_DIR. So, if we set LIBRARY_DIR to a different directory than BINARY_DIR, when linking LLVMFuzzer-DSOTest, cmake will look for the import library LLVMFuzzer-DSO1.lib in LIBRARY_DIR, and won't find it, since it was created in BINARY_DIR. So, for Windows, we need that LIBRARY_DIR and BINARY_DIR are the same directory.

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

 target_link_libraries(LLVMFuzzer-DSOTest
   LLVMFuzzer-DSO1
   LLVMFuzzer-DSO2
   LLVMFuzzer
   )
 
 set_target_properties(LLVMFuzzer-DSOTest PROPERTIES RUNTIME_OUTPUT_DIRECTORY
   "${CMAKE_BINARY_DIR}/lib/Fuzzer/test")
-set_target_properties(LLVMFuzzer-DSO1 PROPERTIES LIBRARY_OUTPUT_DIRECTORY
-  "${CMAKE_BINARY_DIR}/lib/Fuzzer/lib")
-set_target_properties(LLVMFuzzer-DSO2 PROPERTIES LIBRARY_OUTPUT_DIRECTORY
-  "${CMAKE_BINARY_DIR}/lib/Fuzzer/lib")
+
+if(MSVC)
+  set_output_directory(LLVMFuzzer-DSO1
+    BINARY_DIR "${CMAKE_BINARY_DIR}/lib/Fuzzer/test"
+    LIBRARY_DIR "${CMAKE_BINARY_DIR}/lib/Fuzzer/test")
+  set_output_directory(LLVMFuzzer-DSO2
+    BINARY_DIR "${CMAKE_BINARY_DIR}/lib/Fuzzer/test"
+    LIBRARY_DIR "${CMAKE_BINARY_DIR}/lib/Fuzzer/test")
+else(MSVC)
+  set_output_directory(LLVMFuzzer-DSO1
+    LIBRARY_DIR "${CMAKE_BINARY_DIR}/lib/Fuzzer/lib")
+  set_output_directory(LLVMFuzzer-DSO2
+    LIBRARY_DIR "${CMAKE_BINARY_DIR}/lib/Fuzzer/lib")
+endif()
 
 set(TestBinaries ${TestBinaries} LLVMFuzzer-DSOTest)
 
 ###############################################################################
 # Configure lit to run the tests
 #
 # Note this is done after declaring all tests so we can inform lit if any tests
 # need to be disabled.

llvm/trunk/lib/Fuzzer/test/DSO1.cpp

 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 
 // Source code for a simple DSO.
-
+#ifdef _WIN32
+__declspec( dllexport )
+#endif
 int DSO1(int a) {
   if (a < 123456)
     return 0;
   return 1;
 }
 
 void Uncovered1() { }

llvm/trunk/lib/Fuzzer/test/DSO2.cpp

 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 
 // Source code for a simple DSO.
-
+#ifdef _WIN32
+__declspec( dllexport )
+#endif
 int DSO2(int a) {
   if (a < 3598235)
     return 0;
   return 1;
 }
 
 void Uncovered2() {}