[cfe-commits] r157659 - in /cfe/trunk: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaChecking.cpp test/SemaCXX/warn-memset-bad-sizeof-fixit.cpp
Richard Smith
richard at metafoo.co.uk
Tue May 29 18:10:17 PDT 2012
Hi,
On Tue, May 29, 2012 at 5:34 PM, Anna Zaks <ganna at apple.com> wrote:
> Author: zaks
> Date: Tue May 29 19:34:21 2012
> New Revision: 157659
>
> URL: http://llvm.org/viewvc/llvm-project?rev=157659&view=rev
> Log:
> Add fixits for memory access warnings.
>
We have a rule that fix-its on warnings aren't supposed to change the
code's semantics (-Werror -fixit shouldn't change the semantics of a valid
program). Consequently, fix-its such as this are usually placed on an
accompanying note.
> Also, do not display the builtin name and macro expansion when the
> function is a builtin.
>
> Added:
> cfe/trunk/test/SemaCXX/warn-memset-bad-sizeof-fixit.cpp
> Modified:
> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> 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=157659&r1=157658&r2=157659&view=diff
>
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue May 29
> 19:34:21 2012
> @@ -331,7 +331,7 @@
> def note_bad_memaccess_silence : Note<
> "explicitly cast the pointer to silence this warning">;
> def warn_sizeof_pointer_expr_memaccess : Warning<
> - "argument to 'sizeof' in %0 call is the same expression as the "
> + "argument to 'sizeof' in '%0' call is the same expression as the "
> "%select{destination|source}1; did you mean to "
> "%select{dereference it|remove the addressof|provide an explicit
> length}2?">,
> InGroup<DiagGroup<"sizeof-pointer-memaccess">>;
>
> Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=157659&r1=157658&r2=157659&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaChecking.cpp Tue May 29 19:34:21 2012
> @@ -2755,19 +2755,49 @@
> // TODO: For strncpy() and friends, this could suggest
> sizeof(dst)
> // over sizeof(src) as well.
> unsigned ActionIdx = 0; // Default is to suggest dereferencing.
> + FixItHint Fixit = FixItHint(); // Default hint.
> + StringRef ReadableName = FnName->getName();
> +
> + if (isa<DeclRefExpr>(SizeOfArg))
> + Fixit = FixItHint::CreateInsertion(SizeOfArg->getLocStart(),
> "*");
> +
> if (const UnaryOperator *UnaryOp = dyn_cast<UnaryOperator>(Dest))
> - if (UnaryOp->getOpcode() == UO_AddrOf)
> + if (UnaryOp->getOpcode() == UO_AddrOf) {
> + Fixit = FixItHint::CreateRemoval(
> +
> CharSourceRange::getTokenRange(SizeOfArg->getLocStart(),
> +
> SizeOfArg->getLocStart()));
> ActionIdx = 1; // If its an address-of operator, just remove
> it.
> + }
> if (Context.getTypeSize(PointeeTy) == Context.getCharWidth())
> ActionIdx = 2; // If the pointee's size is sizeof(char),
> // suggest an explicit length.
> unsigned DestSrcSelect =
> (BId == Builtin::BIstrndup ? 1 : ArgIdx);
> - DiagRuntimeBehavior(SizeOfArg->getExprLoc(), Dest,
> +
> + // If the function is defined as a builtin macro, do not show
> macro
> + // expansion.
> + SourceLocation SL = SizeOfArg->getExprLoc();
> + SourceRange DSR = Dest->getSourceRange();
> + SourceRange SSR = SizeOfArg->getSourceRange();
> + SourceManager &SM = PP.getSourceManager();
> +
> + if (SM.isMacroArgExpansion(SL)) {
> + ReadableName = Lexer::getImmediateMacroName(SL, SM, LangOpts);
> + SL = SM.getSpellingLoc(SL);
> + DSR = SourceRange(SM.getSpellingLoc(DSR.getBegin()),
> + SM.getSpellingLoc(DSR.getEnd()));
> + SSR = SourceRange(SM.getSpellingLoc(SSR.getBegin()),
> + SM.getSpellingLoc(SSR.getEnd()));
> + }
> +
> + DiagRuntimeBehavior(SL, Dest,
>
> PDiag(diag::warn_sizeof_pointer_expr_memaccess)
> - << FnName << DestSrcSelect << ActionIdx
> - << Dest->getSourceRange()
> - << SizeOfArg->getSourceRange());
> + << ReadableName
> + << DestSrcSelect
> + << ActionIdx
> + << DSR
> + << SSR
> + << Fixit);
> break;
> }
> }
>
> Added: cfe/trunk/test/SemaCXX/warn-memset-bad-sizeof-fixit.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-memset-bad-sizeof-fixit.cpp?rev=157659&view=auto
>
> ==============================================================================
> --- cfe/trunk/test/SemaCXX/warn-memset-bad-sizeof-fixit.cpp (added)
> +++ cfe/trunk/test/SemaCXX/warn-memset-bad-sizeof-fixit.cpp Tue May 29
> 19:34:21 2012
> @@ -0,0 +1,24 @@
> +// RUN: cp %s %t
> +// RUN: not %clang_cc1 -fixit -Werror -x c++ -std=c++98 %t
> +// RUN: %clang_cc1 -fsyntax-only -Werror -x c++ -std=c++98 %t
> +// RUN: cp %s %t
> +// RUN: not %clang_cc1 -DUSE_BUILTINS -fixit -Werror -x c++ -std=c++98 %t
> +// RUN: %clang_cc1 -DUSE_BUILTINS -fsyntax-only -Werror -x c++ -std=c++98
> %t
> +
> +extern "C" void *memcpy(void *s1, const void *s2, unsigned n);
> +
> +#ifdef USE_BUILTINS
> +# define BUILTIN(f) __builtin_ ## f
> +#else
> +# define BUILTIN(f) f
> +#endif
> +
> +#define memcpy BUILTIN(memcpy)
> +
> +int testFixits(int *to, int *from) {
> + memcpy(to, from, sizeof(to)); // \
> + // expected-warning {{argument to 'sizeof' in 'memcpy' call is
> the same expression as the destination; did you mean to dereference it?}}
> + memcpy(0, &from, sizeof(&from)); // \
> + // expected-warning {{argument to 'sizeof' in 'memcpy' call is
> the same expression as the source; did you mean to remove the addressof?}}
> + return 0;
> +}
>
>
> _______________________________________________
> 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/20120529/3880a17e/attachment.html>
More information about the cfe-commits
mailing list