vsk updated this revision to Diff 234142.
vsk added a comment.

Ignore an objc-cast report at a given SourceLocation after it's been reported 
once.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71491/new/

https://reviews.llvm.org/D71491

Files:
  clang/docs/UndefinedBehaviorSanitizer.rst
  clang/include/clang/Basic/Sanitizers.def
  clang/lib/CodeGen/CGObjC.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Driver/SanitizerArgs.cpp
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/test/CodeGenObjC/for-in.m
  compiler-rt/lib/ubsan/ubsan_checks.inc
  compiler-rt/lib/ubsan/ubsan_handlers.cpp
  compiler-rt/lib/ubsan/ubsan_handlers.h
  compiler-rt/lib/ubsan/ubsan_value.cpp
  compiler-rt/lib/ubsan/ubsan_value.h
  compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp
  compiler-rt/test/ubsan/TestCases/Misc/objc-cast.m

Index: compiler-rt/test/ubsan/TestCases/Misc/objc-cast.m
===================================================================
--- /dev/null
+++ compiler-rt/test/ubsan/TestCases/Misc/objc-cast.m
@@ -0,0 +1,22 @@
+// REQUIRES: darwin
+//
+// RUN: %clang -framework Foundation -fsanitize=objc-cast %s -O1 -o %t
+// RUN: %run %t 2>&1 | FileCheck %s
+//
+// RUN: %clang -framework Foundation -fsanitize=objc-cast -fno-sanitize-recover=objc-cast %s -O1 -o %t.trap
+// RUN: not %run %t.trap 2>&1 | FileCheck %s
+
+#include <Foundation/Foundation.h>
+
+int main() {
+  NSArray *array = [NSArray arrayWithObjects: @1, @2, @3, (void *)0];
+  // CHECK: objc-cast.m:[[@LINE+1]]:{{.*}}: runtime error: invalid ObjC cast, object is a '__NSCFNumber', but expected a 'NSString'
+  for (NSString *str in array) {
+    NSLog(@"%@", str);
+  }
+
+  // The diagnostic should only be printed once.
+  // CHECK-NOT: runtime error
+
+  return 0;
+}
Index: compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp
===================================================================
--- compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp
+++ compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp
@@ -109,6 +109,7 @@
 HANDLER(float_cast_overflow, "float-cast-overflow")
 HANDLER(load_invalid_value, "load-invalid-value")
 HANDLER(invalid_builtin, "invalid-builtin")
+HANDLER(invalid_objc_cast, "invalid-objc-cast")
 HANDLER(function_type_mismatch, "function-type-mismatch")
 HANDLER(implicit_conversion, "implicit-conversion")
 HANDLER(nonnull_arg, "nonnull-arg")
Index: compiler-rt/lib/ubsan/ubsan_value.h
===================================================================
--- compiler-rt/lib/ubsan/ubsan_value.h
+++ compiler-rt/lib/ubsan/ubsan_value.h
@@ -135,6 +135,9 @@
 /// \brief An opaque handle to a value.
 typedef uptr ValueHandle;
 
+/// Returns the class name of the given ObjC object, or null if the name
+/// cannot be found.
+const char *getObjCClassName(ValueHandle Pointer);
 
 /// \brief Representation of an operand value provided by the instrumented code.
 ///
Index: compiler-rt/lib/ubsan/ubsan_value.cpp
===================================================================
--- compiler-rt/lib/ubsan/ubsan_value.cpp
+++ compiler-rt/lib/ubsan/ubsan_value.cpp
@@ -16,9 +16,55 @@
 #include "ubsan_value.h"
 #include "sanitizer_common/sanitizer_common.h"
 #include "sanitizer_common/sanitizer_libc.h"
+#include "sanitizer_common/sanitizer_mutex.h"
+
+#if defined(__APPLE__)
+#include <dlfcn.h>
+#endif
 
 using namespace __ubsan;
 
