r335572 - [ubsan] Relax nullability-return for blocks with deduced types

[Vedant Kumar via cfe-commits]

Author: vedantk
Date: Mon Jun 25 19:50:04 2018
New Revision: 335572

URL: http://llvm.org/viewvc/llvm-project?rev=335572&view=rev
Log:
[ubsan] Relax nullability-return for blocks with deduced types

When the return type of an ObjC-style block literals is deduced, pick
the candidate type with the strictest nullability annotation applicable
to every other candidate.

This suppresses a UBSan false-positive in situations where a too-strict
nullability would be deduced, despite the fact that the returned value
would be implicitly cast to _Nullable.

rdar://41317163

Modified:
    cfe/trunk/include/clang/Basic/Specifiers.h
    cfe/trunk/lib/Sema/SemaLambda.cpp
    cfe/trunk/test/CodeGenObjC/ubsan-nullability.m

Modified: cfe/trunk/include/clang/Basic/Specifiers.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Specifiers.h?rev=335572&r1=335571&r2=335572&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/Specifiers.h (original)
+++ cfe/trunk/include/clang/Basic/Specifiers.h Mon Jun 25 19:50:04 2018
@@ -294,6 +294,12 @@ namespace clang {
     Unspecified
   };
 
+  /// Return true if \p L has a weaker nullability annotation than \p R. The
+  /// ordering is: Unspecified < Nullable < NonNull.
+  inline bool operator<(NullabilityKind L, NullabilityKind R) {
+    return uint8_t(L) > uint8_t(R);
+  }
+
   /// Retrieve the spelling of the given nullability kind.
   llvm::StringRef getNullabilitySpelling(NullabilityKind kind,
                                          bool isContextSensitive = false);

Modified: cfe/trunk/lib/Sema/SemaLambda.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaLambda.cpp?rev=335572&r1=335571&r2=335572&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaLambda.cpp (original)
+++ cfe/trunk/lib/Sema/SemaLambda.cpp Mon Jun 25 19:50:04 2018
@@ -707,8 +707,15 @@ void Sema::deduceClosureReturnType(Captu
     QualType ReturnType =
         (RetE ? RetE->getType() : Context.VoidTy).getUnqualifiedType();
     if (Context.getCanonicalFunctionResultType(ReturnType) ==
-          Context.getCanonicalFunctionResultType(CSI.ReturnType))
+          Context.getCanonicalFunctionResultType(CSI.ReturnType)) {
+      // Use the return type with the strictest possible nullability annotation.
+      auto RetTyNullability = ReturnType->getNullability(Ctx);
+      auto BlockNullability = CSI.ReturnType->getNullability(Ctx);
+      if (BlockNullability &&
+          (!RetTyNullability || *RetTyNullability < *BlockNullability))
+        CSI.ReturnType = ReturnType;
       continue;
+    }
 
     // FIXME: This is a poor diagnostic for ReturnStmts without expressions.
     // TODO: It's possible that the *first* return is the divergent one.

Modified: cfe/trunk/test/CodeGenObjC/ubsan-nullability.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjC/ubsan-nullability.m?rev=335572&r1=335571&r2=335572&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenObjC/ubsan-nullability.m (original)
+++ cfe/trunk/test/CodeGenObjC/ubsan-nullability.m Mon Jun 25 19:50:04 2018
@@ -1,6 +1,6 @@
 // REQUIRES: asserts
-// RUN: %clang_cc1 -x objective-c -emit-llvm -triple x86_64-apple-macosx10.10.0 -fsanitize=nullability-arg,nullability-assign,nullability-return -w %s -o - | FileCheck %s
-// RUN: %clang_cc1 -x objective-c++ -emit-llvm -triple x86_64-apple-macosx10.10.0 -fsanitize=nullability-arg,nullability-assign,nullability-return -w %s -o - | FileCheck %s
+// RUN: %clang_cc1 -x objective-c -emit-llvm -triple x86_64-apple-macosx10.10.0 -fblocks -fobjc-arc -fsanitize=nullability-arg,nullability-assign,nullability-return -w %s -o - | FileCheck %s
+// RUN: %clang_cc1 -x objective-c++ -emit-llvm -triple x86_64-apple-macosx10.10.0 -fblocks -fobjc-arc -fsanitize=nullability-arg,nullability-assign,nullability-return -w %s -o - | FileCheck %s
 
 // CHECK: [[NONNULL_RV_LOC1:@.*]] = private unnamed_addr global {{.*}} i32 100, i32 6
 // CHECK: [[NONNULL_ARG_LOC:@.*]] = private unnamed_addr global {{.*}} i32 204, i32 15 {{.*}} i32 190, i32 23
@@ -177,6 +177,37 @@ void call_A(A *a, int *p) {
 
 void dont_crash(int *_Nonnull p, ...) {}
 
+ at protocol NSObject
+- (id)init;
+ at end
+ at interface NSObject <NSObject> {}
+ at end
+
+#pragma clang assume_nonnull begin
+
+/// Create a "NSObject * _Nonnull" instance.
+NSObject *get_nonnull_error() {
+  // Use nil for convenience. The actual object doesn't matter.
+  return (NSObject *)NULL;
+}
+
+NSObject *_Nullable no_null_return_value_diagnostic(int flag) {
+// CHECK-LABEL: define internal {{.*}}no_null_return_value_diagnostic{{i?}}_block_invoke
+// CHECK-NOT: @__ubsan_handle_nullability_return
+  NSObject *_Nullable (^foo)() = ^() {
+    if (flag) {
+      // Clang should not infer a nonnull return value for this block when this
+      // call is present.
+      return get_nonnull_error();
+    } else {
+      return (NSObject *)NULL;
+    }
+  };
+  return foo();
+}
+
+#pragma clang assume_nonnull end
+
 int main() {
   nonnull_retval1(INULL);
   nonnull_retval2(INNULL, INNULL, INULL, (int *_Nullable)NULL, 0, 0, 0, 0);
@@ -188,5 +219,7 @@ int main() {
   nonnull_init2(INULL);
   call_A((A *)NULL, INULL);
   dont_crash(INNULL, NULL);
+  no_null_return_value_diagnostic(0);
+  no_null_return_value_diagnostic(1);
   return 0;
 }