r322774 - [analyzer] operator new: Use the correct region for the constructor.

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


Author: dergachev
Date: Wed Jan 17 14:34:23 2018
New Revision: 322774

URL: http://llvm.org/viewvc/llvm-project?rev=322774&view=rev
Log:
[analyzer] operator new: Use the correct region for the constructor.

The -analyzer-config c++-allocator-inlining experimental option allows the
analyzer to reason about C++ operator new() similarly to how it reasons about
regular functions. In this mode, operator new() is correctly called before the
construction of an object, with the help of a special CFG element.

However, the subsequent construction of the object was still not performed into
the region of memory returned by operator new(). The patch fixes it.

Passing the value from operator new() to the constructor and then to the
new-expression itself was tricky because operator new() has no call site of its
own in the AST. The new expression itself is not a good call site because it
has an incorrect type (operator new() returns 'void *', while the new expression
is a pointer to the allocated object type). Additionally, lifetime of the new
expression in the environment makes it unsuitable for passing the value.
For that reason, an additional program state trait is introduced to keep track
of the return value.

Finally this patch relaxes restrictions on the memory region class that are
required for inlining the constructor. This change affects the old mode as well
(c++-allocator-inlining=false) and seems safe because these restrictions were
an overkill compared to the actual problems observed.

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

Added:
    cfe/trunk/test/Analysis/new-ctor-conservative.cpp
    cfe/trunk/test/Analysis/new-ctor-inlined.cpp
    cfe/trunk/test/Analysis/new-ctor-recursive.cpp
    cfe/trunk/test/Analysis/new-ctor-symbolic.cpp
Modified:
    cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
    cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
    cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
    cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
    cfe/trunk/test/Analysis/inline.cpp

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h?rev=322774&r1=322773&r2=322774&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h Wed Jan 17 14:34:23 2018
@@ -642,8 +642,8 @@ private:
   const CXXConstructExpr *findDirectConstructorForCurrentCFGElement();
 
   /// For a CXXConstructExpr, walk forward in the current CFG block to find the
-  /// CFGElement for the DeclStmt or CXXInitCtorInitializer for which is
-  /// directly constructed by this constructor. Returns None if the current
+  /// CFGElement for the DeclStmt or CXXInitCtorInitializer or CXXNewExpr which
+  /// is directly constructed by this constructor. Returns None if the current
   /// constructor expression did not directly construct into an existing
   /// region.
   Optional<CFGElement> findElementDirectlyInitializedByCurrentConstructor();
@@ -655,6 +655,30 @@ private:
   /// if not.
   const MemRegion *getRegionForConstructedObject(const CXXConstructExpr *CE,
                                                  ExplodedNode *Pred);
+
+  /// Store the region returned by operator new() so that the constructor
+  /// that follows it knew what location to initialize. The value should be
+  /// cleared once the respective CXXNewExpr CFGStmt element is processed.
+  static ProgramStateRef
+  setCXXNewAllocatorValue(ProgramStateRef State, const CXXNewExpr *CNE,
+                          const LocationContext *CallerLC, SVal V);
+
+  /// Retrieve the location returned by the current operator new().
+  static SVal
+  getCXXNewAllocatorValue(ProgramStateRef State, const CXXNewExpr *CNE,
+                          const LocationContext *CallerLC);
+
+  /// Clear the location returned by the respective operator new(). This needs
+  /// to be done as soon as CXXNewExpr CFG block is evaluated.
+  static ProgramStateRef
+  clearCXXNewAllocatorValue(ProgramStateRef State, const CXXNewExpr *CNE,
+                            const LocationContext *CallerLC);
+
+  /// Check if all allocator values are clear for the given context range
+  /// (including FromLC, not including ToLC). This is useful for assertions.
+  static bool areCXXNewAllocatorValuesClear(ProgramStateRef State,
+                                            const LocationContext *FromLC,
+                                            const LocationContext *ToLC);
 };
 
 /// Traits for storing the call processing policy inside GDM.

Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp?rev=322774&r1=322773&r2=322774&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp Wed Jan 17 14:34:23 2018
@@ -63,6 +63,20 @@ typedef std::pair<const CXXBindTemporary
 REGISTER_TRAIT_WITH_PROGRAMSTATE(InitializedTemporariesSet,
                                  llvm::ImmutableSet<CXXBindTemporaryContext>)
 
