[cfe-commits] r159608 - in /cfe/trunk: include/clang/StaticAnalyzer/Core/PathSensitive/Calls.h lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp lib/StaticAnalyzer/Core/ExprEngineCXX.cpp lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp test/Analysis/new.cpp
Anna Zaks
ganna at apple.com
Mon Jul 16 13:33:11 PDT 2012
On Jul 2, 2012, at 3:21 PM, Jordan Rose wrote:
> Author: jrose
> Date: Mon Jul 2 17:21:47 2012
> New Revision: 159608
>
> URL: http://llvm.org/viewvc/llvm-project?rev=159608&view=rev
> Log:
> [analyzer] Introduce CXXAllocatorCall to handle placement arg invalidation.
>
> This is NOT full-blown support for operator new, but removes some nasty
> duplicated code introduced in r158784.
>
> Modified:
> cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/Calls.h
> cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
> cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
> cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
> cfe/trunk/test/Analysis/new.cpp
>
> Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/Calls.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/Calls.h?rev=159608&r1=159607&r2=159608&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/Calls.h (original)
> +++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/Calls.h Mon Jul 2 17:21:47 2012
> @@ -33,8 +33,9 @@
> CE_BEG_SIMPLE_CALLS = CE_Function,
> CE_END_SIMPLE_CALLS = CE_Block,
> CE_CXXConstructor,
> + CE_CXXAllocator,
> CE_BEG_FUNCTION_CALLS = CE_Function,
> - CE_END_FUNCTION_CALLS = CE_CXXConstructor,
> + CE_END_FUNCTION_CALLS = CE_CXXAllocator,
> CE_ObjCMessage,
> CE_ObjCPropertyAccess,
> CE_BEG_OBJC_CALLS = CE_ObjCMessage,
> @@ -337,6 +338,35 @@
> }
> };
>
> +class CXXAllocatorCall : public AnyFunctionCall {
> + const CXXNewExpr *E;
> +
> +public:
> + CXXAllocatorCall(const CXXNewExpr *e, ProgramStateRef St,
> + const LocationContext *LCtx)
> + : AnyFunctionCall(St, LCtx, CE_CXXAllocator), E(e) {}
> +
> + const CXXNewExpr *getOriginExpr() const { return E; }
> + SourceRange getSourceRange() const { return E->getSourceRange(); }
> +
> + const FunctionDecl *getDecl() const {
> + return E->getOperatorNew();
> + }
> +
> + unsigned getNumArgs() const { return E->getNumPlacementArgs() + 1; }
> +
> + const Expr *getArgExpr(unsigned Index) const {
> + // The first argument of an allocator call is the size of the allocation.
> + if (Index == 0)
> + return 0;
> + return E->getPlacementArg(Index - 1);
> + }
> +
> + static bool classof(const CallEvent *CE) {
> + return CE->getKind() == CE_CXXAllocator;
> + }
> +};
> +
> /// \brief Represents any expression that calls an Objective-C method.
> class ObjCMethodCall : public CallEvent {
> const ObjCMessageExpr *Msg;
>
> Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp?rev=159608&r1=159607&r2=159608&view=diff
> ==============================================================================
> --- cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp (original)
> +++ cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp Mon Jul 2 17:21:47 2012
> @@ -947,6 +947,7 @@
> case CE_CXXMember:
> case CE_Block:
> case CE_CXXConstructor:
> + case CE_CXXAllocator:
> // FIXME: These calls are currently unsupported.
> return getPersistentStopSummary();
> case CE_ObjCMessage:
>
> Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp?rev=159608&r1=159607&r2=159608&view=diff
> ==============================================================================
> --- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp (original)
> +++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp Mon Jul 2 17:21:47 2012
> @@ -92,191 +92,51 @@
> Bldr.generateNode(PP, Pred, state);
> }
>
> -static bool isPointerToConst(const ParmVarDecl *ParamDecl) {
> - // FIXME: Copied from ExprEngineCallAndReturn.cpp
> - QualType PointeeTy = ParamDecl->getOriginalType()->getPointeeType();
> - if (PointeeTy != QualType() && PointeeTy.isConstQualified() &&
> - !PointeeTy->isAnyPointerType() && !PointeeTy->isReferenceType()) {
> - return true;
> - }
> - return false;
> -}
> -
> void ExprEngine::VisitCXXNewExpr(const CXXNewExpr *CNE, ExplodedNode *Pred,
> ExplodedNodeSet &Dst) {
> + // FIXME: Much of this should eventually migrate to CXXAllocatorCall.
> + // Also, we need to decide how allocators actually work -- they're not
> + // really part of the CXXNewExpr because they happen BEFORE the
> + // CXXConstructExpr subexpression. See PR12014 for some discussion.
> StmtNodeBuilder Bldr(Pred, Dst, *currentBuilderContext);
>
> unsigned blockCount = currentBuilderContext->getCurrentBlockCount();
> const LocationContext *LCtx = Pred->getLocationContext();
> DefinedOrUnknownSVal symVal =
> - svalBuilder.getConjuredSymbolVal(NULL, CNE, LCtx, CNE->getType(), blockCount);
> - const MemRegion *NewReg = cast<loc::MemRegionVal>(symVal).getRegion();
> - QualType ObjTy = CNE->getType()->getAs<PointerType>()->getPointeeType();
> - const ElementRegion *EleReg =
> - getStoreManager().GetElementZeroRegion(NewReg, ObjTy);
> + svalBuilder.getConjuredSymbolVal(0, CNE, LCtx, CNE->getType(), blockCount);
> ProgramStateRef State = Pred->getState();
>
> + // Invalidate placement args.
> + CXXAllocatorCall Call(CNE, State, LCtx);
> + State = Call.invalidateRegions(blockCount);
> +
> if (CNE->isArray()) {
> // FIXME: allocating an array requires simulating the constructors.
> // For now, just return a symbolicated region.
> + const MemRegion *NewReg = cast<loc::MemRegionVal>(symVal).getRegion();
> + QualType ObjTy = CNE->getType()->getAs<PointerType>()->getPointeeType();
> + const ElementRegion *EleReg =
> + getStoreManager().GetElementZeroRegion(NewReg, ObjTy);
> State = State->BindExpr(CNE, Pred->getLocationContext(),
> loc::MemRegionVal(EleReg));
> Bldr.generateNode(CNE, Pred, State);
> return;
> }
>
> + // FIXME: Once we have proper support for CXXConstructExprs inside
> + // CXXNewExpr, we need to make sure that the constructed object is not
> + // immediately invalidated here. (The placement call should happen before
> + // the constructor call anyway.)
> FunctionDecl *FD = CNE->getOperatorNew();
> if (FD && FD->isReservedGlobalPlacementOperator()) {
> // Non-array placement new should always return the placement location.
> SVal PlacementLoc = State->getSVal(CNE->getPlacementArg(0), LCtx);
> State = State->BindExpr(CNE, LCtx, PlacementLoc);
> - // FIXME: Once we have proper support for CXXConstructExprs inside
> - // CXXNewExpr, we need to make sure that the constructed object is not
> - // immediately invalidated here. (The placement call should happen before
> - // the constructor call anyway.)
> + } else {
> + State = State->BindExpr(CNE, LCtx, symVal);
> }
>
> - // Invalidate placement args.
> -
> - // FIXME: This is largely copied from invalidateArguments, because
> - // CallOrObjCMessage is not general enough to handle new-expressions yet.
> - SmallVector<const MemRegion *, 4> RegionsToInvalidate;
> -
> - unsigned Index = 0;
> - for (CXXNewExpr::const_arg_iterator I = CNE->placement_arg_begin(),
> - E = CNE->placement_arg_end();
> - I != E; ++I) {
> - // Pre-increment the argument index to skip over the implicit size arg.
> - ++Index;
> - if (FD && Index < FD->getNumParams())
> - if (isPointerToConst(FD->getParamDecl(Index)))
> - continue;
> -
> - SVal V = State->getSVal(*I, LCtx);
> -
> - // If we are passing a location wrapped as an integer, unwrap it and
> - // invalidate the values referred by the location.
> - if (nonloc::LocAsInteger *Wrapped = dyn_cast<nonloc::LocAsInteger>(&V))
> - V = Wrapped->getLoc();
> - else if (!isa<Loc>(V))
> - continue;
> -
> - if (const MemRegion *R = V.getAsRegion()) {
> - // Invalidate the value of the variable passed by reference.
> -
> - // Are we dealing with an ElementRegion? If the element type is
> - // a basic integer type (e.g., char, int) and the underlying region
> - // is a variable region then strip off the ElementRegion.
> - // FIXME: We really need to think about this for the general case
> - // as sometimes we are reasoning about arrays and other times
> - // about (char*), etc., is just a form of passing raw bytes.
> - // e.g., void *p = alloca(); foo((char*)p);
> - if (const ElementRegion *ER = dyn_cast<ElementRegion>(R)) {
> - // Checking for 'integral type' is probably too promiscuous, but
> - // we'll leave it in for now until we have a systematic way of
> - // handling all of these cases. Eventually we need to come up
> - // with an interface to StoreManager so that this logic can be
> - // appropriately delegated to the respective StoreManagers while
> - // still allowing us to do checker-specific logic (e.g.,
> - // invalidating reference counts), probably via callbacks.
> - if (ER->getElementType()->isIntegralOrEnumerationType()) {
> - const MemRegion *superReg = ER->getSuperRegion();
> - if (isa<VarRegion>(superReg) || isa<FieldRegion>(superReg) ||
> - isa<ObjCIvarRegion>(superReg))
> - R = cast<TypedRegion>(superReg);
> - }
> - // FIXME: What about layers of ElementRegions?
> - }
> -
> - // Mark this region for invalidation. We batch invalidate regions
> - // below for efficiency.
> - RegionsToInvalidate.push_back(R);
> - } else {
> - // Nuke all other arguments passed by reference.
> - // FIXME: is this necessary or correct? This handles the non-Region
> - // cases. Is it ever valid to store to these?
> - State = State->unbindLoc(cast<Loc>(V));
> - }
> - }
> -
> - // Invalidate designated regions using the batch invalidation API.
> -
> - // FIXME: We can have collisions on the conjured symbol if the
> - // expression *I also creates conjured symbols. We probably want
> - // to identify conjured symbols by an expression pair: the enclosing
> - // expression (the context) and the expression itself. This should
> - // disambiguate conjured symbols.
> - unsigned Count = currentBuilderContext->getCurrentBlockCount();
> -
> - // NOTE: Even if RegionsToInvalidate is empty, we may still invalidate
> - // global variables.
> - State = State->invalidateRegions(RegionsToInvalidate, CNE, Count, LCtx);
> Bldr.generateNode(CNE, Pred, State);
> - return;
> -
> - // FIXME: The below code is long-since dead. However, constructor handling
> - // in new-expressions is far from complete. See PR12014 for more details.
> -#if 0
> - // Evaluate constructor arguments.
> - const FunctionProtoType *FnType = NULL;
> - const CXXConstructorDecl *CD = CNE->getConstructor();
> - if (CD)
> - FnType = CD->getType()->getAs<FunctionProtoType>();
> - ExplodedNodeSet argsEvaluated;
> - Bldr.takeNodes(Pred);
> - evalArguments(CNE->constructor_arg_begin(), CNE->constructor_arg_end(),
> - FnType, Pred, argsEvaluated);
> - Bldr.addNodes(argsEvaluated);
> -
> - // Initialize the object region and bind the 'new' expression.
> - for (ExplodedNodeSet::iterator I = argsEvaluated.begin(),
> - E = argsEvaluated.end(); I != E; ++I) {
> -
> - ProgramStateRef state = (*I)->getState();
> -
> - // Accumulate list of regions that are invalidated.
> - // FIXME: Eventually we should unify the logic for constructor
> - // processing in one place.
> - SmallVector<const MemRegion*, 10> regionsToInvalidate;
> - for (CXXNewExpr::const_arg_iterator
> - ai = CNE->constructor_arg_begin(), ae = CNE->constructor_arg_end();
> - ai != ae; ++ai)
> - {
> - SVal val = state->getSVal(*ai, (*I)->getLocationContext());
> - if (const MemRegion *region = val.getAsRegion())
> - regionsToInvalidate.push_back(region);
> - }
> -
> - if (ObjTy->isRecordType()) {
> - regionsToInvalidate.push_back(EleReg);
> - // Invalidate the regions.
> - // TODO: Pass the call to new information as the last argument, to limit
> - // the globals which will get invalidated.
> - state = state->invalidateRegions(regionsToInvalidate,
> - CNE, blockCount, 0, 0);
> -
> - } else {
> - // Invalidate the regions.
> - // TODO: Pass the call to new information as the last argument, to limit
> - // the globals which will get invalidated.
> - state = state->invalidateRegions(regionsToInvalidate,
> - CNE, blockCount, 0, 0);
> -
> - if (CNE->hasInitializer()) {
> - SVal V = state->getSVal(*CNE->constructor_arg_begin(),
> - (*I)->getLocationContext());
> - state = state->bindLoc(loc::MemRegionVal(EleReg), V);
> - } else {
> - // Explicitly set to undefined, because currently we retrieve symbolic
> - // value from symbolic region.
> - state = state->bindLoc(loc::MemRegionVal(EleReg), UndefinedVal());
> - }
> - }
> - state = state->BindExpr(CNE, (*I)->getLocationContext(),
> - loc::MemRegionVal(EleReg));
> - Bldr.generateNode(CNE, *I, state);
> - }
> -#endif
> }
>
> void ExprEngine::VisitCXXDeleteExpr(const CXXDeleteExpr *CDE,
>
> Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp?rev=159608&r1=159607&r2=159608&view=diff
> ==============================================================================
> --- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp (original)
> +++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp Mon Jul 2 17:21:47 2012
> @@ -240,11 +240,6 @@
> return false;
> }
>
> - // Do not inline constructors until we can model destructors.
> - // This is unfortunate, but basically necessary for smart pointers and such.
> - if (isa<CXXConstructorDecl>(D))
> - return false;
> -
> // It is possible that the live variables analysis cannot be
> // run. If so, bail out.
> if (!CalleeADC->getAnalysis<RelaxedLiveVariables>())
> @@ -267,10 +262,17 @@
>
> switch (Call.getKind()) {
> case CE_Function:
> - case CE_CXXConstructor:
> case CE_CXXMember:
> // These are always at least possible to inline.
> break;
> + case CE_CXXConstructor:
> + // Do not inline constructors until we can model destructors.
> + // This is unfortunate, but basically necessary for smart pointers and such.
> + return false;
> + case CE_CXXAllocator:
> + // Do not inline allocators until we model deallocators.
> + // This is unfortunate, but basically necessary for smart pointers and such.
> + return false;
> case CE_Block: {
> const BlockDataRegion *BR = cast<BlockCall>(Call).getBlockRegion();
> if (!BR)
Not sure where this switch was added, but we seem to rely on it to determine if we should inline the call or not. We have at least 2 other methods that help us to decide that:
CallEvent::mayBeInlined() and shouldInlineDecl() called below. Can we try and refactor this logic into one of those?
>
> Modified: cfe/trunk/test/Analysis/new.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/new.cpp?rev=159608&r1=159607&r2=159608&view=diff
> ==============================================================================
> --- cfe/trunk/test/Analysis/new.cpp (original)
> +++ cfe/trunk/test/Analysis/new.cpp Mon Jul 2 17:21:47 2012
> @@ -49,6 +49,16 @@
> return y; // no-warning
> }
>
> +void *operator new(size_t, void *, void *);
> +void *testCustomNewMalloc() {
> + int *x = (int *)malloc(sizeof(int));
> +
> + // Should be no-warning (the custom allocator could have freed x).
> + void *y = new (0, x) int; // no-warning
> +
> + return y;
> +}
> +
>
> //--------------------------------
> // Incorrectly-modelled behavior
> @@ -69,14 +79,3 @@
> clang_analyzer_eval(*n == 3); // expected-warning{{UNKNOWN}}
> }
>
> -
> -void *operator new(size_t, void *, void *);
> -void *testCustomNewMalloc() {
> - int *x = (int *)malloc(sizeof(int));
> -
> - // Should be no-warning (the custom allocator could have freed x).
> - void *y = new (0, x) int; // expected-warning{{leak of memory pointed to by 'x'}}
> -
> - return y;
> -}
> -
>
>
> _______________________________________________
> 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