[UBSan] Add returns-nonnull sanitizer.

[samsonov]

Summary

This patch adds a runtime check verifying that functions
annotated with "returns_nonnull" attribute do in fact return nonnull pointers.
It is based on suggestion by Jakub Jelinek:
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20140623/223693.html.

[rsmith]

Looks good, a couple of minor comments.

projects/compiler-rt/lib/ubsan/ubsan_handlers.cc

319

Maybe "null pointer returned from function declared to never return null"? If we can produce a note pointing at the non-null attribute, that'd be even better.

tools/clang/lib/CodeGen/CGCall.cpp

2005

I don't think you should perform this address space check. LLVM is going to assume the pointer is non-null regardless of address space (see CodeGenModule::ConstructAttributeList) so we should sanitize it regardless.

[samsonov]

Renamed the -fsanitize= flag value to "-fsanitize=returns-nonnull-attribute" and added the documentation.
Addressed reviewer's comments.

[samsonov]

Thanks! Submitting this now.

projects/compiler-rt/lib/ubsan/ubsan_handlers.cc

319

Changed the error message as suggested.

tools/clang/lib/CodeGen/CGCall.cpp

2005

Done

projects/compiler-rt/lib/ubsan/ubsan_handlers.h

struct FunctionTypeMismatchData {
   SourceLocation Loc;
   const TypeDescriptor &Type;
 };
 
 RECOVERABLE(function_type_mismatch,
             FunctionTypeMismatchData *Data,
             ValueHandle Val)
 
+struct NonNullReturnData {
+  SourceLocation Loc;
+};
+
+/// \brief Handle returning null from function with returns_nonnull attribute.
+RECOVERABLE(nonnull_return, NonNullReturnData *Data)
+
 }
 
 #endif // UBSAN_HANDLERS_H

projects/compiler-rt/lib/ubsan/ubsan_handlers.cc

__ubsan::__ubsan_handle_function_type_mismatch(FunctionTypeMismatchData *Data,
                                                ValueHandle Function) {
   handleFunctionTypeMismatch(Data, Function, false);
 }
 
 void __ubsan::__ubsan_handle_function_type_mismatch_abort(
     FunctionTypeMismatchData *Data, ValueHandle Function) {
   handleFunctionTypeMismatch(Data, Function, true);
 }
+
+static void handleNonnullReturn(NonNullReturnData *Data, bool Abort) {
+  SourceLocation Loc = Data->Loc.acquire();
+  if (Loc.isDisabled())
+    return;
+
+  ScopedReport R(Abort);
+
+  Diag(Loc, DL_Error, "null pointer returned from function declared to never "
+                      "return null");
+}
+
+void __ubsan::__ubsan_handle_nonnull_return(NonNullReturnData *Data) {
+  handleNonnullReturn(Data, false);
+}
+
+void __ubsan::__ubsan_handle_nonnull_return_abort(NonNullReturnData *Data) {
+  handleNonnullReturn(Data, true);
+}

projects/compiler-rt/test/ubsan/TestCases/Misc/nonnull.cpp

+// RUN: %clangxx -fsanitize=returns-nonnull-attribute %s -O3 -o %t
+// RUN: %run %t foo
+// RUN: %run %t 2>&1 | FileCheck %s
+
+__attribute__((returns_nonnull))
+char *foo(char *a) {
+  return a;
+  // CHECK: nonnull.cpp:[[@LINE+1]]:1: runtime error: null pointer returned from function declared to never return null
+}
+
+int main(int argc, char **argv) {
+  return foo(argv[1]) == 0;
+}

tools/clang/docs/UsersManual.rst

-  ``-fsanitize=null``: Use of a null pointer or creation of a null
       reference.
    -  ``-fsanitize=object-size``: An attempt to use bytes which the
       optimizer can determine are not part of the object being
       accessed. The sizes of objects are determined using
       ``__builtin_object_size``, and consequently may be able to detect
       more problems at higher optimization levels.
    -  ``-fsanitize=return``: In C++, reaching the end of a
       value-returning function without returning a value.
