[PATCH] D37631: [libFuzzer] Support using libc++

[Petr Hosek via Phabricator via llvm-commits]

phosek added inline comments.


================
Comment at: CMakeLists.txt:355
 else()
-  set(COMPILER_RT_LIBCXX_PATH ${LLVM_MAIN_SRC_DIR}/../libcxx)
+  set(COMPILER_RT_LIBCXX_PATH ${LLVM_MAIN_SRC_DIR}/runtimes/libcxx)
   if(EXISTS ${COMPILER_RT_LIBCXX_PATH}/)
----------------
kcc wrote:
> Hm... 
> Why ${LLVM_MAIN_SRC_DIR}/runtimes/libcxx ? 
> 
> Also, is this related to libFuzzer? 
> Could this be a separate patch? 
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.


================
Comment at: lib/fuzzer/CMakeLists.txt:35
 
+if(CMAKE_CXX_COMPILER_ID MATCHES Clang AND COMPILER_RT_HAS_LIBCXX_SOURCES)
+  list(APPEND LIBFUZZER_CFLAGS -stdlib=libc++ -D_LIBCPP_ABI_VERSION=Fuzzer)
----------------
kcc wrote:
> Why is this guarded with "CMAKE_CXX_COMPILER_ID MATCHES Clang"? 
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.


================
Comment at: lib/fuzzer/tests/CMakeLists.txt:22
+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()
----------------
kcc wrote:
> Why do we need stdlib=libc++ for unit tests? 
> 
> I'd rather have a separate lit test in test/fuzzer that uses stdlib=libc++. 
This is the same as in the other `CMakeLists.txt` file, the solution mentioned there would work here as well.


Repository:
  rCRT Compiler Runtime

https://reviews.llvm.org/D37631