r305903 - Function with unparsed body is a definition
Serge Pavlov via cfe-commits
cfe-commits at lists.llvm.org
Sat Aug 5 06:04:35 PDT 2017
This problem is addressed by https://reviews.llvm.org/D36353.
Thanks,
--Serge
2017-08-05 8:44 GMT+07:00 Alexander Kornienko <alexfh at google.com>:
> 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/557a2ade/attachment-0001.html>
More information about the cfe-commits
mailing list