+   -  ``-fsanitize=returns-nonnull-attribute``: Returning null pointer
+      from a function which is declared to never return null.
    -  ``-fsanitize=shift``: Shift operators where the amount shifted is
       greater or equal to the promoted bit-width of the left hand side
       or less than zero, or where the left hand side is negative. For a
       signed left shift, also checks for signed overflow in C, and for
       unsigned overflow in C++.
    -  ``-fsanitize=signed-integer-overflow``: Signed integer overflow,
       including all the checks added by ``-ftrapv``, and checking for
       overflow in signed division (``INT_MIN / -1``).

tools/clang/include/clang/Basic/Sanitizers.def

 SANITIZER("enum", Enum)
 SANITIZER("float-cast-overflow", FloatCastOverflow)
 SANITIZER("float-divide-by-zero", FloatDivideByZero)
 SANITIZER("function", Function)
 SANITIZER("integer-divide-by-zero", IntegerDivideByZero)
 SANITIZER("null", Null)
 SANITIZER("object-size", ObjectSize)
 SANITIZER("return", Return)
+SANITIZER("returns-nonnull-attribute", ReturnsNonnullAttribute)
 SANITIZER("shift", Shift)
 SANITIZER("signed-integer-overflow", SignedIntegerOverflow)
 SANITIZER("unreachable", Unreachable)
 SANITIZER("vla-bound", VLABound)
 SANITIZER("vptr", Vptr)
 
 // IntegerSanitizer
 SANITIZER("unsigned-integer-overflow", UnsignedIntegerOverflow)
 
 // DataFlowSanitizer
 SANITIZER("dataflow", DataFlow)
 
 // -fsanitize=undefined includes all the sanitizers which have low overhead, no
 // ABI or address space layout implications, and only catch undefined behavior.
 SANITIZER_GROUP("undefined", Undefined,
                 Alignment | Bool | ArrayBounds | Enum | FloatCastOverflow |
                     FloatDivideByZero | Function | IntegerDivideByZero | Null |
-                ObjectSize | Return | Shift | SignedIntegerOverflow |
-                Unreachable | VLABound | Vptr)
+                    ObjectSize | Return | ReturnsNonnullAttribute | Shift |
+                    SignedIntegerOverflow | Unreachable | VLABound | Vptr)
 
 // -fsanitize=undefined-trap includes
 // all sanitizers included by -fsanitize=undefined, except those that require
 // runtime support.  This group is generally used in conjunction with the
 // -fsanitize-undefined-trap-on-error flag.
 SANITIZER_GROUP("undefined-trap", UndefinedTrap,
                 Alignment | Bool | ArrayBounds | Enum | FloatCastOverflow |
-                FloatDivideByZero | IntegerDivideByZero | Null | ObjectSize |
-                Return | Shift | SignedIntegerOverflow | Unreachable |
-                VLABound)
+                    FloatDivideByZero | IntegerDivideByZero | Null |
+                    ObjectSize | Return | ReturnsNonnullAttribute | Shift |
+                    SignedIntegerOverflow | Unreachable | VLABound)
 
 SANITIZER_GROUP("integer", Integer,
                 SignedIntegerOverflow | UnsignedIntegerOverflow | Shift |
                 IntegerDivideByZero)
 
 SANITIZER("local-bounds", LocalBounds)
 SANITIZER_GROUP("bounds", Bounds, ArrayBounds | LocalBounds)
 
 #undef SANITIZER
 #undef SANITIZER_GROUP

tools/clang/lib/CodeGen/CGCall.cpp

void CodeGenFunction::EmitFunctionEpilog(const CGFunctionInfo &FI,
 
   case ABIArgInfo::Ignore:
     break;
 
   case ABIArgInfo::Expand:
     llvm_unreachable("Invalid ABI kind for return argument");
   }
 
