[PATCH] D61545: [analyzer] Fix a crash in RVO from within blocks.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a_sidorin, rnkovacs, mikhail.ramalho, Szelethus, baloghadamsoftware, Charusso.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, a.sidorin, szepet.
Herald added a project: clang.

While blocks are an Apple extension to C, i'm adding everybody because my mistake that i'm fixing here may hurt us more when we add more kinds of location contexts, so we shouldn't be making it anywhere.

In the attached test case, `getA()` returns an object by value, and therefore requires a construction context. Its construction context is that of a value that's immediately* returned (by value, without any sort of conversion), so copy elision (namely, RVO) applies. This means that we unwind the stack of `LocationContext`s in order to see where is the current callee called from. This way we obtain the construction context for the call site, and from that we can figure out what object is constructed. In this case it's going to be the first-and-only argument of `use()`. This is all good.

In this case RVO is applied to a return value of an anonymous block that's declared only to be immediately called and discarded. The Static Analyzer models block calls by putting two location contexts on the stack: a `BlockInvocationContext` followed by the actual `StackFrameContext`. I don't really know why it does that :) but that's not important, because if we introduce more kinds of location contexts, it'll look similarly anyway.

Therefore the correct idiom for obtaining the parent stack frame is to first obtain the parent, and then don't forget to obtain its stack frame, which is not necessarily the parent itself. This is the mistake that i made in my RVO code that i'm fixing here.

The code was crashing because it was looking up the call site in a `CFGStmtMap` for the wrong CFG (obtained from a wrong location context). This was happening in `CallEvent::getCalleeStackFrame()`.


Repository:
  rC Clang

https://reviews.llvm.org/D61545

Files:
  clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  clang/test/Analysis/copy-elision.mm


Index: clang/test/Analysis/copy-elision.mm
===================================================================
--- /dev/null
+++ clang/test/Analysis/copy-elision.mm
@@ -0,0 +1,19 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -fblocks -verify %s
+
+// expected-no-diagnostics
+
+namespace block_rvo_crash {
+struct A {};
+struct B {};
+
+A getA();
+void use(A a) {}
+
+void foo() {
+  // This used to crash when finding construction context for getA()
+  // (which is use()'s argument due to RVO).
+  use(^{
+    return getA();  // no-crash
+  }());
+}
+} // namespace block_rvo_crash
Index: clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -188,6 +188,8 @@
       // top frame of the analysis.
       const StackFrameContext *SFC = LCtx->getStackFrame();
       if (const LocationContext *CallerLCtx = SFC->getParent()) {
+        // Skip non-stack-frame contexts.
+        const StackFrameContext *CallerSFC = CallerLCtx->getStackFrame();
         auto RTC = (*SFC->getCallSiteBlock())[SFC->getIndex()]
                        .getAs<CFGCXXRecordTypedCall>();
         if (!RTC) {
@@ -197,7 +199,7 @@
           break;
         }
         return prepareForObjectConstruction(
-            cast<Expr>(SFC->getCallSite()), State, CallerLCtx,
+            cast<Expr>(SFC->getCallSite()), State, CallerSFC,
             RTC->getConstructionContext(), CallOpts);
       } else {
         // We are on the top frame of the analysis. We do not know where is the


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D61545.198121.patch
Type: text/x-patch
Size: 1638 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190504/afc40ac1/attachment.bin>