[PATCH] D36810: Minimal runtime for UBSan.

[Evgenii Stepanov via Phabricator via llvm-commits]

eugenis added a comment.





================
Comment at: clang/lib/CodeGen/CGExpr.cpp:2713
       ("__ubsan_handle_" + CheckName +
-       (CheckInfo.Version ? "_v" + llvm::utostr(CheckInfo.Version) : "") +
-       (NeedsAbortSuffix ? "_abort" : ""))
+       ((CheckInfo.Version && !MinimalRuntime)
+            ? "_v" + llvm::utostr(CheckInfo.Version)
----------------
pcc wrote:
> Now that we have three of these I think it would be more readable to change this to:
> ```if (CheckInfo.Version && !MinimalRuntime)
>   FnName += "_v" + llvm::utostr(CheckInfo.Version);```
> and similarly below.
Done. Note that the new code is using std::string concatenation and not Twine - as I understand, Twine can not be used across multiple statements.


================
Comment at: compiler-rt/lib/CMakeLists.txt:37
     add_subdirectory(ubsan)
+    add_subdirectory(ubsan_minimal)
   endif()
----------------
pcc wrote:
> Instead of adding this here it looks like the new way is to add the runtime to `ALL_SANITIZERS`.
OK, this can be done because nothing else depends on ubsan-minimal (unlike ubsan, stats and lsan above).

Also, this will require a clean build to pick up the new sanitizer, because COMPILER_RT_SANITIZERS_TO_BUILD is already expanded to the list that does not include ubsan_minimal in cmake cache. Re-running cmake with just this one flag (=all) in the existing build directory also works.


https://reviews.llvm.org/D36810