r341566 - [Sema] Clean up some __builtin_*_chk diagnostics

Erik Pilkington via cfe-commits cfe-commits at lists.llvm.org
Thu Sep 6 10:19:33 PDT 2018


Author: epilk
Date: Thu Sep  6 10:19:33 2018
New Revision: 341566

URL: http://llvm.org/viewvc/llvm-project?rev=341566&view=rev
Log:
[Sema] Clean up some __builtin_*_chk diagnostics

Namely, print the likely macro name when it's used, and include the actual
computed sizes in the diagnostic message, which are sometimes not obvious.

rdar://43909200

Differential revision: https://reviews.llvm.org/D51697

Modified:
    cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
    cfe/trunk/lib/Sema/SemaChecking.cpp
    cfe/trunk/test/Sema/builtin-object-size.c
    cfe/trunk/test/Sema/builtins.c

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=341566&r1=341565&r2=341566&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu Sep  6 10:19:33 2018
@@ -694,7 +694,8 @@ def warn_assume_side_effects : Warning<
   InGroup<DiagGroup<"assume">>;
 
 def warn_memcpy_chk_overflow : Warning<
-  "%0 will always overflow destination buffer">,
+  "'%0' will always overflow; destination buffer has size %1,"
+  " but size argument is %2">,
   InGroup<DiagGroup<"builtin-memcpy-chk-size">>;
 
 /// main()

Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=341566&r1=341565&r2=341566&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Thu Sep  6 10:19:33 2018
@@ -238,7 +238,8 @@ static bool SemaBuiltinOverflow(Sema &S,
 
 static void SemaBuiltinMemChkCall(Sema &S, FunctionDecl *FDecl,
                                   CallExpr *TheCall, unsigned SizeIdx,
-                                  unsigned DstSizeIdx) {
+                                  unsigned DstSizeIdx,
+                                  StringRef LikelyMacroName) {
   if (TheCall->getNumArgs() <= SizeIdx ||
       TheCall->getNumArgs() <= DstSizeIdx)
     return;
@@ -256,12 +257,21 @@ static void SemaBuiltinMemChkCall(Sema &
   if (Size.ule(DstSize))
     return;
 
-  // confirmed overflow so generate the diagnostic.
-  IdentifierInfo *FnName = FDecl->getIdentifier();
+  // Confirmed overflow, so generate the diagnostic.
+  StringRef FunctionName = FDecl->getName();
   SourceLocation SL = TheCall->getBeginLoc();
-  SourceRange SR = TheCall->getSourceRange();
+  SourceManager &SM = S.getSourceManager();
+  // If we're in an expansion of a macro whose name corresponds to this builtin,
+  // use the simple macro name and location.
+  if (SL.isMacroID() && Lexer::getImmediateMacroName(SL, SM, S.getLangOpts()) ==
+                            LikelyMacroName) {
+    FunctionName = LikelyMacroName;
+    SL = SM.getImmediateMacroCallerLoc(SL);
+  }
 
-  S.Diag(SL, diag::warn_memcpy_chk_overflow) << SR << FnName;
+  S.Diag(SL, diag::warn_memcpy_chk_overflow)
+      << FunctionName << DstSize.toString(/*Radix=*/10)
+      << Size.toString(/*Radix=*/10);
 }
 
 static bool SemaBuiltinCallWithStaticChain(Sema &S, CallExpr *BuiltinCall) {
@@ -1219,21 +1229,37 @@ Sema::CheckBuiltinFunctionCall(FunctionD
   // check secure string manipulation functions where overflows
   // are detectable at compile time
   case Builtin::BI__builtin___memcpy_chk:
+    SemaBuiltinMemChkCall(*this, FDecl, TheCall, 2, 3, "memcpy");
+    break;
   case Builtin::BI__builtin___memmove_chk:
+    SemaBuiltinMemChkCall(*this, FDecl, TheCall, 2, 3, "memmove");
+    break;
   case Builtin::BI__builtin___memset_chk:
+    SemaBuiltinMemChkCall(*this, FDecl, TheCall, 2, 3, "memset");
+    break;
   case Builtin::BI__builtin___strlcat_chk:
+    SemaBuiltinMemChkCall(*this, FDecl, TheCall, 2, 3, "strlcat");
+    break;
   case Builtin::BI__builtin___strlcpy_chk:
+    SemaBuiltinMemChkCall(*this, FDecl, TheCall, 2, 3, "strlcpy");
+    break;
   case Builtin::BI__builtin___strncat_chk:
+    SemaBuiltinMemChkCall(*this, FDecl, TheCall, 2, 3, "strncat");
+    break;
   case Builtin::BI__builtin___strncpy_chk:
+    SemaBuiltinMemChkCall(*this, FDecl, TheCall, 2, 3, "strncpy");
+    break;
   case Builtin::BI__builtin___stpncpy_chk:
-    SemaBuiltinMemChkCall(*this, FDecl, TheCall, 2, 3);
+    SemaBuiltinMemChkCall(*this, FDecl, TheCall, 2, 3, "stpncpy");
     break;
   case Builtin::BI__builtin___memccpy_chk:
-    SemaBuiltinMemChkCall(*this, FDecl, TheCall, 3, 4);
+    SemaBuiltinMemChkCall(*this, FDecl, TheCall, 3, 4, "memccpy");
     break;
   case Builtin::BI__builtin___snprintf_chk:
+    SemaBuiltinMemChkCall(*this, FDecl, TheCall, 1, 3, "snprintf");
+    break;
   case Builtin::BI__builtin___vsnprintf_chk:
-    SemaBuiltinMemChkCall(*this, FDecl, TheCall, 1, 3);
+    SemaBuiltinMemChkCall(*this, FDecl, TheCall, 1, 3, "vsnprintf");
     break;
   case Builtin::BI__builtin_call_with_static_chain:
     if (SemaBuiltinCallWithStaticChain(*this, TheCall))

Modified: cfe/trunk/test/Sema/builtin-object-size.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/builtin-object-size.c?rev=341566&r1=341565&r2=341566&view=diff
==============================================================================
--- cfe/trunk/test/Sema/builtin-object-size.c (original)
+++ cfe/trunk/test/Sema/builtin-object-size.c Thu Sep  6 10:19:33 2018
@@ -23,7 +23,7 @@ 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); // expected-warning {{'__builtin___vsnprintf_chk' will always overflow destination buffer}}
+ __builtin___vsnprintf_chk (0, 42, 0, 11, fmt, args); // expected-warning {{'__builtin___vsnprintf_chk' will always overflow; destination buffer has size 11, but size argument is 42}}
 }
 
 // rdar://18334276
@@ -50,7 +50,7 @@ void f6(void)
   char b[5];
   char buf[10];
   __builtin___memccpy_chk (buf, b, '\0', sizeof(b), __builtin_object_size (buf, 0));
-  __builtin___memccpy_chk (b, buf, '\0', sizeof(buf), __builtin_object_size (b, 0));  // expected-warning {{'__builtin___memccpy_chk' will always overflow destination buffer}}
+  __builtin___memccpy_chk (b, buf, '\0', sizeof(buf), __builtin_object_size (b, 0));  // expected-warning {{'__builtin___memccpy_chk' will always overflow; destination buffer has size 5, but size argument is 10}}
 }
 
 int pr28314(void) {

Modified: cfe/trunk/test/Sema/builtins.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/builtins.c?rev=341566&r1=341565&r2=341566&view=diff
==============================================================================
--- cfe/trunk/test/Sema/builtins.c (original)
+++ cfe/trunk/test/Sema/builtins.c Thu Sep  6 10:19:33 2018
@@ -221,14 +221,14 @@ void Test19(void)
                                     // 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-warning {{'__builtin___strlcpy_chk' will always overflow destination buffer}}
+				    // expected-warning {{'__builtin___strlcpy_chk' will always overflow; destination buffer has size 20, but size argument is 40}}
 
         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-warning {{'__builtin___strlcat_chk' will always overflow destination buffer}}
+				                                                   // expected-warning {{'__builtin___strlcat_chk' will always overflow; destination buffer has size 20, but size argument is 40}}
 }
 
 // rdar://11076881
@@ -236,7 +236,7 @@ char * Test20(char *p, const char *in, u
 {
     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 (&buf[6], in, 5, __builtin_object_size (&buf[6], 0)); // expected-warning {{'__builtin___memcpy_chk' will always overflow; destination buffer has size 4, but size argument is 5}}
 
     __builtin___memcpy_chk (p, "abcde", n, __builtin_object_size (p, 0));
 
@@ -244,7 +244,7 @@ char * Test20(char *p, const char *in, u
 
     __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}}
+    __builtin___memcpy_chk (&buf[6], "abcde", 5, __builtin_object_size (&buf[6], 0)); // expected-warning {{'__builtin___memcpy_chk' will always overflow; destination buffer has size 4, but size argument is 5}}
 
     return buf;
 }
@@ -276,3 +276,14 @@ void test22(void) {
   (void)__builtin_signbitl(1.0f);
   (void)__builtin_signbitl(1.0L);
 }
+
+// rdar://43909200
+#define memcpy(x,y,z) __builtin___memcpy_chk(x,y,z, __builtin_object_size(x,0))
+#define my_memcpy(x,y,z) __builtin___memcpy_chk(x,y,z, __builtin_object_size(x,0))
+
+void test23() {
+  char src[1024];
+  char buf[10];
+  memcpy(buf, src, 11); // expected-warning{{'memcpy' will always overflow; destination buffer has size 10, but size argument is 11}}
+  my_memcpy(buf, src, 11); // expected-warning{{'__builtin___memcpy_chk' will always overflow; destination buffer has size 10, but size argument is 11}}
+}




More information about the cfe-commits mailing list