Patch to check at compile time for overflow when

__builtin___memcpy_chk and similar builtins are
being used. Patch by Jacques Fortier (with added
clang tests).  rdar://11076881

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@218063 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index d955e4eaffcb2ef1784b1eff9ca2948899fbaac2..7e08fce794a98b355ca8c7d7f7f94af6546ec52f 100644 (file)
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -454,6 +454,10 @@ def warn_assume_side_effects : Warning<
   "the argument to %0 has side effects that will be discarded">,
   InGroup<DiagGroup<"assume">>;
 
+def warn_memcpy_chk_overflow : Warning<
+  "%0 will always overflow destination buffer">,
+  InGroup<DiagGroup<"builtin-memcpy-chk-size">>;
+
 /// main()
 // static main() is not an error in C, just in C++.
 def warn_static_main : Warning<"'main' should not be declared static">,

diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h
index 3755256da9621ecd141c04f6af6ef4acc60731d1..b262c45cd7cdedd0ec1235305c28ba2a2c1551a2 100644 (file)
--- a/include/clang/Sema/Sema.h
+++ b/include/clang/Sema/Sema.h
@@ -8355,7 +8355,8 @@ private:
 
   bool CheckObjCString(Expr *Arg);
 
-  ExprResult CheckBuiltinFunctionCall(unsigned BuiltinID, CallExpr *TheCall);
+  ExprResult CheckBuiltinFunctionCall(FunctionDecl *FDecl,
+                                     unsigned BuiltinID, CallExpr *TheCall);
 
   bool CheckARMBuiltinExclusiveCall(unsigned BuiltinID, CallExpr *TheCall,
                                     unsigned MaxWidth);

diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp
index 953b3f67d2f86ecee05c8e8489febf47ef60e883..7462869306340da075b73997bb90dc71d39f4b73 100644 (file)
--- a/lib/Sema/SemaChecking.cpp
+++ b/lib/Sema/SemaChecking.cpp
@@ -111,8 +111,37 @@ static bool SemaBuiltinAddressof(Sema &S, CallExpr *TheCall) {
   return false;
 }
 
+static void SemaBuiltinMemChkCall(Sema &S, FunctionDecl *FDecl,
+                                 CallExpr *TheCall, unsigned SizeIdx,
+                                  unsigned DstSizeIdx) {
+  if (TheCall->getNumArgs() <= SizeIdx ||
+      TheCall->getNumArgs() <= DstSizeIdx)
+    return;
+
+  const Expr *SizeArg = TheCall->getArg(SizeIdx);
+  const Expr *DstSizeArg = TheCall->getArg(DstSizeIdx);
+
+  llvm::APSInt Size, DstSize;
+
+  // find out if both sizes are known at compile time
+  if (!SizeArg->EvaluateAsInt(Size, S.Context) ||
+      !DstSizeArg->EvaluateAsInt(DstSize, S.Context))
+    return;
+
+  if (Size.ule(DstSize))
+    return;
+
+  // confirmed overflow so generate the diagnostic.
+  IdentifierInfo *FnName = FDecl->getIdentifier();
+  SourceLocation SL = TheCall->getLocStart();
+  SourceRange SR = TheCall->getSourceRange();
+
+  S.Diag(SL, diag::warn_memcpy_chk_overflow) << SR << FnName;
+}
+
 ExprResult
