r322777 - [analyzer] operator new: Model the cast of returned pointer into object type.

Artem Dergachev via cfe-commits cfe-commits at lists.llvm.org
Wed Jan 17 14:51:19 PST 2018


Author: dergachev
Date: Wed Jan 17 14:51:19 2018
New Revision: 322777

URL: http://llvm.org/viewvc/llvm-project?rev=322777&view=rev
Log:
[analyzer] operator new: Model the cast of returned pointer into object type.

According to [basic.stc.dynamic.allocation], the return type of any C++
overloaded operator new() is "void *". However, type of the new-expression
"new T()" and the type of "this" during construction of "T" are both "T *".

Hence an implicit cast, which is not present in the AST, needs to be performed
before the construction. This patch adds such cast in the case when the
allocator was indeed inlined. For now, in the case where the allocator was *not*
inlined we still use the same symbolic value (which is a pure SymbolicRegion of
type "T *") because it is consistent with how we represent the casts and causes
less surprise in the checkers after switching to the new behavior.

The better approach would be to represent that value as a cast over a
SymbolicRegion of type "void *", however we have technical difficulties
conjuring such region without any actual expression of type "void *" present in
the AST.

Differential Revision: https://reviews.llvm.org/D41250
rdar://problem/12180598

Modified:
    cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
    cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
    cfe/trunk/test/Analysis/new-ctor-conservative.cpp
    cfe/trunk/test/Analysis/new-ctor-inlined.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp?rev=322777&r1=322776&r2=322777&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp Wed Jan 17 14:51:19 2018
