delcypher added a comment.
Overall approach LGTM. I just have some very minor nits.
================
Comment at: clang/include/clang/Basic/DiagnosticDriverKinds.td:219
+def err_drv_unsupported_static_ubsan_darwin : Error<
+ "Static UBSan runtime is not supported on darwin">;
def err_drv_duplicate_config : Error<
----------------
Nit: Driver messages start with lowercase.
Also please check if "UBSan" is spelt differently in existing driver messages.
It might actually be written more explicitly like "undefined behavior
sanitizer".
================
Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:1441
Sanitize.requiresMinimalRuntime() ? "ubsan_minimal"
- : "ubsan",
- Sanitize.needsSharedRt());
+ : "ubsan");
if (Sanitize.needsTsanRt())
----------------
Maybe add an assert? As the code is constructed right now it should never fail
but in the future someone could change the code and break the assumption.
```lang=c++
if (Sanitize.needsUbsanRt()) {
assert(Sanitize.needsSharedRt() && "Static sanitizer runtimes not
supported");
AddLinkSanitizerLibArgs(Args, CmdArgs,
Sanitize.requiresMinimalRuntime() ? "ubsan_minimal"
: "ubsan");
}
```
================
Comment at: compiler-rt/lib/ubsan/CMakeLists.txt:118
+ if (NOT APPLE)
+ add_compiler_rt_runtime(clang_rt.ubsan
+ STATIC
----------------
I think you may have accidentally added tabs here when re-indenting.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D141550/new/
https://reviews.llvm.org/D141550
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
