r218063 - Patch to check at compile time for overflow when
Fariborz Jahanian
fjahanian at apple.com
Thu Sep 18 10:58:27 PDT 2014
Author: fjahanian
Date: Thu Sep 18 12:58:27 2014
New Revision: 218063
URL: http://llvm.org/viewvc/llvm-project?rev=218063&view=rev
Log:
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
Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Sema/SemaChecking.cpp
cfe/trunk/lib/Sema/SemaExpr.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=218063&r1=218062&r2=218063&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu Sep 18 12:58:27 2014
@@ -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">,
Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=218063&r1=218062&r2=218063&view=diff
==============================================================================
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Thu Sep 18 12:58:27 2014
@@ -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);
Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=218063&r1=218062&r2=218063&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Thu Sep 18 12:58:27 2014
@@ -111,8 +111,37 @@ static bool SemaBuiltinAddressof(Sema &S
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
// 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
Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=218063&r1=218062&r2=218063&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Thu Sep 18 12:58:27 2014
@@ -4665,7 +4665,7 @@ Sema::BuildResolvedCallExpr(Expr *Fn, Na
// 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, Na
return ExprError();
if (BuiltinID)
- return CheckBuiltinFunctionCall(BuiltinID, TheCall);
+ return CheckBuiltinFunctionCall(FDecl, BuiltinID, TheCall);
} else if (NDecl) {
if (CheckPointerCall(NDecl, TheCall, Proto))
return ExprError();
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=218063&r1=218062&r2=218063&view=diff
==============================================================================
--- cfe/trunk/test/Sema/builtin-object-size.c (original)
+++ cfe/trunk/test/Sema/builtin-object-size.c Thu Sep 18 12:58:27 2014
@@ -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}}
}
Modified: cfe/trunk/test/Sema/builtins.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/builtins.c?rev=218063&r1=218062&r2=218063&view=diff
==============================================================================
--- cfe/trunk/test/Sema/builtins.c (original)
+++ cfe/trunk/test/Sema/builtins.c Thu Sep 18 12:58:27 2014
@@ -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;
}
More information about the cfe-commits
mailing list