+typedef const char *(*ObjCGetClassNameTy)(void *);
+
+const char *__ubsan::getObjCClassName(ValueHandle Pointer) {
+#if defined(__APPLE__)
+  // We need to query the ObjC runtime for some information, but do not want
+  // to introduce a static dependency from the ubsan runtime onto ObjC. Try to
+  // grab a handle to the ObjC runtime used by the process.
+  static bool AttemptedDlopen = false;
+  static void *ObjCHandle = nullptr;
+  static void *ObjCObjectGetClassName = nullptr;
+
+  // Prevent threads from racing to dlopen().
+  static __sanitizer::StaticSpinMutex Lock;
+  {
+    __sanitizer::SpinMutexLock Guard(&Lock);
+
+    if (!AttemptedDlopen) {
+      ObjCHandle = dlopen(
+          "/usr/lib/libobjc.A.dylib",
+          RTLD_LAZY         // Only bind symbols when used.
+              | RTLD_LOCAL  // Only make symbols available via the handle.
+              | RTLD_NOLOAD // Do not load the dylib, just grab a handle if the
+                            // image is already loaded.
+              | RTLD_FIRST  // Only search the image pointed-to by the handle.
+      );
+      AttemptedDlopen = true;
+      if (!ObjCHandle)
+        return nullptr;
+      ObjCObjectGetClassName = dlsym(ObjCHandle, "object_getClassName");
+    }
+  }
+
+  if (!ObjCObjectGetClassName)
+    return nullptr;
+
+  return ObjCGetClassNameTy(ObjCObjectGetClassName)((void *)Pointer);
+#else
+  return nullptr;
+#endif
+}
+
 SIntMax Value::getSIntValue() const {
   CHECK(getType().isSignedIntegerTy());
   if (isInlineInt()) {
Index: compiler-rt/lib/ubsan/ubsan_handlers.h
===================================================================
--- compiler-rt/lib/ubsan/ubsan_handlers.h
+++ compiler-rt/lib/ubsan/ubsan_handlers.h
@@ -168,6 +168,14 @@
 /// Handle a builtin called in an invalid way.
 RECOVERABLE(invalid_builtin, InvalidBuiltinData *Data)
 
+struct InvalidObjCCast {
+  SourceLocation Loc;
+  const TypeDescriptor &ExpectedType;
+};
+
+/// Handle an invalid ObjC cast.
+RECOVERABLE(invalid_objc_cast, InvalidObjCCast *Data, ValueHandle Pointer)
+
 struct NonNullReturnData {
   SourceLocation AttrLoc;
 };
Index: compiler-rt/lib/ubsan/ubsan_handlers.cpp
===================================================================
--- compiler-rt/lib/ubsan/ubsan_handlers.cpp
+++ compiler-rt/lib/ubsan/ubsan_handlers.cpp
@@ -16,6 +16,7 @@
 #include "ubsan_diag.h"
 #include "ubsan_flags.h"
 #include "ubsan_monitor.h"
+#include "ubsan_value.h"
 
 #include "sanitizer_common/sanitizer_common.h"
 
@@ -598,6 +599,36 @@
   Die();
 }
 
+static void handleInvalidObjCCast(InvalidObjCCast *Data, ValueHandle Pointer,
+                                  ReportOptions Opts) {
+  SourceLocation Loc = Data->Loc.acquire();
+  ErrorType ET = ErrorType::InvalidObjCCast;
+
+  if (ignoreReport(Loc, Opts, ET))
+    return;
+
+  ScopedReport R(Opts, Loc, ET);
+
+  const char *GivenClass = getObjCClassName(Pointer);
+  const char *GivenClassStr = GivenClass ? GivenClass : "<unknown type>";
+
+  Diag(Loc, DL_Error, ET,
+       "invalid ObjC cast, object is a '%0', but expected a %1")
+      << GivenClassStr << Data->ExpectedType;
+}
+
+void __ubsan::__ubsan_handle_invalid_objc_cast(InvalidObjCCast *Data,
+                                               ValueHandle Pointer) {
+  GET_REPORT_OPTIONS(false);
+  handleInvalidObjCCast(Data, Pointer, Opts);
+}
+void __ubsan::__ubsan_handle_invalid_objc_cast_abort(InvalidObjCCast *Data,
+                                                     ValueHandle Pointer) {
+  GET_REPORT_OPTIONS(true);
+  handleInvalidObjCCast(Data, Pointer, Opts);
+  Die();
+}
+
 static void handleNonNullReturn(NonNullReturnData *Data, SourceLocation *LocPtr,
                                 ReportOptions Opts, bool IsAttr) {
   if (!LocPtr)
Index: compiler-rt/lib/ubsan/ubsan_checks.inc
===================================================================
--- compiler-rt/lib/ubsan/ubsan_checks.inc
+++ compiler-rt/lib/ubsan/ubsan_checks.inc
@@ -35,6 +35,7 @@
             "integer-divide-by-zero")
 UBSAN_CHECK(FloatDivideByZero, "float-divide-by-zero", "float-divide-by-zero")
 UBSAN_CHECK(InvalidBuiltin, "invalid-builtin-use", "invalid-builtin-use")
+UBSAN_CHECK(InvalidObjCCast, "invalid-objc-cast", "invalid-objc-cast")
 UBSAN_CHECK(ImplicitUnsignedIntegerTruncation,
             "implicit-unsigned-integer-truncation",
             "implicit-unsigned-integer-truncation")
Index: clang/test/CodeGenObjC/for-in.m
===================================================================
--- clang/test/CodeGenObjC/for-in.m
+++ clang/test/CodeGenObjC/for-in.m
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -emit-llvm %s -o %t
+// RUN: %clang_cc1 %s -verify -o /dev/null
+// RUN: %clang_cc1 %s -triple x86_64-apple-darwin -emit-llvm -fsanitize=objc-cast -o - | FileCheck %s
 
 void p(const char*, ...);
 
@@ -18,12 +19,26 @@
 #define L5(n) L4(n+0),L4(n+16)
 #define L6(n) L5(n+0),L5(n+32)
 
+// CHECK-LABEL: define void @t0
 void t0() {
   NSArray *array = [NSArray arrayWithObjects: L1(0), (void*)0];
 
   p("array.length: %d\n", [array count]);
   unsigned index = 0;
   for (NSString *i in array) {	// expected-warning {{collection expression type 'NSArray *' may not respond}}
+
+    // CHECK:      [[expectedCls:%.*]] = load %struct._class_t*, {{.*}}, !nosanitize
+    // CHECK-NEXT: [[kindOfClassSel:%.*]] = load i8*, i8** @OBJC_SELECTOR_REFERENCES{{.*}}, !nosanitize
+    // CHECK-NEXT: [[expectedClsI8:%.*]] = bitcast %struct._class_t* [[expectedCls]] to i8*, !nosanitize
+    // CHECK-NEXT: [[isCls:%.*]] = call zeroext i1 bitcast {{.*}}@objc_msgSend to i1 (i8*, i8*, %struct.__objcFastEnumerationState*, {{.*}})(i8* [[theItem:%.*]], i8* [[kindOfClassSel]], %struct.__objcFastEnumerationState* {{.*}}, i8* [[expectedClsI8]]), !nosanitize
+    // CHECK: br i1 [[isCls]]
+
+    // CHECK: ptrtoint i8* [[theItem]] to i64, !nosanitize
+    // CHECK-NEXT: call void @__ubsan_handle_invalid_objc_cast
+    // CHECK-NEXT: unreachable, !nosanitize
+
+    // CHECK: bitcast i8* [[theItem]]
+
     p("element %d: %s\n", index++, [i cString]);
   }
 }
Index: clang/lib/Driver/ToolChains/Darwin.cpp
===================================================================
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -2629,6 +2629,7 @@
   Res |= SanitizerKind::Fuzzer;
   Res |= SanitizerKind::FuzzerNoLink;
   Res |= SanitizerKind::Function;
+  Res |= SanitizerKind::ObjCCast;
 
   // Prior to 10.9, macOS shipped a version of the C++ standard library without
   // C++11 support. The same is true of iOS prior to version 5. These OS'es are
Index: clang/lib/Driver/SanitizerArgs.cpp
===================================================================
--- clang/lib/Driver/SanitizerArgs.cpp
+++ clang/lib/Driver/SanitizerArgs.cpp
@@ -27,7 +27,8 @@
 static const SanitizerMask NeedsUbsanRt =
     SanitizerKind::Undefined | SanitizerKind::Integer |
     SanitizerKind::ImplicitConversion | SanitizerKind::Nullability |
-    SanitizerKind::CFI | SanitizerKind::FloatDivideByZero;
+    SanitizerKind::CFI | SanitizerKind::FloatDivideByZero |
+    SanitizerKind::ObjCCast;
 static const SanitizerMask NeedsUbsanCxxRt =
     SanitizerKind::Vptr | SanitizerKind::CFI;
 static const SanitizerMask NotAllowedWithTrap = SanitizerKind::Vptr;
@@ -47,11 +48,12 @@
     SanitizerKind::ImplicitConversion | SanitizerKind::Nullability |
     SanitizerKind::DataFlow | SanitizerKind::Fuzzer |
     SanitizerKind::FuzzerNoLink | SanitizerKind::FloatDivideByZero |
-    SanitizerKind::SafeStack | SanitizerKind::ShadowCallStack;
+    SanitizerKind::SafeStack | SanitizerKind::ShadowCallStack |
+    SanitizerKind::ObjCCast;
 static const SanitizerMask RecoverableByDefault =
     SanitizerKind::Undefined | SanitizerKind::Integer |
     SanitizerKind::ImplicitConversion | SanitizerKind::Nullability |
-    SanitizerKind::FloatDivideByZero;
+    SanitizerKind::FloatDivideByZero | SanitizerKind::ObjCCast;
 static const SanitizerMask Unrecoverable =
     SanitizerKind::Unreachable | SanitizerKind::Return;
 static const SanitizerMask AlwaysRecoverable =
@@ -63,7 +65,8 @@
     (SanitizerKind::Undefined & ~SanitizerKind::Vptr) |
     SanitizerKind::UnsignedIntegerOverflow | SanitizerKind::ImplicitConversion |
     SanitizerKind::Nullability | SanitizerKind::LocalBounds |
-    SanitizerKind::CFI | SanitizerKind::FloatDivideByZero;
+    SanitizerKind::CFI | SanitizerKind::FloatDivideByZero |
+    SanitizerKind::ObjCCast;
 static const SanitizerMask TrappingDefault = SanitizerKind::CFI;
 static const SanitizerMask CFIClasses =
     SanitizerKind::CFIVCall | SanitizerKind::CFINVCall |
Index: clang/lib/CodeGen/CodeGenFunction.h
===================================================================
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -118,6 +118,7 @@
   SANITIZER_CHECK(FunctionTypeMismatch, function_type_mismatch, 1)             \
   SANITIZER_CHECK(ImplicitConversion, implicit_conversion, 0)                  \
   SANITIZER_CHECK(InvalidBuiltin, invalid_builtin, 0)                          \
+  SANITIZER_CHECK(InvalidObjCCast, invalid_objc_cast, 0)                       \
   SANITIZER_CHECK(LoadInvalidValue, load_invalid_value, 0)                     \
   SANITIZER_CHECK(MissingReturn, missing_return, 0)                            \
   SANITIZER_CHECK(MulOverflow, mul_overflow, 0)                                \
Index: clang/lib/CodeGen/CGObjC.cpp
===================================================================
--- clang/lib/CodeGen/CGObjC.cpp
+++ clang/lib/CodeGen/CGObjC.cpp
@@ -1841,6 +1841,41 @@
   llvm::Value *CurrentItem =
     Builder.CreateAlignedLoad(CurrentItemPtr, getPointerAlign());
 
+  if (SanOpts.has(SanitizerKind::ObjCCast)) {
+    // Before using an item from the collection, check that the implicit cast
+    // from id to the element type is valid. This is done with instrumentation
+    // roughly corresponding to:
+    //
+    //   if (![item isKindOfClass:expectedCls]) { /* emit diagnostic */ }
+    SanitizerScope SanScope(this);
+    const ObjCObjectPointerType *ObjPtrTy =
+        elementType->getAsObjCInterfacePointerType();
+    const ObjCInterfaceType *InterfaceTy =
+        ObjPtrTy ? ObjPtrTy->getInterfaceType() : nullptr;
+    if (InterfaceTy) {
+      auto &C = CGM.getContext();
+      assert(InterfaceTy->getDecl() && "No decl for ObjC interface type");
+      IdentifierInfo *IsKindOfClassII[] = {&C.Idents.get("isKindOfClass")};
+      Selector IsKindOfClassSel = C.Selectors.getSelector(
+          llvm::array_lengthof(IsKindOfClassII), &IsKindOfClassII[0]);
+      CallArgList IsKindOfClassArgs;
+      llvm::Value *Cls =
+          CGM.getObjCRuntime().GetClass(*this, InterfaceTy->getDecl());
+      Args.add(RValue::get(Cls), C.getObjCClassType());
+      llvm::Value *IsClass =
+          CGM.getObjCRuntime()
+              .GenerateMessageSend(*this, ReturnValueSlot(), C.BoolTy,
+                                   IsKindOfClassSel, CurrentItem, Args)
+              .getScalarVal();
+      llvm::Constant *StaticData[] = {
+          EmitCheckSourceLocation(S.getBeginLoc()),
+          EmitCheckTypeDescriptor(QualType(InterfaceTy, 0))};
+      EmitCheck({{IsClass, SanitizerKind::ObjCCast}},
+                SanitizerHandler::InvalidObjCCast,
+                ArrayRef<llvm::Constant *>(StaticData), CurrentItem);
+    }
+  }
+
   // Cast that value to the right type.
   CurrentItem = Builder.CreateBitCast(CurrentItem, convertedElementType,
                                       "currentitem");
Index: clang/include/clang/Basic/Sanitizers.def
===================================================================
--- clang/include/clang/Basic/Sanitizers.def
+++ clang/include/clang/Basic/Sanitizers.def
@@ -156,6 +156,8 @@
                 ImplicitIntegerArithmeticValueChange,
                 ImplicitIntegerSignChange | ImplicitSignedIntegerTruncation)
 
+SANITIZER("objc-cast", ObjCCast)
+
 // FIXME:
 //SANITIZER_GROUP("implicit-integer-conversion", ImplicitIntegerConversion,
 //                ImplicitIntegerArithmeticValueChange |
Index: clang/docs/UndefinedBehaviorSanitizer.rst
===================================================================
--- clang/docs/UndefinedBehaviorSanitizer.rst
+++ clang/docs/UndefinedBehaviorSanitizer.rst
@@ -121,6 +121,10 @@
      is annotated with ``_Nonnull``.
   -  ``-fsanitize=nullability-return``: Returning null from a function with
      a return type annotated with ``_Nonnull``.
+  -  ``-fsanitize=objc-cast``: Invalid implicit cast of an ObjC object pointer
+     to an incompatible type. This is often unintentional, but is not undefined
+     behavior, therefore the check is not a part of the ``undefined`` group.
+     Currently only supported on Darwin.
   -  ``-fsanitize=object-size``: An attempt to potentially use bytes which
      the optimizer can determine are not part of the object being accessed.
      This will also detect some types of undefined behavior that may not
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to