Hi, On Tue, May 29, 2012 at 5:34 PM, Anna Zaks <[email protected]> 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 > [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
