Creduce crashes in CFGBuilder::addAutomaticObjDtors

[Quuxplusone]

Bugzilla Link	PR17665
Status	NEW
Importance	P normal
Reported by	octoploid (octoploid@yandex.com)
Reported on	2013-10-23 12:28:38 -0700
Last modified on	2018-10-25 20:12:03 -0700
Version	trunk
Hardware	PC Linux
CC	alp@nuanti.com, llvm-bugs@lists.llvm.org, richard-llvm@metafoo.co.uk
Fixed by commit(s)
Attachments
Blocks
Blocked by
See also

Creduce crashes quite often when build with current trunk clang.

markus@x4 /tmp % gdb --args /var/tmp/creduce/clang_delta/clang_delta --query-
instances=replace-function-def-with-decl test.ii
Reading symbols from /var/tmp/creduce/clang_delta/clang_delta...done.
(gdb) run
Starting program: /var/tmp/creduce/clang_delta/clang_delta --query-
instances=replace-function-def-with-decl test.ii
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/libthread_db.so.1".

Program received signal SIGSEGV, Segmentation fault.
clang::FunctionDecl::isNoReturn (this=0x0) at
/home/markus/llvm/tools/clang/lib/AST/Decl.cpp:2412
2412      return hasAttr<NoReturnAttr>() || hasAttr<CXX11NoReturnAttr>() ||
(gdb) bt
#0  clang::FunctionDecl::isNoReturn (this=0x0) at
/home/markus/llvm/tools/clang/lib/AST/Decl.cpp:2412
#1  0x0000000000f81099 in (anonymous
namespace)::CFGBuilder::addAutomaticObjDtors (this=0x7fffffffcc50, S=0x1bde4f0,
B=..., E=...)
    at /home/markus/llvm/tools/clang/lib/Analysis/CFG.cpp:854
#2  0x0000000000f815ff in (anonymous
namespace)::CFGBuilder::addLocalScopeAndDtors (this=<optimized out>,
S=<optimized out>)
    at /home/markus/llvm/tools/clang/lib/Analysis/CFG.cpp:1028
#3  0x0000000000f85585 in addLocalScopeAndDtors (S=0x1bde4f0,
this=0x7fffffffcc50) at /home/markus/llvm/tools/clang/lib/Analysis/CFG.cpp:1572
#4  (anonymous namespace)::CFGBuilder::VisitCompoundStmt
(this=this@entry=0x7fffffffcc50, C=C@entry=0x1bde4f0) at
/home/markus/llvm/tools/clang/lib/Analysis/CFG.cpp:1557
#5  0x0000000000f83f43 in (anonymous namespace)::CFGBuilder::Visit
(this=this@entry=0x7fffffffcc50, S=S@entry=0x1bde4f0, asc=..., asc@entry=...)
    at /home/markus/llvm/tools/clang/lib/Analysis/CFG.cpp:1092
#6  0x0000000000f88390 in addStmt (S=0x1bde4f0, this=0x7fffffffcc50) at
/home/markus/llvm/tools/clang/lib/Analysis/CFG.cpp:434
#7  buildCFG (Statement=0x1bde4f0, D=<optimized out>, this=0x7fffffffcc50) at
/home/markus/llvm/tools/clang/lib/Analysis/CFG.cpp:660
#8  clang::CFG::buildCFG (D=<optimized out>, Statement=0x1bde4f0,
C=C@entry=0x1737160, BO=...) at
/home/markus/llvm/tools/clang/lib/Analysis/CFG.cpp:3421
#9  0x0000000000f75c47 in clang::AnalysisDeclContext::getCFG
(this=this@entry=0x7fffffffcfe0) at
/home/markus/llvm/tools/clang/lib/Analysis/AnalysisDeclContext.cpp:179
#10 0x0000000000f68804 in CheckFallThrough (AC=...) at
/home/markus/llvm/tools/clang/lib/Sema/AnalysisBasedWarnings.cpp:102
#11 CheckFallThroughForBody (AC=..., CD=..., blkExpr=<optimized out>,
Body=0x1bde4f0, D=0x1bddce0, S=...)
    at /home/markus/llvm/tools/clang/lib/Sema/AnalysisBasedWarnings.cpp:359
#12 clang::sema::AnalysisBasedWarnings::IssueWarnings
(this=this@entry=0x17584b8, P=..., fscope=fscope@entry=0x1758f80,
D=D@entry=0x1bddce0, blkExpr=blkExpr@entry=0x0)
    at /home/markus/llvm/tools/clang/lib/Sema/AnalysisBasedWarnings.cpp:1714
#13 0x0000000000c526d7 in clang::Sema::PopFunctionScopeInfo
(this=this@entry=0x1756c70, WP=WP@entry=0x7fffffffd280, D=D@entry=0x1bddce0,
blkExpr=blkExpr@entry=0x0)
    at /home/markus/llvm/tools/clang/lib/Sema/Sema.cpp:1052