+typedef llvm::ImmutableMap<std::pair<const CXXNewExpr *,
+                           const LocationContext *>, SVal>
+    CXXNewAllocatorValuesTy;
+
+// Keeps track of return values of various operator new() calls between
+// evaluation of the inlined operator new(), through the constructor call,
+// to the actual evaluation of the CXXNewExpr.
+// TODO: Refactor the key for this trait into a LocationContext sub-class,
+// which would be put on the stack of location contexts before operator new()
+// is evaluated, and removed from the stack when the whole CXXNewExpr
+// is fully evaluated.
+// Probably do something similar to the previous trait as well.
+REGISTER_TRAIT_WITH_PROGRAMSTATE(CXXNewAllocatorValues, CXXNewAllocatorValuesTy)
+
 //===----------------------------------------------------------------------===//
 // Engine construction and deletion.
 //===----------------------------------------------------------------------===//
@@ -308,6 +322,43 @@ ExprEngine::createTemporaryRegionIfNeede
   return State;
 }
 
+ProgramStateRef
+ExprEngine::setCXXNewAllocatorValue(ProgramStateRef State,
+                                    const CXXNewExpr *CNE,
+                                    const LocationContext *CallerLC, SVal V) {
+  assert(!State->get<CXXNewAllocatorValues>(std::make_pair(CNE, CallerLC)) &&
+         "Allocator value already set!");
+  return State->set<CXXNewAllocatorValues>(std::make_pair(CNE, CallerLC), V);
+}
+
+SVal ExprEngine::getCXXNewAllocatorValue(ProgramStateRef State,
+                                         const CXXNewExpr *CNE,
+                                         const LocationContext *CallerLC) {
+  return *State->get<CXXNewAllocatorValues>(std::make_pair(CNE, CallerLC));
+}
+
+ProgramStateRef
+ExprEngine::clearCXXNewAllocatorValue(ProgramStateRef State,
+                                      const CXXNewExpr *CNE,
+                                      const LocationContext *CallerLC) {
+  return State->remove<CXXNewAllocatorValues>(std::make_pair(CNE, CallerLC));
+}
+
+bool ExprEngine::areCXXNewAllocatorValuesClear(ProgramStateRef State,
+                                               const LocationContext *FromLC,
+                                               const LocationContext *ToLC) {
+  const LocationContext *LC = FromLC;
+  do {
+    for (auto I : State->get<CXXNewAllocatorValues>())
+      if (I.first.second == LC)
+        return false;
+
+    LC = LC->getParent();
+    assert(LC && "ToLC must be a parent of FromLC!");
+  } while (LC != ToLC);
+  return true;
+}
+
 //===----------------------------------------------------------------------===//
 // Top-level transfer function logic (Dispatcher).
 //===----------------------------------------------------------------------===//
