[Analyzer] Add `getReturnValueUnderConstruction()` to `CallEvent`

[baloghadamsoftware]

Summary

Checkers should be able to get the return value under construction for a CallEvent. This patch adds a function to achieve this which retrieves the return value from the construction context of the call.

[Szelethus]

I think adding a better getter to StackFrameContext would make the patch a tad nicer, but other than that, I don't have much to add to this patch, unfortunately :) The code looks nice and we definitely need something like this.

clang/lib/StaticAnalyzer/Core/CallEvent.cpp

551

This mustn't be serious. StackFrameContext::getIndex() has no comments at all! So it is an index to the element within a CFGBlock to which it also stores a field for??? Ugh. Shouldn't we just have a getCallSiteCFGElement or something? Especially since we have CFGElementRefs now.

Would you be so nice to hunt this down please? :)

[baloghadamsoftware]

clang/lib/StaticAnalyzer/Core/CallEvent.cpp

551

Do you mean a new StackFrameContext::getCFGElement() member function? I agree. I can do it, however this technology where we get the block and the index separately is used at many places in the code, then it would be appropriate to refactor all these places. Even wrose is the backward direction, where we look for the CFG element of a statement: we find the block from CFGStmtMap and then we search for the index liearly, instrad of storing it in the map as well!!!

[balazske]

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

438

A / is missing (or too much of / above?).
It looks like that the comment tells something about how the function works, not what it does. Or not? (The function works by retrieving the construction context and something from it, but that seems to be the return value, not a region. This is why this comment can be confusing.) This belongs better to the implementation, not to the documentation part. The documentation could contain something about how and when the function can be used to get a non-null value.

[NoQ]

clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp

112

Please instead re-use the code that computes the object under construction. That'll save you ~50 lines of code and will be more future-proof (eg., standalone temporaries without destructor technically have a construction context with 0 items so when we implement them correctly your procedure will stop working).

[Szelethus]

clang/lib/StaticAnalyzer/Core/CallEvent.cpp

551

Nasty. Yeah, I mean not to burden you work refactoring any more then you feel like doing it, but simply adding a StackFrameContext::getCFGElement() method and using it in this patch would be nice enough, and some comments to getIndex(). But this obviously isn't a high prio issue.

[baloghadamsoftware]

Merged retrieveFromConstructionContext() to handleConstructionContext().

[baloghadamsoftware]

clang/lib/StaticAnalyzer/Core/CallEvent.cpp

575

This is extremely ugly and was one of the reasons I originally did not reuse handleConstructionContext(). What should I do here? Create a pure virtual handleConstructionContext() into SubEngine? Or somehow make handleConstructionContext static? Is there maybe a more proper way to access ExprEngine from CallEvent?

clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp

112

That was so my first thought. However, handleConstructionContext() is private and non-static. Now I tried to merge the two methods: if the value is already in the construction context, we return it, if not then we add it. Is this what you suggest? Or did I misunderstand you? At the very beginning I tried to simply use handleConstructionContext(), but it asserted because the value was already in the map.

[martong]

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

438

+1

I'd expect a description about what it does and only then the details.

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

719

Why was it necessary to move this prototype? Is this hunk related at all?

clang/lib/StaticAnalyzer/Core/CallEvent.cpp

575

Seems like SubEngine has only one descendant: ExprEngine. Consequently SubEngine is a false interface that is never ever used polymorphically, perhaps we could just get rid of that? I mean what if we used ExprEngine everywhere and scraped the SubEngine class? SubEngine seems to be a superfluous legacy to me (I can be wrong of course).

clang/unittests/StaticAnalyzer/TestReturnValueUnderConstruction.cpp

30

I think it would make the tests cleaner if we had some member variables set here and then in the test function (addTestReturnValueUnderConstructionChecker) we had the expectations on those variables.
Right now, if we face an assertion it kills the whole unit test suite and it is not obvious why that happened.

48

Please clang-format test code as well! You may also use raw string literals. Please be precise with test code too.

[NoQ]

clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp

112

I'd like to preserve the assertion that objects-under-construction are never filled twice; it's a very useful sanity check. What you need in your checker is a function that computes object-under-construction but doesn't put it into the objects-under-construction map. So you have to separate the computation from filling in the state.

[baloghadamsoftware]

clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp

112

OK, so I (fortunately) misundertood you. Thus I should refactor this function to a calculation and a storing part?

[baloghadamsoftware]

clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp

112

OK, I see what you are speaking about, but I have no idea how to do it properly. The problem is that the control logic of filling in the state also depends on the kind of the construction context. For some kinds we do not fill at all. Every way I try it becomes more complex and less correct:

1. NewAllocatedObjectKind: we do not add this to the state, we only retrieve the original.
2. SimpleReturnedValueKind and CXX17ElidedCopyReturnedValueKind: depending on whether we are in top frame we handle this case recursively or we do not fill at all, just return the value. What is the construction context item here? Maybe the ReturnStmt?
3. ElidedTemporaryObjectKind: this is the most problematic: we first handle it recursively for the construction context after elision, then we also fill it for the elided temporary object construction context as well.

The only thing I can (maybe) do is to retrieve the construction context item. But then the switch is still duplicated for filling, because of the different control logic for different construction context kinds.

[baloghadamsoftware]

clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp

112

The only thing I can (maybe) do is to retrieve the construction context item.

This does not help even for retrieving the value, because its control logic also depends on the construction context kind. If I do it, it will be code triplication instead of duplication and results in a code that is worse to understand than the current one.

[baloghadamsoftware]

clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp

112

Please instead re-use the code that computes the object under construction. That'll save you ~50 lines of code and will be more future-proof (eg., standalone temporaries without destructor technically have a construction context with 0 items so when we implement them correctly your procedure will stop working).

Any solution I can come up with rather adds 100 to 150 lines to the code instead of saving 50. For retrieving we only have to determine the construction context item (the key). For storing we also have to calculate the value. It sounds good to calculate these things in separate functions and then have a simple retriever and store function but there are lots of recursions, double strores, non-stores, retrieving in the store function that make it too complicated.

Also retrieving is more complex than just determining the item and getting the value from the context: if you look at SimpleReturnedValueKind and CXX17ElidedCopyReturnedValueKind you can see that we do not use the construction context we got in the parameter (the construction context of the return value) but the construction context of the call instead. For ElidedTemporaryObjectKind we take the construction context before the elusion.

