[cfe-commits] r130299 - in /cfe/trunk: include/clang/Basic/DiagnosticSemaKinds.td include/clang/Sema/Sema.h lib/Sema/SemaChecking.cpp test/SemaCXX/warn-non-pod-memset.cpp
Nico Weber
thakis at chromium.org
Fri Apr 29 08:07:02 PDT 2011
On Wed, Apr 27, 2011 at 12:05 AM, Chandler Carruth <chandlerc at gmail.com> wrote:
> Author: chandlerc
> Date: Wed Apr 27 02:05:31 2011
> New Revision: 130299
>
> URL: http://llvm.org/viewvc/llvm-project?rev=130299&view=rev
> Log:
> Add a warning (-Wnon-pod-memset) for calls to memset() with
> a destination pointer that points to a non-POD type. This can flag such
> horrible bugs as overwriting vptrs when a previously POD structure is
> suddenly given a virtual method, or creating objects that crash on
> practically any use by zero-ing out a member when its changed from
> a const char* to a std::string, etc.
Hi Chandler,
is this ever useful if the pointer doesn't point to something with
virtual methods?
As is, I'm not a huge fan of this warning, because
a) it produces nothing but noise in my project
b) the warning text is not very informative (a bit like "warning: 5 is
an integer" – the statement is true, but why should this warn)
This warns about 5 instances in chrome. 3/5 of them are of the form:
struct A {
A() { memset(this, 0, sizeof(*this)); }
int ma1;
int ma2;
};
1/5 is:
struct A {
int ma;
};
struct B : public A {
int mb;
};
// somewhere else:
B* b = new B;
memset(b, 0, sizeof(B));
1/5 is:
// RefPtr is a smart pointer class with a private T* and the usual
smart pointer stuff,
// this code is from a std::vector<RefPtr> type of class:
static void initialize(RefPtr* begin, RefPtr* end)
{
memset(begin, 0, reinterpret_cast<char*>(end) -
reinterpret_cast<char*>(begin));
}
Nico
>
> Added:
> cfe/trunk/test/SemaCXX/warn-non-pod-memset.cpp
> Modified:
> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> cfe/trunk/include/clang/Sema/Sema.h
> cfe/trunk/lib/Sema/SemaChecking.cpp
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=130299&r1=130298&r2=130299&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed Apr 27 02:05:31 2011
> @@ -261,6 +261,11 @@
> def err_types_compatible_p_in_cplusplus : Error<
> "__builtin_types_compatible_p is not valid in C++">;
> def warn_builtin_unknown : Warning<"use of unknown builtin %0">, DefaultError;
> +def warn_non_pod_memset : Warning<
> + "destination for this memset call is a pointer to a non-POD type %0">,
> + InGroup<DiagGroup<"non-pod-memset">>;
> +def note_non_pod_memset_silence : Note<
> + "explicitly cast the pointer to silence this warning">;
>
> /// main()
> // static/inline main() are not errors in C, just in C++.
>
> Modified: cfe/trunk/include/clang/Sema/Sema.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=130299&r1=130298&r2=130299&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Sema/Sema.h (original)
> +++ cfe/trunk/include/clang/Sema/Sema.h Wed Apr 27 02:05:31 2011
> @@ -5516,6 +5516,8 @@
> unsigned format_idx, unsigned firstDataArg,
> bool isPrintf);
>
> + void CheckMemsetArguments(const CallExpr *Call);
> +
> void CheckReturnStackAddr(Expr *RetValExp, QualType lhsType,
> SourceLocation ReturnLoc);
> void CheckFloatComparison(SourceLocation loc, Expr* lex, Expr* rex);
>
> Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=130299&r1=130298&r2=130299&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaChecking.cpp Wed Apr 27 02:05:31 2011
> @@ -318,6 +318,10 @@
> TheCall->getCallee()->getLocStart());
> }
>
> + // Memset handling
> + if (FnInfo->isStr("memset"))
> + CheckMemsetArguments(TheCall);
> +
> return false;
> }
>
> @@ -1791,6 +1795,36 @@
> }
> }
>
> +//===--- CHECK: Standard memory functions ---------------------------------===//
> +
> +/// \brief Check for dangerous or invalid arguments to memset().
> +///
> +/// This issues warnings on known problematic or dangerous or unspecified
> +/// arguments to the standard 'memset' function call.
> +///
> +/// \param Call The call expression to diagnose.
> +void Sema::CheckMemsetArguments(const CallExpr *Call) {
> + assert(Call->getNumArgs() == 3 && "Unexpected number of arguments to memset");
> + const Expr *Dest = Call->getArg(0)->IgnoreParenImpCasts();
> +
> + QualType DestTy = Dest->getType();
> + if (const PointerType *DestPtrTy = DestTy->getAs<PointerType>()) {
> + QualType PointeeTy = DestPtrTy->getPointeeType();
> + if (!PointeeTy->isPODType()) {
> + DiagRuntimeBehavior(
> + Dest->getExprLoc(), Dest,
> + PDiag(diag::warn_non_pod_memset)
> + << PointeeTy << Call->getCallee()->getSourceRange());
> +
> + SourceRange ArgRange = Call->getArg(0)->getSourceRange();
> + DiagRuntimeBehavior(
> + Dest->getExprLoc(), Dest,
> + PDiag(diag::note_non_pod_memset_silence)
> + << FixItHint::CreateInsertion(ArgRange.getBegin(), "(void*)"));
> + }
> + }
> +}
> +
> //===--- CHECK: Return Address of Stack Variable --------------------------===//
>
> static Expr *EvalVal(Expr *E, llvm::SmallVectorImpl<DeclRefExpr *> &refVars);
>
> Added: cfe/trunk/test/SemaCXX/warn-non-pod-memset.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-non-pod-memset.cpp?rev=130299&view=auto
> ==============================================================================
> --- cfe/trunk/test/SemaCXX/warn-non-pod-memset.cpp (added)
> +++ cfe/trunk/test/SemaCXX/warn-non-pod-memset.cpp Wed Apr 27 02:05:31 2011
> @@ -0,0 +1,45 @@
> +// RUN: %clang_cc1 -fsyntax-only -verify %s
> +
> +extern void *memset(void *, int, unsigned);
> +
> +// Several POD types that should not warn.
> +struct S1 {} s1;
> +struct S2 { int x; } s2;
> +struct S3 { float x, y; S1 s[4]; void (*f)(S1**); } s3;
> +
> +// Non-POD types that should warn.
> +struct X1 { X1(); } x1;
> +struct X2 { ~X2(); } x2;
> +struct X3 { virtual void f(); } x3;
> +struct X4 : X2 {} x4;
> +struct X5 : virtual S1 {} x5;
> +
> +void test_warn() {
> + memset(&x1, 0, sizeof x1); // \
> + // expected-warning {{destination for this memset call is a pointer to a non-POD type}} \
> + // expected-note {{explicitly cast the pointer to silence this warning}}
> + memset(&x2, 0, sizeof x2); // \
> + // expected-warning {{destination for this memset call is a pointer to a non-POD type}} \
> + // expected-note {{explicitly cast the pointer to silence this warning}}
> + memset(&x3, 0, sizeof x3); // \
> + // expected-warning {{destination for this memset call is a pointer to a non-POD type}} \
> + // expected-note {{explicitly cast the pointer to silence this warning}}
> + memset(&x4, 0, sizeof x4); // \
> + // expected-warning {{destination for this memset call is a pointer to a non-POD type}} \
> + // expected-note {{explicitly cast the pointer to silence this warning}}
> + memset(&x5, 0, sizeof x5); // \
> + // expected-warning {{destination for this memset call is a pointer to a non-POD type}} \
> + // expected-note {{explicitly cast the pointer to silence this warning}}
> +}
> +
> +void test_nowarn() {
> + memset(&s1, 0, sizeof s1);
> + memset(&s2, 0, sizeof s2);
> + memset(&s3, 0, sizeof s3);
> +
> + // Unevaluated code shouldn't warn.
> + (void)sizeof memset(&x1, 0, sizeof x1);
> +
> + // Dead code shouldn't warn.
> + if (false) memset(&x1, 0, sizeof x1);
> +}
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
More information about the cfe-commits
mailing list