@@ -429,6 +480,13 @@ void ExprEngine::removeDead(ExplodedNode
   const StackFrameContext *SFC = LC ? LC->getCurrentStackFrame() : nullptr;
   SymbolReaper SymReaper(SFC, ReferenceStmt, SymMgr, getStoreManager());
 
+  for (auto I : CleanedState->get<CXXNewAllocatorValues>()) {
+    if (SymbolRef Sym = I.second.getAsSymbol())
+      SymReaper.markLive(Sym);
+    if (const MemRegion *MR = I.second.getAsRegion())
+      SymReaper.markElementIndicesLive(MR);
+  }
+
   getCheckerManager().runCheckersForLiveSymbols(CleanedState, SymReaper);
 
   // Create a state in which dead bindings are removed from the environment

Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp?rev=322774&r1=322773&r2=322774&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp Wed Jan 17 14:34:23 2018
@@ -115,14 +115,25 @@ ExprEngine::getRegionForConstructedObjec
 
   if (auto Elem = findElementDirectlyInitializedByCurrentConstructor()) {
     if (Optional<CFGStmt> StmtElem = Elem->getAs<CFGStmt>()) {
-      auto *DS = cast<DeclStmt>(StmtElem->getStmt());
-      if (const auto *Var = dyn_cast<VarDecl>(DS->getSingleDecl())) {
-        if (Var->getInit() && Var->getInit()->IgnoreImplicit() == CE) {
-          SVal LValue = State->getLValue(Var, LCtx);
-          QualType Ty = Var->getType();
-          LValue = makeZeroElementRegion(State, LValue, Ty);
-          return LValue.getAsRegion();
+      if (const CXXNewExpr *CNE = dyn_cast<CXXNewExpr>(StmtElem->getStmt())) {
+        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())
+            return MR;
         }
+      } else if (auto *DS = dyn_cast<DeclStmt>(StmtElem->getStmt())) {
+        if (const auto *Var = dyn_cast<VarDecl>(DS->getSingleDecl())) {
+          if (Var->getInit() && Var->getInit()->IgnoreImplicit() == CE) {
+            SVal LValue = State->getLValue(Var, LCtx);
+            QualType Ty = Var->getType();
+            LValue = makeZeroElementRegion(State, LValue, Ty);
+            return LValue.getAsRegion();
+          }
+        }
+      } else {
+        llvm_unreachable("Unexpected directly initialized element!");
       }
     } else if (Optional<CFGInitializer> InitElem = Elem->getAs<CFGInitializer>()) {
       const CXXCtorInitializer *Init = InitElem->getInitializer();
@@ -166,6 +177,9 @@ static bool canHaveDirectConstructor(CFG
     if (isa<DeclStmt>(StmtElem->getStmt())) {
       return true;
     }
+    if (isa<CXXNewExpr>(StmtElem->getStmt())) {
+      return true;
+    }
   }
 
   if (Elem.getKind() == CFGElement::Initializer) {
@@ -455,12 +469,23 @@ void ExprEngine::VisitCXXNewAllocatorCal
   getCheckerManager().runCheckersForPreCall(DstPreCall, Pred,
                                             *Call, *this);
 
-  ExplodedNodeSet DstInvalidated;
-  StmtNodeBuilder Bldr(DstPreCall, DstInvalidated, *currBldrCtx);
-  for (ExplodedNodeSet::iterator I = DstPreCall.begin(), E = DstPreCall.end();
-       I != E; ++I)
-    defaultEvalCall(Bldr, *I, *Call);
-  getCheckerManager().runCheckersForPostCall(Dst, DstInvalidated,
+  ExplodedNodeSet DstPostCall;
+  StmtNodeBuilder CallBldr(DstPreCall, DstPostCall, *currBldrCtx);
+  for (auto I : DstPreCall)
+    defaultEvalCall(CallBldr, I, *Call);
+
+  // 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) {
+    ProgramStateRef State = I->getState();
+    ValueBldr.generateNode(
+        CNE, I,
+        setCXXNewAllocatorValue(State, CNE, LCtx, State->getSVal(CNE, LCtx)));
+  }
+
+  getCheckerManager().runCheckersForPostCall(Dst, DstPostValue,
                                              *Call, *this);
 }
 
@@ -474,7 +499,7 @@ void ExprEngine::VisitCXXNewExpr(const C
 
   unsigned blockCount = currBldrCtx->blockCount();
   const LocationContext *LCtx = Pred->getLocationContext();
-  DefinedOrUnknownSVal symVal = UnknownVal();
+  SVal symVal = UnknownVal();
   FunctionDecl *FD = CNE->getOperatorNew();
 
   bool IsStandardGlobalOpNewFunction = false;
@@ -490,26 +515,37 @@ void ExprEngine::VisitCXXNewExpr(const C
       IsStandardGlobalOpNewFunction = (FD->getNumParams() == 1);
   }
 
+  ProgramStateRef State = Pred->getState();
+
+  // Retrieve the stored operator new() return value.
+  if (AMgr.getAnalyzerOptions().mayInlineCXXAllocator()) {
+    symVal = getCXXNewAllocatorValue(State, CNE, LCtx);
+    State = clearCXXNewAllocatorValue(State, CNE, LCtx);
+  }
+
   // We assume all standard global 'operator new' functions allocate memory in
   // heap. We realize this is an approximation that might not correctly model
   // a custom global allocator.
-  if (IsStandardGlobalOpNewFunction)
-    symVal = svalBuilder.getConjuredHeapSymbolVal(CNE, LCtx, blockCount);
-  else
-    symVal = svalBuilder.conjureSymbolVal(nullptr, CNE, LCtx, CNE->getType(),
-                                          blockCount);
+  if (symVal.isUnknown()) {
+    if (IsStandardGlobalOpNewFunction)
+      symVal = svalBuilder.getConjuredHeapSymbolVal(CNE, LCtx, blockCount);
+    else
+      symVal = svalBuilder.conjureSymbolVal(nullptr, CNE, LCtx, CNE->getType(),
+                                            blockCount);
+  }
 
-  ProgramStateRef State = Pred->getState();
   CallEventManager &CEMgr = getStateManager().getCallEventManager();
   CallEventRef<CXXAllocatorCall> Call =
     CEMgr.getCXXAllocatorCall(CNE, State, LCtx);
 
-  // Invalidate placement args.
-  // FIXME: Once we figure out how we want allocators to work,
-  // we should be using the usual pre-/(default-)eval-/post-call checks here.
-  State = Call->invalidateRegions(blockCount);
-  if (!State)
-    return;
+  if (!AMgr.getAnalyzerOptions().mayInlineCXXAllocator()) {
+    // Invalidate placement args.
+    // FIXME: Once we figure out how we want allocators to work,
+    // we should be using the usual pre-/(default-)eval-/post-call checks here.
+    State = Call->invalidateRegions(blockCount);
+    if (!State)
+      return;
+  }
 
   // If this allocation function is not declared as non-throwing, failures
   // /must/ be signalled by exceptions, and thus the return value will never be
@@ -522,7 +558,8 @@ void ExprEngine::VisitCXXNewExpr(const C
     QualType Ty = FD->getType();
     if (const FunctionProtoType *ProtoType = Ty->getAs<FunctionProtoType>())
       if (!ProtoType->isNothrow(getContext()))
-        State = State->assume(symVal, true);
+        if (auto dSymVal = symVal.getAs<DefinedOrUnknownSVal>())
+          State = State->assume(*dSymVal, true);
   }
 
   StmtNodeBuilder Bldr(Pred, Dst, *currBldrCtx);

Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp?rev=322774&r1=322773&r2=322774&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp Wed Jan 17 14:34:23 2018
@@ -276,6 +276,14 @@ void ExprEngine::processCallExit(Explode
 
       state = state->BindExpr(CCE, callerCtx, ThisV);
     }
+
+    if (const CXXNewExpr *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));
+    }
   }
 
   // Step 3: BindedRetNode -> CleanedNodes
@@ -315,6 +323,10 @@ void ExprEngine::processCallExit(Explode
     CallExitEnd Loc(calleeCtx, callerCtx);
     bool isNew;
     ProgramStateRef CEEState = (*I == CEBNode) ? state : (*I)->getState();
+
+    // See if we have any stale C++ allocator values.
+    assert(areCXXNewAllocatorValuesClear(CEEState, calleeCtx, callerCtx));
+
     ExplodedNode *CEENode = G.getNode(Loc, CEEState, false, &isNew);
     CEENode->addPredecessor(*I, G);
     if (!isNew)
@@ -596,21 +608,34 @@ static CallInlinePolicy mayInlineCallKin
 
     const CXXConstructorCall &Ctor = cast<CXXConstructorCall>(Call);
 
+    const CXXConstructExpr *CtorExpr = Ctor.getOriginExpr();
+
+    // FIXME: ParentMap is slow and ugly. The callee should provide the
+    // necessary context. Ideally as part of the call event, or maybe as part of
+    // location context.
+    const Stmt *ParentExpr = CurLC->getParentMap().getParent(CtorExpr);
+
+    if (ParentExpr && isa<CXXNewExpr>(ParentExpr) &&
+        !Opts.mayInlineCXXAllocator())
+      return CIP_DisallowedOnce;
+
     // FIXME: We don't handle constructors or destructors for arrays properly.
     // Even once we do, we still need to be careful about implicitly-generated
     // initializers for array fields in default move/copy constructors.
+    // We still allow construction into ElementRegion targets when they don't
+    // represent array elements.
     const MemRegion *Target = Ctor.getCXXThisVal().getAsRegion();
-    if (Target && isa<ElementRegion>(Target))
-      return CIP_DisallowedOnce;
-
-    // FIXME: This is a hack. We don't use the correct region for a new
-    // expression, so if we inline the constructor its result will just be
-    // thrown away. This short-term hack is tracked in <rdar://problem/12180598>
-    // and the longer-term possible fix is discussed in PR12014.
-    const CXXConstructExpr *CtorExpr = Ctor.getOriginExpr();
-    if (const Stmt *Parent = CurLC->getParentMap().getParent(CtorExpr))
-      if (isa<CXXNewExpr>(Parent))
-        return CIP_DisallowedOnce;
+    if (Target && isa<ElementRegion>(Target)) {
+      if (ParentExpr)
+        if (const CXXNewExpr *NewExpr = dyn_cast<CXXNewExpr>(ParentExpr))
+          if (NewExpr->isArray())
+            return CIP_DisallowedOnce;
+
+      if (const TypedValueRegion *TR = dyn_cast<TypedValueRegion>(
+              cast<SubRegion>(Target)->getSuperRegion()))
+        if (TR->getValueType()->isArrayType())
+          return CIP_DisallowedOnce;
+    }
 
     // Inlining constructors requires including initializers in the CFG.
     const AnalysisDeclContext *ADC = CallerSFC->getAnalysisDeclContext();
@@ -629,7 +654,7 @@ static CallInlinePolicy mayInlineCallKin
     // FIXME: This is a hack. We don't handle temporary destructors
     // right now, so we shouldn't inline their constructors.
     if (CtorExpr->getConstructionKind() == CXXConstructExpr::CK_Complete)
-      if (!Target || !isa<DeclRegion>(Target))
+      if (!Target || isa<CXXTempObjectRegion>(Target))
         return CIP_DisallowedOnce;
 
     break;

Modified: cfe/trunk/test/Analysis/inline.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/inline.cpp?rev=322774&r1=322773&r2=322774&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/inline.cpp (original)
+++ cfe/trunk/test/Analysis/inline.cpp Wed Jan 17 14:34:23 2018
@@ -315,17 +315,13 @@ namespace OperatorNew {
     int value;
 
     IntWrapper(int input) : value(input) {
-      // We don't want this constructor to be inlined unless we can actually
-      // use the proper region for operator new.
-      // See PR12014 and <rdar://problem/12180598>.
-      clang_analyzer_checkInlined(false); // no-warning
+      clang_analyzer_checkInlined(true); // expected-warning{{TRUE}}
     }
   };
 
   void test() {
     IntWrapper *obj = new IntWrapper(42);
-    // should be TRUE
-    clang_analyzer_eval(obj->value == 42); // expected-warning{{UNKNOWN}}
+    clang_analyzer_eval(obj->value == 42); // expected-warning{{TRUE}}
     delete obj;
   }
 
@@ -335,8 +331,9 @@ namespace OperatorNew {
 
     clang_analyzer_eval(alias == obj); // expected-warning{{TRUE}}
 
-    // should be TRUE
-    clang_analyzer_eval(obj->value == 42); // expected-warning{{UNKNOWN}}
+    clang_analyzer_eval(obj->value == 42); // expected-warning{{TRUE}}
+    // Because malloc() was never free()d:
+    // expected-warning at -2{{Potential leak of memory pointed to by 'alias'}}
   }
 }
 

Added: cfe/trunk/test/Analysis/new-ctor-conservative.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/new-ctor-conservative.cpp?rev=322774&view=auto
==============================================================================
--- cfe/trunk/test/Analysis/new-ctor-conservative.cpp (added)
+++ cfe/trunk/test/Analysis/new-ctor-conservative.cpp Wed Jan 17 14:34:23 2018
@@ -0,0 +1,14 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-config c++-allocator-inlining=true -std=c++11 -verify %s
+
+void clang_analyzer_eval(bool);
+
+struct S {
+  int x;
+  S() : x(1) {}
+  ~S() {}
+};
+
+void checkConstructorInlining() {
+  S *s = new S;
+  clang_analyzer_eval(s->x == 1); // expected-warning{{TRUE}}
+}

Added: cfe/trunk/test/Analysis/new-ctor-inlined.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/new-ctor-inlined.cpp?rev=322774&view=auto
==============================================================================
--- cfe/trunk/test/Analysis/new-ctor-inlined.cpp (added)
+++ cfe/trunk/test/Analysis/new-ctor-inlined.cpp Wed Jan 17 14:34:23 2018
@@ -0,0 +1,40 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-config c++-allocator-inlining=true -std=c++11 -verify %s
+
+void clang_analyzer_eval(bool);
+
+typedef __typeof__(sizeof(int)) size_t;
+
+void *conjure();
+void exit(int);
+
+void *operator new(size_t size) throw() {
+  void *x = conjure();
+  if (x == 0)
+    exit(1);
+  return x;
+}
+
+struct S {
+  int x;
+  S() : x(1) {}
+  ~S() {}
+};
+
+void checkNewAndConstructorInlining() {
+  S *s = new S;
+  // Check that the symbol for 's' is not dying.
+  clang_analyzer_eval(s != 0); // expected-warning{{TRUE}}
+  // Check that bindings are correct (and also not dying).
+  clang_analyzer_eval(s->x == 1); // expected-warning{{TRUE}}
+}
+
+struct Sp {
+  Sp *p;
+  Sp(Sp *p): p(p) {}
+  ~Sp() {}
+};
+
+void checkNestedNew() {
+  Sp *p = new Sp(new Sp(0));
+  clang_analyzer_eval(p->p->p == 0); // expected-warning{{TRUE}}
+}

Added: cfe/trunk/test/Analysis/new-ctor-recursive.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/new-ctor-recursive.cpp?rev=322774&view=auto
==============================================================================
--- cfe/trunk/test/Analysis/new-ctor-recursive.cpp (added)
+++ cfe/trunk/test/Analysis/new-ctor-recursive.cpp Wed Jan 17 14:34:23 2018
@@ -0,0 +1,118 @@
+// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus.NewDelete,cplusplus.NewDeleteLeaks,debug.ExprInspection -analyzer-config c++-allocator-inlining=true -std=c++11 -verify %s
+
+void clang_analyzer_eval(bool);
+void clang_analyzer_dump(int);
+
+typedef __typeof__(sizeof(int)) size_t;
+
+void *conjure();
+void exit(int);
+
+struct S;
+
+S *global_s;
+
+// Recursive operator kinda placement new.
+void *operator new(size_t size, S *place);
+
+enum class ConstructionKind : char {
+  Garbage,
+  Recursive
+};
+
+struct S {
+public:
+  int x;
+  S(): x(1) {}
+  S(int y): x(y) {}
+
+  S(ConstructionKind k) {
+    switch (k) {
+    case ConstructionKind::Recursive: { // Call one more operator new 'r'ecursively.
+      S *s = new (nullptr) S(5);
+      x = s->x + 1;
+      global_s = s;
+      return;
+    }
+    case ConstructionKind::Garbage: {
+      // Leaves garbage in 'x'.
+    }
+    }
+  }
+  ~S() {}
+};
+
+// Do not try this at home!
+void *operator new(size_t size, S *place) {
+  if (!place)
+    return new S();
+  return place;
+}
+
+void testThatCharConstructorIndeedYieldsGarbage() {
+  S *s = new S(ConstructionKind::Garbage);
+  clang_analyzer_eval(s->x == 0); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(s->x == 1); // expected-warning{{UNKNOWN}}
+  // FIXME: This should warn, but MallocChecker doesn't default-bind regions
+  // returned by standard operator new to garbage.
+  s->x += 1; // no-warning
+  delete s;
+}
+
+
+void testChainedOperatorNew() {
+  S *s;
+  // * Evaluate standard new.
+  // * Evaluate constructor S(3).
+  // * Bind value for standard new.
+  // * Evaluate our custom new.
+  // * Evaluate constructor S(Garbage).
+  // * Bind value for our custom new.
+  s = new (new S(3)) S(ConstructionKind::Garbage);
+  clang_analyzer_eval(s->x == 3); // expected-warning{{TRUE}}
+  // expected-warning at +9{{Potential leak of memory pointed to by 's'}}
+
+  // * Evaluate standard new.
+  // * Evaluate constructor S(Garbage).
+  // * Bind value for standard new.
+  // * Evaluate our custom new.
+  // * Evaluate constructor S(4).
+  // * Bind value for our custom new.
+  s = new (new S(ConstructionKind::Garbage)) S(4);
+  clang_analyzer_eval(s->x == 4); // expected-warning{{TRUE}}
+  delete s;
+
+  // -> Enter our custom new (nullptr).
+  //   * Evaluate standard new.
+  //   * Inline constructor S().
+  //   * Bind value for standard new.
+  // <- Exit our custom new (nullptr).
+  // * Evaluate constructor S(Garbage).
+  // * Bind value for our custom new.
+  s = new (nullptr) S(ConstructionKind::Garbage);
+  clang_analyzer_eval(s->x == 1); // expected-warning{{TRUE}}
+  delete s;
+
+  // -> Enter our custom new (nullptr).
+  //   * Evaluate standard new.
+  //   * Inline constructor S().
+  //   * Bind value for standard new.
+  // <- Exit our custom new (nullptr).
+  // -> Enter constructor S(Recursive).
+  //   -> Enter our custom new (nullptr).
+  //     * Evaluate standard new.
+  //     * Inline constructor S().
+  //     * Bind value for standard new.
+  //   <- Exit our custom new (nullptr).
+  //   * Evaluate constructor S(5).
+  //   * Bind value for our custom new (nullptr).
+  //   * Assign that value to global_s.
+  // <- Exit constructor S(Recursive).
+  // * Bind value for our custom new (nullptr).
+  global_s = nullptr;
+  s = new (nullptr) S(ConstructionKind::Recursive);
+  clang_analyzer_eval(global_s); // expected-warning{{TRUE}}
+  clang_analyzer_eval(global_s->x == 5); // expected-warning{{TRUE}}
+  clang_analyzer_eval(s->x == 6); // expected-warning{{TRUE}}
+  delete s;
+}

Added: cfe/trunk/test/Analysis/new-ctor-symbolic.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/new-ctor-symbolic.cpp?rev=322774&view=auto
==============================================================================
--- cfe/trunk/test/Analysis/new-ctor-symbolic.cpp (added)
+++ cfe/trunk/test/Analysis/new-ctor-symbolic.cpp Wed Jan 17 14:34:23 2018
@@ -0,0 +1,33 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-config c++-allocator-inlining=true -std=c++11 -verify %s
+
+void clang_analyzer_eval(bool);
+void clang_analyzer_warnOnDeadSymbol(int);
+
+typedef __typeof__(sizeof(int)) size_t;
+
+int conjure();
+void exit(int);
+
+struct S {
+  S() {}
+  ~S() {}
+
+  static S buffer[1000];
+
+  // This operator allocates stuff within the buffer. Additionally, it never
+  // places anything at the beginning of the buffer.
+  void *operator new(size_t size) {
+    int i = conjure();
+    if (i == 0)
+      exit(1);
+    // Let's see if the symbol dies before new-expression is evaluated.
+    // It shouldn't.
+    clang_analyzer_warnOnDeadSymbol(i);
+    return buffer + i;
+  }
+};
+
+void testIndexLiveness() {
+  S *s = new S();
+  clang_analyzer_eval(s == S::buffer); // expected-warning{{FALSE}}
+} // expected-warning{{SYMBOL DEAD}}




More information about the cfe-commits mailing list