-  llvm::Instruction *Ret = RV ? Builder.CreateRet(RV) : Builder.CreateRetVoid();
+  llvm::Instruction *Ret;
+  if (RV) {
+    if (SanOpts->ReturnsNonnullAttribute &&
+        CurGD.getDecl()->hasAttr<ReturnsNonNullAttr>()) {
+      SanitizerScope SanScope(this);
+      llvm::Value *Cond =
+          Builder.CreateICmpNE(RV, llvm::Constant::getNullValue(RV->getType()));
+      llvm::Constant *StaticData[] = {
+        EmitCheckSourceLocation(EndLoc)
+      };
+      EmitCheck(Cond, "nonnull_return", StaticData, ArrayRef<llvm::Value *>(),
+                CRK_Recoverable);
+    }
+    Ret = Builder.CreateRet(RV);
+  } else {
+    Ret = Builder.CreateRetVoid();
+  }
+
   if (!RetDbgLoc.isUnknown())
     Ret->setDebugLoc(RetDbgLoc);
 }
 
 static bool isInAllocaArgument(CGCXXABI &ABI, QualType type) {
   const CXXRecordDecl *RD = type->getAsCXXRecordDecl();
   return RD && ABI.getRecordArgABI(RD) == CGCXXABI::RAA_DirectInMemory;
 }

tools/clang/test/CodeGen/catch-undef-behavior.c

-// RUN: %clang_cc1 -fsanitize=alignment,null,object-size,shift,return,signed-integer-overflow,vla-bound,float-cast-overflow,integer-divide-by-zero,bool -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s
-// RUN: %clang_cc1 -fsanitize-undefined-trap-on-error -fsanitize=alignment,null,object-size,shift,return,signed-integer-overflow,vla-bound,float-cast-overflow,integer-divide-by-zero,bool -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s --check-prefix=CHECK-TRAP
+// RUN: %clang_cc1 -fsanitize=alignment,null,object-size,shift,return,signed-integer-overflow,vla-bound,float-cast-overflow,integer-divide-by-zero,bool,returns-nonnull-attribute -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s
+// RUN: %clang_cc1 -fsanitize-undefined-trap-on-error -fsanitize=alignment,null,object-size,shift,return,signed-integer-overflow,vla-bound,float-cast-overflow,integer-divide-by-zero,bool,returns-nonnull-attribute -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s --check-prefix=CHECK-TRAP
 // RUN: %clang_cc1 -fsanitize=null -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s --check-prefix=CHECK-NULL
 // RUN: %clang_cc1 -fsanitize=signed-integer-overflow -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s --check-prefix=CHECK-OVERFLOW
 
 // CHECK: @[[INT:.*]] = private unnamed_addr constant { i16, i16, [6 x i8] } { i16 0, i16 11, [6 x i8] c"'int'\00" }
 
 // FIXME: When we only emit each type once, use [[INT]] more below.
 // CHECK: @[[LINE_100:.*]] = private unnamed_addr global {{.*}}, i32 100, i32 5 {{.*}} @[[INT]], i64 4, i8 1
 // CHECK: @[[LINE_200:.*]] = {{.*}}, i32 200, i32 10 {{.*}}, i64 4, i8 0
_Bool sour_bool(_Bool *p) {
   // CHECK-TRAP: %[[OK:.*]] = icmp ule i8 {{.*}}, 1
   // CHECK-TRAP: br i1 %[[OK]]
 
   // CHECK-TRAP: call void @llvm.trap() [[NR_NUW]]
   // CHECK-TRAP: unreachable
   return *p;
 }
 
+// CHECK-LABEL: @ret_nonnull
+__attribute__((returns_nonnull))
+int *ret_nonnull(int *a) {
+  // CHECK: [[OK:%.*]] = icmp ne i32* {{.*}}, null
+  // CHECK: br i1 [[OK]]
+  // CHECK: call void @__ubsan_handle_nonnull_return
+
+  // CHECK-TRAP: [[OK:%.*]] = icmp ne i32* {{.*}}, null
+  // CHECK-TRAP: br i1 [[OK]]
+  // CHECK-TRAP: call void @llvm.trap() [[NR_NUW]]
+  // CHECK-TRAP: unreachable
+  return a;
+}
+
 // CHECK: ![[WEIGHT_MD]] = metadata !{metadata !"branch_weights", i32 1048575, i32 1}
 
 // CHECK-TRAP: attributes [[NR_NUW]] = { noreturn nounwind }

tools/clang/test/Driver/fsanitize.c

 // RUN: %clang -target x86_64-linux-gnu -fsanitize=undefined-trap -fsanitize-undefined-trap-on-error %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UNDEFINED-TRAP
 // RUN: %clang -target x86_64-linux-gnu -fsanitize-undefined-trap-on-error -fsanitize=undefined-trap %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UNDEFINED-TRAP
