[libFuzzer] Diff 24 - Use clang as linker in Windows, to properly include sanitizer libraries.

[kcc]

lib/Fuzzer/test/CMakeLists.txt

41 ↗

(On Diff #81816)

sounds wrong.
why we can't have this in the clang driver?

[rnk]

lib/Fuzzer/test/CMakeLists.txt

41 ↗

(On Diff #81816)

We can't have it in the clang driver because the clang driver isn't called to perform links on Windows, as explained in this comment. This CL seems to do what you're suggesting: call the driver and keep the logic to find sanitizer runtimes there.

[mpividori]

lib/Fuzzer/test/CMakeLists.txt

41 ↗

(On Diff #81816)

@kcc
If we use clang, it will automatically include the libraries from compiler-rt when using the linker (link.exe)
If we don't add this modification, cmake will directly try to link using link.exe, but it won't include the libraries, so it will fail.
It is different in linux, because the linker receives the compiler flags. As far a I understand.

[zturner]

Try passing -fuse-ld=lld to clang-cl. We basically whitelist certain gcc-style options, and this is one of them. That's why -g doesn't work, but certain other options do. It's kind of a hodgepodge and there's no clearly defined pattern for which ones we accept and which ones we don't.

In any way, in this case -fuse-ld=lld should force it to use lld.

[kcc]

I greatly dislike the need for such a change.
What is the command line with just clang-cl (no cmake) to build libFuzzer.lib?

[rnk]

In D27869#631353, @kcc wrote:

I greatly dislike the need for such a change.
What is the command line with just clang-cl (no cmake) to build libFuzzer.lib?

To build an executable using libFuzzer, or to build the static archive libFuzzer.lib?

[kcc]

In D27869#631524, @rnk wrote:

In D27869#631353, @kcc wrote:

I greatly dislike the need for such a change.
What is the command line with just clang-cl (no cmake) to build libFuzzer.lib?

To build an executable using libFuzzer, or to build the static archive libFuzzer.lib?

I was asking about "build the static archive libFuzzer.lib" but actually I'd like to see both.

[rnk]

In D27869#631527, @kcc wrote:

I was asking about "build the static archive libFuzzer.lib" but actually I'd like to see both.

LLVMFuzzer.lib is basically this command:

link.exe /lib /nologo /machine:x64 /out:lib\LLVMFuzzer.lib lib\Fuzzer\CMakeFiles\LLVMFuzzerNoMainObjects.dir\*.obj lib\Fuzzer\CMakeFiles\LLVMFuzzer.dir\FuzzerMain.cpp.obj

LLVMFuzzer-NullDerefTest links like this:

link.exe /nologo lib\Fuzzer\test\CMakeFiles\LLVMFuzzer-NullDerefTest.dir\NullDerefTest.cpp.obj  /out:lib\Fuzzer\test\LLVMFuzzer-NullDerefTest.exe /implib:lib\LLVMFuzzer-NullDerefTest.lib /pdb:lib\Fuzzer\test\LLVMFuzzer-NullDerefTest.pdb /version:0.0  /machine:x64 /MANIFEST:NO /STACK:10000000 -debug /INCREMENTAL:NO /subsystem:console  lib\LLVMFuzzer.lib kernel32.lib user32.lib gdi32.lib winspool.lib shell32.lib ole32.lib oleaut32.lib uuid.lib comdlg32.lib advapi32.lib

Many of those arguments are unnecessary, and would maybe not be present without cmake, but the important point is that executables are not linked by calling the clang driver on Windows. This is true in every Windows build system I've ever encountered that wasn't ported from Unix.

The link step above fails because the ASan RTL is not listed on the link line. We can solve this problem in either of these ways:

1. Break convention as Marcos is doing here and call clang-cl to do the link manually.
2. Duplicate the clang driver logic to pass the appropriate asan runtimes into LLVM cmake. Chromium already duplicates this logic itself.

[mpividori]

@kcc
To build libFuzer.lib this changes are not needed, they are only needed when compiling the code to be fuzzed with cmake, which means, it is only necessary for the tests. Because of that, I include this changes in the CMake file in the test folder only.
If we want to compile the tests on command line with clang-cl, it would be:

clang-cl -fsanitize-coverage=trace-pc-guard,....  -fsanitize=address Test.cpp LLVMFuzzer.lib /MD /link /SUBSYSTEM:CONSOLE

Cmake split this command in 2 steps, compiling and linking:

clang-cl -fsanitize-coverage=trace-pc-guard -fsanitize=address /c Test.cpp
link.exe /MD Test.obj LLVMFuzzer.lib /SUBSYSTEM:CONSOLE

The last command fails, because we don't provide compiler-rt libraries to the linker.

So, basically, I change the link command, so cmake execute this:

clang-cl -fsanitize-coverage=trace-pc-guard -fsanitize=address /c Test.cpp
clang-cl -fsanitize-coverage=trace-pc-guard -fsanitize=address /MD Test.obj LLVMFuzzer.lib /link /SUBSYSTEM:CONSOLE

Now, clang-cl will include compiler-rt libraries.

[kcc]

How do we handle asan tests on windows?
They should have the same problem.

[rnk]

In D27869#631541, @kcc wrote:

How do we handle asan tests on windows?
They should have the same problem.

The ASan lit tests? They invoke clang directly to perform the link, so the driver logic does it for us:

// RUN: %clangxx_asan -O0 %s -o %t && not %run %t 2>&1 | FileCheck %s

The "unit" tests I think also invoke clang directly.

[kcc]

LGTM
Ok, (I think) I understand now.
Please leave a TODO saying that once we have a monorepo we should simply require the just-built clang.

[george.karpenkov]

@kcc @rnk I am looking at this change now, as it makes porting to compiler-rt more difficult.
In all other tests (asan/tsan/etc) test runtime is linked together with the executable instead, which contains all the libraries injected by the driver.
If we do that, would it be possible to not apply this change?

[kcc]

AFAICT, libFuzzer on Windows has no active users or contributors.
If you ask me, I would prefer to

a) disable check-fuzzer on the windows bot
b) complete the migration from llvm to compiler-rt, while preserving the status quo only on Linux and Mac
c) let anyone who cares about windows (if anyone left) fix it once the dust settles.