@@ -119,9 +119,17 @@ ExprEngine::getRegionForConstructedObjec
         if (AMgr.getAnalyzerOptions().mayInlineCXXAllocator()) {
           // TODO: Detect when the allocator returns a null pointer.
           // Constructor shall not be called in this case.
-          if (const MemRegion *MR =
-                  getCXXNewAllocatorValue(State, CNE, LCtx).getAsRegion())
+          if (const SubRegion *MR = dyn_cast_or_null<SubRegion>(
+                  getCXXNewAllocatorValue(State, CNE, LCtx).getAsRegion())) {
+            if (CNE->isArray()) {
+              // TODO: This code exists only to trigger the suppression for
+              // array constructors. In fact, we need to call the constructor
+              // for every allocated element, not just the first one!
+              return getStoreManager().GetElementZeroRegion(
+                  MR, CNE->getType()->getPointeeType());
+            }
             return MR;
+          }
         }
       } else if (auto *DS = dyn_cast<DeclStmt>(StmtElem->getStmt())) {
         if (const auto *Var = dyn_cast<VarDecl>(DS->getSingleDecl())) {
@@ -473,12 +481,22 @@ void ExprEngine::VisitCXXNewAllocatorCal
   StmtNodeBuilder CallBldr(DstPreCall, DstPostCall, *currBldrCtx);
   for (auto I : DstPreCall)
     defaultEvalCall(CallBldr, I, *Call);
+  // If the call is inlined, DstPostCall will be empty and we bail out now.
 
   // Store return value of operator new() for future use, until the actual
   // CXXNewExpr gets processed.
   ExplodedNodeSet DstPostValue;
   StmtNodeBuilder ValueBldr(DstPostCall, DstPostValue, *currBldrCtx);
   for (auto I : DstPostCall) {
+    // FIXME: Because CNE serves as the "call site" for the allocator (due to
+    // lack of a better expression in the AST), the conjured return value symbol
+    // is going to be of the same type (C++ object pointer type). Technically
+    // this is not correct because the operator new's prototype always says that
+    // it returns a 'void *'. So we should change the type of the symbol,
+    // and then evaluate the cast over the symbolic pointer from 'void *' to
+    // the object pointer type. But without changing the symbol's type it
+    // is breaking too much to evaluate the no-op symbolic cast over it, so we
+    // skip it for now.
     ProgramStateRef State = I->getState();
     ValueBldr.generateNode(
         CNE, I,
@@ -564,16 +582,20 @@ void ExprEngine::VisitCXXNewExpr(const C
 
   StmtNodeBuilder Bldr(Pred, Dst, *currBldrCtx);
 
+  SVal Result = symVal;
+
   if (CNE->isArray()) {
     // FIXME: allocating an array requires simulating the constructors.
     // For now, just return a symbolicated region.
-    const SubRegion *NewReg =
-        symVal.castAs<loc::MemRegionVal>().getRegionAs<SubRegion>();
-    QualType ObjTy = CNE->getType()->getAs<PointerType>()->getPointeeType();
-    const ElementRegion *EleReg =
-      getStoreManager().GetElementZeroRegion(NewReg, ObjTy);
-    State = State->BindExpr(CNE, Pred->getLocationContext(),
-                            loc::MemRegionVal(EleReg));
+    if (!AMgr.getAnalyzerOptions().mayInlineCXXAllocator()) {
+      const SubRegion *NewReg =
+          symVal.castAs<loc::MemRegionVal>().getRegionAs<SubRegion>();
+      QualType ObjTy = CNE->getType()->getAs<PointerType>()->getPointeeType();
+      const ElementRegion *EleReg =
+          getStoreManager().GetElementZeroRegion(NewReg, ObjTy);
+      Result = loc::MemRegionVal(EleReg);
+    }
+    State = State->BindExpr(CNE, Pred->getLocationContext(), Result);
     Bldr.generateNode(CNE, Pred, State);
     return;
   }
@@ -582,7 +604,6 @@ void ExprEngine::VisitCXXNewExpr(const C
   // 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.)
-  SVal Result = symVal;
   if (FD && FD->isReservedGlobalPlacementOperator()) {
     // Non-array placement new should always return the placement location.
     SVal PlacementLoc = State->getSVal(CNE->getPlacementArg(0), LCtx);

Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp?rev=322777&r1=322776&r2=322777&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp Wed Jan 17 14:51:19 2018
@@ -277,12 +277,19 @@ void ExprEngine::processCallExit(Explode
       state = state->BindExpr(CCE, callerCtx, ThisV);
     }
 
-    if (const CXXNewExpr *CNE = dyn_cast<CXXNewExpr>(CE)) {
+    if (const auto *CNE = dyn_cast<CXXNewExpr>(CE)) {
       // We are currently evaluating a CXXNewAllocator CFGElement. It takes a
       // while to reach the actual CXXNewExpr element from here, so keep the
       // region for later use.
-      state = setCXXNewAllocatorValue(state, CNE, calleeCtx->getParent(),
-                                      state->getSVal(CE, callerCtx));
+      // Additionally cast the return value of the inlined operator new
+      // (which is of type 'void *') to the correct object type.
+      SVal AllocV = state->getSVal(CNE, callerCtx);
+      AllocV = svalBuilder.evalCast(
+          AllocV, CNE->getType(),
+          getContext().getPointerType(getContext().VoidTy));
+
+      state =
+          setCXXNewAllocatorValue(state, CNE, calleeCtx->getParent(), AllocV);
     }
   }
 

Modified: cfe/trunk/test/Analysis/new-ctor-conservative.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/new-ctor-conservative.cpp?rev=322777&r1=322776&r2=322777&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/new-ctor-conservative.cpp (original)
+++ cfe/trunk/test/Analysis/new-ctor-conservative.cpp Wed Jan 17 14:51:19 2018
@@ -12,3 +12,18 @@ void checkConstructorInlining() {
   S *s = new S;
   clang_analyzer_eval(s->x == 1); // expected-warning{{TRUE}}
 }
+
+void checkNewPOD() {
+  int *i = new int;
+  clang_analyzer_eval(*i == 0); // expected-warning{{UNKNOWN}}
+  int *j = new int();
+  clang_analyzer_eval(*j == 0); // expected-warning{{TRUE}}
+  int *k = new int(5);
+  clang_analyzer_eval(*k == 5); // expected-warning{{TRUE}}
+}
+
+void checkNewArray() {
+  S *s = new S[10];
+  // FIXME: Should be true once we inline array constructors.
+  clang_analyzer_eval(s[0].x == 1); // expected-warning{{UNKNOWN}}
+}

Modified: cfe/trunk/test/Analysis/new-ctor-inlined.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/new-ctor-inlined.cpp?rev=322777&r1=322776&r2=322777&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/new-ctor-inlined.cpp (original)
+++ cfe/trunk/test/Analysis/new-ctor-inlined.cpp Wed Jan 17 14:51:19 2018
@@ -38,3 +38,18 @@ void checkNestedNew() {
   Sp *p = new Sp(new Sp(0));
   clang_analyzer_eval(p->p->p == 0); // expected-warning{{TRUE}}
 }
+
+void checkNewPOD() {
+  int *i = new int;
+  clang_analyzer_eval(*i == 0); // expected-warning{{UNKNOWN}}
+  int *j = new int();
+  clang_analyzer_eval(*j == 0); // expected-warning{{TRUE}}
+  int *k = new int(5);
+  clang_analyzer_eval(*k == 5); // expected-warning{{TRUE}}
+}
+
+void checkTrivialCopy() {
+  S s;
+  S *t = new S(s); // no-crash
+  clang_analyzer_eval(t->x == 1); // expected-warning{{TRUE}}
+}




More information about the cfe-commits mailing list