On Jan 13, 2012, at 1:52 PM, Anna Zaks wrote: > Author: zaks > Date: Fri Jan 13 15:52:01 2012 > New Revision: 148142 > > URL: http://llvm.org/viewvc/llvm-project?rev=148142&view=rev > Log: > Move identification of memory setting and copying functions (memset, > memcmp, strncmp,..) out of Sema and into FunctionDecl so that the logic > could be reused in the analyzer.
Hi Anna, Is there a reason not to just use Builtin ID's directly? Why have another enum? -Chris > > Modified: > cfe/trunk/include/clang/AST/Decl.h > cfe/trunk/include/clang/Sema/Sema.h > cfe/trunk/lib/AST/Decl.cpp > cfe/trunk/lib/Sema/SemaChecking.cpp > > Modified: cfe/trunk/include/clang/AST/Decl.h > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Decl.h?rev=148142&r1=148141&r2=148142&view=diff > ============================================================================== > --- cfe/trunk/include/clang/AST/Decl.h (original) > +++ cfe/trunk/include/clang/AST/Decl.h Fri Jan 13 15:52:01 2012 > @@ -1968,6 +1968,27 @@ > /// definition of a member function. > virtual bool isOutOfLine() const; > > + /// \brief Enumeration used to identify memory setting or copying functions > + /// identified by getMemoryFunctionKind(). > + enum MemoryFunctionKind { > + MFK_Memset, > + MFK_Memcpy, > + MFK_Memmove, > + MFK_Memcmp, > + MFK_Strncpy, > + MFK_Strncmp, > + MFK_Strncasecmp, > + MFK_Strncat, > + MFK_Strndup, > + MFK_Strlcpy, > + MFK_Strlcat, > + MFK_Invalid > + }; > + > + /// \brief If the given function is a memory copy or setting function, > return > + /// it's kind. If the function is not a memory function, returns > MFK_Invalid. > + MemoryFunctionKind getMemoryFunctionKind(); > + > // Implement isa/cast/dyncast/etc. > static bool classof(const Decl *D) { return classofKind(D->getKind()); } > static bool classof(const FunctionDecl *D) { return true; } > > Modified: cfe/trunk/include/clang/Sema/Sema.h > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=148142&r1=148141&r2=148142&view=diff > ============================================================================== > --- cfe/trunk/include/clang/Sema/Sema.h (original) > +++ cfe/trunk/include/clang/Sema/Sema.h Fri Jan 13 15:52:01 2012 > @@ -6311,21 +6311,8 @@ > unsigned format_idx, unsigned firstDataArg, > bool isPrintf); > > - /// \brief Enumeration used to describe which of the memory setting or > copying > - /// functions is being checked by \c CheckMemaccessArguments(). > - enum CheckedMemoryFunction { > - CMF_Memset, > - CMF_Memcpy, > - CMF_Memmove, > - CMF_Memcmp, > - CMF_Strncpy, > - CMF_Strncmp, > - CMF_Strncasecmp, > - CMF_Strncat, > - CMF_Strndup > - }; > - > - void CheckMemaccessArguments(const CallExpr *Call, CheckedMemoryFunction > CMF, > + void CheckMemaccessArguments(const CallExpr *Call, > + FunctionDecl::MemoryFunctionKind CMF, > IdentifierInfo *FnName); > > void CheckStrlcpycatArguments(const CallExpr *Call, > > Modified: cfe/trunk/lib/AST/Decl.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Decl.cpp?rev=148142&r1=148141&r2=148142&view=diff > ============================================================================== > --- cfe/trunk/lib/AST/Decl.cpp (original) > +++ cfe/trunk/lib/AST/Decl.cpp Fri Jan 13 15:52:01 2012 > @@ -2302,6 +2302,83 @@ > return SourceRange(getOuterLocStart(), EndRangeLoc); > } > > +FunctionDecl::MemoryFunctionKind FunctionDecl::getMemoryFunctionKind() { > + IdentifierInfo *FnInfo = getIdentifier(); > + > + if (!FnInfo) > + return MFK_Invalid; > + > + // Builtin handling. > + switch (getBuiltinID()) { > + case Builtin::BI__builtin_memset: > + case Builtin::BI__builtin___memset_chk: > + case Builtin::BImemset: > + return MFK_Memset; > + > + case Builtin::BI__builtin_memcpy: > + case Builtin::BI__builtin___memcpy_chk: > + case Builtin::BImemcpy: > + return MFK_Memcpy; > + > + case Builtin::BI__builtin_memmove: > + case Builtin::BI__builtin___memmove_chk: > + case Builtin::BImemmove: > + return MFK_Memmove; > + > + case Builtin::BIstrlcpy: > + return MFK_Strlcpy; > + case Builtin::BIstrlcat: > + return MFK_Strlcat; > + > + case Builtin::BI__builtin_memcmp: > + return MFK_Memcmp; > + > + case Builtin::BI__builtin_strncpy: > + case Builtin::BI__builtin___strncpy_chk: > + case Builtin::BIstrncpy: > + return MFK_Strncpy; > + > + case Builtin::BI__builtin_strncmp: > + return MFK_Strncmp; > + > + case Builtin::BI__builtin_strncasecmp: > + return MFK_Strncasecmp; > + > + case Builtin::BI__builtin_strncat: > + case Builtin::BIstrncat: > + return MFK_Strncat; > + > + case Builtin::BI__builtin_strndup: > + case Builtin::BIstrndup: > + return MFK_Strndup; > + > + default: > + if (getLinkage() == ExternalLinkage && > + (!getASTContext().getLangOptions().CPlusPlus || isExternC())) { > + if (FnInfo->isStr("memset")) > + return MFK_Memset; > + else if (FnInfo->isStr("memcpy")) > + return MFK_Memcpy; > + else if (FnInfo->isStr("memmove")) > + return MFK_Memmove; > + else if (FnInfo->isStr("memcmp")) > + return MFK_Memcmp; > + else if (FnInfo->isStr("strncpy")) > + return MFK_Strncpy; > + else if (FnInfo->isStr("strncmp")) > + return MFK_Strncmp; > + else if (FnInfo->isStr("strncasecmp")) > + return MFK_Strncasecmp; > + else if (FnInfo->isStr("strncat")) > + return MFK_Strncat; > + else if (FnInfo->isStr("strndup")) > + return MFK_Strndup; > + } > + break; > + } > + return MFK_Invalid; > +} > + > //===----------------------------------------------------------------------===// > // FieldDecl Implementation > //===----------------------------------------------------------------------===// > > Modified: cfe/trunk/lib/Sema/SemaChecking.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=148142&r1=148141&r2=148142&view=diff > ============================================================================== > --- cfe/trunk/lib/Sema/SemaChecking.cpp (original) > +++ cfe/trunk/lib/Sema/SemaChecking.cpp Fri Jan 13 15:52:01 2012 > @@ -480,88 +480,15 @@ > TheCall->getCallee()->getLocStart()); > } > > - // Builtin handling > - int CMF = -1; > - switch (FDecl->getBuiltinID()) { > - case Builtin::BI__builtin_memset: > - case Builtin::BI__builtin___memset_chk: > - case Builtin::BImemset: > - CMF = CMF_Memset; > - break; > - > - case Builtin::BI__builtin_memcpy: > - case Builtin::BI__builtin___memcpy_chk: > - case Builtin::BImemcpy: > - CMF = CMF_Memcpy; > - break; > - > - case Builtin::BI__builtin_memmove: > - case Builtin::BI__builtin___memmove_chk: > - case Builtin::BImemmove: > - CMF = CMF_Memmove; > - break; > + FunctionDecl::MemoryFunctionKind CMF = FDecl->getMemoryFunctionKind(); > + if (CMF == FunctionDecl::MFK_Invalid) > + return false; > > - case Builtin::BIstrlcpy: > - case Builtin::BIstrlcat: > + // Handle memory setting and copying functions. > + if (CMF == FunctionDecl::MFK_Strlcpy || CMF == FunctionDecl::MFK_Strlcat) > CheckStrlcpycatArguments(TheCall, FnInfo); > - break; > - > - case Builtin::BI__builtin_memcmp: > - CMF = CMF_Memcmp; > - break; > - > - case Builtin::BI__builtin_strncpy: > - case Builtin::BI__builtin___strncpy_chk: > - case Builtin::BIstrncpy: > - CMF = CMF_Strncpy; > - break; > - > - case Builtin::BI__builtin_strncmp: > - CMF = CMF_Strncmp; > - break; > - > - case Builtin::BI__builtin_strncasecmp: > - CMF = CMF_Strncasecmp; > - break; > - > - case Builtin::BI__builtin_strncat: > - case Builtin::BIstrncat: > - CMF = CMF_Strncat; > - break; > - > - case Builtin::BI__builtin_strndup: > - case Builtin::BIstrndup: > - CMF = CMF_Strndup; > - break; > - > - default: > - if (FDecl->getLinkage() == ExternalLinkage && > - (!getLangOptions().CPlusPlus || FDecl->isExternC())) { > - if (FnInfo->isStr("memset")) > - CMF = CMF_Memset; > - else if (FnInfo->isStr("memcpy")) > - CMF = CMF_Memcpy; > - else if (FnInfo->isStr("memmove")) > - CMF = CMF_Memmove; > - else if (FnInfo->isStr("memcmp")) > - CMF = CMF_Memcmp; > - else if (FnInfo->isStr("strncpy")) > - CMF = CMF_Strncpy; > - else if (FnInfo->isStr("strncmp")) > - CMF = CMF_Strncmp; > - else if (FnInfo->isStr("strncasecmp")) > - CMF = CMF_Strncasecmp; > - else if (FnInfo->isStr("strncat")) > - CMF = CMF_Strncat; > - else if (FnInfo->isStr("strndup")) > - CMF = CMF_Strndup; > - } > - break; > - } > - > - // Memset/memcpy/memmove handling > - if (CMF != -1) > - CheckMemaccessArguments(TheCall, CheckedMemoryFunction(CMF), FnInfo); > + else > + CheckMemaccessArguments(TheCall, CMF, FnInfo); > > return false; > } > @@ -2500,16 +2427,17 @@ > /// > /// \param Call The call expression to diagnose. > void Sema::CheckMemaccessArguments(const CallExpr *Call, > - CheckedMemoryFunction CMF, > + FunctionDecl::MemoryFunctionKind CMF, > IdentifierInfo *FnName) { > // It is possible to have a non-standard definition of memset. Validate > // we have enough arguments, and if not, abort further checking. > - unsigned ExpectedNumArgs = (CMF == CMF_Strndup ? 2 : 3); > + unsigned ExpectedNumArgs = (CMF == FunctionDecl::MFK_Strndup ? 2 : 3); > if (Call->getNumArgs() < ExpectedNumArgs) > return; > > - unsigned LastArg = (CMF == CMF_Memset || CMF == CMF_Strndup ? 1 : 2); > - unsigned LenArg = (CMF == CMF_Strndup ? 1 : 2); > + unsigned LastArg = (CMF == FunctionDecl::MFK_Memset || > + CMF == FunctionDecl::MFK_Strndup ? 1 : 2); > + unsigned LenArg = (CMF == FunctionDecl::MFK_Strndup ? 1 : 2); > const Expr *LenExpr = Call->getArg(LenArg)->IgnoreParenImpCasts(); > > // We have special checking when the length is a sizeof expression. > @@ -2553,7 +2481,8 @@ > if (Context.getTypeSize(PointeeTy) == Context.getCharWidth()) > ActionIdx = 2; // If the pointee's size is sizeof(char), > // suggest an explicit length. > - unsigned DestSrcSelect = (CMF == CMF_Strndup ? 1 : ArgIdx); > + unsigned DestSrcSelect = > + (CMF == FunctionDecl::MFK_Strndup ? 1 : ArgIdx); > DiagRuntimeBehavior(SizeOfArg->getExprLoc(), Dest, > PDiag(diag::warn_sizeof_pointer_expr_memaccess) > << FnName << DestSrcSelect << ActionIdx > @@ -2583,12 +2512,15 @@ > DiagRuntimeBehavior( > Dest->getExprLoc(), Dest, > PDiag(diag::warn_dyn_class_memaccess) > - << (CMF == CMF_Memcmp ? ArgIdx + 2 : ArgIdx) << FnName << > PointeeTy > + << (CMF == FunctionDecl::MFK_Memcmp ? ArgIdx + 2 : ArgIdx) > + << FnName << PointeeTy > // "overwritten" if we're warning about the destination for any > call > // but memcmp; otherwise a verb appropriate to the call. > - << (ArgIdx == 0 && CMF != CMF_Memcmp ? 0 : (unsigned)CMF) > + << (ArgIdx == 0 && > + CMF != FunctionDecl::MFK_Memcmp ? 0 : (unsigned)CMF) > << Call->getCallee()->getSourceRange()); > - else if (PointeeTy.hasNonTrivialObjCLifetime() && CMF != CMF_Memset) > + else if (PointeeTy.hasNonTrivialObjCLifetime() && > + CMF != FunctionDecl::MFK_Memset) > DiagRuntimeBehavior( > Dest->getExprLoc(), Dest, > PDiag(diag::warn_arc_object_memaccess) > > > _______________________________________________ > 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
