[NFCI][analyzer] invalidateRegions: require explicit State (llvm#164434)

The method `CallEvent::invalidateRegions()` takes (an unsigned
`BlockCount` and) a `ProgramStateRef` which was previously defaulted to
`nullptr` -- and when the state argument was `nullptr`, the method used
the "inner" state of the `CallEvent` as a starting point for the
invalidation.

My recent commit 0f6f13b turned the
"inner" state of the `CallEvent` into a hidden `protected`
implementation detail; so this commit follows that direction by removing
the "defaults to the inner state" behavior from `invalidateRegions` to
avoid exposing the inner state through this channel.

The method `CallEvent::invalidateRegions()` was only called in two
locations, only one of those was relying on the existence of the default
argument value, and even there it was easy to explicitly pass the
`State` object.

This commit also eliminates a footgun: with the old logic, if some code
had tried to pass a state explicitly (presumably because the "inner"
state of the call was obsolete) but that "new" state happened to be
`nullptr`, then `invalidateRegions` would have silently used the inner
state attached to the call. However, I reviewed the call sites and don't
think that it was actually possible to reach this buggy execution path.

Author: NagyDonat

clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h

@@ -372,12 +372,10 @@ class CallEvent {
   ProgramPoint getProgramPoint(bool IsPreVisit = false,
                                const ProgramPointTag *Tag = nullptr) const;
 
-  /// Returns a new state with all argument regions invalidated.
-  ///
-  /// This accepts an alternate state in case some processing has already
-  /// occurred.
+  /// Invalidates the regions (arguments, globals, special regions like 'this')
+  /// that may have been written by this call, returning the updated state.
   ProgramStateRef invalidateRegions(unsigned BlockCount,
-                                    ProgramStateRef Orig = nullptr) const;
+                                    ProgramStateRef State) const;
 
   using FrameBindingTy = std::pair<SVal, SVal>;
   using BindingsTy = SmallVectorImpl<FrameBindingTy>;

clang/lib/StaticAnalyzer/Core/CallEvent.cpp

@@ -230,13 +230,11 @@ static void findPtrToConstParams(llvm::SmallSet<unsigned, 4> &PreserveArgs,
 }
 
 ProgramStateRef CallEvent::invalidateRegions(unsigned BlockCount,
-                                             ProgramStateRef Orig) const {
-  ProgramStateRef Result = (Orig ? Orig : getState());
-
+                                             ProgramStateRef State) const {
   // Don't invalidate anything if the callee is marked pure/const.
-  if (const Decl *callee = getDecl())
-    if (callee->hasAttr<PureAttr>() || callee->hasAttr<ConstAttr>())
-      return Result;
+  if (const Decl *Callee = getDecl())
+    if (Callee->hasAttr<PureAttr>() || Callee->hasAttr<ConstAttr>())
+      return State;
 
   SmallVector<SVal, 8> ValuesToInvalidate;
   RegionAndSymbolInvalidationTraits ETraits;
@@ -278,10 +276,10 @@ ProgramStateRef CallEvent::invalidateRegions(unsigned BlockCount,
   // Invalidate designated regions using the batch invalidation API.
   // NOTE: Even if RegionsToInvalidate is empty, we may still invalidate
   //  global variables.
-  return Result->invalidateRegions(ValuesToInvalidate, getCFGElementRef(),
-                                   BlockCount, getLocationContext(),
-                                   /*CausedByPointerEscape*/ true,
-                                   /*Symbols=*/nullptr, this, &ETraits);
+  return State->invalidateRegions(ValuesToInvalidate, getCFGElementRef(),
+                                  BlockCount, getLocationContext(),
+                                  /*CausedByPointerEscape*/ true,
+                                  /*Symbols=*/nullptr, this, &ETraits);
 }
 
 ProgramPoint CallEvent::getProgramPoint(bool IsPreVisit,

clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp

@@ -1013,7 +1013,7 @@ void ExprEngine::VisitCXXNewExpr(const CXXNewExpr *CNE, ExplodedNode *Pred,
     // FIXME: Once we figure out how we want allocators to work,
     // we should be using the usual pre-/(default-)eval-/post-call checkers
     // here.
-    State = Call->invalidateRegions(blockCount);
+    State = Call->invalidateRegions(blockCount, State);
     if (!State)
       return;