Cool! Do these warnings fire on plain memcpy if the system headers don't arrange for memcpy to route to __builtin__memcpy_chk? If so, can you add tests for plain prototyped memcpy as you did for strlcpy in the previous test?
On Thu, Sep 18, 2014 at 10:58 AM, Fariborz Jahanian <[email protected]> wrote: > 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; > } > > > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
