[PATCH] D36810: Minimal runtime for UBSan.

[Peter Collingbourne via Phabricator via llvm-commits]

pcc accepted this revision.
pcc added a comment.
This revision is now accepted and ready to land.

LGTM



================
Comment at: compiler-rt/lib/CMakeLists.txt:37
     add_subdirectory(ubsan)
+    add_subdirectory(ubsan_minimal)
   endif()
----------------
eugenis wrote:
> 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.
> COMPILER_RT_SANITIZERS_TO_BUILD is already expanded to the list that does not include ubsan_minimal in cmake cache

That doesn't seem right. I think that in a separate change we should change the default value to `all`.


================
Comment at: compiler-rt/lib/ubsan_minimal/CMakeLists.txt:17
+
+if(APPLE)
+
----------------
I don't think you need to test for this now that the inclusion of this file is conditional on `COMPILER_RT_HAS_UBSAN_MINIMAL`.


https://reviews.llvm.org/D36810