[cfe-commits] r148142 - in /cfe/trunk: include/clang/AST/Decl.h include/clang/Sema/Sema.h lib/AST/Decl.cpp lib/Sema/SemaChecking.cpp

Chris Lattner clattner at apple.com
Fri Jan 13 22:18:48 PST 2012


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
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits




More information about the cfe-commits mailing list