[PATCH] D16428: [CMake] Provide options for toggling on and off various runtime libraries.

Chris Bieneman via llvm-commits llvm-commits at lists.llvm.org
Thu Jan 21 14:48:09 PST 2016


beanz added inline comments.

================
Comment at: cmake/config-ix.cmake:630
@@ +629,3 @@
+endif()
+
+set(COMPILER_RT_RUNTIMES_TO_BUILD "all"
----------------
samsonov wrote:
> I think you should do the same for "stats" library. It's not a part of sanitizer_common, it uses sanitizer_common. So, this
> should be smth. like
> 
> ```
> if (OS_NAME MATCHES "Darwin|Linux|FreeBSD|Windows")
>   list(APPEND DEFAULT_RUNTIMES stats)
>   list(APPEND REQUIRES_COMMON stats)
> endif()
> ```
> 
> and remove adding `stats` below from "SHOULD_BUILD_COMMON".
I can make that change.

Additional explanation in my next comment.

================
Comment at: cmake/config-ix.cmake:646
@@ +645,3 @@
+  append_list_unique(COMPILER_RT_RUNTIMES_TO_BUILD lsan)
+  append_list_unique(COMPILER_RT_RUNTIMES_TO_BUILD ubsan)
+  append_list_unique(COMPILER_RT_RUNTIMES_TO_BUILD stats)
----------------
samsonov wrote:
> I don't fully understand how it works: you seem to add "ubsan" to COMPILER_RT_RUNTIMES_TO_BUILD always if you need sanitizer_common, which will lead to `COMPILER_RT_HAS_UBSAN` being true in that case. That's not true: e.g. if UBSAN_SUPPORTED_ARCH is empty you don't need to create UBSan runtime, or execute UBSan tests.
I had based this on the `if(COMPILER_RT_HAS_SANITIZER_COMMON)` block that exists in lib/CMakeLists.txt today.

The complication is that some of these libraries (like ubsan and lsan) produce object libs even if you're not actually building the sanitizer, and those object libraries get used all over the place without checking to make sure the corresponding sanitizer is supported.

I probably need to re-think how to handle those. If you have suggestions I'd appreciate them, but I'll head back to the drawing board.


http://reviews.llvm.org/D16428





More information about the llvm-commits mailing list