#14 0x0000000000ce131e in clang::Sema::ActOnFinishFunctionBody (this=0x1756c70,
dcl=dcl@entry=0x1bddce0, Body=0x1bde4f0,
IsInstantiation=IsInstantiation@entry=false)
    at /home/markus/llvm/tools/clang/lib/Sema/SemaDecl.cpp:9806
#15 0x0000000000ce1b87 in clang::Sema::ActOnFinishFunctionBody (this=<optimized
out>, D=D@entry=0x1bddce0, BodyArg=<optimized out>)
    at /home/markus/llvm/tools/clang/lib/Sema/SemaDecl.cpp:9654
#16 0x0000000000c2de95 in clang::Parser::ParseFunctionStatementBody
(this=this@entry=0x1771f90, Decl=0x1bddce0, BodyScope=...)
    at /home/markus/llvm/tools/clang/lib/Parse/ParseStmt.cpp:2419
#17 0x0000000000c3b6e4 in clang::Parser::ParseLexedMethodDef (this=0x1771f90,
LM=...) at /home/markus/llvm/tools/clang/lib/Parse/ParseCXXInlineMethods.cpp:461
#18 0x0000000000c3b500 in clang::Parser::ParseLexedMethodDefs (this=0x1771f90,
Class=...) at
/home/markus/llvm/tools/clang/lib/Parse/ParseCXXInlineMethods.cpp:398
#19 0x0000000000bf3e00 in clang::Parser::ParseCXXMemberSpecification
(this=this@entry=0x1771f90, RecordLoc=..., AttrFixitLoc=...,
AttrFixitLoc@entry=..., Attrs=...,
    TagType=TagType@entry=18, TagDecl=TagDecl@entry=0x1bdd0b0) at /home/markus/llvm/tools/clang/lib/Parse/ParseDeclCXX.cpp:2713
#20 0x0000000000bf585a in clang::Parser::ParseClassSpecifier
(this=this@entry=0x1771f90, TagTokKind=TagTokKind@entry=clang::tok::kw_class,
StartLoc=..., DS=...,
    TemplateInfo=..., AS=AS@entry=clang::AS_none, EnteringContext=EnteringContext@entry=true, DSC=DSC@entry=clang::Parser::DSC_top_level, Attributes=...)
    at /home/markus/llvm/tools/clang/lib/Parse/ParseDeclCXX.cpp:1614
#21 0x0000000000bd9555 in clang::Parser::ParseDeclarationSpecifiers
(this=this@entry=0x1771f90, DS=..., TemplateInfo=...,
AS=AS@entry=clang::AS_none,
    DSContext=DSContext@entry=clang::Parser::DSC_top_level, LateAttrs=LateAttrs@entry=0x0) at /home/markus/llvm/tools/clang/lib/Parse/ParseDecl.cpp:3043
#22 0x0000000000bcbdf4 in clang::Parser::ParseDeclOrFunctionDefInternal
(this=this@entry=0x1771f90, attrs=..., DS=..., AS=AS@entry=clang::AS_none)
    at /home/markus/llvm/tools/clang/lib/Parse/Parser.cpp:852
#23 0x0000000000bcc4f9 in clang::Parser::ParseDeclarationOrFunctionDefinition
(this=0x1771f90, attrs=..., AS=clang::AS_none, DS=0x0)
    at /home/markus/llvm/tools/clang/lib/Parse/Parser.cpp:918
#24 0x0000000000bcc52f in clang::Parser::ParseDeclarationOrFunctionDefinition
(this=<optimized out>, attrs=..., DS=<optimized out>, AS=<optimized out>)
    at /home/markus/llvm/tools/clang/lib/Parse/Parser.cpp:920
#25 0x0000000000bd0403 in clang::Parser::ParseExternalDeclaration
(this=this@entry=0x1771f90, attrs=..., DS=DS@entry=0x0)
    at /home/markus/llvm/tools/clang/lib/Parse/Parser.cpp:783
#26 0x0000000000bd0ce2 in clang::Parser::ParseTopLevelDecl
(this=this@entry=0x1771f90, Result=...) at
/home/markus/llvm/tools/clang/lib/Parse/Parser.cpp:588
#27 0x0000000000bc72eb in clang::ParseAST (S=...,
PrintStats=PrintStats@entry=false,
SkipFunctionBodies=SkipFunctionBodies@entry=false)
    at /home/markus/llvm/tools/clang/lib/Parse/ParseAST.cpp:134
#28 0x0000000000a32602 in TransformationManager::doTransformation
(this=0x170d5c0, ErrorMsg="", ErrorCode=@0x167e170: -1) at
TransformationManager.cpp:200
#29 0x000000000046ec34 in main (argc=<optimized out>, argv=<optimized out>) at
ClangDelta.cpp:211
(gdb) q

