r348276 - [AST][NFC] Make ArrayTypeTraitExpr non polymorphic

Bruno Ricci via cfe-commits cfe-commits at lists.llvm.org
Wed Dec 12 05:47:00 PST 2018



On 12/12/2018 13:25, Mikael Holmén wrote:
> 
> 
> On 12/12/18 2:17 PM, Bruno Ricci wrote:
>>
>>
>> On 12/12/2018 07:27, Mikael Holmén wrote:
>>> Hi,
>>>
>>> On 12/11/18 5:49 PM, Bruno Ricci wrote:
>>>> Hi Mikael,
>>>>
>>>> I can indeed reproduce this with gcc 5.5.0 when doing a Release
>>>> build with assertions. I cannot reproduce this with gcc 6.5.0
>>>> (also with a Release build with assertions), nor can I reproduce
>>>> this with clang 7 (also with a Release build with assertions).
>>>>
>>>> I tried to instrument StringRefCheckerVisitor::VisitChildren but this
>>>> just causes the stack overflow to move to CGBuilder.
>>>>
>>>> What is stranger is that the reproducer do not even causes
>>>> an ArrayTypeTraitExpr node to be created.
>>>>
>>>> Increasing the maximum stack size with ulimit by a small amount fixes
>>>> the problem, so I don't think this is a mis-compilation either.
>>>
>>> Oh, I didn't realize it actually passed with a larger stack, I thought
>>> it recursed forever.
>>>
>>> I see now that before the change clang-tidy compiled with gcc 5.4 passed
>>> with stacksize limited to 2288k and after it needs 9372k.
>>>
>>> If I use clang 3.6 to compile I needed 1747k before the change and 1750k
>>> after.
>>
>> Huh ? This is a huge increase.
>>
> 
> Yes, well compared to the insane increase for gcc 5.4 it's nothing :)
> 
> But perhaps the increase with clang is interesting as well, I don't know.

I was talking about the increase with gcc :)

> 
>>>
>>>>
>>>> I think that this change is correct since no one is using the polymorphism
>>>> of ArrayTypeTraitExpr. Indeed taking the blunt step of making it final do
>>>> not causes any failure.
>>>>
>>>> The only possible explaination I can think of is that for one reason
>>>> or another, with this particular compiler + build settings, this change
>>>> causes an increase of stack usage which exceeed the default limit ?
>>>>
>>>
>>> Yes perhaps it's just some deficiency in old gcc versions that is
>>> exposed by this then.
>>>
>>> Thanks,
>>> Mikael
>>
>> Perhaps it is just indeed a deficiency in gcc 5.4, but this is strange.
>>
> 
> Yes. I'm not sure what to do with this, apart from that we will speed up 
> our team's transition from gcc 5.4 to something newer. That would be 
> needed anyway due to the fact that llvm wants to start using C++17, but 
> we'll try to do that sooner rather than later now.
> 
> Regards,
> Mikael

Well there is always the possibility to revert it if this a blocking issue.
I can try to investigate this problem this week-end if I have some time.

Regards,
Bruno

> 
>> Bruno
>>>
>>>
>>>> Regards,
>>>> Bruno
> 


More information about the cfe-commits mailing list