Future proofness: I agree, this is a problem to solve. In cases where the construction context is empty maybe we can move the calculation of the values to a separate function that is called by both the "handler" and the "retriever".

[martong]

clang/unittests/StaticAnalyzer/TestReturnValueUnderConstruction.cpp

25

I assume this tests this call expression: returnC(1) . But this is absolutely not trivial from the test code, could you please make this cleaner?

[balazske]

clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp

112

Do I think correctly that retrieveFromConstructionContext (in the previous version) should return the same value as second part of handleConstructionContext? It does not look obvious at first. Or do we need a function with that purpose? If yes it looks a difficult task because the similar "logic" to get the value and update the state. Probably it is enough to add a flag to handleConstructionContext to not make new state. The current code looks bad because calling a "handle" type of function (that was private before) only to compute a value.

[martong]

clang/unittests/StaticAnalyzer/TestReturnValueUnderConstruction.cpp

29

How do we know that 0 is the proper block count?

[martong]

In this version of the patch you are using handleConstructionContext to "retrieve" or read an SVal for the construction. However, it seems like that function was designed to store values in the GDM for individual construction context items. To mix a read-only functionality into a setter seems to be error-prone to me.

Please instead re-use the code that computes the object under construction. That'll save you ~50 lines of code and will be more future-proof

I am not sure that this is the best option here. In my viewpoint, ConstructionContext and ConstructionContextItem are both "variants" (i.e. tagged unions) and we perform operations on these. Each operation is actually a visitation on the different kind of values (i.e. a switch). One operation is transforming a ConstructionContext to a ConstructionContextItem. This is what is needed to be able to call the getObjectUnderConstruction function. And this operation - the reading - is way different than the other - the store -, see Adam's concerns. This yields to different functions for each op. But this is perfectly natural once we work with "variants". I mean we can easily extend the operations on a variant with a new operation as a form of a visitation of the kinds (contrary to traditional class hierarchies where adding a new op is hard, but adding a new type is easy).

[martong]

In D80366#2059765, @martong wrote:

In this version of the patch you are using handleConstructionContext to "retrieve" or read an SVal for the construction. However, it seems like that function was designed to store values in the GDM for individual construction context items. To mix a read-only functionality into a setter seems to be error-prone to me.

Please instead re-use the code that computes the object under construction. That'll save you ~50 lines of code and will be more future-proof

I am not sure that this is the best option here. In my viewpoint, ConstructionContext and ConstructionContextItem are both "variants" (i.e. tagged unions) and we perform operations on these. Each operation is actually a visitation on the different kind of values (i.e. a switch). One operation is transforming a ConstructionContext to a ConstructionContextItem. This is what is needed to be able to call the getObjectUnderConstruction function. And this operation - the reading - is way different than the other - the store -, see Adam's concerns. This yields to different functions for each op. But this is perfectly natural once we work with "variants". I mean we can easily extend the operations on a variant with a new operation as a form of a visitation of the kinds (contrary to traditional class hierarchies where adding a new op is hard, but adding a new type is easy).

Just one more thing. It may turn out that the visitation should be done the same way in both cases, then we have to have a Visitor factored out. But, now it seems that even the visitation strategy is different for the individual operations.

[NoQ]

clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp

112

Any solution I can come up with rather adds 100 to 150 lines to the code instead of saving 50.

I think the following is a fairly literal implementation of my suggestion:

diff.diff19 KBDownload

It's 26 lines shorter than your retrieveFromConstructionContext() (ok, not 50, duplicating the switch was more annoying than i expected), it adds zero new logic (the only new piece of logic is to track constructors that were modeled correctly but their elision wasn't; previously we could fit it into control flow), it no longer tries to reverse-engineer the original behavior by duplicating its logic, and it's more future-proof for the reasons explained above.

[baloghadamsoftware]

clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp

112

Thank you for working on this. The point what I did not see (and I still no not see it) is that in your solution (and your proposal) instead of retrieving the value from the construction context we actually recalculate it, thus we do not use the construction context at all (except for NewAllocatedObjectKind) to retrieve the location of the return value. Can we guarantee that we always recalculate exactly the same symbolic value as the symbolic values we store in the map? For example, for SimpleReturnedValueKind et. al. we conjure a new symbolic value. Is it really the same value than that the one in the map, thus the one which is the real location where the object is constructed? If not, then the checkers might not work correctly because they store into the GDM using a different region as key than they use for retrieving the value.

[martong]

clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp

112

I think the following is a fairly literal implementation of my suggestion: ... It's 26 lines shorter than your retrieveFromConstructionContext() (ok, not 50, duplicating the switch was more annoying than i expected), it adds zero new logic (the only new piece of logic is to track constructors that were modeled correctly but their elision wasn't; previously we could fit it into control flow), it no longer tries to reverse-engineer the original behavior by duplicating its logic, and it's more future-proof for the reasons explained above.

+1 for this version. Even though Adam has valid concerns, this version contains functions with clear responsibilities.

Can we guarantee that we always recalculate exactly the same symbolic value as the symbolic values we store in the map? For example, for SimpleReturnedValueKind et. al. we conjure a new symbolic value. Is it really the same value than that the one in the map, thus the one which is the real location where the object is constructed?

I think the guarantee is that in Artem's implementation the switch is duplicated. That would be okay for now, but I still consider this error-prone, exactly because of the duplication. So, maybe this yields for a common Visitor.

Guys, would it be possible to have a common Visitor for these? Could be somewhat similar to DeclVisitor with return type as type parameter. And we could pass a lambda that is called in every case? Or would that be too overkill

[baloghadamsoftware]

clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp

112

I think the guarantee is that in Artem's implementation the switch is duplicated.

That is not my point. My point is that handleConstructionContext() is called, a value is calculated and stored in the map. This calculation may be the creation of a new conjured symbol. Thereafter, whenever a checker tries to retrieve the location of the return value from the construction context in Artem's solution we do not retrieve the value previously stored in the map, but calculate a new one, which is not necessarily the same as the one we stored in the map, because a in some cases a new symbol is conjured. Then the checker uses this new symbol as the index in GDM, but the object is constructed to the location stored in the map. Therefore, later when the checker tries to retrieve data from the GDM using the location of the existing object as a key it cannot find the data because it was stored using the wrong key.

getReturnValueUnderConstruction() must always return the location where the object is to be constructed. Therefore I think it must not recalculate the location which may involve creation of new conjured symbols but first it must look into the map of objects under construction. This guarantees that the returned location is really the location where the object is constructed.

