[analyzer] Escape symbols stored into specific region after a conservative evalcall.

[xazax.hun]

Summary

This patch introduced additional PointerEscape callbacks after conservative calls for output parameters.
This should not really affect the current checkers but the upcoming FuchsiaHandleChecker relies on this heavily.

[xazax.hun]

clang/include/clang/StaticAnalyzer/Core/CheckerManager.h

409

Actually, it is possible I went too far with plumbing RegionAndSymbolInvalidationTraits.

[xazax.hun]

clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp

715

After some offline conversation it is very likely that we want to move the runCheckersForPointerEscape here.

The main question is, how should we get all the data?

We should know about:

• What regions are output params.
• What regions are considered escaped.
• What regions have traits that prevents escaping.

Is there anything else?

[NoQ]

clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp

715

What regions are output params.

That should be obvious from the AST. Like, parameters of non-const pointer/reference types.

What regions are considered escaped.

Output parameter regions (as TopLevelInvalidated) and whatever's reachable from them.

What regions have traits that prevents escaping.

Currently the only trait that affects escaping (as opposed to invalidation) is TK_SuppressEscape and it is never applied to out-parameters.

[xazax.hun]

Also, it looks like for example CStringChecker calls ProgramState::InvalidateRegions directly when modeling function calls. So it looks like if checkers are using evalCall, pointerEscape will not be triggered automatically. I am not sure if that would be a bug or feature :D

[NoQ]

In D71224#1776052, @xazax.hun wrote:

Also, it looks like for example CStringChecker calls ProgramState::InvalidateRegions directly when modeling function calls. So it looks like if checkers are using evalCall, pointerEscape will not be triggered automatically. I am not sure if that would be a bug or feature :D

I'd say it's a feature, because evalCall is the official way for checkers to opt out of babysitting.

[xazax.hun]

• Prototype a new approach.

[xazax.hun]

clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp

622

How much do we care about the escape kind? For each symbol we need to check if it was directly passed to the callee. It is not too bad I guess, but I was wondering.

715

After some offline conversation it is very likely that we want to move the runCheckersForPointerEscape here.

Nope, we do not want to move it here. We want the pointerEscape to happen AFTER postCall callbacks.

1127

Hmm. Should I introduce a very short lived trait or do we have a better way to deal with stuff like this? A short lived map?

[NoQ]

I think you don't need to smuggle WasConservative all the way up. It's implied that if the evaluation was not conservative, then the respective ExplodedNodeSet is going to be empty, as all nodes will be put directly into the worklist instead. Eg., checkPostCall isn't going to be invoked immediately after inlineCall, but only after enqueueEndOfFunction.

clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp

622

Dunno, just introduce a new PSK_ item and use it here. It isn't supposed to be per-symbol, it's just to notify checkers that we're in this new post-call invocation for out-parameters, so that they could opt out of the whole callback.

[xazax.hun]

• Do not track explicitly if a call was conservative.

[xazax.hun]

• Added a test. More rigorous tests will come in the FuchsiaHandleChecker.
• Added a new PSK_ entry.

[NoQ]

clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp

604

WDYT about re-using ExprEngine::escapeValue() by changing it to accept ArrayRef<SVal> instead of a single SVal?

[xazax.hun]

clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp

604

It is not entirely the same. Here we only collect symbols from non-stack regions. There (as far as I understand) we collect all symbols. I could add a flag but at that point I wonder if it is worth the change.

[NoQ]

clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp

604

Umm, wait, right, so what are you doing in this callback to begin with? The code says "gather all symbols that are encountered as immediate values stored in traversed regions". Why not simply "gather all symbols that are traversed from parameter regions"?

649

I guess technically, for our own sanity, it's worth it to-rescan the symbols for every node in dstPostCall. But i'll be very surprised if they are actually going to yield different results for every predecessor node.

[xazax.hun]

clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp

604

My understanding is the following but correct me if I am wrong:

int *getConjuredSymbol();

call(getConjuredSymbol());

So we have can talk about two symbols here. One symbol is what was returned by getConjuredSymbol and the other is the pointee, the symbol that we get when we dereference the result of getConjuredSymbol. And my understanding is that we want to invoke escape for the latter and not the former. ExprEngine::escapeValue invalidates the former but not the latter. The visitor here invalidates the latter but not the former. This behavior solves most of my test cases in FuchsiaHandleChecker.

[xazax.hun]

clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp

649

I think it is possible. We use the state to get the pointee of some pointers, so in case the PostCall splits the state on the value of the outputs it would be reasonable to get different results.

[NoQ]

clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp

604

How about escapeValue(getSVal(getArgSVal()))? (the type for getSVal() could be obtained from the AST).

[xazax.hun]

clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp

604

Hmm, I believe that could work, thanks!

[xazax.hun]

• Reuse ExprEngine::escapeValue.

