r305903 - Function with unparsed body is a definition
Alexander Kornienko via cfe-commits
cfe-commits at lists.llvm.org
Fri Aug 4 18:44:27 PDT 2017
Thank you for the analysis! It would be nice if you could implement a short
term workaround to avoid crashes. Maybe Richard has ideas on what a proper
solution should look like.
On Wed, Aug 2, 2017 at 8:05 PM, Serge Pavlov <sepavloff at gmail.com> wrote:
> At first thank you for the nice test case.
>
> This crash is not caused by r305903. The root cause is that instantiation
> of f<int> is triggered when parse of f<T> is not finished yet. This is the
> case just addressed by that change.
> The instantiation is requested by the code (https://github.com/llvm-mirro
> r/clang/blob/master/lib/Sema/SemaExpr.cpp#L13622):
>
> else if (Func->isConstexpr())
> // Do not defer instantiations of constexpr functions, to avoid the
> // expression evaluator needing to call back into Sema if it sees a
> // call to such a function.
> InstantiateFunctionDefinition(PointOfInstantiation, Func);
>
>
> It looks like a violation of C++ Standard, as function is instantiated
> when there is no use of it. However if this check is removed and
> instantiation is postponed as for non-constexpr functions, lots of tests
> fail.
>
> Without r305903 the call to `getDefinition` for templated declaration of
> f<T> returned null, declaration f<int> was moved to the list of pending
> instantiations and crash is not observed. It does not mean however that
> original source code would be compiled correctly.
>
> It this is urgent problem, I can prepare a patch that will mimic behavior
> before r305903 for instantiation of constexpr functions. If there is some
> time I would investigate this problem in depth, as it manifests itself in
> number of cases, for instance in https://bugs.llvm.org/show_
> bug.cgi?id=33561.
>
> Thanks,
> --Serge
>
> 2017-08-01 23:28 GMT+07:00 Serge Pavlov <sepavloff at gmail.com>:
>
>> Yes, sure, I will investigate it.
>>
>> Thanks,
>> --Serge
>>
>> 2017-08-01 21:32 GMT+07:00 Alexander Kornienko <alexfh at google.com>:
>>
>>> This change causes an assertion failure on valid code. Could you take a
>>> look at fixing this?
>>>
>>> A reduced test case:
>>>
>>> $ cat /tmp/SemaTemplateInstantiateDecl-crash2.cpp
>>> template <class T>
>>> constexpr void f(T) {
>>> f(0);
>>> }
>>> $ clang -fsyntax-only -std=c++11 /tmp/SemaTemplateInstantiateDe
>>> cl-crash2.cpp
>>> assert.h assertion failed at llvm/tools/clang/lib/Sema/Sema
>>> TemplateInstantiateDecl.cpp:3840 in void clang::Sema::InstantiateFuncti
>>> onDefinition(clang::SourceLocation, clang::FunctionDecl *, bool, bool,
>>> bool): (Pattern |
>>> | PatternDecl->isDefaulted()) && "unexpected kind of function template
>>> definition"
>>> *** Check failure stack trace: ***
>>> @ 0x6094c4a __assert_fail
>>> @ 0x2705bfe clang::Sema::InstantiateFunctionDefinition()
>>> @ 0x2c1fb13 clang::Sema::MarkFunctionReferenced()
>>> @ 0x2bec07b clang::Sema::MarkAnyDeclReferenced()
>>> @ 0x2c2327f MarkExprReferenced()
>>> @ 0x2be8f0a clang::Sema::MarkDeclRefReferenced()
>>> @ 0x2980ac0 clang::Sema::FixOverloadedFunctionReference()
>>> @ 0x2982e66 FinishOverloadedCallExpr()
>>> @ 0x2982b39 clang::Sema::BuildOverloadedCallExpr()
>>> @ 0x2be461b clang::Sema::ActOnCallExpr()
>>> @ 0x2571e90 clang::Parser::ParsePostfixExpressionSuffix()
>>> @ 0x25784cb clang::Parser::ParseCastExpression()
>>> @ 0x2570865 clang::Parser::ParseCastExpression()
>>> @ 0x256f1e3 clang::Parser::ParseAssignmentExpression()
>>> @ 0x256f0bf clang::Parser::ParseExpression()
>>> @ 0x2517eeb clang::Parser::ParseExprStatement()
>>> @ 0x2517074 clang::Parser::ParseStatement
>>> OrDeclarationAfterAttributes()
>>> @ 0x2516b50 clang::Parser::ParseStatementOrDeclaration()
>>> @ 0x251deb4 clang::Parser::ParseCompoundStatementBody()
>>> @ 0x251ea53 clang::Parser::ParseFunctionStatementBody()
>>> @ 0x24f5b23 clang::Parser::ParseFunctionDefinition()
>>> @ 0x25082a5 clang::Parser::ParseSingleDec
>>> larationAfterTemplate()
>>> @ 0x2507652 clang::Parser::ParseTemplateD
>>> eclarationOrSpecialization()
>>> @ 0x2506fa5 clang::Parser::ParseDeclarati
>>> onStartingWithTemplate()
>>> @ 0x25b6853 clang::Parser::ParseDeclaration()
>>> @ 0x24f36d5 clang::Parser::ParseExternalDeclaration()
>>> @ 0x24f2739 clang::Parser::ParseTopLevelDecl()
>>> @ 0x24f220e clang::Parser::ParseFirstTopLevelDecl()
>>> @ 0x24ed582 clang::ParseAST()
>>>
>>> On Wed, Jun 21, 2017 at 2:46 PM, Serge Pavlov via cfe-commits <
>>> cfe-commits at lists.llvm.org> wrote:
>>>
>>>> Author: sepavloff
>>>> Date: Wed Jun 21 07:46:57 2017
>>>> New Revision: 305903
>>>>
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=305903&view=rev
>>>> Log:
>>>> Function with unparsed body is a definition
>>>>
>>>> While a function body is being parsed, the function declaration is not
>>>> considered
>>>> as a definition because it does not have a body yet. In some cases it
>>>> leads to
>>>> incorrect interpretation, the case is presented in
>>>> https://bugs.llvm.org/show_bug.cgi?id=14785:
>>>> ```
>>>> template<typename T> struct Somewhat {
>>>> void internal() const {}
>>>> friend void operator+(int const &, Somewhat<T> const &) {}
>>>> };
>>>> void operator+(int const &, Somewhat<char> const &x) { x.internal(); }
>>>> ```
>>>> When statement `x.internal()` in the body of global `operator+` is
>>>> parsed, the type
>>>> of `x` must be completed, so the instantiation of `Somewhat<char>` is
>>>> started. It
>>>> instantiates the declaration of `operator+` defined inline, and makes a
>>>> check for
>>>> redefinition. The check does not detect another definition because the
>>>> declaration
>>>> of `operator+` is still not defining as does not have a body yet.
>>>>
>>>> To solves this problem the function `isThisDeclarationADefinition`
>>>> considers
>>>> a function declaration as a definition if it has flag `WillHaveBody`
>>>> set.
>>>>
>>>> This change fixes PR14785.
>>>>
>>>> Differential Revision: https://reviews.llvm.org/D30375
>>>>
>>>> This is a recommit of 305379, reverted in 305381, with small changes.
>>>>
>>>> Modified:
>>>> cfe/trunk/include/clang/AST/Decl.h
>>>> cfe/trunk/lib/Sema/SemaCUDA.cpp
>>>> cfe/trunk/lib/Sema/SemaDecl.cpp
>>>> cfe/trunk/lib/Sema/SemaDeclCXX.cpp
>>>> cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
>>>> cfe/trunk/test/SemaCXX/friend2.cpp
>>>>
>>>> Modified: cfe/trunk/include/clang/AST/Decl.h
>>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/
>>>> AST/Decl.h?rev=305903&r1=305902&r2=305903&view=diff
>>>> ============================================================
>>>> ==================
>>>> --- cfe/trunk/include/clang/AST/Decl.h (original)
>>>> +++ cfe/trunk/include/clang/AST/Decl.h Wed Jun 21 07:46:57 2017
>>>> @@ -1874,7 +1874,7 @@ public:
>>>> ///
>>>> bool isThisDeclarationADefinition() const {
>>>> return IsDeleted || IsDefaulted || Body || IsLateTemplateParsed ||
>>>> - hasDefiningAttr();
>>>> + WillHaveBody || hasDefiningAttr();
>>>> }
>>>>
>>>> /// doesThisDeclarationHaveABody - Returns whether this specific
>>>>
>>>> Modified: cfe/trunk/lib/Sema/SemaCUDA.cpp
>>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaC
>>>> UDA.cpp?rev=305903&r1=305902&r2=305903&view=diff
>>>> ============================================================
>>>> ==================
>>>> --- cfe/trunk/lib/Sema/SemaCUDA.cpp (original)
>>>> +++ cfe/trunk/lib/Sema/SemaCUDA.cpp Wed Jun 21 07:46:57 2017
>>>> @@ -629,12 +629,6 @@ static bool IsKnownEmitted(Sema &S, Func
>>>> // emitted, because (say) the definition could include "inline".
>>>> FunctionDecl *Def = FD->getDefinition();
>>>>
>>>> - // We may currently be parsing the body of FD, in which case
>>>> - // FD->getDefinition() will be null, but we still want to treat FD
>>>> as though
>>>> - // it's a definition.
>>>> - if (!Def && FD->willHaveBody())
>>>> - Def = FD;
>>>> -
>>>> if (Def &&
>>>> !isDiscardableGVALinkage(S.getASTContext().GetGVALinkageFor
>>>> Function(Def)))
>>>> return true;
>>>>
>>>> Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
>>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaD
>>>> ecl.cpp?rev=305903&r1=305902&r2=305903&view=diff
>>>> ============================================================
>>>> ==================
>>>> --- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
>>>> +++ cfe/trunk/lib/Sema/SemaDecl.cpp Wed Jun 21 07:46:57 2017
>>>> @@ -12232,6 +12232,7 @@ Decl *Sema::ActOnFinishFunctionBody(Decl
>>>>
>>>> if (FD) {
>>>> FD->setBody(Body);
>>>> + FD->setWillHaveBody(false);
>>>>
>>>> if (getLangOpts().CPlusPlus14) {
>>>> if (!FD->isInvalidDecl() && Body && !FD->isDependentContext() &&
>>>>
>>>> Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
>>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaD
>>>> eclCXX.cpp?rev=305903&r1=305902&r2=305903&view=diff
>>>> ============================================================
>>>> ==================
>>>> --- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
>>>> +++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Wed Jun 21 07:46:57 2017
>>>> @@ -13878,6 +13878,9 @@ void Sema::SetDeclDeleted(Decl *Dcl, Sou
>>>> return;
>>>> }
>>>>
>>>> + // Deleted function does not have a body.
>>>> + Fn->setWillHaveBody(false);
>>>> +
>>>> if (const FunctionDecl *Prev = Fn->getPreviousDecl()) {
>>>> // Don't consider the implicit declaration we generate for explicit
>>>> // specializations. FIXME: Do not generate these implicit
>>>> declarations.
>>>>
>>>> Modified: cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
>>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaT
>>>> emplateInstantiateDecl.cpp?rev=305903&r1=305902&r2=305903&view=diff
>>>> ============================================================
>>>> ==================
>>>> --- cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp (original)
>>>> +++ cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp Wed Jun 21
>>>> 07:46:57 2017
>>>> @@ -1782,6 +1782,9 @@ Decl *TemplateDeclInstantiator::VisitFun
>>>> Previous.clear();
>>>> }
>>>>
>>>> + if (isFriend)
>>>> + Function->setObjectOfFriendDecl();
>>>> +
>>>> SemaRef.CheckFunctionDeclaration(/*Scope*/ nullptr, Function,
>>>> Previous,
>>>> isExplicitSpecialization);
>>>>
>>>>
>>>> Modified: cfe/trunk/test/SemaCXX/friend2.cpp
>>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/f
>>>> riend2.cpp?rev=305903&r1=305902&r2=305903&view=diff
>>>> ============================================================
>>>> ==================
>>>> --- cfe/trunk/test/SemaCXX/friend2.cpp (original)
>>>> +++ cfe/trunk/test/SemaCXX/friend2.cpp Wed Jun 21 07:46:57 2017
>>>> @@ -170,3 +170,40 @@ struct Test {
>>>> template class Test<int>;
>>>>
>>>> }
>>>> +
>>>> +namespace pr14785 {
>>>> +template<typename T>
>>>> +struct Somewhat {
>>>> + void internal() const { }
>>>> + friend void operator+(int const &, Somewhat<T> const &) {} //
>>>> expected-error{{redefinition of 'operator+'}}
>>>> +};
>>>> +
>>>> +void operator+(int const &, Somewhat<char> const &x) { //
>>>> expected-note {{previous definition is here}}
>>>> + x.internal(); // expected-note{{in instantiation of template class
>>>> 'pr14785::Somewhat<char>' requested here}}
>>>> +}
>>>> +}
>>>> +
>>>> +namespace D30375 {
>>>> +template <typename K> struct B {
>>>> + template <typename A> bool insert(A &);
>>>> +};
>>>> +
>>>> +template <typename K>
>>>> +template <typename A> bool B<K>::insert(A &x) { return x < x; }
>>>> +
>>>> +template <typename K> class D {
>>>> + B<K> t;
>>>> +
>>>> +public:
>>>> + K x;
>>>> + bool insert() { return t.insert(x); }
>>>> + template <typename K1> friend bool operator<(const D<K1> &, const
>>>> D<K1> &);
>>>> +};
>>>> +
>>>> +template <typename K> bool operator<(const D<K> &, const D<K> &);
>>>> +
>>>> +void func() {
>>>> + D<D<int>> cache;
>>>> + cache.insert();
>>>> +}
>>>> +}
>>>>
>>>>
>>>> _______________________________________________
>>>> cfe-commits mailing list
>>>> cfe-commits at lists.llvm.org
>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>>
>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170805/27146813/attachment-0001.html>
More information about the cfe-commits
mailing list