[NoQ]

clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp

112

Yes it should always return the same value, because there is exactly one correct solution to the problem that it's solving. If it may return different values for the same (E, State, LCtx, CC) tuple, it's a bug. This method should ultimately be turned into a static method.

Loading a stored value if it exists will only sweep the problem under the rug. It'd be much better for us to assert that the freshly recomputed value is equal to the stored value if the latter is present.

The code that constructs symbolic region for the return value of a top frame is indeed suspicious. I'm not sure it can actually return different regions twice but this code is clearly incorrect because SymbolConjured is never the correct solution anyway. The only reason SymbolConjured is used anywhere is because the author (in this case, i) was too lazy to define a new symbol class that has the right identity. SymbolConjured is like UnknownVal that's a bit less unknown but still screams "not implemented yet". I think this definitely deserves a fixme.

I think the guarantee is that in Artem's implementation the switch is duplicated. That would be okay for now, but I still consider this error-prone, exactly because of the duplication. So, maybe this yields for a common Visitor.
Guys, would it be possible to have a common Visitor for these?

Mmm, i don't think i understand this argument. A visitor is a switch with a fancy interface and wildcards and lacking checks for unhandled cases.

Generally though i wouldn't mind having a ConstructionContextVisitor. For instance, that'll allow us to replace code like

case ConstructionContext::CXX17ElidedCopyVariableKind:
case ConstructionContext::SimpleVariableKind: {
}

with a single

VisitVariable(VariableConstructionContext *CC) {
}

I wouldn't like a visitor for ConstructionContextItems because they are fairly meaningless when taken out of, well, context.

[NoQ]

+GSoC gang because it'll ultimately help Nithin's future checker track smart pointers returned from functions.

[vsavchenko]

clang/lib/StaticAnalyzer/Core/CallEvent.cpp

551

+1
I do believe that clearer interface functions and better segregation of different functionalities will make it much easier for us in the future

553

nit: space after if

And I would also prefer putting const auto *Ctor

clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp

225–227

Maybe here (and further) we can use a widespread pattern of bool-castable things declared in the if condition (like you do with dyn_casts):

if (Optional<SVal> ExistingVal = getObjectUnderConstruction(State, CE, LCtx))
  return std::make_pair(State, *ExistingVal);

284–299

It seems like a code duplication to me.

First of all we can first compose a ConstructionContextItem from either CE, CCE, or ME and then call for getObjectUnderConstruction.
Additionally, I think we can hide even that by introducing an overload for getObjectUnderConstruction that takes an expression and an index.

clang/unittests/StaticAnalyzer/TestReturnValueUnderConstruction.cpp

25

I think that this function, the meat and bones of the test, should be properly commented and state explicitly what are the assumption and what is the expected outcome.

30

Why not gtests assertions? Those will also interrupt the execution, but will be much clearer in the output.

[baloghadamsoftware]

Incorporated @NoQ's solution.

[baloghadamsoftware]

Updated according to the comments.

[baloghadamsoftware]

clang/lib/StaticAnalyzer/Core/CallEvent.cpp

551

I fully agree, but StackFrameContext is not in the StaticAnalyzer part, but the Analysis part. I think that should be a separate patch. Who is the code owner for that part?

553

Ctor is not a pointer, but an llvm::Optional.

[NoQ]

clang/lib/StaticAnalyzer/Core/CallEvent.cpp

543

It is clear that the final return value of getConstructionContext() does not depend on the current block count; in fact the information you're trying to obtain is purely syntactic. You probably might as well pass 0 here. As i mentioned before, the ideal solution would be to make CallEvent carry a CFGElementRef to begin with (as a replacement for the origin expr), so that it didn't need to be recovered like this.

We should absolutely eliminate the BlockCount parameter of getReturnValueUnderConstruction() as well. Checker developers shouldn't understand what it is, especially given that it doesn't matter at all which value do you pass.

551

It's us anyway. StackFrameContext is essentially analyzer-exclusive but even for things like CFG and analysis-based warnings in Clang, there's nobody else who maintains them except for the analyzer gang.

553

Ah, classic.

[baloghadamsoftware]

Updated according to the comments.

[baloghadamsoftware]

clang/lib/StaticAnalyzer/Core/CallEvent.cpp

543

OK, I eliminated it. Do we need BlockCount for getParameterLocation() (the other patch)?

clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp

112

A Visitor is a good idea. However, is it used anywhere else? If it is, then it is worth to do it, but I think it can go into a new patch.

[NoQ]

Great, thank you!

clang/lib/StaticAnalyzer/Core/CallEvent.cpp

539

This looks useful. Let's turn it into a CallEvent method as well.

543

We absolutely need BlockCount in the other patch because it's part of the identity of the parameter region.

That said, the API that requires the user to provide BlockCount manually is still terrible and extremely error-prone. Namely, the contract of getParameterLocation() would be "The behavior is undefined unless you provide the same block count that is there when call happens". Which is a good indication that we should have stored BlockCount in CallEvent from the start.

[thakis]

Hello, this breaks check-clang everywhere, e.g. http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/69062/steps/test-check-all/logs/FAIL%3A%20Clang-Unit%3A%3ATestReturnValueUnderConstructionChecker.ReturnValueUnderConstructionChecker

Please take a look, and revert for now if the fix isn't obvious.

Please watch the tree a bit after landing; it's been red for 4 hours now.

[baloghadamsoftware]

In D80366#2082100, @thakis wrote:

Hello, this breaks check-clang everywhere, e.g. http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/69062/steps/test-check-all/logs/FAIL%3A%20Clang-Unit%3A%3ATestReturnValueUnderConstructionChecker.ReturnValueUnderConstructionChecker

Please take a look, and revert for now if the fix isn't obvious.

Please watch the tree a bit after landing; it's been red for 4 hours now.

Hello,

Thank you, I think I fixed it now. I worked on it for hours, because it involved lots of git branch changes and the the rebuild was slow.

[thakis]

Thank you, I think I fixed it now. I worked on it for hours, because it involved lots of git branch changes and the the rebuild was slow.

Thanks for the fix! If you notice that fixing a bot will take more than a few minutes, please revert the commit while you work on the fix :)

[riccibruno]

