[prev in list] [next in list] [prev in thread] [next in thread]
List: cfe-commits
Subject: [PATCH] D58797: [Sema] Add some compile time _FORTIFY_SOURCE diagnostics
From: Erik Pilkington via Phabricator via cfe-commits <cfe-commits () lists ! llvm ! org>
Date: 2019-02-28 23:04:53
Message-ID: differential-rev-PHID-DREV-eogp5wx5nqgkisdgbad7-req () reviews ! llvm ! org
[Download RAW message or body]
erik.pilkington created this revision.
erik.pilkington added reviewers: george.burgess.iv, rsmith, aaron.ballman.
Herald added subscribers: jdoerfert, dexonsmith, jkorous.
Herald added a project: clang.
These diagnose overflowing calls to subset of fortifiable functions. Some f=
unctions, like `sprintf` or `strcpy` aren't supported right not, but we sho=
uld probably support these in the future. We previously supported this kind=
of functionality with `-Wbuiltin-memcpy-chk-size`, but that diagnose doesn=
't work with `_FORTIFY` implementations that use wrapper functions. Also un=
like that diagnostic, we emit these warnings regardless of whether `_FORTIF=
Y_SOURCE` is actually enabled, which is nice for programs that don't enable=
the runtime checks.
Why not just use diagnose_if, like Bionic does? We can get better diagnosti=
cs in the compiler (i.e. mention the sizes), and we have the potential to d=
iagnose `sprintf` and `strcpy` which is impossible with diagnose_if (at lea=
st, in languages that don't support C++14 constexpr). This approach also sa=
ves standard libraries from having to add diagnose_if.
rdar://48006655
Thanks for taking a look!
Erik
Repository:
rC Clang
https://reviews.llvm.org/D58797
Files:
clang/include/clang/AST/Decl.h
clang/include/clang/Basic/DiagnosticGroups.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Sema/Sema.h
clang/lib/AST/Decl.cpp
clang/lib/Sema/SemaChecking.cpp
clang/lib/Sema/SemaExpr.cpp
clang/test/Analysis/bstring.c
clang/test/Analysis/null-deref-ps-region.c
clang/test/Analysis/pr22954.c
clang/test/Analysis/string.c
clang/test/Sema/builtin-object-size.c
clang/test/Sema/builtins.c
clang/test/Sema/transpose-memset.c
clang/test/Sema/warn-fortify-source.c
clang/test/Sema/warn-strncat-size.c
["D58797.188793.patch" (text/x-patch)]
Index: clang/test/Sema/warn-strncat-size.c
===================================================================
--- clang/test/Sema/warn-strncat-size.c
+++ clang/test/Sema/warn-strncat-size.c
@@ -39,7 +39,7 @@
strncat(dest, "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA", sizeof(dest) - strlen(dest)); // \
expected-warning{{the value of the size argument in 'strncat' is too large, might \
lead to a buffer overflow}} expected-note {{change the argument to be the free space \
in the destination buffer minus the terminating null byte}}
strncat((*s5)->f2[x], s2, sizeof(s2)); // expected-warning {{size argument in \
'strncat' call appears to be size of the source}} expected-note {{change the argument \
to be the free space in the destination buffer minus the terminating \
null byte}}
- strncat(s1+3, s2, sizeof(s2)); // expected-warning {{size argument in 'strncat' \
call appears to be size of the source}} + strncat(s1+3, s2, sizeof(s2)); // \
expected-warning {{size argument in 'strncat' call appears to be size of the source}} \
expected-warning {{strncat' size argument is too large; destination buffer has size \
97, but size argument is 200}} strncat(s4.f1, s2, sizeof(s2)); // expected-warning \
{{size argument in 'strncat' call appears to be size of the source}} expected-note \
{{change the argument to be the free space in the destination buffer minus the \
terminating null byte}} }
Index: clang/test/Sema/warn-fortify-source.c
===================================================================
--- /dev/null
+++ clang/test/Sema/warn-fortify-source.c
@@ -0,0 +1,83 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 %s -verify
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 %s -verify \
-DUSE_PASS_OBJECT_SIZE +// RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 %s \
-verify -DUSE_BUILTINS +
+typedef unsigned long size_t;
+
+#if defined(USE_PASS_OBJECT_SIZE)
+void *memcpy(void *dst, const void *src, size_t c);
+static void *memcpy(void *dst __attribute__((pass_object_size(1))), const void *src, \
size_t c) __attribute__((overloadable)) __asm__("merp"); +static void *memcpy(void \
*const dst __attribute__((pass_object_size(1))), const void *src, size_t c) \
__attribute__((overloadable)) { + return 0;
+}
+#elif defined(USE_BUILTINS)
+#define memcpy(x,y,z) __builtin_memcpy(x,y,z)
+#else
+void *memcpy(void *dst, const void *src, size_t c);
+#endif
+
+void call_memcpy() {
+ char dst[10];
+ char src[20];
+ memcpy(dst, src, 20); // expected-warning {{memcpy' will always overflow; \
destination buffer has size 10, but size argument is 20}} +}
+
+void call_memcpy_type() {
+ struct pair {
+ int first;
+ int second;
+ };
+ struct pair p;
+ char buf[20];
+ memcpy(&p.first, buf, 20);
+#ifdef USE_PASS_OBJECT_SIZE
+ // Use the more strict checking mode on the pass_object_size attribute:
+ // expected-warning@-3 {{memcpy' will always overflow; destination buffer has size \
4, but size argument is 20}} +#else
+ // Or just fallback to type 0:
+ // expected-warning@-6 {{memcpy' will always overflow; destination buffer has size \
8, but size argument is 20}} +#endif
+}
+
+void call_strncat() {
+ char s1[10], s2[20];
+ __builtin_strncat(s2, s1, 20);
+ __builtin_strncat(s1, s2, 20); // expected-warning {{'__builtin_strncat' size \
argument is too large; destination buffer has size 10, but size argument is 20}} +}
+
+void call_strncpy() {
+ char s1[10], s2[20];
+ __builtin_strncpy(s2, s1, 20);
+ __builtin_strncpy(s1, s2, 20); // expected-warning {{'__builtin_strncpy' size \
argument is too large; destination buffer has size 10, but size argument is 20}} +}
+
+void call_stpncpy() {
+ char s1[10], s2[20];
+ __builtin_stpncpy(s2, s1, 20);
+ __builtin_stpncpy(s1, s2, 20); // expected-warning {{'__builtin_stpncpy' size \
argument is too large; destination buffer has size 10, but size argument is 20}} +}
+
+void call_memmove() {
+ char s1[10], s2[20];
+ __builtin_memmove(s2, s1, 20);
+ __builtin_memmove(s1, s2, 20); // expected-warning {{'__builtin_memmove' will \
always overflow; destination buffer has size 10, but size argument is 20}} +}
+
+void call_memset() {
+ char buf[10];
+ __builtin_memset(buf, 0xff, 10);
+ __builtin_memset(buf, 0xff, 11); // expected-warning {{'__builtin_memset' will \
always overflow; destination buffer has size 10, but size argument is 11}} +}
+
+void call_snprintf() {
+ char buf[10];
+ __builtin_snprintf(buf, 10, "merp");
+ __builtin_snprintf(buf, 11, "merp"); // expected-warning {{'__builtin_snprintf' \
size argument is too large; destination buffer has size 10, but size argument is 11}} \
+} +
+void call_vsnprintf() {
+ char buf[10];
+ __builtin_va_list list;
+ __builtin_vsnprintf(buf, 10, "merp", list);
+ __builtin_vsnprintf(buf, 11, "merp", list); // expected-warning \
{{'__builtin_vsnprintf' size argument is too large; destination buffer has size 10, \
but size argument is 11}} +}
Index: clang/test/Sema/transpose-memset.c
===================================================================
--- clang/test/Sema/transpose-memset.c
+++ clang/test/Sema/transpose-memset.c
@@ -10,7 +10,7 @@
int main() {
memset(array, sizeof(array), 0); // expected-warning{{'size' argument to memset is \
'0'; did you mean to transpose the last two arguments?}} expected-note{{parenthesize \
the third argument to silence}}
- memset(array, sizeof(array), 0xff); // expected-warning{{setting buffer to a \
'sizeof' expression; did you mean to transpose the last two arguments?}} \
expected-note{{cast the second argument to 'int' to silence}} + memset(array, \
sizeof(array), 0xff); // expected-warning{{setting buffer to a 'sizeof' expression; \
did you mean to transpose the last two arguments?}} expected-note{{cast the second \
argument to 'int' to silence}} expected-warning{{'__builtin_memset' will always \
overflow; destination buffer has size 40, but size argument is 255}} memset(ptr, \
sizeof(ptr), 0); // expected-warning{{'size' argument to memset is '0'; did you mean \
to transpose the last two arguments?}} expected-note{{parenthesize the third argument \
to silence}} memset(ptr, sizeof(*ptr) * 10, 1); // expected-warning{{setting buffer \
to a 'sizeof' expression; did you mean to transpose the last two arguments?}} \
expected-note{{cast the second argument to 'int' to silence}} memset(ptr, 10 * \
sizeof(int *), 1); // expected-warning{{setting buffer to a 'sizeof' expression; did \
you mean to transpose the last two arguments?}} expected-note{{cast the second \
argument to 'int' to silence}}
Index: clang/test/Sema/builtins.c
===================================================================
--- clang/test/Sema/builtins.c
+++ clang/test/Sema/builtins.c
@@ -230,14 +230,14 @@
// 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 has size 20, but size argument is 40}} + // \
expected-warning {{'strlcpy' 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 has size 20, but \
size argument is 40}} + // \
expected-warning {{'strlcat' will always overflow; destination buffer has size 20, \
but size argument is 40}} }
// rdar://11076881
@@ -245,7 +245,7 @@
{
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 \
has size 4, but size argument is 5}} + __builtin___memcpy_chk (&buf[6], in, 5, \
__builtin_object_size (&buf[6], 0)); // expected-warning {{'memcpy' will always \
overflow; destination buffer has size 4, but size argument is 5}}
__builtin___memcpy_chk (p, "abcde", n, __builtin_object_size (p, 0));
@@ -253,7 +253,7 @@
__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 has size 4, but size argument is 5}} + __builtin___memcpy_chk (&buf[6], \
"abcde", 5, __builtin_object_size (&buf[6], 0)); // expected-warning {{'memcpy' will \
always overflow; destination buffer has size 4, but size argument is 5}}
return buf;
}
@@ -312,5 +312,5 @@
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}} + my_memcpy(buf, \
src, 11); // expected-warning{{'memcpy' 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
@@ -30,7 +30,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 has size 11, \
but size argument is 42}} + __builtin___vsnprintf_chk (0, 42, 0, 11, fmt, args); // \
expected-warning {{'vsnprintf' will always overflow; destination buffer has size 11, \
but size argument is 42}} }
// rdar://18334276
@@ -57,7 +57,7 @@
char b[5];
char buf[10];
__builtin___memccpy_chk (buf, b, '\0', sizeof(b), OBJECT_SIZE_BUILTIN (buf, 0));
- __builtin___memccpy_chk (b, buf, '\0', sizeof(buf), OBJECT_SIZE_BUILTIN (b, 0)); \
// expected-warning {{'__builtin___memccpy_chk' will always overflow; destination \
buffer has size 5, but size argument is 10}} + __builtin___memccpy_chk (b, buf, \
'\0', sizeof(buf), OBJECT_SIZE_BUILTIN (b, 0)); // expected-warning {{'memccpy' will \
always overflow; destination buffer has size 5, but size argument is 10}} }
int pr28314(void) {
Index: clang/test/Analysis/string.c
===================================================================
--- clang/test/Analysis/string.c
+++ clang/test/Analysis/string.c
@@ -517,12 +517,18 @@
char x[4];
if (strlen(y) == 4)
strncpy(x, y, 5); // expected-warning{{Size argument is greater than the length \
of the destination buffer}} +#ifndef VARIANT
+ // expected-warning@-2{{size argument is too large; destination buffer has size 4, \
but size argument is 5}} +#endif
}
void strncpy_no_overflow(char *y) {
char x[4];
if (strlen(y) == 3)
strncpy(x, y, 5); // expected-warning{{Size argument is greater than the length \
of the destination buffer}} +#ifndef VARIANT
+ // expected-warning@-2{{size argument is too large; destination buffer has size 4, \
but size argument is 5}} +#endif
}
void strncpy_no_overflow2(char *y, int n) {
@@ -1247,7 +1253,7 @@
void memset8_char_array_nonnull() {
char str[5] = "abcd";
clang_analyzer_eval(strlen(str) == 4); // expected-warning{{TRUE}}
- memset(str, '0', 10);
+ memset(str, '0', 10); // expected-warning{{'memset' will always overflow; \
destination buffer has size 5, but size argument is 10}} clang_analyzer_eval(str[0] \
!= '0'); // expected-warning{{UNKNOWN}} clang_analyzer_eval(strlen(str) >= 10); \
// expected-warning{{TRUE}} clang_analyzer_eval(strlen(str) < 10); // \
expected-warning{{FALSE}} @@ -1284,7 +1290,7 @@
struct POD_memset pod;
pod.num = 1;
pod.c = '1';
- memset(&pod.c, 0, sizeof(struct POD_memset));
+ memset(&pod.c, 0, sizeof(struct POD_memset)); // expected-warning {{'memset' will \
always overflow; destination buffer has size 4, but size argument is 8}} \
clang_analyzer_eval(pod.num == 0); // expected-warning{{UNKNOWN}} \
clang_analyzer_eval(pod.c == 0); // expected-warning{{UNKNOWN}} }
Index: clang/test/Analysis/pr22954.c
===================================================================
--- clang/test/Analysis/pr22954.c
+++ clang/test/Analysis/pr22954.c
@@ -303,7 +303,7 @@
i18.j = 11;
i18.s2 = strdup("hello");
char input[100] = {3};
- memcpy(i18.s1, input, 100);
+ memcpy(i18.s1, input, 100); // expected-warning {{'memcpy' will always overflow; \
destination buffer has size 24, but size argument is 100}} \
clang_analyzer_eval(i18.s1[0] == 1); // expected-warning{{UNKNOWN}}\ \
expected-warning{{Potential leak of memory pointed to by 'i18.s2'}} \
clang_analyzer_eval(i18.s1[1] == 2); // expected-warning{{UNKNOWN}} @@ -534,7 +534,7 \
@@ struct aa a262 = {{1, 2, 3, 4}, 0};
a262.s2 = strdup("hello");
char input[] = {'a', 'b', 'c', 'd'};
- memcpy(a262.s1, input, -1);
+ memcpy(a262.s1, input, -1); // expected-warning{{'memcpy' will always overflow; \
destination buffer has size 16, but size argument is 18446744073709551615}} \
clang_analyzer_eval(a262.s1[0] == 1); // expected-warning{{UNKNOWN}}\ \
expected-warning{{Potential leak of memory pointed to by 'a262.s2'}} \
clang_analyzer_eval(a262.s1[1] == 1); // expected-warning{{UNKNOWN}}
Index: clang/test/Analysis/null-deref-ps-region.c
===================================================================
--- clang/test/Analysis/null-deref-ps-region.c
+++ clang/test/Analysis/null-deref-ps-region.c
@@ -51,7 +51,7 @@
void testStackArrayOutOfBound() {
char buf[1];
- memset(buf, 0, 1024); // expected-warning {{Memory set function accesses \
out-of-bound array element}} + memset(buf, 0, 1024); // expected-warning {{Memory \
set function accesses out-of-bound array element}} expected-warning {{'memset' will \
always overflow; destination buffer has size 1, but size argument is 1024}} }
void testHeapSymbolOutOfBound() {
Index: clang/test/Analysis/bstring.c
===================================================================
--- clang/test/Analysis/bstring.c
+++ clang/test/Analysis/bstring.c
@@ -72,7 +72,10 @@
char src[] = {1, 2, 3, 4};
char dst[1];
- memcpy(dst, src, 4); // expected-warning{{Memory copy function overflows \
destination buffer}} + memcpy(dst, src, 4); // expected-warning{{Memory copy \
function overflows destination buffer}} +#ifndef VARIANT
+ // expected-warning@-2{{memcpy' will always overflow; destination buffer has size \
1, but size argument is 4}} +#endif
}
void memcpy3 () {
@@ -94,6 +97,9 @@
char dst[3];
memcpy(dst+2, src+2, 2); // expected-warning{{Memory copy function overflows \
destination buffer}} +#ifndef VARIANT
+ // expected-warning@-2{{memcpy' will always overflow; destination buffer has size \
1, but size argument is 2}} +#endif
}
void memcpy6() {
@@ -351,7 +357,10 @@
char src[] = {1, 2, 3, 4};
char dst[1];
- memmove(dst, src, 4); // expected-warning{{overflow}}
+ memmove(dst, src, 4); // expected-warning{{Memory copy function overflows \
destination buffer}} +#ifndef VARIANT
+ // expected-warning@-2{{memmove' will always overflow; destination buffer has size \
1, but size argument is 4}} +#endif
}
//===----------------------------------------------------------------------===
Index: clang/lib/Sema/SemaExpr.cpp
===================================================================
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -5926,6 +5926,8 @@
if (CheckFunctionCall(FDecl, TheCall, Proto))
return ExprError();
+ checkFortifiedBuiltinMemoryFunction(FDecl, TheCall);
+
if (BuiltinID)
return CheckBuiltinFunctionCall(FDecl, BuiltinID, TheCall);
} else if (NDecl) {
Index: clang/lib/Sema/SemaChecking.cpp
===================================================================
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -235,47 +235,6 @@
return false;
}
-static void SemaBuiltinMemChkCall(Sema &S, FunctionDecl *FDecl,
- CallExpr *TheCall, unsigned SizeIdx,
- unsigned DstSizeIdx,
- StringRef LikelyMacroName) {
- if (TheCall->getNumArgs() <= SizeIdx ||
- TheCall->getNumArgs() <= DstSizeIdx)
- return;
-
- const Expr *SizeArg = TheCall->getArg(SizeIdx);
- const Expr *DstSizeArg = TheCall->getArg(DstSizeIdx);
-
- Expr::EvalResult SizeResult, DstSizeResult;
-
- // find out if both sizes are known at compile time
- if (!SizeArg->EvaluateAsInt(SizeResult, S.Context) ||
- !DstSizeArg->EvaluateAsInt(DstSizeResult, S.Context))
- return;
-
- llvm::APSInt Size = SizeResult.Val.getInt();
- llvm::APSInt DstSize = DstSizeResult.Val.getInt();
-
- if (Size.ule(DstSize))
- return;
-
- // Confirmed overflow, so generate the diagnostic.
- StringRef FunctionName = FDecl->getName();
- SourceLocation SL = TheCall->getBeginLoc();
- 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)
- << FunctionName << DstSize.toString(/*Radix=*/10)
- << Size.toString(/*Radix=*/10);
-}
-
static bool SemaBuiltinCallWithStaticChain(Sema &S, CallExpr *BuiltinCall) {
if (checkArgCount(S, BuiltinCall, 2))
return true;
@@ -339,6 +298,139 @@
return false;
}
+/// Check a call to BuiltinID for buffer overflows. If BuiltinID is a
+/// __builtin_*_chk function, then use the object size argument specified in the
+/// source. Otherwise, infer the object size using __builtin_object_size.
+void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD,
+ CallExpr *TheCall) {
+ // FIXME: There are some more useful checks we could be doing here:
+ // - Analyze the format string of sprintf to see how much of buffer is used.
+ // - Evaluate strlen of strcpy arguments, use as object size.
+
+ unsigned BuiltinID = FD->getBuiltinID(/*ConsiderWrappers=*/true);
+ if (!BuiltinID)
+ return;
+ llvm::APSInt UsedSize, ComputedSize;
+
+ // If this is a call to a non-fortified function, then determine what the
+ // buffer size would be.
+ auto computeObjectSize = [&](unsigned ObjIdx) {
+ // If the parameter has a pass_object_size attribute, then we should use
+ // it's (potentially) more strict checking mode. Otherwise, conservatively
+ // assume type 0.
+ int BOSType = 0;
+ if (auto *POS = FD->getParamDecl(ObjIdx)->getAttr<PassObjectSizeAttr>())
+ BOSType = POS->getType();
+
+ Expr *ObjArg = TheCall->getArg(ObjIdx);
+ uint64_t Result;
+ if (!ObjArg->tryEvaluateObjectSize(Result, getASTContext(), BOSType))
+ return false;
+ // Get the object size in the target's size_t width.
+ const TargetInfo &TI = getASTContext().getTargetInfo();
+ unsigned SizeTypeWidth = TI.getTypeWidth(TI.getSizeType());
+ ComputedSize = llvm::APSInt::getUnsigned(Result).extOrTrunc(SizeTypeWidth);
+ return true;
+ };
+
+ auto evaluateObjectSize = [&](unsigned Idx) {
+ Expr *SizeArg = TheCall->getArg(Idx);
+ Expr::EvalResult Result;
+ if (!SizeArg->EvaluateAsInt(Result, getASTContext()))
+ return false;
+ ComputedSize = Result.Val.getInt();
+ return true;
+ };
+
+ auto evaluateSizeArg = [&](unsigned Idx) {
+ Expr *SizeArg = TheCall->getArg(Idx);
+ Expr::EvalResult Result;
+ if (!SizeArg->EvaluateAsInt(Result, getASTContext()))
+ return false;
+ UsedSize = Result.Val.getInt();
+ return true;
+ };
+
+ unsigned DiagID = 0;
+ switch (BuiltinID) {
+ default: return;
+ case Builtin::BI__builtin___memcpy_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:
+ case Builtin::BI__builtin___memccpy_chk: {
+ DiagID = diag::warn_memcpy_chk_overflow;
+ if (!evaluateObjectSize(TheCall->getNumArgs()-1) ||
+ !evaluateSizeArg(TheCall->getNumArgs()-2))
+ return;
+ break;
+ }
+ case Builtin::BI__builtin___snprintf_chk:
+ case Builtin::BI__builtin___vsnprintf_chk: {
+ DiagID = diag::warn_memcpy_chk_overflow;
+ if (!evaluateObjectSize(3) || !evaluateSizeArg(1))
+ return;
+ break;
+ }
+
+ case Builtin::BIstrncat:
+ case Builtin::BI__builtin_strncat:
+ case Builtin::BIstrncpy:
+ case Builtin::BI__builtin_strncpy:
+ case Builtin::BIstpncpy:
+ case Builtin::BI__builtin_stpncpy: {
+ if (!computeObjectSize(0) || !evaluateSizeArg(TheCall->getNumArgs() - 1))
+ return;
+ // Whether these functions overflow depend on the runtime strlen of the
+ // string, not just the buffer size, so emitting the "always overflow"
+ // diagnostic isn't quite right. We should still diagnose passing a buffer
+ // size larger than the destination buffer though; this is a runtime abort
+ // in _FORTIFY_SOURCE mode, and is quite suspicious otherwise.
+ DiagID = diag::warn_fortify_source_size_mismatch;
+ break;
+ }
+
+ case Builtin::BImemcpy:
+ case Builtin::BI__builtin_memcpy:
+ case Builtin::BImemmove:
+ case Builtin::BI__builtin_memmove:
+ case Builtin::BImemset:
+ case Builtin::BI__builtin_memset: {
+ if (!computeObjectSize(0) || !evaluateSizeArg(TheCall->getNumArgs() - 1))
+ return;
+ DiagID = diag::warn_fortify_source_overflow;
+ break;
+ }
+ case Builtin::BIsnprintf:
+ case Builtin::BI__builtin_snprintf:
+ case Builtin::BIvsnprintf:
+ case Builtin::BI__builtin_vsnprintf: {
+ if (!computeObjectSize(0) || !evaluateSizeArg(1))
+ return;
+ DiagID = diag::warn_fortify_source_size_mismatch;
+ break;
+ }
+ }
+
+ if (UsedSize.ule(ComputedSize))
+ return;
+
+ StringRef FunctionName = getASTContext().BuiltinInfo.getName(BuiltinID);
+ if (DiagID == diag::warn_memcpy_chk_overflow) {
+ // __builtin___memcpy_chk -> memcpy
+ FunctionName = FunctionName.drop_front(std::strlen("__builtin___"));
+ FunctionName = FunctionName.drop_back(std::strlen("_chk"));
+ }
+
+ Diag(TheCall->getBeginLoc(), DiagID)
+ << FunctionName << ComputedSize.toString(/*Radix=*/10)
+ << UsedSize.toString(/*Radix=*/10);
+}
+
static bool SemaBuiltinSEHScopeCheck(Sema &SemaRef, CallExpr *TheCall,
Scope::ScopeFlags NeededScopeFlags,
unsigned DiagID) {
@@ -1302,42 +1394,6 @@
TheCall->setType(Context.IntTy);
break;
}
-
- // 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, "stpncpy");
- break;
- case Builtin::BI__builtin___memccpy_chk:
- 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, "vsnprintf");
- break;
case Builtin::BI__builtin_call_with_static_chain:
if (SemaBuiltinCallWithStaticChain(*this, TheCall))
return ExprError();
Index: clang/lib/AST/Decl.cpp
===================================================================
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -2981,16 +2981,20 @@
FunctionDecl *FunctionDecl::getCanonicalDecl() { return getFirstDecl(); }
-/// Returns a value indicating whether this function
-/// corresponds to a builtin function.
+/// Returns a value indicating whether this function corresponds to a builtin
+/// function.
///
-/// The function corresponds to a built-in function if it is
-/// declared at translation scope or within an extern "C" block and
-/// its name matches with the name of a builtin. The returned value
-/// will be 0 for functions that do not correspond to a builtin, a
-/// value of type \c Builtin::ID if in the target-independent range
-/// \c [1,Builtin::First), or a target-specific builtin value.
-unsigned FunctionDecl::getBuiltinID() const {
+/// The function corresponds to a built-in function if it is declared at
+/// translation scope or within an extern "C" block and its name matches with
+/// the name of a builtin. The returned value will be 0 for functions that do
+/// not correspond to a builtin, a value of type \c Builtin::ID if in the
+/// target-independent range \c [1,Builtin::First), or a target-specific builtin
+/// value.
+///
+/// \param ConsiderWrapperFunctions If we should consider wrapper functions as
+/// their wrapped builtins. This shouldn't be done in general, but its useful in
+/// Sema to diagnose calls to wrappers based on their semantics.
+unsigned FunctionDecl::getBuiltinID(bool ConsiderWrapperFunctions) const {
if (!getIdentifier())
return 0;
@@ -3018,7 +3022,7 @@
// If the function is marked "overloadable", it has a different mangled name
// and is not the C library function.
- if (hasAttr<OverloadableAttr>())
+ if (!ConsiderWrapperFunctions && hasAttr<OverloadableAttr>())
return 0;
if (!Context.BuiltinInfo.isPredefinedLibFunction(BuiltinID))
@@ -3029,7 +3033,7 @@
// function or whether it just has the same name.
// If this is a static function, it's not a builtin.
- if (getStorageClass() == SC_Static)
+ if (!ConsiderWrapperFunctions && getStorageClass() == SC_Static)
return 0;
// OpenCL v1.2 s6.9.f - The library functions defined in
Index: clang/include/clang/Sema/Sema.h
===================================================================
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -10665,6 +10665,7 @@
ExprResult CheckBuiltinFunctionCall(FunctionDecl *FDecl,
unsigned BuiltinID, CallExpr *TheCall);
+ void checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, CallExpr *TheCall);
bool CheckARMBuiltinExclusiveCall(unsigned BuiltinID, CallExpr *TheCall,
unsigned MaxWidth);
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -677,6 +677,12 @@
" but size argument is %2">,
InGroup<DiagGroup<"builtin-memcpy-chk-size">>;
+def warn_fortify_source_overflow
+ : Warning<warn_memcpy_chk_overflow.Text>, InGroup<FortifySource>;
+def warn_fortify_source_size_mismatch : Warning<
+ "'%0' size argument is too large; destination buffer has size %1,"
+ " but size argument is %2">, InGroup<FortifySource>;
+
/// main()
// static main() is not an error in C, just in C++.
def warn_static_main : Warning<"'main' should not be declared static">,
Index: clang/include/clang/Basic/DiagnosticGroups.td
===================================================================
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -1052,3 +1052,5 @@
def CrossTU : DiagGroup<"ctu">;
def CTADMaybeUnsupported : DiagGroup<"ctad-maybe-unsupported">;
+
+def FortifySource : DiagGroup<"fortify-source">;
Index: clang/include/clang/AST/Decl.h
===================================================================
--- clang/include/clang/AST/Decl.h
+++ clang/include/clang/AST/Decl.h
@@ -2255,7 +2255,7 @@
return const_cast<FunctionDecl*>(this)->getCanonicalDecl();
}
- unsigned getBuiltinID() const;
+ unsigned getBuiltinID(bool ConsiderWrapperFunctions = false) const;
// ArrayRef interface to parameters.
ArrayRef<ParmVarDecl *> parameters() const {
[Attachment #4 (text/plain)]
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic