[PATCH] D91387: [Driver] Support UBSan multilib

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added inline comments.


================
Comment at: clang/cmake/caches/Fuchsia-stage2.cmake:214-215
 
+    set(RUNTIMES_${target}-unknown-fuchsia+ubsan+noexcept_LLVM_BUILD_COMPILER_RT OFF CACHE BOOL "")
+    set(RUNTIMES_${target}-unknown-fuchsia+ubsan+noexcept_LLVM_USE_SANITIZER "Undefined" CACHE STRING "")
+    set(RUNTIMES_${target}-unknown-fuchsia+ubsan+noexcept_LIBCXXABI_ENABLE_NEW_DELETE_DEFINITIONS OFF CACHE BOOL "")
----------------
Don't you need plain +ubsan instances of these like for +asan above?



================
Comment at: clang/cmake/caches/Fuchsia-stage2.cmake:216-217
+    set(RUNTIMES_${target}-unknown-fuchsia+ubsan+noexcept_LLVM_USE_SANITIZER "Undefined" CACHE STRING "")
+    set(RUNTIMES_${target}-unknown-fuchsia+ubsan+noexcept_LIBCXXABI_ENABLE_NEW_DELETE_DEFINITIONS OFF CACHE BOOL "")
+    set(RUNTIMES_${target}-unknown-fuchsia+ubsan+noexcept_LIBCXX_ENABLE_NEW_DELETE_DEFINITIONS OFF CACHE BOOL "")
+    set(RUNTIMES_${target}-unknown-fuchsia+ubsan+noexcept_LIBCXXABI_ENABLE_EXCEPTIONS OFF CACHE BOOL "")
----------------
Standalone ubsan doesn't replace the allocator, so these shouldn't be overridden for it like they are for asan.



================
Comment at: clang/lib/Driver/ToolChains/Fuchsia.cpp:205
                           .flag("+fno-exceptions"));
+  // UBSan has lower priority than ASan so we pick the latter when both are set.
+  Multilibs.push_back(Multilib("ubsan", {}, {}, 2)
----------------
This being the case, should we be adding `-fsanitize=undefined` to the libc++ (et al) build for asan?
I guess that's an orthogonal change, really.



================
Comment at: clang/lib/Driver/ToolChains/Fuchsia.cpp:217
   // Use the asan+noexcept variant with ASan and -fno-exceptions.
-  Multilibs.push_back(Multilib("asan+noexcept", {}, {}, 3)
+  Multilibs.push_back(Multilib("asan+noexcept", {}, {}, 5)
                           .flag("+fsanitize=address")
----------------
This is getting to be a lot of individual ordered integer constants to maintain.
Can we just define a local enum to represent the ordering and use symbolic names in each call?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91387/new/

https://reviews.llvm.org/D91387