erik.pilkington created this revision.
erik.pilkington added reviewers: aaron.ballman, rsmith.
Herald added subscribers: kristina, dexonsmith.
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
Thanks for taking a look!
Repository:
rC Clang
https://reviews.llvm.org/D51697
Files:
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaChecking.cpp
clang/test/Sema/builtin-object-size.c
clang/test/Sema/builtins.c
Index: clang/test/Sema/builtins.c
===================================================================
--- clang/test/Sema/builtins.c
+++ clang/test/Sema/builtins.c
@@ -221,30 +221,30 @@
// 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
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 (&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));
__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}}
+ __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)__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}}
+}
Index: clang/test/Sema/builtin-object-size.c
===================================================================
--- clang/test/Sema/builtin-object-size.c
+++ clang/test/Sema/builtin-object-size.c
@@ -23,7 +23,7 @@
// 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 @@
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) {
Index: clang/lib/Sema/SemaChecking.cpp
===================================================================
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -238,7 +238,8 @@
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 @@
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 @@
// 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))
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -694,7 +694,8 @@
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()
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
