[PATCH] D27607: [ubsan] Treat ObjC's BOOL as if its range is always {0, 1}

[Vedant Kumar via Phabricator via cfe-commits]

vsk created this revision.
vsk added a reviewer: ahatanak.
vsk added subscribers: kubabrecka, cfe-commits.

On some Apple platforms, the ObjC BOOL type is defined as a signed char.
When performing instrumentation for -fsanitize=bool, we'd like to treat
the range of BOOL like it's always {0, 1}. While we can't change clang's
IRGen for char-backed BOOL's due to ABI compatibility concerns, we can
teach ubsan to catch potential abuses of this type.

rdar://problem/29502773


https://reviews.llvm.org/D27607

Files:
  lib/CodeGen/CGExpr.cpp
  test/CodeGenObjC/ubsan-bool.m


Index: test/CodeGenObjC/ubsan-bool.m
===================================================================
--- /dev/null
+++ test/CodeGenObjC/ubsan-bool.m
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -x objective-c -emit-llvm -triple x86_64-apple-macosx10.10.0 -fsanitize=bool %s -o - | FileCheck %s -check-prefixes=SHARED,OBJC
+// RUN: %clang_cc1 -x objective-c++ -emit-llvm -triple x86_64-apple-macosx10.10.0 -fsanitize=bool %s -o - | FileCheck %s -check-prefixes=SHARED,OBJC
+// RUN: %clang_cc1 -x c -emit-llvm -triple x86_64-apple-macosx10.10.0 -fsanitize=bool %s -o - | FileCheck %s -check-prefixes=SHARED,C
+
+#ifdef IS_SYSHEADER
+
+// Create a system definition of ObjC's BOOL.
+#pragma clang system_header
+typedef signed char BOOL;
+
+#else
+
+// Import an official-looking definition of BOOL.
+#define IS_SYSHEADER
+#include __FILE__
+
+// SHARED-LABEL: f1
+BOOL f1() {
+  // OBJC: call void @__ubsan_handle_load_invalid_value
+  // C-NOT: call void @__ubsan_handle_load_invalid_value
+  BOOL a = 2;
+  return a + 1;
+}
+
+#endif // IS_SYSHEADER
Index: lib/CodeGen/CGExpr.cpp
===================================================================
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -1217,13 +1217,29 @@
   return false;
 }
 
+/// Check if \p Ty is defined as BOOL in a system header. In ObjC language
+/// modes, it's safe to treat such a type as 'the builtin bool'.
+static bool isObjCBool(QualType Ty, const SourceManager &SM,
+                       const LangOptions &LO) {
+  if (!LO.ObjC1 && !LO.ObjC2)
+    return false;
+
+  const auto *TT = Ty.getTypePtr()->getAs<TypedefType>();
+  if (!TT)
+    return false;
+
+  const TypedefNameDecl *TND = TT->getDecl();
+  return TND->getName() == "BOOL" &&
+         SM.isInSystemHeader(
+             TND->getTypeSourceInfo()->getTypeLoc().getBeginLoc());
+}
+
 static bool getRangeForType(CodeGenFunction &CGF, QualType Ty,
                             llvm::APInt &Min, llvm::APInt &End,
-                            bool StrictEnums) {
+                            bool StrictEnums, bool IsBool) {
   const EnumType *ET = Ty->getAs<EnumType>();
   bool IsRegularCPlusPlusEnum = CGF.getLangOpts().CPlusPlus && StrictEnums &&
                                 ET && !ET->getDecl()->isFixed();
-  bool IsBool = hasBooleanRepresentation(Ty);
   if (!IsBool && !IsRegularCPlusPlusEnum)
     return false;
 
@@ -1253,8 +1269,8 @@
 
 llvm::MDNode *CodeGenFunction::getRangeForLoadFromType(QualType Ty) {
   llvm::APInt Min, End;
-  if (!getRangeForType(*this, Ty, Min, End,
-                       CGM.getCodeGenOpts().StrictEnums))
+  if (!getRangeForType(*this, Ty, Min, End, CGM.getCodeGenOpts().StrictEnums,
+                       hasBooleanRepresentation(Ty)))
     return nullptr;
 
   llvm::MDBuilder MDHelper(getLLVMContext());
@@ -1313,14 +1329,16 @@
                                       false /*ConvertTypeToTag*/);
   }
 
-  bool NeedsBoolCheck =
-      SanOpts.has(SanitizerKind::Bool) && hasBooleanRepresentation(Ty);
+  bool IsBool =
+      hasBooleanRepresentation(Ty) ||
+      isObjCBool(Ty, CGM.getContext().getSourceManager(), CGM.getLangOpts());
+  bool NeedsBoolCheck = SanOpts.has(SanitizerKind::Bool) && IsBool;
   bool NeedsEnumCheck =
       SanOpts.has(SanitizerKind::Enum) && Ty->getAs<EnumType>();
   if (NeedsBoolCheck || NeedsEnumCheck) {
     SanitizerScope SanScope(this);
     llvm::APInt Min, End;
-    if (getRangeForType(*this, Ty, Min, End, true)) {
+    if (getRangeForType(*this, Ty, Min, End, /*StrictEnums=*/true, IsBool)) {
       --End;
       llvm::Value *Check;
       if (!Min)


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D27607.80856.patch
Type: text/x-patch
Size: 3592 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20161209/38506275/attachment-0001.bin>