[libFuzzer] Support using libc++

[phosek]

Summary

This is needed in case the users of libFuzzer use libc++ in their code, which the fuzz target (libFuzzer) will be linked against. When libc++ source is available, we build a private version of it and link it against libFuzzer which allows using the same static library against codebases which use both libc++ and libstdc++.

[george.karpenkov]

Minor nitpick: maybe print a message if user ask for LIBCXX but that's not available?

[kcc]

Hm. Shouldn't we build two versions of libFuzzer by default?
(Oh my, this is one more trouble with using STL in a low-level library)

CMakeLists.txt

143 ↗

(On Diff #114392)

Why do we need it here?
What other parts of compiler-rt use C++ stdlib?

[george.karpenkov]

@kcc terrible suggestion: can we just copy'n paste vector/set/string implementations into libFuzzer from libc++? IIRC the license should be fine with it, and header libraries are recompiled with the source anyway.

[kcc]

Copy-paste -- no.
But if we can completely privatize the use of STL with some trick, that would be great (e.g. #define the namespace name to be something other than std).
This may work with STL containers, likely to work with STL algorithms, and unlikely to work with STL streams. If streams are the only problem, we could probably get rid of them.

[phosek]

In D37631#865032, @kcc wrote:

Hm. Shouldn't we build two versions of libFuzzer by default?
(Oh my, this is one more trouble with using STL in a low-level library)

Building two versions would work. We would need to alter the naming scheme and modify the driver to pick up the right version depending on the value -stdlib. We'll need some platform specific code to handle Darwin and Fuchsia which only support libc++.

[phosek]

In D37631#865062, @kcc wrote:

Copy-paste -- no.
But if we can completely privatize the use of STL with some trick, that would be great (e.g. #define the namespace name to be something other than std).
This may work with STL containers, likely to work with STL algorithms, and unlikely to work with STL streams. If streams are the only problem, we could probably get rid of them.

We would probably need a special configuration in libcxx/include/__config.

[kcc]

If we can privatize STL this would be a much more preferable solution -- no driver changes and multiple versions.
This may also solve libFuzzer+msan (right now we have to recompile libFuzzer for msan).

[phosek]

In D37631#865187, @kcc wrote:

If we can privatize STL this would be a much more preferable solution -- no driver changes and multiple versions.
This may also solve libFuzzer+msan (right now we have to recompile libFuzzer for msan).

I managed to "internalize" the names using _LIBCPP_ABI_VERSION, but now libFuzzer is breaking at link time because of undefined references (as expected). To solve that, we would probably have recompile libc++ using the internalized names and then package all the object files (or at least the necessary subset) into libFuzzer archive.

[kcc]

What symbols are missing?

[phosek]

The ones I'm seeing in our build are references to basic_string and vector.

[kcc]

In D37631#865376, @phosek wrote:

The ones I'm seeing in our build are references to basic_string and vector.

Weird. I would expect those to be fully inlined.

[phosek]

We can probably enforce by adding an explicit instantiations inside libFuzzer akin to https://github.com/llvm-mirror/libcxx/blob/master/src/vector.cpp#L14 and https://github.com/llvm-mirror/libcxx/blob/master/src/string.cpp#L22. I'll give it a try.

[phosek]

Hmm, it does seem to work, but there are more things that need to be pulled in so I'm somewhat concerned that if I keep pulling the thread, I'll eventually pull the entire libc++.

[kcc]

Let's see how many things you need first :)

[phosek]

I need std::thread which is mostly implemented in thread.cpp, not in headers.

[kcc]

What else?

std::thread is a syntax sugar, we can reimplement it with pthread easily.

[phosek]

Here is the full list: https://reviews.llvm.org/P8035. Mostly streams, some system stuff, mutex and thread.

[kcc]

If we can privatize vector, string, the other containers, and algorithms,
Will we be able to privatize sort? (I can see it in your output)
I'm ok to rewrite thread/mutex/stream/clock using C library (I can even do part of that). Will this be enough?

[kcc]

replacing std::istringstream with a hand-written-stuff would be very unfortunate :(

[kcc]

So, how about linking a full private version of libc++?
Is that even possible?

[phosek]

In D37631#865602, @kcc wrote:

So, how about linking a full private version of libc++?
Is that even possible?

I think it's possible but it won't be pretty. It's also complicated by the fact that libFuzzer is built as static library. What we would have to do is to build custom libc++ using a custom namespace, then build libFuzzer against that library, and finally combine the two static libraries together similarly to what libc++ is doing when merging libc++abi (https://github.com/llvm-mirror/libcxx/blob/master/lib/CMakeLists.txt#L268). I can give it a try if you're fine with that solution.

[kcc]

finally combine the two static libraries

libFuzzer.a is an implementation detail of -fsanitize=fuzzer
so why not just have two .a libs (libFuzzer.a and libFuzzer-private-libc++.a) and use both of them at link time?

[EricWF]

In D37631#869128, @phosek wrote:

In D37631#865602, @kcc wrote:

So, how about linking a full private version of libc++?
Is that even possible?

I think it's possible but it won't be pretty. It's also complicated by the fact that libFuzzer is built as static library. What we would have to do is to build custom libc++ using a custom namespace, then build libFuzzer against that library, and finally combine the two static libraries together similarly to what libc++ is doing when merging libc++abi (https://github.com/llvm-mirror/libcxx/blob/master/lib/CMakeLists.txt#L268). I can give it a try if you're fine with that solution.

This sounds reasonable to me. Libc++ could add an option that allows you to control the inline namespace it uses, hence giving you a version of libc++ that isn't necessarily *private* but is, at least, local to libFuzzer. After merging the archives, you shouldn't depend or conflict with external libc++ versions (**except for non-versioned types like exceptions, i'm not sure how relevant that is)

[phosek]

This does seem to work. Right now it's only used if COMPILER_RT_ENABLE_LIBCXX is set, but we can make it the default whenever libc++ source is available if that's fine with you.

[kcc]

In D37631#870913, @phosek wrote:

This does seem to work. Right now it's only used if COMPILER_RT_ENABLE_LIBCXX is set, but we can make it the default whenever libc++ source is available if that's fine with you.

I would rather require libc++ for libFuzzer to build. Isn't it simpler?

Vitaly, please help me review the cmake code.

We'll also need tests for any such change.

[vitalybuka]

Can we just rely on LLVM_ENABLE_LIBCXX instead of introducing COMPILER_RT_ENABLE_LIBCXX?

[phosek]

In D37631#874773, @vitalybuka wrote:

Can we just rely on LLVM_ENABLE_LIBCXX instead of introducing COMPILER_RT_ENABLE_LIBCXX?

Those fulfill different purpose, LLVM_ENABLE_LIBCXX enables building LLVM itself against libc++ (i.e. it passes -stdlib=libc++ to the host compiler), but it doesn't necessarily mean that the libc++ source is available. I think we should just rely on COMPILER_RT_HAS_LIBCXX_SOURCES, i.e. use libc++ anytime its sources are available (and we're building with Clang).

[phosek]

In D37631#876077, @phosek wrote:

In D37631#874773, @vitalybuka wrote:

Can we just rely on LLVM_ENABLE_LIBCXX instead of introducing COMPILER_RT_ENABLE_LIBCXX?

Those fulfill different purpose, LLVM_ENABLE_LIBCXX enables building LLVM itself against libc++ (i.e. it passes -stdlib=libc++ to the host compiler), but it doesn't necessarily mean that the libc++ source is available. I think we should just rely on COMPILER_RT_HAS_LIBCXX_SOURCES, i.e. use libc++ anytime its sources are available (and we're building with Clang).

After thinking about it a bit more, I think LLVM_ENABLE_LIBCXX is fine for this purpose. The only question is whether you want to have separate toggle, e.g. for testing? Maybe we could keep the separate option but set LLVM_ENABLE_LIBCXX as its default value?

[phosek]

In D37631#871554, @kcc wrote:

I would rather require libc++ for libFuzzer to build. Isn't it simpler?

There still the case when you're building compiler-rt without having libc++ around in which case you need to fallback onto whatever C++ library is available is on host. Using libc++ whenever the source is available and we're building with Clang, independently of other options, would be fine with me.

We'll also need tests for any such change.

I was thinking about the right testing strategy. The lit setup already links explicitly against libstdc++, so if we use libc++ whenever possible, the existing tests should be sufficient to check the two libraries work together. We may consider adding additional lit configuration which also uses libc++.

[vitalybuka]

So actually if LLVM_ENABLE_LIBCXX=NO it already builds libfuzzer with libc++. There are issues with tests. We need to add --stdlib=libc++ there and maybe rpath.

After thinking about it a bit more, I think LLVM_ENABLE_LIBCXX is fine for this purpose. The only question is whether you want to have separate toggle, e.g. for testing? Maybe we could keep the separate option but set LLVM_ENABLE_LIBCXX as its default value?

We should not test different variations of cmake flags and environments using cmake itself. This can get complicated very quickly.
Unless we are going to ship both fuzzer versions. Are we?

We have tests and we can just make them work with any value of LLVM_ENABLE_LIBCXX.
Then we just add another step into build bot config.
WDYT?

[phosek]

In D37631#876650, @vitalybuka wrote:

So actually if LLVM_ENABLE_LIBCXX=NO it already builds libfuzzer with libc++. There are issues with tests. We need to add --stdlib=libc++ there and maybe rpath.

The problem is that this only covers the case when compiler-rt is in llvm/projects, when built as part of llvm/runtimes we currently don't set LLVM_ENABLE_LIBCXX (although maybe we should when it's available).

After thinking about it a bit more, I think LLVM_ENABLE_LIBCXX is fine for this purpose. The only question is whether you want to have separate toggle, e.g. for testing? Maybe we could keep the separate option but set LLVM_ENABLE_LIBCXX as its default value?

We should not test different variations of cmake flags and environments using cmake itself. This can get complicated very quickly.
Unless we are going to ship both fuzzer versions. Are we?

Ideally no.

We have tests and we can just make them work with any value of LLVM_ENABLE_LIBCXX.
Then we just add another step into build bot config.
WDYT?

Yeah, that' what I was thinking. I can update the lit configuration to use -lc++ even on Linux if LLVM_ENABLE_LIBCXX is set so we get coverage on bots. Is that fine with you?

[vitalybuka]

Yeah, that' what I was thinking. I can update the lit configuration to use -lc++ even on Linux if LLVM_ENABLE_LIBCXX is set so we get coverage on bots. Is that fine with you?

SGTM
but is any reason to use -lc++ instead of --stdlib=libc++ ?

[phosek]

In D37631#876860, @vitalybuka wrote:

Yeah, that' what I was thinking. I can update the lit configuration to use -lc++ even on Linux if LLVM_ENABLE_LIBCXX is set so we get coverage on bots. Is that fine with you?

SGTM
but is any reason to use -lc++ instead of --stdlib=libc++ ?

One issue with LLVM_ENABLE_LIBCXX that I just ran into is that it uses libc++ from the host compiler, but when building and running tests, we're using the just built Clang which might have a different version (e.g. in my case the host has 4.0.0 while target is 6.0.0dev) which causes issues. This is not a problem with runtimes/ build where compiler-rt is always built using just built Clang, but it's a problem when compiler-rt is built in projects/. I don't think we can rely on LLVM_ENABLE_LIBCXX or just -stdlib=libc++ for that reason, we always have explicitly point out to the right libc++ which should be used.

[vitalybuka]

Seems LLVM_ENABLE_LIBCXX is not helpful here.

In D37631#876567, @phosek wrote:

In D37631#871554, @kcc wrote:

I would rather require libc++ for libFuzzer to build. Isn't it simpler?

There still the case when you're building compiler-rt without having libc++ around in which case you need to fallback onto whatever C++ library is available is on host. Using libc++ whenever the source is available and we're building with Clang, independently of other options, would be fine with me.

I am not sure that I understand your reply.

I think Kostya suggests to remove COMPILER_RT_ENABLE_LIBCXX and go with just:

if(COMPILER_RT_HAS_LIBCXX_SOURCES)
  use libc++
else
  whatever is on the host
endif

Why is this bad?

[kcc]

So, folks, what's the plan?

[phosek]

I apologize for the delay, I was stuck working on other things but I finally I have to finish this. I've updated the patch to always use libc++ when available and I've also updated the fuzzer unit tests.

[kcc]

Matt, please also take a look.

CMakeLists.txt

355 ↗

(On Diff #124848)

Hm...
Why ${LLVM_MAIN_SRC_DIR}/runtimes/libcxx ?

Also, is this related to libFuzzer?
Could this be a separate patch?

lib/fuzzer/CMakeLists.txt

35

Why is this guarded with "CMAKE_CXX_COMPILER_ID MATCHES Clang"?

lib/fuzzer/tests/CMakeLists.txt

22

Why do we need stdlib=libc++ for unit tests?

I'd rather have a separate lit test in test/fuzzer that uses stdlib=libc++.

[phosek]

CMakeLists.txt

355 ↗

(On Diff #124848)

That's an alternate location for libc++ when cross-compiling runtimes (e.g. this is used by Fuchsia's toolchain build). Another one would be for monorepo layout. I'm fine splitting this into a separate change.

lib/fuzzer/CMakeLists.txt

35

This is so we can use -stdlib=libc++ with Clang so it automatically includes the correct path to libc++ headers. Alternative would be to use -nostdinc++ and manually add the path to libc++ headers to CFLAGS. That would work for GCC as well.

lib/fuzzer/tests/CMakeLists.txt

22

This is the same as in the other CMakeLists.txt file, the solution mentioned there would work here as well.

[kcc]

could you please also add a lit test that uses -stdlib=libc++?

CMakeLists.txt

355 ↗

(On Diff #124848)

Yes, please.
This looks like completely independent from libFuzzer and I'd like someone from libcxx to review it.

lib/fuzzer/CMakeLists.txt

35

I see...

lib/fuzzer/tests/CMakeLists.txt

22

Ah, yea, the unit tests are linked with the guts of libFuzzer and so they need -stdlib=libc++. Got it.

[phosek]

In D37631#939960, @kcc wrote:

could you please also add a lit test that uses -stdlib=libc++?

I've added the test but I don't think this is sufficient, we also need to check if the host clang supports it, so I need to add an extra CMake check.

[morehouse]

LGTM for recent clang. Not sure how long ago -stdlib=libc++ support was added. If it was long enough ago, maybe the lit config is OK? Otherwise might need to check clang version or something.

test/fuzzer/lit.cfg

57

What is the difference between -stdlib=libc++ and -lc++? Why does the original code use -lc++ instead?

[phosek]

I managed to get this also working with GCC, but while testing the change and I noticed that 3 unit tests are currently failing, all with the same segfault: P8047, P8048, P8049. I'm debugging this now trying to figure what's the problem.

test/fuzzer/lit.cfg

57

-stdlib=libc++ is handled by the driver which sets up the header include paths (i.e. the C++ library header path which is different for lib++ and stdlibc++) appropriately and adds all necessary libraries which could be more than just -lc++, e.g. -lc++abi or -lunwind. I don't know why the original code is setting the library explicitly since this should normally be handled by the driver appropriately for each host.

[vitalybuka]

lib/fuzzer/CMakeLists.txt

81

why do we need check for Clang here and below?

test/fuzzer/lit.cfg

84

Is any patch with tests which shows how this substitutions are going to be used?
I'd probably keep this part of change with actual tests using this.

[phosek]

lib/fuzzer/CMakeLists.txt

81

Remove, I've reworked the patch to explicitly include libc++ headers so this now works with GCC as well.

test/fuzzer/lit.cfg

84

This is no longer used so removed.

[phosek]

Turned out the problem was an incompatibility between libc++abi (which libc++ assumes as the default C++ ABI) and libstdc++ (which is what is being used when linking against libstdc++). Setting -DLIBCXX_CXX_ABI=none seems to be the solution. I'm still trying to figure what's the best way to tests against libc++, simply linking them isn't enough because libc++.so.1 is not in the default dynamic linker search path.

[vitalybuka]

In D37631#947752, @phosek wrote:

Turned out the problem was an incompatibility between libc++abi (which libc++ assumes as the default C++ ABI) and libstdc++ (which is what is being used when linking against libstdc++). Setting -DLIBCXX_CXX_ABI=none seems to be the solution. I'm still trying to figure what's the best way to tests against libc++, simply linking them isn't enough because libc++.so.1 is not in the default dynamic linker search path.

I assume because of this the patch is not finished yet?

If I apply the patch, check-fuzzer does not work for me. I see output like following and I don't see test command-line has path to libc++

FAIL: LLVMFuzzer :: fuzzer-customcrossover.test (19 of 147)
******************** TEST 'LLVMFuzzer :: fuzzer-customcrossover.test' FAILED ********************
Script:
--
./bin/clang -std=c++11 -lstdc++ -gline-tables-only  -fsanitize=address,fuzzer -Icompiler-rt/lib/fuzzer compiler-rt/test/fuzzer/CustomCrossOverTest.cpp -o projects/compiler-rt/test/fuzzer/Output/fuzzer-customcrossover.test.tmp-CustomCrossOverTest
not projects/compiler-rt/test/fuzzer/Output/fuzzer-customcrossover.test.tmp-CustomCrossOverTest -seed=1 -runs=1000000                2>&1 | FileCheck compiler-rt/test/fuzzer/fuzzer-customcrossover.test --check-prefix=CHECK_CO
projects/compiler-rt/test/fuzzer/Output/fuzzer-customcrossover.test.tmp-CustomCrossOverTest -seed=1 -runs=1000000 -cross_over=0 2>&1 | FileCheck compiler-rt/test/fuzzer/fuzzer-customcrossover.test --check-prefix=CHECK_NO_CO
--
Exit Code: 1

Command Output (stderr):
--
compiler-rt/test/fuzzer/fuzzer-customcrossover.test:8:11: error: expected string not found in input
CHECK_CO: BINGO
          ^
<stdin>:9:30: note: scanning from here
In LLVMFuzzerCustomCrossover uncaught_exceptions not yet implemented
                             ^
<stdin>:17:142: note: possible intended match here
 #6 0x45a425 in std::uncaught_exceptions() (projects/compiler-rt/test/fuzzer/Output/fuzzer-customcrossover.test.tmp-CustomCrossOverTest+0x45a425)
                                                                                                                                            ^

lib/fuzzer/CMakeLists.txt

93

it would be nice if add_custom_libcxx set some variable with include dir
but it should be done in separate patch

97

alignment of arguments is very unusual. could you make it more consistent with existing code?

test/fuzzer/CMakeLists.txt

22 ↗

(On Diff #125889)

could you please remove loop until we have more than one time?

[phosek]

In D37631#947770, @vitalybuka wrote:

In D37631#947752, @phosek wrote:

Turned out the problem was an incompatibility between libc++abi (which libc++ assumes as the default C++ ABI) and libstdc++ (which is what is being used when linking against libstdc++). Setting -DLIBCXX_CXX_ABI=none seems to be the solution. I'm still trying to figure what's the best way to tests against libc++, simply linking them isn't enough because libc++.so.1 is not in the default dynamic linker search path.

I assume because of this the patch is not finished yet?

If I apply the patch, check-fuzzer does not work for me. I see output like following and I don't see test command-line has path to libc++

FAIL: LLVMFuzzer :: fuzzer-customcrossover.test (19 of 147)
******************** TEST 'LLVMFuzzer :: fuzzer-customcrossover.test' FAILED ********************
Script:
--
./bin/clang -std=c++11 -lstdc++ -gline-tables-only  -fsanitize=address,fuzzer -Icompiler-rt/lib/fuzzer compiler-rt/test/fuzzer/CustomCrossOverTest.cpp -o projects/compiler-rt/test/fuzzer/Output/fuzzer-customcrossover.test.tmp-CustomCrossOverTest
not projects/compiler-rt/test/fuzzer/Output/fuzzer-customcrossover.test.tmp-CustomCrossOverTest -seed=1 -runs=1000000                2>&1 | FileCheck compiler-rt/test/fuzzer/fuzzer-customcrossover.test --check-prefix=CHECK_CO
projects/compiler-rt/test/fuzzer/Output/fuzzer-customcrossover.test.tmp-CustomCrossOverTest -seed=1 -runs=1000000 -cross_over=0 2>&1 | FileCheck compiler-rt/test/fuzzer/fuzzer-customcrossover.test --check-prefix=CHECK_NO_CO
--
Exit Code: 1

Command Output (stderr):
--
compiler-rt/test/fuzzer/fuzzer-customcrossover.test:8:11: error: expected string not found in input
CHECK_CO: BINGO
          ^
<stdin>:9:30: note: scanning from here
In LLVMFuzzerCustomCrossover uncaught_exceptions not yet implemented
                             ^
<stdin>:17:142: note: possible intended match here
 #6 0x45a425 in std::uncaught_exceptions() (projects/compiler-rt/test/fuzzer/Output/fuzzer-customcrossover.test.tmp-CustomCrossOverTest+0x45a425)
                                                                                                                                            ^

Yes, I'm still debugging the test failures. I see the same error, but only when using GCC as a host compiler. There's another issue when using Clang with libc++ as the host C++ library. These errors are related to exceptions implementation which uses non-versioned types unlike the rest of libc++'s implementation which is something that @EricWF mentioned in his comment. I'm trying to figure if I could disable exceptions altogether from the private libc++ since those shouldn't be needed for libFuzzer.

[vitalybuka]

Yes, I'm still debugging the test failures. I see the same error, but only when using GCC as a host compiler. There's another issue when using Clang with libc++ as the host C++ library.

Mine host compiler was clang. libcxx sources were there but I didn't add them into include path. Something like this:
cmake -GNinja -DCMAKE_C_COMPILER=build0/bin/clang -DCMAKE_CXX_COMPILER=build0/bin/clang++ -DLLVM_ENABLE_LLD=ON -DCMAKE_BUILD_TYPE=Release -DLLVM_ENABLE_ASSERTIONS=ON -DLLVM_ENABLE_WERROR=ON -DLLVM_INCLUDE_GO_TESTS=OFF -DLLVM_CCACHE_BUILD=ON -DLLVM_BINUTILS_INCDIR=/usr/include '-DLLVM_ENABLE_PROJECTS=clang;lld;compiler-rt;libcxx;libcxxabi' /usr/local/google/home/vitalybuka/src/llvm.git/llvm-project/llvm

[phosek]

So I finally ended up with something that works reliably with both libstdc++ and libc++. I plan on doing a bit more cleanup, but a question is what to do on Darwin and Windows. I'm relying on partial linking. That appears to be supported by ld64, but there doesn't seem be the --whole-archive option that we need as well (unless we upgrade CMake to 3.9). On Windows, I don't know if partial linking is supported at all. However, I also don't know if we even need this on Darwin and Windows rather than simply relying on the host C++ library?

[kcc]

In D37631#954580, @phosek wrote:

So I finally ended up with something that works reliably with both libstdc++ and libc++. I plan on doing a bit more cleanup, but a question is what to do on Darwin and Windows. I'm relying on partial linking. That appears to be supported by ld64, but there doesn't seem be the --whole-archive option that we need as well (unless we upgrade CMake to 3.9). On Windows, I don't know if partial linking is supported at all. However, I also don't know if we even need this on Darwin and Windows rather than simply relying on the host C++ library?

I don't think we need this on OSX.
libFuzzer is not supported on Windows currently at all.

[beanz]

@phosek On darwin ld64 supports the flags -all_load and -force_load which work similarly to --whole-archive. The ld64 manpage has the documentation for those options.

[phosek]

In D37631#958562, @beanz wrote:

@phosek On darwin ld64 supports the flags -all_load and -force_load which work similarly to --whole-archive. The ld64 manpage has the documentation for those options.

Thanks, that's useful. Do you know what would be an equivalent of objcopy --localize-hidden on Darwin?

[beanz]

@phosek not sure if there is an equivalent. Probably strip -x is the closest.

[phosek]

I've added another test case to check this work with both statically and dynamically linked libc++ (only if built as part of LLVM). Everything seems to be working fine, PTAL.

[phosek]

ping?

[vitalybuka]

I see a lot of these on "ninja check-fuzzer"

******************** TEST 'LLVMFuzzer :: value-profile-set.test' FAILED ********************
Script:
--
/usr/local/google/home/vitalybuka/src/llvm.git/out/build-clang-none/./bin/clang -std=c++11 -stdlib=libc++ -lc++ -static-libstdc++ -gline-tables-only  -fsanitize=address,fuzzer -I/usr/local/google/home/vitalybuka/src/llvm.git/llvm-project/compiler-rt/lib/fuzzer /usr/local/google/home/vitalybuka/src/llvm.git/llvm-project/compiler-rt/test/fuzzer/FourIndependentBranchesTest.cpp -o /usr/local/google/home/vitalybuka/src/llvm.git/out/build-clang-none/projects/compiler-rt/test/fuzzer/Output/value-profile-set.test.tmp-FourIndependentBranchesTest
not /usr/local/google/home/vitalybuka/src/llvm.git/out/build-clang-none/projects/compiler-rt/test/fuzzer/Output/value-profile-set.test.tmp-FourIndependentBranchesTest -seed=1 -use_cmp=0 -use_value_profile=1 -runs=100000000 2>&1 | FileCheck /usr/local/google/home/vitalybuka/src/llvm.git/llvm-project/compiler-rt/test/fuzzer/value-profile-set.test
--
Exit Code: 1

Command Output (stderr):
--
clang-6.0: warning: argument unused during compilation: '-static-libstdc++' [-Wunused-command-line-argument]
/usr/bin/ld: cannot find -lc++
/usr/bin/ld: cannot find -lc++
clang-6.0: error: linker command failed with exit code 1 (use -v to see invocation)

lib/fuzzer/FuzzerInterface.h

25 ↗

(On Diff #128303)

this is header so it may have side effects. could you please use attribute instead of pragma?

[vitalybuka]

It was configured with:

cmake -GNinja -DCMAKE_C_COMPILER=/usr/local/google/home/vitalybuka/src/llvm.git/out/build/bin//clang -DCMAKE_CXX_COMPILER=/usr/local/google/home/vitalybuka/src/llvm.git/out/build/bin//clang++ -DLLVM_ENABLE_LLD=ON -DCMAKE_BUILD_TYPE=Release -DLLVM_ENABLE_ASSERTIONS=ON -DLLVM_ENABLE_WERROR=OFF -DLLVM_INCLUDE_GO_TESTS=OFF -DLLVM_CCACHE_BUILD=ON -DLLVM_BINUTILS_INCDIR=/usr/include '-DLLVM_ENABLE_PROJECTS=clang;lld;compiler-rt;libcxx;libcxxabi' /usr/local/google/home/vitalybuka/src/llvm.git/llvm-project/llvm

[morehouse]

test/fuzzer/lit.cfg

52

Is this line necessary considering config.environment['LD_LIBRARY_PATH'] is already set above?

[phosek]

In D37631#972553, @vitalybuka wrote:

I see a lot of these on "ninja check-fuzzer"

I forgot to include a dependency on libc++ in tests using it, that should be fixed now.

[morehouse]

check-fuzzer tests clean for me now with both clang and gcc.

lib/fuzzer/FuzzerInterface.h

67 ↗

(On Diff #129575)

AFAICT, all libFuzzer code puts attributes before the function name. Could you follow that convention please?

test/fuzzer/lit.cfg

57

Now we are specifying both -stdlib=libc++ and -lc++. Why is -lc++ necessary now?

[phosek]

test/fuzzer/lit.cfg

57

Turned the problem is that we're always C compiler even when building C++ code so -stdlib=libc++ isn't sufficient. I've updated the script to also add --driver-mode=g++ in C++ mode so this is no longer needed.

[morehouse]

Vitaly, does this test clean on your end now?

test/fuzzer/lit.cfg

61

What does -static-libstdc++ do here? Does it mix libstdc++ with libc++?

[phosek]

test/fuzzer/lit.cfg

61

static-libstdc++ works with both libc++ and libstdc++ (confusingly), the choice of C++ library is controlled by -stdlib= flag. This branch is used to check that libFuzzer can be used with statically linked C++ library (i.e. to ensure that the symbols from the internalized C++ library are properly hidden and localized to don't cause collision).

[vitalybuka]

In D37631#977283, @morehouse wrote:

Vitaly, does this test clean on your end now?

It works for me now.

lib/fuzzer/FuzzerMain.cpp

19 ↗

(On Diff #129722)

please remove space after "visibility"

[hintonda]

This change cause cross-compiles to fail if the target compiler can't run on the host machine -- in my case, I'm compiling Linux executables on Darwin, and the new relink step fails.

Perhaps it should be guarded by COMPILER_RT_CAN_EXECUTE_TESTS.

[Hahnfeld]

One disadvantage of this change is that the libc++ variant is reconfigured on every invocation of make / ninja, even if there is nothing to do:

[1/3] No force-reconfigure step for 'libcxx_fuzzer_x86_64'
[2/3] Performing configure step for 'libcxx_fuzzer_x86_64'
-- libcxx_fuzzer_x86_64 configure command succeeded.  See also <...>/projects/compiler-rt/lib/fuzzer/libcxx_fuzzer_x86_64/src/libcxx_fuzzer_x86_64-stamp/libcxx_fuzzer_x86_64-configure-*.log
[3/3] Performing build step for 'libcxx_fuzzer_x86_64'
-- libcxx_fuzzer_x86_64 build command succeeded.  See also <...>/projects/compiler-rt/lib/fuzzer/libcxx_fuzzer_x86_64/src/libcxx_fuzzer_x86_64-stamp/libcxx_fuzzer_x86_64-build-*.log

Can we avoid this and have ninja say that there is no work to do?

[hintonda]

In D37631#983999, @Hahnfeld wrote:

One disadvantage of this change is that the libc++ variant is reconfigured on every invocation of make / ninja, even if there is nothing to do:

[1/3] No force-reconfigure step for 'libcxx_fuzzer_x86_64'
[2/3] Performing configure step for 'libcxx_fuzzer_x86_64'
-- libcxx_fuzzer_x86_64 configure command succeeded.  See also <...>/projects/compiler-rt/lib/fuzzer/libcxx_fuzzer_x86_64/src/libcxx_fuzzer_x86_64-stamp/libcxx_fuzzer_x86_64-configure-*.log
[3/3] Performing build step for 'libcxx_fuzzer_x86_64'
-- libcxx_fuzzer_x86_64 build command succeeded.  See also <...>/projects/compiler-rt/lib/fuzzer/libcxx_fuzzer_x86_64/src/libcxx_fuzzer_x86_64-stamp/libcxx_fuzzer_x86_64-build-*.log

Can we avoid this and have ninja say that there is no work to do?

That's because add_custom_libcxx() calls ExternalProject_Add() directly instead of llvm_ExternalProject_Add(), which already has logic to handle this.

In fact, it looks like this is partial reimplementation of code in llvm/runtimes/CMakeLists.txt. Perhaps that code, mostly runtime_register_target(), could be moved to LLVMExternalProjectUtils.cmake and reused here?

lib/fuzzer/CMakeLists.txt

CHECK_CXX_SOURCE_COMPILES("
   static thread_local int blah;
   int main() {
   return 0;
   }
   " HAS_THREAD_LOCAL)
 
 set(LIBFUZZER_CFLAGS ${SANITIZER_COMMON_CFLAGS})
 
+if(CMAKE_CXX_COMPILER_ID MATCHES Clang AND COMPILER_RT_HAS_LIBCXX_SOURCES)
+  list(APPEND LIBFUZZER_CFLAGS -stdlib=libc++ -D_LIBCPP_ABI_VERSION=Fuzzer)
+endif()
+
 append_list_if(COMPILER_RT_HAS_OMIT_FRAME_POINTER_FLAG -fno-omit-frame-pointer LIBFUZZER_CFLAGS)
 
 if (CMAKE_CXX_FLAGS MATCHES "fsanitize-coverage")
   list(APPEND LIBFUZZER_CFLAGS -fno-sanitize-coverage=trace-pc-guard,edge,trace-cmp,indirect-calls,8bit-counters)
 endif()
 
 if(NOT HAS_THREAD_LOCAL)
   list(APPEND LIBFUZZER_CFLAGS -Dthread_local=__thread)

 add_compiler_rt_runtime(clang_rt.fuzzer_no_main
   STATIC
   OS ${FUZZER_SUPPORTED_OS}
   ARCHS ${FUZZER_SUPPORTED_ARCH}
   OBJECT_LIBS RTfuzzer
   CFLAGS ${LIBFUZZER_CFLAGS}
   PARENT_TARGET fuzzer)
 
+if(CMAKE_CXX_COMPILER_ID MATCHES Clang AND COMPILER_RT_HAS_LIBCXX_SOURCES)
+  foreach(arch ${FUZZER_SUPPORTED_ARCH})
+    get_target_flags_for_arch(${arch} TARGET_CFLAGS)
+    set(LIBCXX_PREFIX ${CMAKE_CURRENT_BINARY_DIR}/libcxx_fuzzer_${arch})
+    add_custom_libcxx(libcxx_fuzzer_${arch} ${LIBCXX_PREFIX}
+      CFLAGS ${TARGET_CFLAGS} -D_LIBCPP_ABI_VERSION=Fuzzer)
+    add_dependencies(clang_rt.fuzzer-${arch} libcxx_fuzzer_${arch})
+    add_custom_command(TARGET clang_rt.fuzzer-${arch} POST_BUILD
+      COMMAND
+        ${PYTHON_EXECUTABLE} ${COMPILER_RT_LIBCXX_PATH}/utils/merge_archives.py
+      ARGS
+        -o $<TARGET_LINKER_FILE:clang_rt.fuzzer-${arch}>
+        "$<TARGET_LINKER_FILE:clang_rt.fuzzer-${arch}>"
+        "${CMAKE_CURRENT_BINARY_DIR}/libcxx_fuzzer_${arch}/lib/libc++.a"
+    )
+  endforeach()
+endif()
+
 if(COMPILER_RT_INCLUDE_TESTS)
   add_subdirectory(tests)
 endif()

lib/fuzzer/tests/CMakeLists.txt

 list(APPEND LIBFUZZER_UNITTEST_LINK_FLAGS --driver-mode=g++)
 
 if(APPLE)
   list(APPEND LIBFUZZER_UNITTEST_LINK_FLAGS -lc++)
 else()
   list(APPEND LIBFUZZER_UNITTEST_LINK_FLAGS -lstdc++ -lpthread)
 endif()
 
+if(CMAKE_CXX_COMPILER_ID MATCHES Clang AND COMPILER_RT_HAS_LIBCXX_SOURCES)
+  list(APPEND LIBFUZZER_UNITTEST_CFLAGS -stdlib=libc++ -D_LIBCPP_ABI_VERSION=Fuzzer)
+endif()
+
 foreach(arch ${FUZZER_SUPPORTED_ARCH})
   set(LIBFUZZER_TEST_RUNTIME RTFuzzerTest.${arch})
   if(APPLE)
     set(LIBFUZZER_TEST_RUNTIME_OBJECTS
       $<TARGET_OBJECTS:RTfuzzer.osx>)
   else()
     set(LIBFUZZER_TEST_RUNTIME_OBJECTS
       $<TARGET_OBJECTS:RTfuzzer.${arch}>)
   endif()
   add_library(${LIBFUZZER_TEST_RUNTIME} STATIC
     ${LIBFUZZER_TEST_RUNTIME_OBJECTS})
   set_target_properties(${LIBFUZZER_TEST_RUNTIME} PROPERTIES
     ARCHIVE_OUTPUT_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
     FOLDER "Compiler-RT Runtime tests")
 
+  if(CMAKE_CXX_COMPILER_ID MATCHES Clang AND COMPILER_RT_HAS_LIBCXX_SOURCES)
+    set(LIBFUZZER_TEST_RUNTIME_DEPS libcxx_fuzzer_${arch})
+    set(LIBFUZZER_TEST_RUNTIME_LINK_FLAGS
+      "${CMAKE_CURRENT_BINARY_DIR}/../libcxx_fuzzer_${arch}/lib/libc++.a")
+  endif()
+
   set(FuzzerTestObjects)
   generate_compiler_rt_tests(FuzzerTestObjects
     FuzzerUnitTests "Fuzzer-${arch}-Test" ${arch}
     SOURCES FuzzerUnittest.cpp ${COMPILER_RT_GTEST_SOURCE}
     RUNTIME ${LIBFUZZER_TEST_RUNTIME}
-    DEPS gtest
+    DEPS gtest ${LIBFUZZER_TEST_RUNTIME_DEPS}
     CFLAGS ${LIBFUZZER_UNITTEST_CFLAGS}
-    LINK_FLAGS ${LIBFUZZER_UNITTEST_LINK_FLAGS})
+    LINK_FLAGS ${LIBFUZZER_UNITTEST_LINK_FLAGS} ${LIBFUZZER_TEST_RUNTIME_LINK_FLAGS})
   set_target_properties(FuzzerUnitTests PROPERTIES
     RUNTIME_OUTPUT_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR})
 endforeach()

test/fuzzer/lit.cfg

if sys.platform.startswith('linux'):
   config.available_features.add('linux')
 else:
   lit_config.note('linux feature unavailable')
 
 config.substitutions.append(('%build_dir', config.cmake_binary_dir))
 libfuzzer_src_root = os.path.join(config.compiler_rt_src_root, "lib", "fuzzer")
 config.substitutions.append(('%libfuzzer_src', libfuzzer_src_root))
 
-def generate_compiler_cmd(is_cpp=True, fuzzer_enabled=True):
+def generate_compiler_cmd(is_cpp=True, libcxx=False, fuzzer_enabled=True):
   compiler_cmd = config.c_compiler
+  if config.clang and libcxx:
+    link_cmd = '-stdlib=libc++'
+  else:
     link_cmd = '-lc++' if 'darwin' in config.target_triple else '-lstdc++'
   std_cmd = '-std=c++11' if is_cpp else ''
   sanitizers = ['address']
   if fuzzer_enabled:
     sanitizers.append('fuzzer')
   sanitizers_cmd = ('-fsanitize=%s' % ','.join(sanitizers))
   isysroot_cmd = config.osx_sysroot_flag if config.osx_sysroot_flag else ''
   include_cmd = '-I%s' % libfuzzer_src_root
   return '%s %s %s -gline-tables-only %s %s %s' % (
       compiler_cmd, std_cmd, link_cmd, isysroot_cmd, sanitizers_cmd, include_cmd)
 
 config.substitutions.append(('%cpp_compiler',
       generate_compiler_cmd(is_cpp=True, fuzzer_enabled=True)
       ))
 
 config.substitutions.append(('%c_compiler',
       generate_compiler_cmd(is_cpp=False, fuzzer_enabled=True)
       ))
 
 config.substitutions.append(('%no_fuzzer_cpp_compiler',
       generate_compiler_cmd(is_cpp=True, fuzzer_enabled=False)
       ))
 
 config.substitutions.append(('%no_fuzzer_c_compiler',
       generate_compiler_cmd(is_cpp=False, fuzzer_enabled=False)
       ))
+
+config.substitutions.append(('%libcxx_c_compiler',
+      generate_compiler_cmd(is_cpp=False, libcxx=True, fuzzer_enabled=True)
+      ))
+
+config.substitutions.append(('%libcxx_cxx_compiler',
+      generate_compiler_cmd(is_cpp=True, libcxx=True, fuzzer_enabled=True)
+      ))