[NoQ]

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

637

Dunno, should we rename to escapeValues()?

clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp

615–617

I encourage Call.parameters(). This way you won't need to obtain a FuncDecl. In fact you won't even need it to be a FunctionDecl.

625

Ok, so this patch interacts with D71152 in a non-trivial manner. We should re-use the logic that decides whether an escape on bind occurs.

[xazax.hun]

clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp

625

Yeah, I just started to look into it and some skeletons fell out.

Let's consider the following example:

void leak() {
  zx_handle_t sa, sb;
  if (zx_channel_create(0, &sa, &sb))
    return;
  zx_handle_close(sb);
}

After using the same logic we can no longer detect the leak. The reason is, after zx_channel_create both sa and sb regions are escaped (and they become escaped during the conservative call) and then after the PostCall callback, when we attached the traits to our symbols, we will trigger pointer escape immediately, and lose the traits we just added.

I am not immediately sure what to do here. I feel like the main source of the problem is the fact that zx_channel_create should not escape anything and the checker has now way to communicate that to the analyzer core. Any ideas?

[xazax.hun]

clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp

625

Suddenly, I see four ways forward, none of which is optimal. Hopefully, you can come up with something better.

1. Do not care about escaped local regions in this code path. So basically we would tune the invalidation down a bit.
2. Make it possible for modeling checkers to prevent escaping. It would be great if there would be a way to do that without doing EvalCall.
3. Require the users to annotate out parameters. And we could assume that out parameters do not escape.
4. Do some heuristics like if a checker just attached some info in a PostCall, it is likely that we should not escape that symbol.

[xazax.hun]

clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp

625

Or:

5. Since output arguments might be more common than escapes in practice, maybe we could just not escape local regions at all by default only if there is a special annotation or a modelling check asks for it. So users would get one more tool to suppress false positives but we would not overdo invalidation by default.

[NoQ]

In any case, every checker is allowed to make their own decisions about escaping. Escape on its own is not material, it's all about how the checker reacts to escapes. Say, it's up to MallocChecker to decide whether the function may or may not release memory that escapes on call.

I think a valid approach would be to simply look up the function in your CallDescriptionMap and then abort the checkPointerEscape callback when it's found.

Yet, it annoys me a bit that we didn't make everything magically work in an "out of the box" manner. Can we eliminate the first pointer escape (that happens before PostCall) but only keep the secondary escape?

[xazax.hun]

In D71224#1778179, @NoQ wrote:

In any case, every checker is allowed to make their own decisions about escaping. Escape on its own is not material, it's all about how the checker reacts to escapes. Say, it's up to MallocChecker to decide whether the function may or may not release memory that escapes on call.

I think a valid approach would be to simply look up the function in your CallDescriptionMap and then abort the checkPointerEscape callback when it's found.

Yet, it annoys me a bit that we didn't make everything magically work in an "out of the box" manner. Can we eliminate the first pointer escape (that happens before PostCall) but only keep the secondary escape?

I don't think this is a good enough model currently. The problem is that, it does not play well with annotations. E.g. the checker can see a symbol escaping, but it does not have a whole lot of information how. For example, currently, there is no way to check if the output parameter through which the escape happened was annotated somehow.

[NoQ]

In D71224#1778204, @xazax.hun wrote:

I don't think this is a good enough model currently. The problem is that, it does not play well with annotations. E.g. the checker can see a symbol escaping, but it does not have a whole lot of information how. For example, currently, there is no way to check if the output parameter through which the escape happened was annotated somehow.

Hmm. If the function is annotated, it is hopefully "fully" annotated, or at least the programmer doesn't mind adding more annotations to it. Given that you have your CallEvent structure in checkPointerEscape, i hope you can easily see if there are any annotations at all on the function, and if so, suppress the current escape entirely. Or at least scan the annotated parameters and suppress the escape for them.

I guess it's still a problem if the *same* handle is also passed through a parameter that *cannot* be annotated (eg., as part of a structure passed into the call) and then actually getting released inside the call, but is it a real problem for you?

[xazax.hun]

In D71224#1778231, @NoQ wrote:

In D71224#1778204, @xazax.hun wrote:

I don't think this is a good enough model currently. The problem is that, it does not play well with annotations. E.g. the checker can see a symbol escaping, but it does not have a whole lot of information how. For example, currently, there is no way to check if the output parameter through which the escape happened was annotated somehow.

Hmm. If the function is annotated, it is hopefully "fully" annotated, or at least the programmer doesn't mind adding more annotations to it. Given that you have your CallEvent structure in checkPointerEscape, i hope you can easily see if there are any annotations at all on the function, and if so, suppress the current escape entirely. Or at least scan the annotated parameters and suppress the escape for them.

I guess it's still a problem if the *same* handle is also passed through a parameter that *cannot* be annotated (eg., as part of a structure passed into the call) and then actually getting released inside the call, but is it a real problem for you?