-Sema::CheckBuiltinFunctionCall(unsigned BuiltinID, CallExpr *TheCall) {
+Sema::CheckBuiltinFunctionCall(FunctionDecl *FDecl, unsigned BuiltinID,
+                               CallExpr *TheCall) {
   ExprResult TheCallResult(TheCall);
 
   // Find out if any arguments are required to be integer constant expressions.
@@ -332,6 +361,24 @@ Sema::CheckBuiltinFunctionCall(unsigned BuiltinID, CallExpr *TheCall) {
     // so ensure that they are declared.
     DeclareGlobalNewDelete();
     break;
+
+  // check secure string manipulation functions where overflows
+  // are detectable at compile time
+  case Builtin::BI__builtin___memcpy_chk:
+  case Builtin::BI__builtin___memccpy_chk:
+  case Builtin::BI__builtin___memmove_chk:
+  case Builtin::BI__builtin___memset_chk:
+  case Builtin::BI__builtin___strlcat_chk:
+  case Builtin::BI__builtin___strlcpy_chk:
+  case Builtin::BI__builtin___strncat_chk:
+  case Builtin::BI__builtin___strncpy_chk:
+  case Builtin::BI__builtin___stpncpy_chk:
+    SemaBuiltinMemChkCall(*this, FDecl, TheCall, 2, 3);
+    break;
+  case Builtin::BI__builtin___snprintf_chk:
+  case Builtin::BI__builtin___vsnprintf_chk:
+    SemaBuiltinMemChkCall(*this, FDecl, TheCall, 1, 3);
+    break;
   }
 
   // Since the target specific builtins for each arch overlap, only check those

diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp
index 6b01b1f4b0ca41ab9c2a1aab8b8befb819cc2104..50bc4d2ac8bb015dcf03929ae4eefb0a25bc6e69 100644 (file)
--- a/lib/Sema/SemaExpr.cpp
+++ b/lib/Sema/SemaExpr.cpp
@@ -4665,7 +4665,7 @@ Sema::BuildResolvedCallExpr(Expr *Fn, NamedDecl *NDecl,
 
   // Bail out early if calling a builtin with custom typechecking.
   if (BuiltinID && Context.BuiltinInfo.hasCustomTypechecking(BuiltinID))
-    return CheckBuiltinFunctionCall(BuiltinID, TheCall);
+    return CheckBuiltinFunctionCall(FDecl, BuiltinID, TheCall);
 
  retry:
   const FunctionType *FuncT;
@@ -4793,7 +4793,7 @@ Sema::BuildResolvedCallExpr(Expr *Fn, NamedDecl *NDecl,
       return ExprError();
 
     if (BuiltinID)
-      return CheckBuiltinFunctionCall(BuiltinID, TheCall);
+      return CheckBuiltinFunctionCall(FDecl, BuiltinID, TheCall);
   } else if (NDecl) {
     if (CheckPointerCall(NDecl, TheCall, Proto))
       return ExprError();

diff --git a/test/Sema/builtin-object-size.c b/test/Sema/builtin-object-size.c
index 0abc27ba187a66c43381a67adf0042ff2f619401..db4832e347a08943365731734faa75b8507fc6ed 100644 (file)
--- a/test/Sema/builtin-object-size.c
+++ b/test/Sema/builtin-object-size.c
@@ -23,6 +23,6 @@ int f3() {
 // rdar://6252231 - cannot call vsnprintf with va_list on x86_64
 void f4(const char *fmt, ...) {
  __builtin_va_list args;
- __builtin___vsnprintf_chk (0, 42, 0, 11, fmt, args);
+ __builtin___vsnprintf_chk (0, 42, 0, 11, fmt, args); // expected-warning {{'__builtin___vsnprintf_chk' will always overflow destination buffer}}
 }
 

diff --git a/test/Sema/builtins.c b/test/Sema/builtins.c
index 8e3a60ab0678e1f7d983580048991676e635119a..f8df2f85cf640084198a10b8375b001e87fdae24 100644 (file)
--- a/test/Sema/builtins.c
+++ b/test/Sema/builtins.c
@@ -215,10 +215,31 @@ void Test19(void)
         strlcpy(buf, b, sizeof(b)); // expected-warning {{size argument in 'strlcpy' call appears to be size of the source; expected the size of the destination}} \\
                                     // expected-note {{change size argument to be the size of the destination}}
         __builtin___strlcpy_chk(buf, b, sizeof(b), __builtin_object_size(buf, 0)); // expected-warning {{size argument in '__builtin___strlcpy_chk' call appears to be size of the source; expected the size of the destination}} \
-                                    // expected-note {{change size argument to be the size of the destination}}
+                                    // expected-note {{change size argument to be the size of the destination}} \
+                                   // expected-warning {{'__builtin___strlcpy_chk' will always overflow destination buffer}}
 
         strlcat(buf, b, sizeof(b)); // expected-warning {{size argument in 'strlcat' call appears to be size of the source; expected the size of the destination}} \
                                     // expected-note {{change size argument to be the size of the destination}}
+                                   
         __builtin___strlcat_chk(buf, b, sizeof(b), __builtin_object_size(buf, 0)); // expected-warning {{size argument in '__builtin___strlcat_chk' call appears to be size of the source; expected the size of the destination}} \
-                                                                                   // expected-note {{change size argument to be the size of the destination}}
+                                                                                   // expected-note {{change size argument to be the size of the destination}} \
+                                                                                  // expected-warning {{'__builtin___strlcat_chk' will always overflow destination buffer}}
+}
+
+// rdar://11076881
+char * Test20(char *p, const char *in, unsigned n)
+{
+    static char buf[10];
+
+    __builtin___memcpy_chk (&buf[6], in, 5, __builtin_object_size (&buf[6], 0)); // expected-warning {{'__builtin___memcpy_chk' will always overflow destination buffer}}
+
+    __builtin___memcpy_chk (p, "abcde", n, __builtin_object_size (p, 0));
+
+    __builtin___memcpy_chk (&buf[5], "abcde", 5, __builtin_object_size (&buf[5], 0));
+
+    __builtin___memcpy_chk (&buf[5], "abcde", n, __builtin_object_size (&buf[5], 0));
+
+    __builtin___memcpy_chk (&buf[6], "abcde", 5, __builtin_object_size (&buf[6], 0)); // expected-warning {{'__builtin___memcpy_chk' will always overflow destination buffer}}
+
+    return buf;
 }