This is an archive of the discontinued LLVM Phabricator instance.

[CMake] Fix USE_LLVM_SANITIZER configuration for out-of-tree builds.
ClosedPublic

Authored by EricWF on Feb 7 2017, 2:28 PM.

Details

Summary

r291918 changed HandleLLVMOptions.cmake to add -fsanitize-blacklist=<llvm-file> when LLVM_USE_SANITIZER=Undefined is specified. This breaks out-of-tree users of LLVM_USE_SANITIZER since that file is not present.

This patch fixes the issue by checking if the file exists first.

Diff Detail

Event Timeline

EricWF created this revision.Feb 7 2017, 2:28 PM
krasin edited edge metadata.Feb 7 2017, 2:40 PM

Hi Eric,

sorry for breaking this. I still don't fully understand what do you mean by out-of-tree builds here. Can you please provide more specific instructions on how to reproduce the issue?

As for the change, while it might fix compilation issues, it does not address the fact that compiling Clang under UBSan without the blacklist will raise UBSan reports due to known a undefined behavior in libstdc++ in stl_tree. So, I would really like to understand the conditions under which it fails.

EricWF added a comment.Feb 7 2017, 2:52 PM

Hi Ivan,

I'm doing libc++/libc++abi out-of-tree builds (although only libc++abi has the problem currently). The installed HandleLLVMOptions.cmake is included by libc++abi, which results in configuration errors because that file is missing. If you're really interested here is what my configuration script looks lie.

I understand this doesn't solve the problem of out-of-tree Clang builds which target libstdc++, but that seems like a separate problem/fix.

krasin added a comment.Feb 7 2017, 2:56 PM

Hi Eric,

thank you for the explanation. After seeing your use case, I agree that the patch makes sense. After all, libc++ does not depend on libstdc++.

In the unlikely case when someone builds something like that with UBSan and do depends on libstdc++, they are welcome to provide a path to the blacklist in CMAKE_C_FLAGS / CMAKE_CXX_FLAGS or add another condition here.

krasin accepted this revision.Feb 7 2017, 2:57 PM
This revision is now accepted and ready to land.Feb 7 2017, 2:57 PM
EricWF closed this revision.Feb 7 2017, 2:59 PM