Yeah, this was one of my idea as well. I think one of my main concerns is that I would except the majority of the escapes are simply being output parameters and only a minority are legitimate. So I was wondering if we got the default right. Maybe a checker should do more work to get the escaping rather than more work preventing it?

[NoQ]

In D71224#1778284, @xazax.hun wrote:

So I was wondering if we got the default right. Maybe a checker should do more work to get the escaping rather than more work preventing it?

But that's exactly how it works right now(?) If you don't define checkPointerEscape you get no escaping, if you do extra work of defining it you get the exact amount of escaping that you want.

[xazax.hun]

In D71224#1778303, @NoQ wrote:

In D71224#1778284, @xazax.hun wrote:

So I was wondering if we got the default right. Maybe a checker should do more work to get the escaping rather than more work preventing it?

But that's exactly how it works right now(?) If you don't define checkPointerEscape you get no escaping, if you do extra work of defining it you get the exact amount of escaping that you want.

So basically what I am wonder/worrying about is the following:
The analyzer core will decide that the stack region is escaped and the checkers has no word about this. And from that time on the checkers have to do extra work each time there is a store or conservative call to find out if this escape corresponds to a region that was escaped earlier unwillingly (from the checker's point of view).

[xazax.hun]

I think I found the main problem with the current model, at least for the FuchsiaHandleCheck.

Consider the following two snippets:

zx_handle_t *get_handle_address();
void escape_store_to_escaped_region01() {
  zx_handle_t sb;
  if (zx_channel_create(0, get_handle_address(), &sb))
    return;
  zx_handle_close(sb);
}

void leak() {
  zx_handle_t sa, sb;
  if (zx_channel_create(0, &sa, &sb))
    return;
  zx_handle_close(sb);
}

In the first one I want the first handle to be escaped in the second one I do not want it to be escaped.

With my current proposed changes the checker will receive a pointer escape callback for both but it does not have enough info to differentiate between the two cases.

If I do not act upon this kind of escape I end up reporting a false positive in the first case. If I act on this escape I end up missing a true positive in the second case.

[xazax.hun]

• First take on trying to reuse processPointerEscapedOnBind.

[NoQ]

In D71224#1778332, @xazax.hun wrote:

So basically what I am wonder/worrying about is the following:
The analyzer core will decide that the stack region is escaped and the checkers has no word about this.

Yup, you got me. Pre-escaped locals are indeed material and beyond the checker's control. I don't seem to have any immediate solutions. I think we could postpone the work on pre-escaped locals (until we figure out how to do them correctly) if they're not immediately necessary to you (after all i was the one who suggested it). Or ignore the problem (depending on how we do our FP vs. FN trade-off).

In D71224#1778357, @xazax.hun wrote:

Consider the following two snippets:

Mm, these snippets don't have pre-escaped locals. Like, they accidentally do, but above i proposed to work around this by removing the first escape invocation (that happens during the call) and only doing it after the call. This way these locals don't have time to become pre-escaped. I think these are not a problem.

[xazax.hun]

• Rebasing after reverting the pre-escaping.

[NoQ]

Thanks!! Nothing controversial remains here, right? :)

clang/include/clang/StaticAnalyzer/Core/CheckerManager.h

91

All three escape-on-calls are on conservatively evaluated calls only, so this name isn't very descriptive.

Maybe PSK_EscapeOutParameters?

clang/lib/StaticAnalyzer/Core/ExprEngine.cpp

2736–2738

Dunno, make_pair?

clang/include/clang/StaticAnalyzer/Checkers/Checkers.td

CmdLineOption<Boolean,
                   Hide>,
     CmdLineOption<Boolean,
                   "RegionChanges",
                   "",
                   "false",
                   Released,
                   Hide>,
     CmdLineOption<Boolean,
+                  "PointerEscape",
+                  "",
+                  "false",
+                  Released,
+                  Hide>,
+    CmdLineOption<Boolean,
                   "*",
                   "Enables all callbacks.",
                   "false",
                   Released,
                   Hide>
   ]>,
   Documentation<NotDocumented>;
 

clang/include/clang/StaticAnalyzer/Core/CheckerManager.h

enum PointerEscapeKind {
   /// The pointer has been passed to a function call directly.
   PSK_DirectEscapeOnCall,
 
   /// The pointer has been passed to a function indirectly.
   /// For example, the pointer is accessible through an
   /// argument to a function.
   PSK_IndirectEscapeOnCall,
 
+
+  /// Escape for a new symbol that was generated into a region
+  /// that the analyzer cannot follow during a conservative call.
+  PSK_EscapeOutParameters,
+
   /// The reason for pointer escape is unknown. For example,
   /// a region containing this pointer is invalidated.
   PSK_EscapeOther
 };
 
 /// This wrapper is used to ensure that only StringRefs originating from the
 /// CheckerRegistry are used as check names. We want to make sure all checker
 /// name strings have a lifetime that keeps them alive at least until the path