Warning (from eg: http://lab.llvm.org:8011/builders/clang-armv7-linux-build-cache/builds/32043):

/home/buildslave/buildslave/clang-armv7-linux-build-cache/llvm/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h:1102:23: warning: 'getDecl' overrides a member function but is not marked 'override' [-Winconsistent-missing-override]

Edit: From D80522

clang/include/clang/Analysis/AnalysisDeclContext.h

public:
   const Stmt *getCallSite() const { return CallSite; }
 
   const CFGBlock *getCallSiteBlock() const { return Block; }
 
   bool inTopFrame() const override { return getParent() == nullptr; }
 
   unsigned getIndex() const { return Index; }
 
+  CFGElement getCallSiteCFGElement() const { return (*Block)[Index]; }
+  
   void Profile(llvm::FoldingSetNodeID &ID) override;
 
   static void Profile(llvm::FoldingSetNodeID &ID, AnalysisDeclContext *ADC,
                       const LocationContext *ParentLC, const Stmt *S,
                       const CFGBlock *Block, unsigned BlockCount,
                       unsigned Index) {
     ProfileCommon(ID, StackFrame, ADC, ParentLC, S);
     ID.AddPointer(Block);

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

public:
 
   /// Some call event sub-classes conveniently adjust mismatching AST indices
   /// to match parameter indices. This function converts an argument index
   /// as understood by CallEvent to the argument index as understood by the AST.
   virtual unsigned getASTArgumentIndex(unsigned CallArgumentIndex) const {
     return CallArgumentIndex;
   }
 
+  /// Returns the construction context of the call, if it is a C++ constructor
+  /// call or a call of a function returning a C++ class instance. Otherwise
+  /// return nullptr.
+  const ConstructionContext *getConstructionContext() const;
+
+  /// If the call returns a C++ record type then the region of its return value
+  /// can be retrieved from its construction context.
+  Optional<SVal> getReturnValueUnderConstruction() const;
+
   // Iterator access to formal parameters and their types.
 private:
   struct GetTypeFn {
     QualType operator()(ParmVarDecl *PD) const { return PD->getType(); }
   };
 
 public:
   /// Return call's formal parameters.

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

struct EvalCallOptions {
     /// This call is a constructor or a destructor of a temporary value.
     bool IsTemporaryCtorOrDtor = false;
 
     /// This call is a constructor for a temporary that is lifetime-extended
     /// by binding it to a reference-type field within an aggregate,
     /// for example 'A { const C &c; }; A a = { C() };'
     bool IsTemporaryLifetimeExtendedViaAggregate = false;
 
+    /// This call is a pre-C++17 elidable constructor that we failed to elide
+    /// because we failed to compute the target region into which
+    /// this constructor would have been ultimately elided. Analysis that
+    /// we perform in this case is still correct but it behaves differently,
+    /// as if copy elision is disabled.
+    bool IsElidableCtorThatHasNotBeenElided = false;
+
     EvalCallOptions() {}
   };
 
 private:
   cross_tu::CrossTranslationUnitContext &CTU;
 
   AnalysisManager &AMgr;
 
public:
   void evalCall(ExplodedNodeSet &Dst, ExplodedNode *Pred,
                 const CallEvent &Call);
 
   /// Default implementation of call evaluation.
   void defaultEvalCall(NodeBuilder &B, ExplodedNode *Pred,
                        const CallEvent &Call,
                        const EvalCallOptions &CallOpts = {});
 
+  /// Find location of the object that is being constructed by a given
+  /// constructor. This should ideally always succeed but due to not being
+  /// fully implemented it sometimes indicates that it failed via its
+  /// out-parameter CallOpts; in such cases a fake temporary region is
+  /// returned, which is better than nothing but does not represent
+  /// the actual behavior of the program.
+  SVal computeObjectUnderConstruction(
+      const Expr *E, ProgramStateRef State, const LocationContext *LCtx,
+      const ConstructionContext *CC, EvalCallOptions &CallOpts);
+
+  /// Update the program state with all the path-sensitive information
+  /// that's necessary to perform construction of an object with a given
+  /// syntactic construction context. V and CallOpts have to be obtained from
+  /// computeObjectUnderConstruction() invoked with the same set of
+  /// the remaining arguments (E, State, LCtx, CC).
+  ProgramStateRef updateObjectsUnderConstruction(
+      SVal V, const Expr *E, ProgramStateRef State, const LocationContext *LCtx,
+      const ConstructionContext *CC, const EvalCallOptions &CallOpts);
+
+  /// A convenient wrapper around computeObjectUnderConstruction
+  /// and updateObjectsUnderConstruction.
+  std::pair<ProgramStateRef, SVal> handleConstructionContext(
+      const Expr *E, ProgramStateRef State, const LocationContext *LCtx,
+      const ConstructionContext *CC, EvalCallOptions &CallOpts) {
+    SVal V = computeObjectUnderConstruction(E, State, LCtx, CC, CallOpts);
+    return std::make_pair(
+        updateObjectsUnderConstruction(V, E, State, LCtx, CC, CallOpts), V);
+  }
+
 private:
   ProgramStateRef finishArgumentConstruction(ProgramStateRef State,
                                              const CallEvent &Call);
   void finishArgumentConstruction(ExplodedNodeSet &Dst, ExplodedNode *Pred,
                                   const CallEvent &Call);
 
   void evalLoadCommon(ExplodedNodeSet &Dst,
                       const Expr *NodeEx,  /* Eventually will be a CFGStmt */
private:
 
   /// For a DeclStmt or CXXInitCtorInitializer, walk backward in the current CFG
   /// block to find the constructor expression that directly constructed into
   /// the storage for this statement. Returns null if the constructor for this
   /// statement created a temporary object region rather than directly
   /// constructing into an existing region.
   const CXXConstructExpr *findDirectConstructorForCurrentCFGElement();
 
-  /// Update the program state with all the path-sensitive information
-  /// that's necessary to perform construction of an object with a given
-  /// syntactic construction context. If the construction context is unavailable
-  /// or unusable for any reason, a dummy temporary region is returned, and the
-  /// IsConstructorWithImproperlyModeledTargetRegion flag is set in \p CallOpts.
-  /// Returns the updated program state and the new object's this-region.
-  std::pair<ProgramStateRef, SVal> handleConstructionContext(
-      const Expr *E, ProgramStateRef State, const LocationContext *LCtx,
-      const ConstructionContext *CC, EvalCallOptions &CallOpts);
-
   /// Common code that handles either a CXXConstructExpr or a
   /// CXXInheritedCtorInitExpr.
   void handleConstructor(const Expr *E, ExplodedNode *Pred,
                          ExplodedNodeSet &Dst);
 
   /// Store the location of a C++ object corresponding to a statement
   /// until the statement is actually encountered. For example, if a DeclStmt
   /// has CXXConstructExpr as its initializer, the object would be considered

clang/lib/StaticAnalyzer/Core/CallEvent.cpp

if (!ArgVal.isUnknown()) {
       Loc ParamLoc = SVB.makeLoc(MRMgr.getVarRegion(ParamDecl, CalleeCtx));
       Bindings.push_back(std::make_pair(ParamLoc, ArgVal));
     }
   }
 
   // FIXME: Variadic arguments are not handled at all right now.
 }
 
+const ConstructionContext *CallEvent::getConstructionContext() const {
+  const StackFrameContext *StackFrame = getCalleeStackFrame(0);
+  if (!StackFrame)
+    return nullptr;
+
+  const CFGElement Element = StackFrame->getCallSiteCFGElement();
+  if (const auto Ctor = Element.getAs<CFGConstructor>()) {
+    return Ctor->getConstructionContext();
+  }
+
+  if (const auto RecCall = Element.getAs<CFGCXXRecordTypedCall>()) {
+    return RecCall->getConstructionContext();
+  }
+
+  return nullptr;
+}
+
+Optional<SVal>
+CallEvent::getReturnValueUnderConstruction() const {
+  const auto *CC = getConstructionContext();
+  if (!CC)
+    return None;
+
+  ExprEngine::EvalCallOptions CallOpts;
+  ExprEngine &Engine = getState()->getStateManager().getOwningEngine();
+  SVal RetVal =
+    Engine.computeObjectUnderConstruction(getOriginExpr(), getState(),
+                                          getLocationContext(), CC, CallOpts);
+  return RetVal;
+}
+
 ArrayRef<ParmVarDecl*> AnyFunctionCall::parameters() const {
   const FunctionDecl *D = getDecl();
   if (!D)
     return None;
   return D->parameters();
 }
 
 RuntimeDefinition AnyFunctionCall::getRuntimeDefinition() const {
   const FunctionDecl *FD = getDecl();
   if (!FD)
     return {};
 
   // Note that the AnalysisDeclContext will have the FunctionDecl with
   // the definition (if one exists).
   AnalysisDeclContext *AD =

clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp

while (const ArrayType *AT = Ctx.getAsArrayType(Ty)) {
     Ty = AT->getElementType();
     LValue = State->getLValue(Ty, SVB.makeZeroArrayIndex(), LValue);
     IsArray = true;
   }
 
   return LValue;
 }
 
-std::pair<ProgramStateRef, SVal> ExprEngine::handleConstructionContext(
+SVal ExprEngine::computeObjectUnderConstruction(
     const Expr *E, ProgramStateRef State, const LocationContext *LCtx,
     const ConstructionContext *CC, EvalCallOptions &CallOpts) {
   SValBuilder &SVB = getSValBuilder();
   MemRegionManager &MRMgr = SVB.getRegionManager();
   ASTContext &ACtx = SVB.getContext();
 
-  // See if we're constructing an existing region by looking at the
-  // current construction context.
+  // Compute the target region by exploring the construction context.
   if (CC) {
     switch (CC->getKind()) {
     case ConstructionContext::CXX17ElidedCopyVariableKind:
     case ConstructionContext::SimpleVariableKind: {
       const auto *DSCC = cast<VariableConstructionContext>(CC);
       const auto *DS = DSCC->getDeclStmt();
       const auto *Var = cast<VarDecl>(DS->getSingleDecl());
-      SVal LValue = State->getLValue(Var, LCtx);
       QualType Ty = Var->getType();
-      LValue =
-          makeZeroElementRegion(State, LValue, Ty, CallOpts.IsArrayCtorOrDtor);
-      State =
-          addObjectUnderConstruction(State, DSCC->getDeclStmt(), LCtx, LValue);
-      return std::make_pair(State, LValue);
+      return makeZeroElementRegion(State, State->getLValue(Var, LCtx), Ty,
+                                   CallOpts.IsArrayCtorOrDtor);
     }
     case ConstructionContext::CXX17ElidedCopyConstructorInitializerKind:
     case ConstructionContext::SimpleConstructorInitializerKind: {
       const auto *ICC = cast<ConstructorInitializerConstructionContext>(CC);
       const auto *Init = ICC->getCXXCtorInitializer();
       assert(Init->isAnyMemberInitializer());
       const CXXMethodDecl *CurCtor = cast<CXXMethodDecl>(LCtx->getDecl());
-      Loc ThisPtr =
-      SVB.getCXXThis(CurCtor, LCtx->getStackFrame());
+      Loc ThisPtr = SVB.getCXXThis(CurCtor, LCtx->getStackFrame());
       SVal ThisVal = State->getSVal(ThisPtr);
 
       const ValueDecl *Field;
       SVal FieldVal;
       if (Init->isIndirectMemberInitializer()) {
         Field = Init->getIndirectMember();
         FieldVal = State->getLValue(Init->getIndirectMember(), ThisVal);
       } else {
         Field = Init->getMember();
         FieldVal = State->getLValue(Init->getMember(), ThisVal);
       }
 
       QualType Ty = Field->getType();
-      FieldVal = makeZeroElementRegion(State, FieldVal, Ty,
-                                       CallOpts.IsArrayCtorOrDtor);
-      State = addObjectUnderConstruction(State, Init, LCtx, FieldVal);
-      return std::make_pair(State, FieldVal);
+      return makeZeroElementRegion(State, FieldVal, Ty,
     }
     case ConstructionContext::NewAllocatedObjectKind: {
       if (AMgr.getAnalyzerOptions().MayInlineCXXAllocator) {
         const auto *NECC = cast<NewAllocatedObjectConstructionContext>(CC);
         const auto *NE = NECC->getCXXNewExpr();
         SVal V = *getObjectUnderConstruction(State, NE, LCtx);
         if (const SubRegion *MR =
                 dyn_cast_or_null<SubRegion>(V.getAsRegion())) {
           if (NE->isArray()) {
             // TODO: In fact, we need to call the constructor for every
             // allocated element, not just the first one!
             CallOpts.IsArrayCtorOrDtor = true;
-            return std::make_pair(
-                State, loc::MemRegionVal(getStoreManager().GetElementZeroRegion(
-                           MR, NE->getType()->getPointeeType())));
+            return loc::MemRegionVal(getStoreManager().GetElementZeroRegion(
+                MR, NE->getType()->getPointeeType()));
           }
-          return std::make_pair(State, V);
+          return  V;
         }
         // TODO: Detect when the allocator returns a null pointer.
         // Constructor shall not be called in this case.
       }
       break;
     }
     case ConstructionContext::SimpleReturnedValueKind:
     case ConstructionContext::CXX17ElidedCopyReturnedValueKind: {
case ConstructionContext::CXX17ElidedCopyReturnedValueKind: {
           break;
         }
         if (isa<BlockInvocationContext>(CallerLCtx)) {
           // Unwrap block invocation contexts. They're mostly part of
           // the current stack frame.
           CallerLCtx = CallerLCtx->getParent();
           assert(!isa<BlockInvocationContext>(CallerLCtx));
         }
-        return handleConstructionContext(
+        return computeObjectUnderConstruction(
             cast<Expr>(SFC->getCallSite()), State, CallerLCtx,
             RTC->getConstructionContext(), CallOpts);
       } else {
         // We are on the top frame of the analysis. We do not know where is the
         // object returned to. Conjure a symbolic region for the return value.
         // TODO: We probably need a new MemRegion kind to represent the storage
         // of that SymbolicRegion, so that we cound produce a fancy symbol
         // instead of an anonymous conjured symbol.
         // TODO: Do we need to track the region to avoid having it dead
         // too early? It does die too early, at least in C++17, but because
         // putting anything into a SymbolicRegion causes an immediate escape,
         // it doesn't cause any leak false positives.
         const auto *RCC = cast<ReturnedValueConstructionContext>(CC);
         // Make sure that this doesn't coincide with any other symbol
         // conjured for the returned expression.
         static const int TopLevelSymRegionTag = 0;
         const Expr *RetE = RCC->getReturnStmt()->getRetValue();
         assert(RetE && "Void returns should not have a construction context");
         QualType ReturnTy = RetE->getType();
         QualType RegionTy = ACtx.getPointerType(ReturnTy);
-        SVal V = SVB.conjureSymbolVal(&TopLevelSymRegionTag, RetE, SFC,
-                                      RegionTy, currBldrCtx->blockCount());
-        return std::make_pair(State, V);
+        return SVB.conjureSymbolVal(&TopLevelSymRegionTag, RetE, SFC, RegionTy,
+                                    currBldrCtx->blockCount());
       }
       llvm_unreachable("Unhandled return value construction context!");
     }
     case ConstructionContext::ElidedTemporaryObjectKind: {
       assert(AMgr.getAnalyzerOptions().ShouldElideConstructors);
       const auto *TCC = cast<ElidedTemporaryObjectConstructionContext>(CC);
-      const CXXBindTemporaryExpr *BTE = TCC->getCXXBindTemporaryExpr();
-      const MaterializeTemporaryExpr *MTE = TCC->getMaterializedTemporaryExpr();
-      const CXXConstructExpr *CE = TCC->getConstructorAfterElision();
 
       // Support pre-C++17 copy elision. We'll have the elidable copy
       // constructor in the AST and in the CFG, but we'll skip it
       // and construct directly into the final object. This call
       // also sets the CallOpts flags for us.
-      SVal V;
       // If the elided copy/move constructor is not supported, there's still
       // benefit in trying to model the non-elided constructor.
       // Stash our state before trying to elide, as it'll get overwritten.
       ProgramStateRef PreElideState = State;
       EvalCallOptions PreElideCallOpts = CallOpts;
 
-      std::tie(State, V) = handleConstructionContext(
-          CE, State, LCtx, TCC->getConstructionContextAfterElision(), CallOpts);
+      SVal V = computeObjectUnderConstruction(
+          TCC->getConstructorAfterElision(), State, LCtx,
+          TCC->getConstructionContextAfterElision(), CallOpts);
 
       // FIXME: This definition of "copy elision has not failed" is unreliable.
       // It doesn't indicate that the constructor will actually be inlined
-      // later; it is still up to evalCall() to decide.
-      if (!CallOpts.IsCtorOrDtorWithImproperlyModeledTargetRegion) {
-        // Remember that we've elided the constructor.
-        State = addObjectUnderConstruction(State, CE, LCtx, V);
-
-        // Remember that we've elided the destructor.
-        if (BTE)
-          State = elideDestructor(State, BTE, LCtx);
-
-        // Instead of materialization, shamelessly return
-        // the final object destination.
-        if (MTE)
-          State = addObjectUnderConstruction(State, MTE, LCtx, V);
+      // later; this is still up to evalCall() to decide.
+      if (!CallOpts.IsCtorOrDtorWithImproperlyModeledTargetRegion)
+        return V;
 
-        return std::make_pair(State, V);
-      } else {
-        // Copy elision failed. Revert the changes and proceed as if we have
-        // a simple temporary.
-        State = PreElideState;
-        CallOpts = PreElideCallOpts;
-      }
+      CallOpts.IsElidableCtorThatHasNotBeenElided = true;
       LLVM_FALLTHROUGH;
     }
     case ConstructionContext::SimpleTemporaryObjectKind: {
       const auto *TCC = cast<TemporaryObjectConstructionContext>(CC);
-      const CXXBindTemporaryExpr *BTE = TCC->getCXXBindTemporaryExpr();
       const MaterializeTemporaryExpr *MTE = TCC->getMaterializedTemporaryExpr();
-      SVal V = UnknownVal();
 
+      CallOpts.IsTemporaryCtorOrDtor = true;
       if (MTE) {
         if (const ValueDecl *VD = MTE->getExtendingDecl()) {
           assert(MTE->getStorageDuration() != SD_FullExpression);
           if (!VD->getType()->isReferenceType()) {
             // We're lifetime-extended by a surrounding aggregate.
             // Automatic destructors aren't quite working in this case
             // on the CFG side. We should warn the caller about that.
             // FIXME: Is there a better way to retrieve this information from
             // the MaterializeTemporaryExpr?
             CallOpts.IsTemporaryLifetimeExtendedViaAggregate = true;
           }
         }
 
         if (MTE->getStorageDuration() == SD_Static ||
             MTE->getStorageDuration() == SD_Thread)
-          V = loc::MemRegionVal(MRMgr.getCXXStaticTempObjectRegion(E));
+          return loc::MemRegionVal(MRMgr.getCXXStaticTempObjectRegion(E));
       }
 
-      if (V.isUnknown())
-        V = loc::MemRegionVal(MRMgr.getCXXTempObjectRegion(E, LCtx));
-
-      if (BTE)
-        State = addObjectUnderConstruction(State, BTE, LCtx, V);
-
-      if (MTE)
-        State = addObjectUnderConstruction(State, MTE, LCtx, V);
-
-      CallOpts.IsTemporaryCtorOrDtor = true;
-      return std::make_pair(State, V);
+      return loc::MemRegionVal(MRMgr.getCXXTempObjectRegion(E, LCtx));
     }
     case ConstructionContext::ArgumentKind: {
       // Arguments are technically temporaries.
       CallOpts.IsTemporaryCtorOrDtor = true;
 
       const auto *ACC = cast<ArgumentConstructionContext>(CC);
       const Expr *E = ACC->getCallLikeExpr();
       unsigned Idx = ACC->getIndex();
-      const CXXBindTemporaryExpr *BTE = ACC->getCXXBindTemporaryExpr();
 
       CallEventManager &CEMgr = getStateManager().getCallEventManager();
-      SVal V = UnknownVal();
       auto getArgLoc = [&](CallEventRef<> Caller) -> Optional<SVal> {
         const LocationContext *FutureSFC =
             Caller->getCalleeStackFrame(currBldrCtx->blockCount());
         // Return early if we are unable to reliably foresee
         // the future stack frame.
         if (!FutureSFC)
           return None;
 
         // This should be equivalent to Caller->getDecl() for now, but
         // FutureSFC->getDecl() is likely to support better stuff (like
         // virtual functions) earlier.
         const Decl *CalleeD = FutureSFC->getDecl();
 
         // FIXME: Support for variadic arguments is not implemented here yet.
         if (CallEvent::isVariadic(CalleeD))
           return None;
 
         // Operator arguments do not correspond to operator parameters
         // because this-argument is implemented as a normal argument in
         // operator call expressions but not in operator declarations.
         const VarRegion *VR = Caller->getParameterLocation(
             *Caller->getAdjustedParameterIndex(Idx), currBldrCtx->blockCount());
         if (!VR)
           return None;
 
         return loc::MemRegionVal(VR);
       };
 
       if (const auto *CE = dyn_cast<CallExpr>(E)) {
         CallEventRef<> Caller = CEMgr.getSimpleCall(CE, State, LCtx);
-        if (auto OptV = getArgLoc(Caller))
-          V = *OptV;
+        if (Optional<SVal> V = getArgLoc(Caller))
+          return *V;
         else
           break;
-        State = addObjectUnderConstruction(State, {CE, Idx}, LCtx, V);
       } else if (const auto *CCE = dyn_cast<CXXConstructExpr>(E)) {
         // Don't bother figuring out the target region for the future
         // constructor because we won't need it.
         CallEventRef<> Caller =
             CEMgr.getCXXConstructorCall(CCE, /*Target=*/nullptr, State, LCtx);
-        if (auto OptV = getArgLoc(Caller))
-          V = *OptV;
+        if (Optional<SVal> V = getArgLoc(Caller))
+          return *V;
         else
           break;
-        State = addObjectUnderConstruction(State, {CCE, Idx}, LCtx, V);
       } else if (const auto *ME = dyn_cast<ObjCMessageExpr>(E)) {
         CallEventRef<> Caller = CEMgr.getObjCMethodCall(ME, State, LCtx);
-        if (auto OptV = getArgLoc(Caller))
-          V = *OptV;
+        if (Optional<SVal> V = getArgLoc(Caller))
+          return *V;
         else
           break;
-        State = addObjectUnderConstruction(State, {ME, Idx}, LCtx, V);
-      }
+      }
+    }
+    } // switch (CC->getKind())
 
-      assert(!V.isUnknown());
+  // If we couldn't find an existing region to construct into, assume we're
+  // constructing a temporary. Notify the caller of our failure.
+  CallOpts.IsCtorOrDtorWithImproperlyModeledTargetRegion = true;
+  return loc::MemRegionVal(MRMgr.getCXXTempObjectRegion(E, LCtx));
+}
 
-      if (BTE)
+ProgramStateRef ExprEngine::updateObjectsUnderConstruction(
+    SVal V, const Expr *E, ProgramStateRef State, const LocationContext *LCtx,
+    const ConstructionContext *CC, const EvalCallOptions &CallOpts) {
+  if (CallOpts.IsCtorOrDtorWithImproperlyModeledTargetRegion) {
+    // Sounds like we failed to find the target region and therefore
+    // copy elision failed. There's nothing we can do about it here.
+    return State;
+  }
+
+  // See if we're constructing an existing region by looking at the
+  // current construction context.
+  assert(CC && "Computed target region without construction context?");
+  switch (CC->getKind()) {
+  case ConstructionContext::CXX17ElidedCopyVariableKind:
+  case ConstructionContext::SimpleVariableKind: {
+    const auto *DSCC = cast<VariableConstructionContext>(CC);
+    return addObjectUnderConstruction(State, DSCC->getDeclStmt(), LCtx, V);
+    }
+    case ConstructionContext::CXX17ElidedCopyConstructorInitializerKind:
+    case ConstructionContext::SimpleConstructorInitializerKind: {
+      const auto *ICC = cast<ConstructorInitializerConstructionContext>(CC);
+      return addObjectUnderConstruction(State, ICC->getCXXCtorInitializer(),
+                                        LCtx, V);
+    }
+    case ConstructionContext::NewAllocatedObjectKind: {
+      return State;
+    }
+    case ConstructionContext::SimpleReturnedValueKind:
+    case ConstructionContext::CXX17ElidedCopyReturnedValueKind: {
+      const StackFrameContext *SFC = LCtx->getStackFrame();
+      const LocationContext *CallerLCtx = SFC->getParent();
+      if (!CallerLCtx) {
+        // No extra work is necessary in top frame.
+        return State;
+      }
+
+      auto RTC = (*SFC->getCallSiteBlock())[SFC->getIndex()]
+                     .getAs<CFGCXXRecordTypedCall>();
+      assert(RTC && "Could not have had a target region without it");
+      if (isa<BlockInvocationContext>(CallerLCtx)) {
+        // Unwrap block invocation contexts. They're mostly part of
+        // the current stack frame.
+        CallerLCtx = CallerLCtx->getParent();
+        assert(!isa<BlockInvocationContext>(CallerLCtx));
+      }
+
+      return updateObjectsUnderConstruction(V,
+          cast<Expr>(SFC->getCallSite()), State, CallerLCtx,
+          RTC->getConstructionContext(), CallOpts);
+    }
+    case ConstructionContext::ElidedTemporaryObjectKind: {
+      assert(AMgr.getAnalyzerOptions().ShouldElideConstructors);
+      if (!CallOpts.IsElidableCtorThatHasNotBeenElided) {
+        const auto *TCC = cast<ElidedTemporaryObjectConstructionContext>(CC);
+        State = updateObjectsUnderConstruction(
+            V, TCC->getConstructorAfterElision(), State, LCtx,
+            TCC->getConstructionContextAfterElision(), CallOpts);
+
+        // Remember that we've elided the constructor.
+        State = addObjectUnderConstruction(
+            State, TCC->getConstructorAfterElision(), LCtx, V);
+
+        // Remember that we've elided the destructor.
+        if (const auto *BTE = TCC->getCXXBindTemporaryExpr())
+          State = elideDestructor(State, BTE, LCtx);
+
+        // Instead of materialization, shamelessly return
+        // the final object destination.
+        if (const auto *MTE = TCC->getMaterializedTemporaryExpr())
+          State = addObjectUnderConstruction(State, MTE, LCtx, V);
+
+        return State;
+      }
+      // If we decided not to elide the constructor, proceed as if
+      // it's a simple temporary.
+      LLVM_FALLTHROUGH;
+    }
+    case ConstructionContext::SimpleTemporaryObjectKind: {
+      const auto *TCC = cast<TemporaryObjectConstructionContext>(CC);
+      if (const auto *BTE = TCC->getCXXBindTemporaryExpr())
         State = addObjectUnderConstruction(State, BTE, LCtx, V);
 
-      return std::make_pair(State, V);
+      if (const auto *MTE = TCC->getMaterializedTemporaryExpr())
+        State = addObjectUnderConstruction(State, MTE, LCtx, V);
+
+      return State;
     }
+    case ConstructionContext::ArgumentKind: {
+      const auto *ACC = cast<ArgumentConstructionContext>(CC);
+      if (const auto *BTE = ACC->getCXXBindTemporaryExpr())
+        State = addObjectUnderConstruction(State, BTE, LCtx, V);
+
+      return addObjectUnderConstruction(
+          State, {ACC->getCallLikeExpr(), ACC->getIndex()}, LCtx, V);
     }
   }
-  // If we couldn't find an existing region to construct into, assume we're
-  // constructing a temporary. Notify the caller of our failure.
-  CallOpts.IsCtorOrDtorWithImproperlyModeledTargetRegion = true;
-  return std::make_pair(
-      State, loc::MemRegionVal(MRMgr.getCXXTempObjectRegion(E, LCtx)));
+  llvm_unreachable("Unhandled construction context!");
 }
 
 void ExprEngine::handleConstructor(const Expr *E,
                                    ExplodedNode *Pred,
                                    ExplodedNodeSet &destNodes) {
   const auto *CE = dyn_cast<CXXConstructExpr>(E);
   const auto *CIE = dyn_cast<CXXInheritedCtorInitExpr>(E);
   assert(CE || CIE);

clang/unittests/StaticAnalyzer/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS
   FrontendOpenMP
   Support
   )
 
 add_clang_unittest(StaticAnalysisTests
   AnalyzerOptionsTest.cpp
   CallDescriptionTest.cpp
   CallEventTest.cpp
-  StoreTest.cpp
+  RangeSetTest.cpp
   RegisterCustomCheckersTest.cpp
+  StoreTest.cpp 
   SymbolReaperTest.cpp
-  RangeSetTest.cpp
+  TestReturnValueUnderConstruction.cpp
   )
 
 clang_target_link_libraries(StaticAnalysisTests
   PRIVATE
   clangBasic
   clangAnalysis
   clangAST
   clangASTMatchers
   clangCrossTU
   clangFrontend
   clangSerialization
   clangStaticAnalyzerCore
   clangStaticAnalyzerFrontend
   clangTooling
   )

clang/unittests/StaticAnalyzer/TestReturnValueUnderConstruction.cpp

+//===- unittests/StaticAnalyzer/TestReturnValueUnderConstruction.cpp ------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#include "CheckerRegistration.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "clang/StaticAnalyzer/Frontend/AnalysisConsumer.h"
+#include "clang/StaticAnalyzer/Frontend/CheckerRegistry.h"
+#include "clang/Tooling/Tooling.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace ento {
+namespace {
+
+class TestReturnValueUnderConstructionChecker
+  : public Checker<check::PostCall> {
+public:
+  void checkPostCall(const CallEvent &Call, CheckerContext &C) const {
+    // We are checking the invocation of `returnC` which returns an object
+    // by value.
+    const IdentifierInfo *ID = Call.getCalleeIdentifier();
+    if (ID->getName() != "returnC")
+      return;
+
+    // Since `returnC` returns an object by value, the invocation results
+    // in an object of type `C` constructed into variable `c`. Thus the
+    // return value of `CallEvent::getReturnValueUnderConstruction()` must
+    // be non-empty and has to be a `MemRegion`.
+    Optional<SVal> RetVal = Call.getReturnValueUnderConstruction();
+    ASSERT_TRUE(RetVal);
+    ASSERT_TRUE(RetVal->getAsRegion());
+  }
+};
+
+void addTestReturnValueUnderConstructionChecker(
+    AnalysisASTConsumer &AnalysisConsumer, AnalyzerOptions &AnOpts) {
+  AnOpts.CheckersAndPackages =
+    {{"test.TestReturnValueUnderConstruction", true}};
+  AnalysisConsumer.AddCheckerRegistrationFn([](CheckerRegistry &Registry) {
+      Registry.addChecker<TestReturnValueUnderConstructionChecker>(
+          "test.TestReturnValueUnderConstruction", "", "");
+    });
+}
+
+TEST(TestReturnValueUnderConstructionChecker,
+     ReturnValueUnderConstructionChecker) {
+  EXPECT_TRUE(runCheckerOnCode<addTestReturnValueUnderConstructionChecker>(
+                  R"(class C {
+                     public:
+                       C(int nn): n(nn) {}
+                       virtual ~C() {}
+                     private:
+                       int n;
+                     };
+
+                     C returnC(int m) {
+                       C c(m);
+                       return c;
+                     }
+
+                     void foo() {
+                       C c = returnC(1); 
+                     })"));
+}
+
+} // namespace
+} // namespace ento
+} // namespace clang