r348276 - [AST][NFC] Make ArrayTypeTraitExpr non polymorphic
Mikael Holmén via cfe-commits
cfe-commits at lists.llvm.org
Tue Dec 11 23:27:09 PST 2018
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.
>
> 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
> Regards,
> Bruno
>
> On 11/12/2018 14:13, Mikael Holmén wrote:
>> Hi Bruno,
>>
>> I've no idea if this is really related to your change, but with this
>> commit, the following starts crashing:
>>
>> clang-tidy -allow-enabling-analyzer-alpha-checkers
>> -checks='-*,clang-analyzer-*' ./reduced.c --
>>
>> It seems like it recurses forever for some reason, and then we run out
>> of stack and it crashes:
>>
>> elxhw7c132-n7[llvm]: build-all-bbigcc/bin/clang-tidy
>> -allow-enabling-analyzer-alpha-checkers -checks='-*,clang-analyzer-*'
>> ./reduced.c --
>> #0 0x000000000074fbda llvm::sys::PrintStackTrace(llvm::raw_ostream&)
>> (build-all-bbigcc/bin/clang-tidy+0x74fbda)
>> #1 0x000000000074e0aa llvm::sys::RunSignalHandlers()
>> (build-all-bbigcc/bin/clang-tidy+0x74e0aa)
>> #2 0x000000000074e1d7 SignalHandler(int)
>> (build-all-bbigcc/bin/clang-tidy+0x74e1d7)
>> #3 0x00007f6cd7f84330 __restore_rt
>> (/lib/x86_64-linux-gnu/libpthread.so.0+0x10330)
>> #4 0x0000000000f06c63 clang::StmtVisitorBase<std::add_pointer,
>> (anonymous namespace)::StringRefCheckerVisitor,
>> void>::Visit(clang::Stmt*) (build-all-bbigcc/bin/clang-tidy+0xf06c63)
>> #5 0x0000000000f0a282 (anonymous
>> namespace)::StringRefCheckerVisitor::VisitChildren(clang::Stmt*)
>> (build-all-bbigcc/bin/clang-tidy+0xf0a282)
>> #6 0x0000000000f0527f clang::StmtVisitorBase<std::add_pointer,
>> (anonymous namespace)::StringRefCheckerVisitor,
>> void>::Visit(clang::Stmt*) (.part.132)
>> (build-all-bbigcc/bin/clang-tidy+0xf0527f)
>> #7 0x0000000000f06c68 clang::StmtVisitorBase<std::add_pointer,
>> (anonymous namespace)::StringRefCheckerVisitor,
>> void>::Visit(clang::Stmt*) (build-all-bbigcc/bin/clang-tidy+0xf06c68)
>> #8 0x0000000000f0a282 (anonymous
>> namespace)::StringRefCheckerVisitor::VisitChildren(clang::Stmt*)
>> (build-all-bbigcc/bin/clang-tidy+0xf0a282)
>> #9 0x0000000000f03fcf clang::StmtVisitorBase<std::add_pointer,
>> (anonymous namespace)::StringRefCheckerVisitor,
>> void>::Visit(clang::Stmt*) (.part.132)
>> (build-all-bbigcc/bin/clang-tidy+0xf03fcf)
>> #10 0x0000000000f06c68 clang::StmtVisitorBase<std::add_pointer,
>> (anonymous namespace)::StringRefCheckerVisitor,
>> void>::Visit(clang::Stmt*) (build-all-bbigcc/bin/clang-tidy+0xf06c68)
>> #11 0x0000000000f0a282 (anonymous
>> namespace)::StringRefCheckerVisitor::VisitChildren(clang::Stmt*)
>> (build-all-bbigcc/bin/clang-tidy+0xf0a282)
>> #12 0x0000000000f03fcf clang::StmtVisitorBase<std::add_pointer,
>> (anonymous namespace)::StringRefCheckerVisitor,
>> void>::Visit(clang::Stmt*) (.part.132)
>> (build-all-bbigcc/bin/clang-tidy+0xf03fcf)
>> #13 0x0000000000f06c68 clang::StmtVisitorBase<std::add_pointer,
>> (anonymous namespace)::StringRefCheckerVisitor,
>> void>::Visit(clang::Stmt*) (build-all-bbigcc/bin/clang-tidy+0xf06c68)
>> #14 0x0000000000f0a282 (anonymous
>> namespace)::StringRefCheckerVisitor::VisitChildren(clang::Stmt*)
>> (build-all-bbigcc/bin/clang-tidy+0xf0a282)
>> #15 0x0000000000f03fcf clang::StmtVisitorBase<std::add_pointer,
>> (anonymous namespace)::StringRefCheckerVisitor,
>> void>::Visit(clang::Stmt*) (.part.132)
>> (build-all-bbigcc/bin/clang-tidy+0xf03fcf)
>>
>> etc
>>
>> I've only seen this when I compile clang-tidy with gcc 5.4.0, assertions
>> on, with optimizations. Simply turning on debug info when compiling
>> clang-tidy makes the crash go away so it's quite fragile.
>>
>> Regards,
>> Mikael
>>
>> On 12/4/18 5:01 PM, Bruno Ricci via cfe-commits wrote:
>>> Author: brunoricci
>>> Date: Tue Dec 4 08:01:24 2018
>>> New Revision: 348276
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=348276&view=rev
>>> Log:
>>> [AST][NFC] Make ArrayTypeTraitExpr non polymorphic
>>>
>>> ArrayTypeTraitExpr is the only expression class which is polymorphic.
>>> As far as I can tell this is completely pointless.
>>>
>>> Differential Revision: https://reviews.llvm.org/D55221
>>>
>>> Reviewed By: aaron.ballman
>>>
>>>
>>> Modified:
>>> cfe/trunk/include/clang/AST/ExprCXX.h
>>> cfe/trunk/lib/AST/ExprCXX.cpp
>>>
>>> Modified: cfe/trunk/include/clang/AST/ExprCXX.h
>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ExprCXX.h?rev=348276&r1=348275&r2=348276&view=diff
>>> ==============================================================================
>>> --- cfe/trunk/include/clang/AST/ExprCXX.h (original)
>>> +++ cfe/trunk/include/clang/AST/ExprCXX.h Tue Dec 4 08:01:24 2018
>>> @@ -2455,8 +2455,6 @@ class ArrayTypeTraitExpr : public Expr {
>>> /// The type being queried.
>>> TypeSourceInfo *QueriedType = nullptr;
>>>
>>> - virtual void anchor();
>>> -
>>> public:
>>> friend class ASTStmtReader;
>>>
>>> @@ -2474,8 +2472,6 @@ public:
>>> explicit ArrayTypeTraitExpr(EmptyShell Empty)
>>> : Expr(ArrayTypeTraitExprClass, Empty), ATT(0) {}
>>>
>>> - virtual ~ArrayTypeTraitExpr() = default;
>>> -
>>> SourceLocation getBeginLoc() const LLVM_READONLY { return Loc; }
>>> SourceLocation getEndLoc() const LLVM_READONLY { return RParen; }
>>>
>>>
>>> Modified: cfe/trunk/lib/AST/ExprCXX.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprCXX.cpp?rev=348276&r1=348275&r2=348276&view=diff
>>> ==============================================================================
>>> --- cfe/trunk/lib/AST/ExprCXX.cpp (original)
>>> +++ cfe/trunk/lib/AST/ExprCXX.cpp Tue Dec 4 08:01:24 2018
>>> @@ -1443,5 +1443,3 @@ TypeTraitExpr *TypeTraitExpr::CreateDese
>>> void *Mem = C.Allocate(totalSizeToAlloc<TypeSourceInfo *>(NumArgs));
>>> return new (Mem) TypeTraitExpr(EmptyShell());
>>> }
>>> -
>>> -void ArrayTypeTraitExpr::anchor() {}
>>>
>>>
>>> _______________________________________________
>>> cfe-commits mailing list
>>> cfe-commits at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>
More information about the cfe-commits
mailing list