//===----------------------------------------------------------------------===//
   ///
   /// Warning: Currently, the CallEvent MUST come from a CallExpr!
   void runCheckersForEvalCall(ExplodedNodeSet &Dst,
                               const ExplodedNodeSet &Src,
                               const CallEvent &CE, ExprEngine &Eng);
 
   /// Run checkers for the entire Translation Unit.
   void runCheckersOnEndOfTranslationUnit(const TranslationUnitDecl *TU,
                                          AnalysisManager &mgr,
                                          BugReporter &BR);
 
   /// Run checkers for debug-printing a ProgramState.
   ///
   /// Unlike most other callbacks, any checker can simply implement the virtual
   /// method CheckerBase::printState if it has custom data to print.
   ///
   /// \param Out   The output stream

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

 protected:
   /// evalBind - Handle the semantics of binding a value to a specific location.
   ///  This method is used by evalStore, VisitDeclStmt, and others.
   void evalBind(ExplodedNodeSet &Dst, const Stmt *StoreE, ExplodedNode *Pred,
                 SVal location, SVal Val, bool atDeclInit = false,
                 const ProgramPoint *PP = nullptr);
 
   /// Call PointerEscape callback when a value escapes as a result of bind.
-  ProgramStateRef processPointerEscapedOnBind(ProgramStateRef State,
-                                              SVal Loc,
-                                              SVal Val,
-                                              const LocationContext *LCtx) override;
+  ProgramStateRef processPointerEscapedOnBind(
+      ProgramStateRef State, ArrayRef<std::pair<SVal, SVal>> LocAndVals,
+      const LocationContext *LCtx, PointerEscapeKind Kind,
+      const CallEvent *Call) override;
+
+  ProgramStateRef
+  processPointerEscapedOnBind(ProgramStateRef State,
+                              SVal Loc, SVal Val,
+                              const LocationContext *LCtx);
+
   /// Call PointerEscape callback when a value escapes as a result of
   /// region invalidation.
   /// \param[in] ITraits Specifies invalidation traits for regions/symbols.
   ProgramStateRef notifyCheckersOfPointerEscape(
                            ProgramStateRef State,
                            const InvalidatedSymbols *Invalidated,
                            ArrayRef<const MemRegion *> ExplicitRegions,
                            const CallEvent *Call,
                            RegionAndSymbolInvalidationTraits &ITraits) override;
 
   /// A simple wrapper when you only need to notify checkers of pointer-escape
-  /// of a single value.
-  ProgramStateRef escapeValue(ProgramStateRef State, SVal V,
-                              PointerEscapeKind K) const;
+  /// of some values.
+  ProgramStateRef escapeValues(ProgramStateRef State, ArrayRef<SVal> Vs,
+                               PointerEscapeKind K,
+                               const CallEvent *Call = nullptr) const;
 
 public:
   // FIXME: 'tag' should be removed, and a LocationContext should be used
   // instead.
   // FIXME: Comment on the meaning of the arguments, when 'St' may not
   // be the same as Pred->state, and when 'location' may not be the
   // same as state->getLValue(Ex).
   /// Simulate a read of the result of Ex.

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

 //
 //===----------------------------------------------------------------------===//
 #ifndef LLVM_CLANG_STATICANALYZER_CORE_PATHSENSITIVE_SUBENGINE_H
 #define LLVM_CLANG_STATICANALYZER_CORE_PATHSENSITIVE_SUBENGINE_H
 
 #include "clang/Analysis/ProgramPoint.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/Store.h"