The same also applies to our recent discussion around r309646 where the windows part is blocking me.

Reid, Zack, WDYT?

[george.karpenkov]

@kcc I strongly agree with your idea of dropping the support, at least until someone is prepared to heavily invest into this (and invest using good code style, without writing scary CMake hacks)

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

foreach (flag ${BUILD_FLAGS_AS_LIST})
     endif()
   endforeach()
   set(${VARNAME} "${new_flags}")
 endforeach()
 
 # Enable the coverage instrumentation (it is disabled for the Fuzzer lib).
 set(CMAKE_CXX_FLAGS "${LIBFUZZER_FLAGS_BASE} -fsanitize-coverage=trace-pc-guard,indirect-calls,trace-cmp,trace-div,trace-gep -gline-tables-only")
 
+if(MSVC)
+  # In order to use the sanitizers in Windows, we need to link against many
+  # runtime libraries which will depend on the target being created
+  # (executable or dll) and the c runtime library used (MT/MD).
+  # By default, cmake uses link.exe for linking, which fails because we don't
+  # specify the appropiate dependencies.
+  # As we don't want to consider all of that possible situations which depends
+  # on the implementation of the compiler-rt, the simplest option is to change
+  # the rules for linking executables and shared libraries, using the compiler
+  # instead of link.exe. Clang will consider the sanitizer flags, and
+  # automatically provide the required libraries to the linker.
+  set(CMAKE_CXX_LINK_EXECUTABLE "<CMAKE_CXX_COMPILER> <FLAGS> ${CMAKE_CXX_FLAGS} <OBJECTS> -o <TARGET> <LINK_LIBRARIES> /link <CMAKE_CXX_LINK_FLAGS> <LINK_FLAGS>")
+  set(CMAKE_CXX_CREATE_SHARED_LIBRARY "<CMAKE_CXX_COMPILER> ${CMAKE_CXX_FLAGS} /LD <CMAKE_SHARED_LIBRARY_CXX_FLAGS> <CMAKE_SHARED_LIBRARY_CREATE_CXX_FLAGS> <CMAKE_SHARED_LIBRARY_SONAME_CXX_FLAG> <TARGET_SONAME> -o <TARGET> <OBJECTS> <LINK_LIBRARIES> /link <LINK_FLAGS>")
+endif()
+
 # add_libfuzzer_test(<name>
 #   SOURCES source0.cpp [source1.cpp ...]
 #   )
 #
 #   Declares a LibFuzzer test executable with target name LLVMFuzzer-<name>.
 #
 #   One or more source files to be compiled into the binary must be declared
 #   after the SOURCES keyword.