[cfe-commits] [cfe-dev] Clang crash on infinite template instantiation

[Richard Smith]

On Fri, Sep 21, 2012 at 2:58 PM, Richard Smith <richard at metafoo.co.uk>wrote:

> On Fri, Sep 21, 2012 at 1:42 PM, Pan, Wei <wei.pan at intel.com> wrote:
>
>>  Hello Richard,****
>>
>> ** **
>>
>> Does the attached patch look good for this bug? All your suggestions were
>> applied.
>>
>
> I'm surprised that we now produce four (fatal!) errors for
> instantiation-depth-subst.cpp. We shouldn't be emitting diagnostics after
> the first fatal error. Do you know why it's happening? Please add a FIXME
> to that test for that.
>

I've fixed this in r164437.


> Please merge your new test file (test/SemaCXX/PR12053.cpp) into
> test/SemaCXX/trailing-return-0x.cpp (put the contents into a namespace
> PR12053 in that file). Then this LGTM.
>
> Do you need me to commit this for you?
>
>
>>
>>
>> Thanks,****
>>
>> ** **
>>
>> Wei****
>>
>> ** **
>>
>> *From:* metafoo at gmail.com [mailto:metafoo at gmail.com] *On Behalf Of *Richard
>> Smith
>> *Sent:* Friday, September 07, 2012 4:26 PM
>>
>> *To:* Pan, Wei
>> *Cc:* cfe-dev at cs.uiuc.edu
>> *Subject:* Re: [cfe-dev] Clang crash on infinite template instantiation**
>> **
>>
>>  ** **
>>
>> > --- a/include/clang/Sema/Sema.h****
>>
>> > +++ b/include/clang/Sema/Sema.h****
>>
>> ** **
>>
>> For your future consideration: Convention on these lists is to use -p0
>> patches instead of -p1 patches. You can configure git with '[diff] noprefix
>> = true' for this. Also, patches should generally be cfe-commits@, not
>> cfe-dev at .****
>>
>> ** **
>>
>> ** **
>>
>> The patch generally looks fine. A few minor things:****
>>
>> ** **
>>
>> > @@ -1939,6 +1939,10 @@ public:****
>>
>> >      BEF_end****
>>
>> >    };****
>>
>> >  ****
>>
>> > +  /// IsBuildingRecoveryCallExpr - True if Sema is building a recovery
>> call****
>>
>> ** **
>>
>> Please use \brief instead of repeating the variable name.****
>>
>> ** **
>>
>> > +  /// expression.****
>>
>> > +  bool IsBuildingRecoveryCallExpr;****
>>
>> > +****
>>
>> >    ForRangeStatus BuildForRangeBeginEndCall(Scope *S, SourceLocation
>> Loc,****
>>
>> ** **
>>
>> Please move this to the top of the class, with the other member variables.
>> ****
>>
>> ** **
>>
>> ** **
>>
>> > +  // template <typename T> auto foo(T t) -> decltpye(foo(t)) {}****
>>
>> > +  // template <typename T> auto foo(T t) -> decltpye(foo(&t)) {}****
>>
>> ** **
>>
>> Typo "decltpye".****
>>
>> ** **
>>
>> ** **
>>
>> > +++ b/test/SemaTemplate/instantiation-depth-subst.cpp****
>>
>> > @@ -1,9 +1,6 @@****
>>
>> >  // RUN: %clang_cc1 -std=c++11 -verify %s -ftemplate-depth 2****
>>
>> >  ****
>>
>> >  // PR9793****
>>
>> > -template<typename T> auto f(T t) -> decltype(f(t)); // \****
>>
>> > -// expected-error {{recursive template instantiation exceeded maximum
>> depth of 2}} \****
>>
>> > -// expected-note 3 {{while substituting}} \****
>>
>> > -// expected-note {{candidate}}****
>>
>> > +template<typename T> auto f(T t) -> decltype(f(t)); // expected-note
>> {{candidate template ignored}}****
>>
>> > ****
>>
>> >  int k = f(0); // expected-error {{no matching function for call to
>> 'f'}}****
>>
>> ** **
>>
>> This test is no longer testing what it was intended to test (that we have
>> a depth limit for pure substitution). Please instead change the test as
>> follows (so 'f' can be found within its own return type via ADL):****
>>
>> ** **
>>
>> -int k = f(0);****
>>
>> +struct S {};****
>>
>> +int k = f(S());****
>>
>> ** **
>>
>> On Fri, Sep 7, 2012 at 9:39 AM, Pan, Wei <wei.pan at intel.com> wrote:****
>>
>> Hi Richard,****
>>
>>  ****
>>
>> Disabling BuildRecoveryCallExpr seems correct to solving this problem. Do
>> you think this patch is correct?****
>>
>>  ****
>>
>> Thanks!****
>>
>>  ****
>>
>> Wei****
>>
>>  ****
>>
>> *From:* metafoo at gmail.com [mailto:metafoo at gmail.com] *On Behalf Of *Richard
>> Smith
>> *Sent:* Thursday, September 06, 2012 6:26 PM
>> *To:* Pan, Wei
>> *Cc:* cfe-dev at cs.uiuc.edu
>> *Subject:* Re: [cfe-dev] Clang crash on infinite template instantiation**
>> **
>>
>>  ****
>>
>> On Thu, Sep 6, 2012 at 1:50 PM, Pan, Wei <wei.pan at intel.com> wrote:****
>>
>>  Hello Clang Dev,****
>>
>>  ****
>>
>> Recently I looked into the clang bug 12053,****
>>
>>  ****
>>
>> template <typename T> auto foo(T t) -> decltype(foo(t)) {}****
>>
>>  ****
>>
>> http://llvm.org/bugs/show_bug.cgi?id=12053****
>>
>>  ****
>>
>> which crashes clang (trunk)  (and gcc 4.6 too).****
>>
>>  ****
>>
>> As far as I know,  Clang does realize that there is no candidate ****
>>
>> available while resolving “decltype(foo(t))”, however
>> BuildRecoveryCallExpr ****
>>
>> will find  template foo (in DiagnoseTwoPhaseLookup) ****
>>
>> and try to instantiate it again. This leads to the crash.****
>>
>>  ****
>>
>> I am wondering if the following is the right way to fix this. The basic
>> idea is: ****
>>
>>  ****
>>
>> Before starting instantiating a function template, we check if the same
>> function****
>>
>> template instantiation *with the same template arguments* is already
>> in-progress. ****
>>
>> If yes, then clang is not making any progress and should lead an infinite
>> loop.****
>>
>> We treat it as an SFINAE.****
>>
>>  ****
>>
>> The attached patch will fix the clang crashing on the above test and
>> other similar tests like****
>>
>>  ****
>>
>> template <typename T> auto foo(T t) -> decltype(bar(t)) {}****
>>
>> template <typename T> auto bar(T t) -> decltype(foo(t)) {}****
>>
>> int x = foo(0);****
>>
>>  ****
>>
>> This is not a final patch since this change will affect two tests (only
>> these two) ****
>>
>> instantiation-depth-subst.cpp and instantiation-depth-subst-2.cpp.****
>>
>> Any thoughts?****
>>
>>   ****
>>
>> I generally like the idea of checking for cyclic function template
>> instantiations, but I'm hesitant about this approach -- scanning through
>> the instantiation stack each time introduces overhead which grows
>> quadratically in the instantiation depth, which could be too slow for deep
>> instantiation stacks.****
>>
>>  ****
>>
>> Also, this approach doesn't solve the entire problem. For instance, in
>> this case, there is no cycle:****
>>
>>  ****
>>
>>   template<typename T> auto foo(T t) -> decltype(foo(&t)) {}****
>>
>>  ****
>>
>> Perhaps we can solve this more directly, by just disabling
>> BuildRecoveryCallExpr when we're already in the middle of recovery.****
>>
>> ** **
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120921/c91bbbe1/attachment.html>