+#include "clang/StaticAnalyzer/Core/CheckerManager.h"
 
 namespace clang {
 
 class CFGBlock;
 class CFGElement;
 class LocationContext;
 class Stmt;
 
public:
 
   inline ProgramStateRef
   processRegionChange(ProgramStateRef state,
                       const MemRegion* MR,
                       const LocationContext *LCtx) {
     return processRegionChanges(state, nullptr, MR, MR, LCtx, nullptr);
   }
 
-  virtual ProgramStateRef
-  processPointerEscapedOnBind(ProgramStateRef State, SVal Loc, SVal Val, const LocationContext *LCtx) = 0;
+  virtual ProgramStateRef processPointerEscapedOnBind(
+      ProgramStateRef State, ArrayRef<std::pair<SVal, SVal>> LocAndVals,
+      const LocationContext *LCtx, PointerEscapeKind Kind,
+      const CallEvent *Call) = 0;
 
   virtual ProgramStateRef
   notifyCheckersOfPointerEscape(ProgramStateRef State,
                            const InvalidatedSymbols *Invalidated,
                            ArrayRef<const MemRegion *> ExplicitRegions,
                            const CallEvent *Call,
                            RegionAndSymbolInvalidationTraits &HTraits) = 0;
 

clang/lib/StaticAnalyzer/Checkers/AnalysisOrderChecker.cpp

: public Checker<check::PreStmt<CastExpr>,
                      check::PostStmt<CXXNewExpr>,
                      check::PreStmt<OffsetOfExpr>,
                      check::PostStmt<OffsetOfExpr>,
                      check::PreCall,
                      check::PostCall,
                      check::EndFunction,
                      check::NewAllocator,
                      check::Bind,
+                     check::PointerEscape,
                      check::RegionChanges,
                      check::LiveSymbols> {
 
   bool isCallbackEnabled(AnalyzerOptions &Opts, StringRef CallbackName) const {
     return Opts.getCheckerBooleanOption(this, "*") ||
            Opts.getCheckerBooleanOption(this, CallbackName);
   }
 
checkRegionChanges(ProgramStateRef State,
                      const InvalidatedSymbols *Invalidated,
                      ArrayRef<const MemRegion *> ExplicitRegions,
                      ArrayRef<const MemRegion *> Regions,
                      const LocationContext *LCtx, const CallEvent *Call) const {
     if (isCallbackEnabled(State, "RegionChanges"))
       llvm::errs() << "RegionChanges\n";
     return State;
   }
+
+  ProgramStateRef checkPointerEscape(ProgramStateRef State,
+                                     const InvalidatedSymbols &Escaped,
+                                     const CallEvent *Call,
+                                     PointerEscapeKind Kind) const {
+    if (isCallbackEnabled(State, "PointerEscape"))
+      llvm::errs() << "PointerEscape\n";
+    return State;
+  }
 };
 } // end anonymous namespace
 
 //===----------------------------------------------------------------------===//
 // Registration.
 //===----------------------------------------------------------------------===//
 
 void ento::registerAnalysisOrderChecker(CheckerManager &mgr) {
   mgr.registerChecker<AnalysisOrderChecker>();
 }
 
 bool ento::shouldRegisterAnalysisOrderChecker(const LangOptions &LO) {
   return true;
 }

clang/lib/StaticAnalyzer/Core/ExprEngine.cpp

if (!getObjectUnderConstruction(State, BTE, LC)) {
       // those, we currently skip the CXXBindTemporaryExpr but rely on adding
       // temporary destructor nodes.
       State = addObjectUnderConstruction(State, BTE, LC, UnknownVal());
     }
     StmtBldr.generateNode(BTE, Node, State);
   }
 }
 
