vsk updated this revision to Diff 271508.
vsk added a comment.
Herald added projects: clang, Sanitizers.
Herald added a subscriber: Sanitizers.
Rebase.
Repository:
rG LLVM Github Monorepo
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,57 @@
#include "ubsan_value.h"
#include "sanitizer_common/sanitizer_common.h"
#include "sanitizer_common/sanitizer_libc.h"
+#include "sanitizer_common/sanitizer_mutex.h"
+
+// TODO(dliew): Prefer '__APPLE__' here over 'SANITIZER_MAC', as the latter is
+// unclear. rdar://58124919 tracks using a more obviously portable guard.
+#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"
@@ -640,6 +641,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
@@ -37,6 +37,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
@@ -2678,6 +2678,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;
@@ -48,11 +49,11 @@
SanitizerKind::DataFlow | SanitizerKind::Fuzzer |
SanitizerKind::FuzzerNoLink | SanitizerKind::FloatDivideByZero |
SanitizerKind::SafeStack | SanitizerKind::ShadowCallStack |
- SanitizerKind::Thread;
+ SanitizerKind::Thread | 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 =
@@ -62,7 +63,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
@@ -122,6 +122,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
@@ -1837,6 +1837,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 */ }
+ const ObjCObjectPointerType *ObjPtrTy =
+ elementType->getAsObjCInterfacePointerType();
+ const ObjCInterfaceType *InterfaceTy =
+ ObjPtrTy ? ObjPtrTy->getInterfaceType() : nullptr;
+ if (InterfaceTy) {
+ SanitizerScope SanScope(this);
+ 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
@@ -127,6 +127,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
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
