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
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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
