r218063 - Patch to check at compile time for overflow when
Reid Kleckner
rnk at google.com
Thu Sep 18 11:33:40 PDT 2014
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 <fjahanian at apple.com>
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
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140918/2fd3571d/attachment.html>
More information about the cfe-commits
mailing list