Allow building with LLVM_USE_SANITIZER=“Address;Undefined” (and “Undefined;Address”).
Details
Diff Detail
Event Timeline
test/libcxx/test/config.py | ||
---|---|---|
636 | Why don't you sanitize those? IIRC, vptr needs stuff from libc++abi, not libc++. Is there a problem with this sanitizer on libcxx tests? Please add a comment explaining these (also, remove any -fno-sanitize=... that you can :-) ) |
test/libcxx/test/config.py | ||
---|---|---|
636 | It’s the same flags that are used for UBSan-only builds, see a few lines below. I don’t know why exactly we’re using that and it’s not the point of this patch to change them. We can take a look at which flags should be enabled/disabled and document that, but that should happen in a separate revision. |
Factoring out the UBSan flags so they’re not duplicated.
Shouldn't this be '-fsanitize="undefined;address"'?
We’re appending to the flags (with +=), so this should work as well (having -fsanitize=address -fsanitize=undefined).
LGTM too. We should check if those -fno-sanitize flags are actually needed, later.
lib/CMakeLists.txt | ||
---|---|---|
49 | Please add these to line 40, above (easy to miss on Duncan's message). |
Duncan sent some more comments in mail:
It'd be nice to factor this out more somehow. Can't UBSan be combined with all the others?
Yes, UBSan can be used at least with TSan and MSan, not sure about the other sanitizers and -fsanitize options. However, LLVM currently only allows some very specific options in LLVM_USE_SANITIZER (see llvm/cmake/modules/HandleLLVMOptions.cmake), and currently it only allows ASan+UBSan together. Let’s leave generalizing this for later, when someone’s actually interested in running a different sanitizer combination.
Please add these to line 40, above (easy to miss on Duncan's message).