<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Wed, Mar 7, 2018 at 1:55 PM, Reid Kleckner via cfe-commits <span dir="ltr"><<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: rnk<br>
Date: Wed Mar 7 10:55:10 2018<br>
New Revision: 326926<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=326926&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project?rev=326926&view=rev</a><br>
Log:<br>
Push a function scope when parsing function bodies without a declaration<br>
<br>
Summary:<br>
This is PR36536.<br>
<br>
There are a few ways to reach Sema::ActOnStartOfFunctionDef with a null<br>
Decl. Currently, the parser continues on to attempt to parse the<br>
statements in the function body without pushing a function scope or<br>
declaration context. However, lots of statement parsing logic relies on<br>
getCurFunction() returning something reasonable. It turns out that<br>
getCurFunction() will never return null today because of an optimization<br>
where Sema pre-allocates one FunctionScopeInfo and reuses it when<br>
possible. This goes wrong when something inside the function body causes<br>
us to push another function scope, such as requiring an implicit<br>
definition of a special member function. Reusing the state clears it<br>
out, which will lead to bugs. In PR36536, we found that the SwitchStack<br>
gets unbalanced, because we push a switch, clear out the stack, and then<br>
try to pop a switch that isn't there.<br>
<br>
As a follow-up, I plan to move the pre-allocated FunctionScopeInfo out<br>
of the FunctionScopes stack. This means the FunctionScopes stack will<br>
often be empty, and callers of getCurFunction() will need to check for<br>
null.<br>
<br>
Reviewers: thakis<br>
<br>
Subscribers: cfe-commits<br>
<br>
Differential Revision: <a href="https://reviews.llvm.org/D43980" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D43980</a><br>
<br>
Added:<br>
cfe/trunk/test/SemaCXX/<wbr>pr36536.cpp<br>
Modified:<br>
cfe/trunk/lib/Sema/SemaDecl.<wbr>cpp<br>
<br>
Modified: cfe/trunk/lib/Sema/SemaDecl.<wbr>cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=326926&r1=326925&r2=326926&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/cfe/trunk/lib/Sema/<wbr>SemaDecl.cpp?rev=326926&r1=<wbr>326925&r2=326926&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/lib/Sema/SemaDecl.<wbr>cpp (original)<br>
+++ cfe/trunk/lib/Sema/SemaDecl.<wbr>cpp Wed Mar 7 10:55:10 2018<br>
@@ -12406,8 +12406,13 @@ static void RebuildLambdaScopeInfo(CXXMe<br>
<br>
Decl *Sema::<wbr>ActOnStartOfFunctionDef(Scope *FnBodyScope, Decl *D,<br>
SkipBodyInfo *SkipBody) {<br>
- if (!D)<br>
+ if (!D) {<br>
+ // Parsing the function declaration failed in some way. Push on a fake scope<br>
+ // anyway so we can try to parse the function body.<br>
+ PushFunctionScope();<br>
return D;<br>
+ }<br>
+<br>
FunctionDecl *FD = nullptr;<br>
<br>
if (FunctionTemplateDecl *FunTmpl = dyn_cast<FunctionTemplateDecl><wbr>(D))<br>
@@ -12816,6 +12821,9 @@ Decl *Sema::<wbr>ActOnFinishFunctionBody(Decl<br>
getCurFunction()-><wbr>ObjCWarnForNoInitDelegation = false;<br>
}<br>
} else {<br>
+ // Parsing the function declaration failed in some way. Pop the fake scope<br>
+ // we pushed on.<br>
+ PopFunctionScopeInfo(<wbr>ActivePolicy, dcl);<br>
return nullptr;<br>
}<br>
<br>
<br>
Added: cfe/trunk/test/SemaCXX/<wbr>pr36536.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/pr36536.cpp?rev=326926&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/cfe/trunk/test/<wbr>SemaCXX/pr36536.cpp?rev=<wbr>326926&view=auto</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/test/SemaCXX/<wbr>pr36536.cpp (added)<br>
+++ cfe/trunk/test/SemaCXX/<wbr>pr36536.cpp Wed Mar 7 10:55:10 2018<br>
@@ -0,0 +1,44 @@<br>
+// RUN: %clang_cc1 -std=c++11 %s -verify -fno-spell-checking<br>
+<br>
+// These test cases are constructed to make clang call ActOnStartOfFunctionDef<br>
+// with nullptr.<br>
+<br>
+struct ImplicitDefaultCtor1 {};<br>
+struct Foo {<br>
+ typedef int NameInClass;<br>
+ void f();<br>
+};<br>
+namespace bar {<br>
+// FIXME: Improved our recovery to make this a redeclaration of Foo::f,<br></blockquote><div><br></div><div>(nit: s/Improved/Improve/)</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+// even though this is in the wrong namespace. That will allow name lookup to<br>
+// find NameInClass below. Users are likely to hit this when they forget to<br>
+// close namespaces.<br>
+// expected-error@+1 {{cannot define or redeclare 'f' here}}<br>
+void Foo::f() {<br>
+ switch (0) { case 0: ImplicitDefaultCtor1 o; }<br>
+ // expected-error@+1 {{unknown type name 'NameInClass'}}<br>
+ NameInClass var;<br>
+}<br>
+} // namespace bar<br>
+<br>
+struct ImplicitDefaultCtor2 {};<br>
+template <typename T> class TFoo { void f(); };<br>
+// expected-error@+1 {{nested name specifier 'decltype(TFoo<T>())::'}}<br>
+template <typename T> void decltype(TFoo<T>())::f() {<br>
+ switch (0) { case 0: ImplicitDefaultCtor1 o; }<br>
+}<br>
+<br>
+namespace tpl2 {<br>
+struct ImplicitDefaultCtor3 {};<br>
+template <class T1> class A {<br>
+ template <class T2> class B {<br>
+ void mf2();<br>
+ };<br>
+};<br>
+template <class Y><br>
+template <><br>
+// expected-error@+1 {{nested name specifier 'A<Y>::B<double>::'}}<br>
+void A<Y>::B<double>::mf2() {<br>
+ switch (0) { case 0: ImplicitDefaultCtor3 o; }<br>
+}<br>
+}<br>
<br>
<br>
______________________________<wbr>_________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br></div></div>