-// CHECK-UNDEFINED-TRAP: "-fsanitize={{((signed-integer-overflow|integer-divide-by-zero|float-divide-by-zero|shift|unreachable|return|vla-bound|alignment|null|object-size|float-cast-overflow|array-bounds|enum|bool),?){14}"}}
+// CHECK-UNDEFINED-TRAP: "-fsanitize={{((signed-integer-overflow|integer-divide-by-zero|float-divide-by-zero|shift|unreachable|return|vla-bound|alignment|null|object-size|float-cast-overflow|array-bounds|enum|bool|returns-nonnull-attribute),?){15}"}}
 // CHECK-UNDEFINED-TRAP: "-fsanitize-undefined-trap-on-error"
 
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=undefined %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UNDEFINED
-// CHECK-UNDEFINED: "-fsanitize={{((signed-integer-overflow|integer-divide-by-zero|float-divide-by-zero|function|shift|unreachable|return|vla-bound|alignment|null|vptr|object-size|float-cast-overflow|array-bounds|enum|bool),?){16}"}}
+// CHECK-UNDEFINED: "-fsanitize={{((signed-integer-overflow|integer-divide-by-zero|float-divide-by-zero|function|shift|unreachable|return|vla-bound|alignment|null|vptr|object-size|float-cast-overflow|array-bounds|enum|bool|returns-nonnull-attribute),?){17}"}}
 
 // RUN: %clang -target x86_64-apple-darwin10 -fsanitize=undefined %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UNDEFINED-DARWIN
-// CHECK-UNDEFINED-DARWIN: "-fsanitize={{((signed-integer-overflow|integer-divide-by-zero|float-divide-by-zero|shift|unreachable|return|vla-bound|alignment|null|vptr|object-size|float-cast-overflow|array-bounds|enum|bool),?){15}"}}
+// CHECK-UNDEFINED-DARWIN: "-fsanitize={{((signed-integer-overflow|integer-divide-by-zero|float-divide-by-zero|shift|unreachable|return|vla-bound|alignment|null|vptr|object-size|float-cast-overflow|array-bounds|enum|bool|returns-nonnull-attribute),?){16}"}}
 
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=integer %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-INTEGER
 // CHECK-INTEGER: "-fsanitize={{((signed-integer-overflow|unsigned-integer-overflow|integer-divide-by-zero|shift),?){4}"}}
 
 // RUN: %clang -fsanitize=bounds -### -fsyntax-only %s 2>&1 | FileCheck %s --check-prefix=CHECK-BOUNDS
 // CHECK-BOUNDS: "-fsanitize={{((array-bounds|local-bounds),?){2}"}}
 
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=thread,undefined -fno-sanitize=thread -fno-sanitize=float-cast-overflow,vptr,bool,enum %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-PARTIAL-UNDEFINED
-// CHECK-PARTIAL-UNDEFINED: "-fsanitize={{((signed-integer-overflow|integer-divide-by-zero|float-divide-by-zero|function|shift|unreachable|return|vla-bound|alignment|null|object-size|array-bounds),?){12}"}}
+// CHECK-PARTIAL-UNDEFINED: "-fsanitize={{((signed-integer-overflow|integer-divide-by-zero|float-divide-by-zero|function|shift|unreachable|return|vla-bound|alignment|null|object-size|array-bounds|returns-nonnull-attribute),?){13}"}}
 
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=undefined -fsanitize-undefined-trap-on-error %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UNDEFINED-TRAP-ON-ERROR-UNDEF
 // CHECK-UNDEFINED-TRAP-ON-ERROR-UNDEF: '-fsanitize=undefined' not allowed with '-fsanitize-undefined-trap-on-error'
 
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=vptr -fsanitize-undefined-trap-on-error %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UNDEFINED-TRAP-ON-ERROR-VPTR
 // CHECK-UNDEFINED-TRAP-ON-ERROR-VPTR: '-fsanitize=vptr' not allowed with '-fsanitize-undefined-trap-on-error'
 
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=vptr -fno-rtti %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-VPTR-NO-RTTI