As you can see dtor is NULL in CFGBuilder::addAutomaticObjDtors
lib/Analysis/CFG.cpp.

The following patch fixes the issue for me:

diff --git a/lib/Analysis/CFG.cpp b/lib/Analysis/CFG.cpp
index 5b48a15def0e..18931334cdc6 100644
--- a/lib/Analysis/CFG.cpp
+++ b/lib/Analysis/CFG.cpp
@@ -851,7 +851,7 @@ void
CFGBuilder::addAutomaticObjDtors(LocalScope::const_iterator B,
     Ty = Context->getBaseElementType(Ty);

     const CXXDestructorDecl *Dtor = Ty->getAsCXXRecordDecl()->getDestructor();
-    if (Dtor->isNoReturn())
+    if (Dtor && Dtor->isNoReturn())
       Block = createNoReturnBlock();
     else
       autoCreateBlock();

[Quuxplusone]

Could you provide some code that reproduces the issue?

[Quuxplusone]

(In reply to comment #1)
> Could you provide some code that reproduces the issue?

Sorry, no. As I wrote above you need clang_delta from creduce to
trigger the issue.

I've been using the following patch as a workaround without any issues:

diff --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp
index d6361e8767c7..d0335d1a3068 100644
--- a/clang/lib/Analysis/CFG.cpp
+++ b/clang/lib/Analysis/CFG.cpp
@@ -1112,7 +1112,7 @@ void
CFGBuilder::addAutomaticObjDtors(LocalScope::const_iterator B,
     Ty = Context->getBaseElementType(Ty);

     const CXXDestructorDecl *Dtor = Ty->getAsCXXRecordDecl()->getDestructor();
-    if (Dtor->isNoReturn())
+    if (Dtor && Dtor->isNoReturn())
       Block = createNoReturnBlock();
     else
       autoCreateBlock();
@@ -3596,7 +3596,7 @@ CFGBlock
*CFGBuilder::VisitCXXBindTemporaryExprForTemporaryDtors(
     // a new block for the destructor which does not have as a successor
     // anything built thus far. Control won't flow out of this block.
     const CXXDestructorDecl *Dtor = E->getTemporary()->getDestructor();
-    if (Dtor->isNoReturn()) {
+    if (Dtor && Dtor->isNoReturn()) {
       Succ = B;
       Block = createNoReturnBlock();
     } else {

[Quuxplusone]

(In reply to comment #2)
> (In reply to comment #1)
> > Could you provide some code that reproduces the issue?
>
> Sorry, no. As I wrote above you need clang_delta from creduce to
> trigger the issue.

If there's no way to reproduce this with ordinary source input to clang,
getDestructor() is guaranteed to return non-null here and the fault lies with
the third-party tool.

Perhaps your clang_delta tool isn't synthesising the necessary implicit
destructor to form a valid AST?

[Quuxplusone]

(In reply to comment #3)
> (In reply to comment #2)
> > (In reply to comment #1)
> > > Could you provide some code that reproduces the issue?
> >
> > Sorry, no. As I wrote above you need clang_delta from creduce to
> > trigger the issue.
>
> If there's no way to reproduce this with ordinary source input to clang,
> getDestructor() is guaranteed to return non-null here and the fault lies
> with the third-party tool.
>
> Perhaps your clang_delta tool isn't synthesising the necessary implicit
> destructor to form a valid AST?

Yes, that's possible. But isn't robustness when used with third-party tools
something to strive for?

[Quuxplusone]

(In reply to comment #4)
> (In reply to comment #3)
> > (In reply to comment #2)
> > > (In reply to comment #1)
> > > > Could you provide some code that reproduces the issue?
> > >
> > > Sorry, no. As I wrote above you need clang_delta from creduce to
> > > trigger the issue.
> >
> > If there's no way to reproduce this with ordinary source input to clang,
> > getDestructor() is guaranteed to return non-null here and the fault lies
> > with the third-party tool.
> >
> > Perhaps your clang_delta tool isn't synthesising the necessary implicit
> > destructor to form a valid AST?
>
> Yes, that's possible. But isn't robustness when used with third-party tools
> something to strive for?

Ordinarily I'd agree and say a null-check is pretty harmless.

In this case though I'm concerned that the tool could well be dropping a valid
declaration. Silently skipping here with a null check isn't necessarily the
right thing to do, especially since clang CodeGen also makes the same non-null
assumption.

So it's not a question of shifting the blame, but rather that this appears to
be uncovering some genuine undefined behaviour in the tool.

[Quuxplusone]

In this case it may be the nature of the tool. Creduce will
(as the name implies) try to reduce the input file as much as
possible. It never uses the CodeGen. It is a purely source to
source transformer.