[cfe-commits] r148142 - in /cfe/trunk: include/clang/AST/Decl.h include/clang/Sema/Sema.h lib/AST/Decl.cpp lib/Sema/SemaChecking.cpp
Anna Zaks
ganna at apple.com
Sat Jan 14 09:16:31 PST 2012
On Jan 13, 2012, at 10:18 PM, Chris Lattner wrote:
>
> 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?
>
Good point.
The reason for the new enum is that it provides better readability: documents what the method handles; and in all the places where the value is used, makes it obvious that it's not just a single builtin.
On the other hand, it's probably more undesirable to grow FunctionDecl.. I'll patch to use the built-in ID.
Thanks!
Anna.
> -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
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
More information about the cfe-commits
mailing list