vsk added inline comments.
================
Comment at: clang/lib/Driver/ToolChain.cpp:953
+ SanitizerKind::ImplicitConversion | SanitizerKind::Nullability |
+ SanitizerKind::LocalBounds | SanitizerKind::ObjCCast;
if (getTriple().getArch() == llvm::Triple::x86 ||
----------------
delcypher wrote:
> `SanitizerKind::ObjCCast` doesn't seem to fit the comment above. It is
> platform dependent (only really works for Apple platforms) and it **does**
> require runtime support (i.e. the Objective-C runtime).
The runtime dependency is optional, and there's nothing inherently
Apple-specific about this check. However, perhaps it's best not to
inadvertently advertise support for the GNU objc runtime before it's in tree.
================
Comment at: compiler-rt/lib/ubsan/ubsan_value.cpp:29
+const char *__ubsan::getObjCClassName(ValueHandle Pointer) {
+#if defined(__APPLE__)
+ // We need to query the ObjC runtime for some information, but do not want
----------------
delcypher wrote:
> The compiler-rt codebase tends to use `SANITIZER_MAC` macro (defined to be 1
> if Apple otherwise it's 0) rather than `__APPLE__`.
I see. That seems problematic, as it makes it tricky to add macOS-specific (or
iOS-specific) functionality down the road. I don't think SANITIZER_MAC should
be defined unless TARGET_OS_MACOS is true.
================
Comment at: compiler-rt/lib/ubsan/ubsan_value.cpp:37
+
+ if (!AttemptedDlopen) {
+ ObjCHandle = dlopen(
----------------
delcypher wrote:
> You might want some sort of lock here (or atomic variable) to ensure we don't
> race and try to `dlopen()` multiple times.
Yes, thanks.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D71491/new/
https://reviews.llvm.org/D71491
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
