[PATCH] D36810: Minimal runtime for UBSan.

[Evgenii Stepanov via Phabricator via llvm-commits]

eugenis added inline comments.


================
Comment at: compiler-rt/lib/CMakeLists.txt:37
     add_subdirectory(ubsan)
+    add_subdirectory(ubsan_minimal)
   endif()
----------------
pcc wrote:
> 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`.
Good point, r311824.


================
Comment at: compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cc:53
+    if (!report_this_error(__builtin_return_address(0))) return; \
+    message("ubsan: " msg "\n");                                 \
+  }                                                              \
----------------
vsk wrote:
> Printing out the return address here would aid debugging. It's extra complexity but would be worth it when triaging bugs, or when using the minimal runtime in low-level software.
Even if it's just an absolute address, w/o module base? It's does not give you much in the presence of ASLR.



https://reviews.llvm.org/D36810