[analyzer] PtrArithChecker: Update to use check::NewAllocator

[NoQ]

Summary

Make use of the new callback introduced in D41406 for tracking values allocated by operator new() in -analyzer-config c++-allocator-inlining=true mode. Most of the patch actually has no intended functional changes, apart from the StripCasts part, which is similar to D41796 and fixes two tests in ptr-arith.cpp (checkNew() and getArray()).

(@xazax.hun - this is an alpha checker last touched by you, do you still have plans for it?)

[xazax.hun]

(@xazax.hun - this is an alpha checker last touched by you, do you still have plans for it?)

I think this check could be very useful. Last I checked it was too noisy (e.g.: around 850 hits on LLVM), and I did plan to look into what are the main sources of false positives, but I did not have time to do so yet.

[NoQ]

D41250#971888 reverts the change in how casts work, and the checker doesn't really need the casted value, because it only tracks the pointer.

This still leaves us with the problem of PostStmt<CXXNewExpr> being called twice, so i'd keep this revision around so that i didn't forget to fix this; same probably needs to be done for dynamic type checker.

lib/StaticAnalyzer/Checkers/PointerArithChecker.cpp

 } // end namespace llvm
 
 namespace {
 class PointerArithChecker
     : public Checker<
           check::PreStmt<BinaryOperator>, check::PreStmt<UnaryOperator>,
           check::PreStmt<ArraySubscriptExpr>, check::PreStmt<CastExpr>,
           check::PostStmt<CastExpr>, check::PostStmt<CXXNewExpr>,
-          check::PostStmt<CallExpr>, check::DeadSymbols> {
+          check::PostStmt<CallExpr>, check::DeadSymbols, check::NewAllocator> {
   AllocKind getKindOfNewOp(const CXXNewExpr *NE, const FunctionDecl *FD) const;
   const MemRegion *getArrayRegion(const MemRegion *Region, bool &Polymorphic,
                                   AllocKind &AKind, CheckerContext &C) const;
   const MemRegion *getPointedRegion(const MemRegion *Region,
                                     CheckerContext &C) const;
   void reportPointerArithMisuse(const Expr *E, CheckerContext &C,
                                 bool PointedNeeded = false) const;
   void initAllocIdentifiers(ASTContext &C) const;
 
+  void processNewAllocatorAux(const CXXNewExpr *NE, SVal Target,
+                              CheckerContext &C) const;
+
   mutable std::unique_ptr<BuiltinBug> BT_pointerArith;
   mutable std::unique_ptr<BuiltinBug> BT_polyArray;
   mutable llvm::SmallSet<IdentifierInfo *, 8> AllocFunctions;
 
 public:
   void checkPreStmt(const UnaryOperator *UOp, CheckerContext &C) const;
   void checkPreStmt(const BinaryOperator *BOp, CheckerContext &C) const;
   void checkPreStmt(const ArraySubscriptExpr *SubExpr, CheckerContext &C) const;
   void checkPreStmt(const CastExpr *CE, CheckerContext &C) const;
   void checkPostStmt(const CastExpr *CE, CheckerContext &C) const;
   void checkPostStmt(const CXXNewExpr *NE, CheckerContext &C) const;
   void checkPostStmt(const CallExpr *CE, CheckerContext &C) const;
   void checkDeadSymbols(SymbolReaper &SR, CheckerContext &C) const;
+  void checkNewAllocator(const CXXNewExpr *NE, SVal Target,
+                         CheckerContext &C) const;
 };
 } // end namespace
 
 REGISTER_MAP_WITH_PROGRAMSTATE(RegionState, const MemRegion *, AllocKind)
 
 void PointerArithChecker::checkDeadSymbols(SymbolReaper &SR,
                                            CheckerContext &C) const {
   // TODO: intentional leak. Some information is garbage collected too early,
void PointerArithChecker::checkPostStmt(const CallExpr *CE,
   // Assume that C allocation functions allocate arrays to avoid false
   // positives.
   // TODO: Add heuristics to distinguish alloc calls that allocates single
   // objecs.
   State = State->set<RegionState>(Region, AllocKind::Array);
   C.addTransition(State);
 }
 
-void PointerArithChecker::checkPostStmt(const CXXNewExpr *NE,
+void PointerArithChecker::processNewAllocatorAux(const CXXNewExpr *NE,
+                                                 SVal Target,
                                                  CheckerContext &C) const {
   const FunctionDecl *FD = NE->getOperatorNew();
   if (!FD)
     return;
 
-  AllocKind Kind = getKindOfNewOp(NE, FD);
-
-  ProgramStateRef State = C.getState();
-  SVal AllocedVal = State->getSVal(NE, C.getLocationContext());
-  const MemRegion *Region = AllocedVal.getAsRegion();
+  const MemRegion *Region = Target.getAsRegion();
   if (!Region)
     return;
-  State = State->set<RegionState>(Region, Kind);
-  C.addTransition(State);
+
+  Region = Region->StripCasts();
+
+  AllocKind Kind = getKindOfNewOp(NE, FD);
+
+  C.addTransition(C.getState()->set<RegionState>(Region, Kind));
+}
+
+void PointerArithChecker::checkPostStmt(const CXXNewExpr *NE,
+                                        CheckerContext &C) const {
+  if (!C.getAnalysisManager().getAnalyzerOptions().mayInlineCXXAllocator())
+    processNewAllocatorAux(NE, C.getSVal(NE), C);
+}
+
+void PointerArithChecker::checkNewAllocator(const CXXNewExpr *NE,
+    SVal Target, CheckerContext &C) const {
+  if (!C.wasInlined)
+    processNewAllocatorAux(NE, Target, C);
 }
 
 void PointerArithChecker::checkPostStmt(const CastExpr *CE,
                                         CheckerContext &C) const {
   if (CE->getCastKind() != CastKind::CK_BitCast)
     return;
 
   const Expr *CastedExpr = CE->getSubExpr();

test/Analysis/ptr-arith.cpp

 // RUN: %clang_analyze_cc1 -Wno-unused-value -std=c++14 -analyzer-checker=core,debug.ExprInspection,alpha.core.PointerArithm -verify %s
+// RUN: %clang_analyze_cc1 -Wno-unused-value -std=c++14 -analyzer-checker=core,debug.ExprInspection,alpha.core.PointerArithm -analyzer-config c++-allocator-inlining=true -verify %s
 struct X {
   int *p;
   int zero;
   void foo () {
     reset(p - 1);
   }
   void reset(int *in) {
     while (in != p) // Loop must be entered.