-ProgramStateRef ExprEngine::escapeValue(ProgramStateRef State, SVal V,
-                                        PointerEscapeKind K) const {
+ProgramStateRef ExprEngine::escapeValues(ProgramStateRef State,
+                                         ArrayRef<SVal> Vs,
+                                         PointerEscapeKind K,
+                                         const CallEvent *Call) const {
   class CollectReachableSymbolsCallback final : public SymbolVisitor {
-    InvalidatedSymbols Symbols;
+    InvalidatedSymbols &Symbols;
 
   public:
-    explicit CollectReachableSymbolsCallback(ProgramStateRef) {}
+    explicit CollectReachableSymbolsCallback(InvalidatedSymbols &Symbols)
+        : Symbols(Symbols) {}
 
     const InvalidatedSymbols &getSymbols() const { return Symbols; }
 
     bool VisitSymbol(SymbolRef Sym) override {
       Symbols.insert(Sym);
       return true;
     }
   };
+  InvalidatedSymbols Symbols;
+  CollectReachableSymbolsCallback CallBack(Symbols);
+  for (SVal V : Vs)
+    State->scanReachableSymbols(V, CallBack);
 
-  const CollectReachableSymbolsCallback &Scanner =
-      State->scanReachableSymbols<CollectReachableSymbolsCallback>(V);
   return getCheckerManager().runCheckersForPointerEscape(
-      State, Scanner.getSymbols(), /*CallEvent*/ nullptr, K, nullptr);
+      State, CallBack.getSymbols(), Call, K, nullptr);
 }
 
 void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred,
                        ExplodedNodeSet &DstTop) {
   PrettyStackTraceLoc CrashInfo(getContext().getSourceManager(),
                                 S->getBeginLoc(), "Error evaluating statement");
   ExplodedNodeSet Dst;
   StmtNodeBuilder Bldr(Pred, DstTop, *currBldrCtx);
case Expr::ObjCBoxedExprClass: {
         // Escape pointers passed into the list, unless it's an ObjC boxed
         // expression which is not a boxable C structure.
         if (!(isa<ObjCBoxedExpr>(Ex) &&
               !cast<ObjCBoxedExpr>(Ex)->getSubExpr()
                                       ->getType()->isRecordType()))
           for (auto Child : Ex->children()) {
             assert(Child);
             SVal Val = State->getSVal(Child, LCtx);
-            State = escapeValue(State, Val, PSK_EscapeOther);
+            State = escapeValues(State, Val, PSK_EscapeOther);
           }
 
         Bldr2.generateNode(S, N, State);
       }
 
       getCheckerManager().runCheckersForPostStmt(Dst, Tmp, S, *this);
       Bldr.addNodes(Dst);
       break;

 
 // A value escapes in four possible cases:
 // (1) We are binding to something that is not a memory region.
 // (2) We are binding to a MemRegion that does not have stack storage.
 // (3) We are binding to a top-level parameter region with a non-trivial
 //     destructor. We won't see the destructor during analysis, but it's there.
 // (4) We are binding to a MemRegion with stack storage that the store
 //     does not understand.
-ProgramStateRef
-ExprEngine::processPointerEscapedOnBind(ProgramStateRef State, SVal Loc,
-                                        SVal Val, const LocationContext *LCtx) {
-
+ProgramStateRef ExprEngine::processPointerEscapedOnBind(
+    ProgramStateRef State, ArrayRef<std::pair<SVal, SVal>> LocAndVals,
+    const LocationContext *LCtx, PointerEscapeKind Kind,
+    const CallEvent *Call) {
+  SmallVector<SVal, 8> Escaped;
+  for (const std::pair<SVal, SVal> &LocAndVal : LocAndVals) {
     // Cases (1) and (2).
-  const MemRegion *MR = Loc.getAsRegion();
-  if (!MR || !MR->hasStackStorage())
-    return escapeValue(State, Val, PSK_EscapeOnBind);
+    const MemRegion *MR = LocAndVal.first.getAsRegion();
+    if (!MR || !MR->hasStackStorage()) {
+      Escaped.push_back(LocAndVal.second);
+      continue;
+    }
 
     // Case (3).
     if (const auto *VR = dyn_cast<VarRegion>(MR->getBaseRegion()))
       if (VR->hasStackParametersStorage() && VR->getStackFrame()->inTopFrame())
         if (const auto *RD = VR->getValueType()->getAsCXXRecordDecl())
-        if (!RD->hasTrivialDestructor())
-          return escapeValue(State, Val, PSK_EscapeOnBind);
+          if (!RD->hasTrivialDestructor()) {
+            Escaped.push_back(LocAndVal.second);
+            continue;
+          }
 
     // Case (4): in order to test that, generate a new state with the binding
     // added. If it is the same state, then it escapes (since the store cannot
     // represent the binding).
     // Do this only if we know that the store is not supposed to generate the
     // same state.
     SVal StoredVal = State->getSVal(MR);
-  if (StoredVal != Val)
-    if (State == (State->bindLoc(loc::MemRegionVal(MR), Val, LCtx)))
-      return escapeValue(State, Val, PSK_EscapeOnBind);
+    if (StoredVal != LocAndVal.second)
+      if (State ==
+          (State->bindLoc(loc::MemRegionVal(MR), LocAndVal.second, LCtx)))
+        Escaped.push_back(LocAndVal.second);
+  }
 
+  if (Escaped.empty())
     return State;
+
+  return escapeValues(State, Escaped, Kind, Call);
+}
+
+ProgramStateRef
+ExprEngine::processPointerEscapedOnBind(ProgramStateRef State, SVal Loc,
+                                        SVal Val, const LocationContext *LCtx) {
+  std::pair<SVal, SVal> LocAndVal(Loc, Val);
+  return processPointerEscapedOnBind(State, LocAndVal, LCtx, PSK_EscapeOnBind,
+                                     nullptr);
 }
 
 ProgramStateRef
 ExprEngine::notifyCheckersOfPointerEscape(ProgramStateRef State,
     const InvalidatedSymbols *Invalidated,
     ArrayRef<const MemRegion *> ExplicitRegions,
     const CallEvent *Call,
     RegionAndSymbolInvalidationTraits &ITraits) {

clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp

if (!B->isAssignmentOp()) {
 
       // Process non-assignments except commas or short-circuited
       // logical expressions (LAnd and LOr).
       SVal Result = evalBinOp(state, Op, LeftV, RightV, B->getType());
       if (!Result.isUnknown()) {
         state = state->BindExpr(B, LCtx, Result);
       } else {
         // If we cannot evaluate the operation escape the operands.
-        state = escapeValue(state, LeftV, PSK_EscapeOther);
-        state = escapeValue(state, RightV, PSK_EscapeOther);
+        state = escapeValues(state, LeftV, PSK_EscapeOther);
+        state = escapeValues(state, RightV, PSK_EscapeOther);
       }
 
       Bldr.generateNode(B, *it, state);
       continue;
     }
 
     assert (B->isCompoundAssignmentOp());
 
ProgramStateRef ExprEngine::handleLValueBitCast(
   // Delegate to SValBuilder to process.
   SVal OrigV = state->getSVal(Ex, LCtx);
   SVal V = svalBuilder.evalCast(OrigV, T, ExTy);
   // Negate the result if we're treating the boolean as a signed i1
   if (CastE->getCastKind() == CK_BooleanToSignedIntegral)
     V = evalMinus(V);
   state = state->BindExpr(CastE, LCtx, V);
   if (V.isUnknown() && !OrigV.isUnknown()) {
-    state = escapeValue(state, OrigV, PSK_EscapeOther);
+    state = escapeValues(state, OrigV, PSK_EscapeOther);
   }
   Bldr.generateNode(CastE, Pred, state);
 
   return state;
 }
 
 ProgramStateRef ExprEngine::handleLVectorSplat(
     ProgramStateRef state, const LocationContext* LCtx, const CastExpr* CastE,

clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp

 //=-- ExprEngineCallAndReturn.cpp - Support for call/return -----*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 //  This file defines ExprEngine's support for calls and returns.
 //
 //===----------------------------------------------------------------------===//
 
+#include "clang/AST/Decl.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
 #include "PrettyStackTraceLocationContext.h"
 #include "clang/AST/CXXInheritance.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/Analysis/Analyses/LiveVariables.h"
 #include "clang/Analysis/ConstructionContext.h"
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
getCheckerManager().runCheckersForEvalCall(dstCallEvaluated, dstPreVisit,
                                              Call, *this);
 
   // If there were other constructors called for object-type arguments
   // of this call, clean them up.
   ExplodedNodeSet dstArgumentCleanup;
   for (auto I : dstCallEvaluated)
     finishArgumentConstruction(dstArgumentCleanup, I, Call);
 
-  // Finally, run any post-call checks.
-  getCheckerManager().runCheckersForPostCall(Dst, dstArgumentCleanup,
+  ExplodedNodeSet dstPostCall;
+  getCheckerManager().runCheckersForPostCall(dstPostCall, dstArgumentCleanup,
                                              Call, *this);
+
+  // Escaping symbols conjured during invalidating the regions above.
+  // Note that, for inlined calls the nodes were put back into the worklist,
+  // so we can assume that every node belongs to a conservative call at this
+  // point.
+
+  // Run pointerEscape callback with the newly conjured symbols.
+  SmallVector<std::pair<SVal, SVal>, 8> Escaped;
+  for (auto I : dstPostCall) {
+    NodeBuilder B(I, Dst, *currBldrCtx);
+    ProgramStateRef State = I->getState();
+    Escaped.clear();
+    {
+      unsigned Arg = -1;
+      for (const ParmVarDecl *PVD : Call.parameters()) {
+        ++Arg;
+        QualType ParamTy = PVD->getType();
+        if (ParamTy.isNull() ||
+            (!ParamTy->isPointerType() && !ParamTy->isReferenceType()))
+          continue;
+        QualType Pointee = ParamTy->getPointeeType();
+        if (Pointee.isConstQualified() || Pointee->isVoidType())
+          continue;
+        if (const MemRegion *MR = Call.getArgSVal(Arg).getAsRegion())
+          Escaped.emplace_back(loc::MemRegionVal(MR), State->getSVal(MR, Pointee));
+      }
+    }
+
+    State = processPointerEscapedOnBind(State, Escaped, I->getLocationContext(),
+                                        PSK_EscapeOutParameters, &Call);
+
+    if (State == I->getState())
+      Dst.insert(I);
+    else
+      B.generateNode(I->getLocation(), State, I);
+  }
 }
 
 ProgramStateRef ExprEngine::bindReturnValue(const CallEvent &Call,
                                             const LocationContext *LCtx,
                                             ProgramStateRef State) {
   const Expr *E = Call.getOriginExpr();
   if (!E)
     return State;
 
   // Some method families have known return values.
   if (const ObjCMethodCall *Msg = dyn_cast<ObjCMethodCall>(&Call)) {
     switch (Msg->getMethodFamily()) {
     default:
       break;
     case OMF_autorelease:
     case OMF_retain:
     case OMF_self: {
       // These methods return their receivers.
       return State->BindExpr(E, LCtx, Msg->getReceiverSVal());
     }
     }
   } else if (const CXXConstructorCall *C = dyn_cast<CXXConstructorCall>(&Call)){
     SVal ThisV = C->getCXXThisVal();
R = IsHeapPointer ? svalBuilder.getConjuredHeapSymbolVal(E, LCtx, Count)
                                                      Count);
   }
   return State->BindExpr(E, LCtx, R);
 }
 
 // Conservatively evaluate call by invalidating regions and binding
 // a conjured return value.
 void ExprEngine::conservativeEvalCall(const CallEvent &Call, NodeBuilder &Bldr,
-                                      ExplodedNode *Pred,
-                                      ProgramStateRef State) {
+                                      ExplodedNode *Pred, ProgramStateRef State) {
   State = Call.invalidateRegions(currBldrCtx->blockCount(), State);
   State = bindReturnValue(Call, Pred->getLocationContext(), State);
 
   // And make the result node.
   Bldr.generateNode(Call.getProgramPoint(), State, Pred);
 }
 
 ExprEngine::CallInlinePolicy
 ExprEngine::mayInlineCallKind(const CallEvent &Call, const ExplodedNode *Pred,
                               AnalyzerOptions &Opts,
                               const ExprEngine::EvalCallOptions &CallOpts) {
   const LocationContext *CurLC = Pred->getLocationContext();
   const StackFrameContext *CallerSFC = CurLC->getStackFrame();
void ExprEngine::BifurcateCall(const MemRegion *BifurReg,
 
   ProgramStateRef NoIState =
       State->set<DynamicDispatchBifurcationMap>(BifurReg,
                                                DynamicDispatchModeConservative);
   conservativeEvalCall(Call, Bldr, Pred, NoIState);
 
   NumOfDynamicDispatchPathSplits++;
 }
 
 void ExprEngine::VisitReturnStmt(const ReturnStmt *RS, ExplodedNode *Pred,
                                  ExplodedNodeSet &Dst) {
   ExplodedNodeSet dstPreVisit;
   getCheckerManager().runCheckersForPreStmt(dstPreVisit, Pred, RS, *this);
 
   StmtNodeBuilder B(dstPreVisit, Dst, *currBldrCtx);
 
   if (RS->getRetValue()) {
     for (ExplodedNodeSet::iterator it = dstPreVisit.begin(),
                                   ei = dstPreVisit.end(); it != ei; ++it) {
       B.generateNode(RS, *it, (*it)->getState());
     }
   }
 }

clang/test/Analysis/analyzer-config.c

 // CHECK-NEXT: ctu-index-name = externalDefMap.txt
 // CHECK-NEXT: deadcode.DeadStores:ShowFixIts = false
 // CHECK-NEXT: deadcode.DeadStores:WarnForDeadNestedAssignments = true
 // CHECK-NEXT: debug.AnalysisOrder:* = false
 // CHECK-NEXT: debug.AnalysisOrder:Bind = false
 // CHECK-NEXT: debug.AnalysisOrder:EndFunction = false
 // CHECK-NEXT: debug.AnalysisOrder:LiveSymbols = false
 // CHECK-NEXT: debug.AnalysisOrder:NewAllocator = false
+// CHECK-NEXT: debug.AnalysisOrder:PointerEscape = false
 // CHECK-NEXT: debug.AnalysisOrder:PostCall = false
 // CHECK-NEXT: debug.AnalysisOrder:PostStmtArraySubscriptExpr = false
 // CHECK-NEXT: debug.AnalysisOrder:PostStmtCXXNewExpr = false
 // CHECK-NEXT: debug.AnalysisOrder:PostStmtCastExpr = false
 // CHECK-NEXT: debug.AnalysisOrder:PostStmtOffsetOfExpr = false
 // CHECK-NEXT: debug.AnalysisOrder:PreCall = false
 // CHECK-NEXT: debug.AnalysisOrder:PreStmtArraySubscriptExpr = false
 // CHECK-NEXT: debug.AnalysisOrder:PreStmtCXXNewExpr = false

 // CHECK-NEXT: suppress-inlined-defensive-checks = true
 // CHECK-NEXT: suppress-null-return-paths = true
 // CHECK-NEXT: track-conditions = true
 // CHECK-NEXT: track-conditions-debug = false
 // CHECK-NEXT: unix.DynamicMemoryModeling:Optimistic = false
 // CHECK-NEXT: unroll-loops = false
 // CHECK-NEXT: widen-loops = false
 // CHECK-NEXT: [stats]
-// CHECK-NEXT: num-entries = 94
+// CHECK-NEXT: num-entries = 95

clang/test/Analysis/expr-inspection.c

 // CHECK-NEXT: }
 
 struct S {
   int x, y;
 };
 
 void test_field_dumps(struct S s, struct S *p) {
   clang_analyzer_dump_pointer(&s.x); // expected-warning{{&s.x}}
-  clang_analyzer_dump_pointer(&p->x); // expected-warning{{&SymRegion{reg_$0<struct S * p>}.x}}
+  clang_analyzer_dump_pointer(&p->x); // expected-warning{{&SymRegion{reg_$1<struct S * p>}.x}}
 }

clang/test/Analysis/pointer-escape-on-conservative-calls.c

+// RUN: %clang_cc1 -analyze -analyzer-checker=debug.AnalysisOrder -analyzer-config debug.AnalysisOrder:PointerEscape=true -analyzer-config debug.AnalysisOrder:PostCall=true %s 2>&1 | FileCheck %s
+
+
+void f(int *);
+int *getMem();
+
+int main() {
+    f(getMem());
+    return 0;
+}
+
+// CHECK: PostCall (f)
+// CHECK-NEXT: PointerEscape