Closed GoogleCodeExporter closed 9 years ago
Please, provide also LLVM/Clang revision.
Original comment by vsap...@gmail.com
on 22 Aug 2012 at 11:45
The current git master branch of llvm + clang (built without polly or lld).
Just updated everything since posting and it is broken on llvm master branch
revision fdeb9fe5e08146d9cb953000cb893eda80329a08 + clang master branch
revision 54c2f88540994726032bfbf1d087bb67767432af.
A quick gdb session reveals that the line "return
Base::TraverseTypeLoc(typeloc);" in iwyu.cc at line 395 is the source of the
crash. Stepping into the clang code shows that the
RecursiveASTVisitor::TraverseAutoTypeLoc call results in the call to
include_what_you_use::BaseAstVisitor<include_what_you_use::IwyuAstConsumer>::Tra
verseType.
That call resolves to QualType::getTypePtr() from clang/AST/Type.h and there,
getCommonPtr() returns NULL, and is unconditionally dereferenced: "return
getCommonPtr()->BaseType"
I know nothing about clang internals, so I am unqualified to comment further,
but I hope that helps. I will try to reproduce this crash without requiring
40MB of our headers so there is a test case, but for now that's as far as I can
go.
Original comment by supercilious.dude@gmail.com
on 22 Aug 2012 at 12:30
Just to add to my previous comment, a debug build of clang, with assertions
enabled confirms the findings and does indeed abort with:
include-what-you-use:
/home/user/software/repositories/llvm/tools/clang/include/clang/AST/Type.h:503:
const clang::ExtQualsTypeCommonBase* clang::QualType::getCommonPtr() const:
Assertion `!isNull() && "Cannot retrieve a NULL type pointer"' failed.
Stack dump:
0. <eof> parser at end of file
Original comment by supercilious.dude@gmail.com
on 22 Aug 2012 at 12:46
AutoTypeLoc seems to be used for the C++11 'auto' keyword.
As far as I know, IWYU only does C++03 (and barely that), I expect C++11 adds
new semantics for IWYU, especially around what constitutes a 'use'.
That said, it would be nice if it didn't segfault until we get as far as C++11
support.
Anyway, using 'auto' and --std=c++11 might make it easier to come up with a
narrow repro case.
Original comment by kim.gras...@gmail.com
on 22 Aug 2012 at 7:11
Can you, please, run IWYU with -Xiwyu --verbose=6? The entire command line
looks like
$ ../../../../../build/Debug+Asserts/bin/include-what-you-use -Xiwyu
--verbose=6 -std=c++11 -I . tests/t.cc
And post here the piece of log prior to segfault. I am looking for something
like
tests/t.cc:11:7: (4) [ VarDecl ] Foo i = bar()
tests/t.cc:11:2: (5) [ AutoType ] class Foo
tests/t.cc:11:2: (5) [ AutoTypeLoc ] class Foo
tests/t.cc:11:2: (6) [ RecordType ] class Foo
I think it will point at a piece of source code causing segfault and will help
to create a repro case.
Original comment by vsap...@gmail.com
on 26 Aug 2012 at 7:08
I posted this on issue 57, but it looks like it's more relevant to this one
actually.
I just ran into this problem, and spent a while debugging it. I don't know what
the exact cause is, but here's an extremely simple test case that demonstrates
the issue.
Tested with Clang 3.1, and IWYU r371 with the addCommentHandler change reverted
so that it compiles against 3.1.
Run with the following command line:
./iwyu -std=gnu++0x bad.cpp
Here's the contents of bad.cpp:
template<typename T>
class Foo {
};
template<typename T>
void g(Foo<T> x) {
auto y = x;
}
int main() {
return 0;
}
Original comment by christop...@gmail.com
on 6 Nov 2012 at 6:28
Original comment by vsap...@gmail.com
on 25 Nov 2012 at 10:16
We are hitting the assertion `!isNull() && "Cannot retrieve a NULL type
pointer"' when we are trying to traverse deduced type of AutoType. But
according to AutoType documentation [1]
> However, within templates and before the initializer is attached, there is no
deduced type
> and an auto type is type-dependent and canonical.
So it is normal for deduced type to be null.
Resolved assertion failure by getting Type* only when traversed QualType is not
null. Also in testing code added possibility to specify Clang flags for some
files in order to use "-std=c++11". Patches are attached.
[1] http://clang.llvm.org/doxygen/classclang_1_1AutoType.html
Original comment by vsap...@gmail.com
on 9 Dec 2012 at 4:00
Attachments:
Nice!
Two comments:
1) I think it would be better to delegate to the base RecursiveASTVisitor
behavior if the QualType is null, i.e.
if (!qualtype.isNull()) {
const Type* type = qualtype.getTypePtr();
if (current_ast_node_->StackContainsContent(type))
return true; // avoid recursion
ASTNode node(type, *GlobalSourceManager());
CurrentASTNodeUpdater canu(¤t_ast_node_, &node);
}
return Base::TraverseType(qualtype);
It currently only returns true as well, but if that ever changes, we'll benefit
from the new behavior.
2) '-I .' in the flags between run_iwyu_tests.py and iwyu_test_util.py could
probably be simplified by always starting clang_flags with '-I .'. In
iwyu_test_util.py:
clang_flags = clang_flags or []
clang_flags = ['-I .'] + clang_flags
and then get rid of -I from run_iwyu_tests.py. I think we'll be hard pressed to
find a test that doesn't use '-I .'.
Otherwise, looks good!
Original comment by kim.gras...@gmail.com
on 10 Dec 2012 at 11:30
I've attached the modified patch. I haven't put CurrentASTNodeUpdater inside if
body because it will be destroyed too early (before Base::TraverseType).
Original comment by vsap...@gmail.com
on 2 Jan 2013 at 9:48
Attachments:
> I haven't put CurrentASTNodeUpdater inside if body
> because it will be destroyed too early (before Base::TraverseType).
Oh, silly of me!
This looks good to me and the new test passes on my Windows setup. Please
commit!
Original comment by kim.gras...@gmail.com
on 2 Jan 2013 at 10:46
Oh, one thing I noticed;
+ clang_flags: Extra command-line flags to pass to clang, for example "-I .".
"-I ." seems like a bad example now that it's always added.
Original comment by kim.gras...@gmail.com
on 2 Jan 2013 at 10:07
This issue was closed by revision r421.
Original comment by vsap...@gmail.com
on 3 Jan 2013 at 9:07
Changed the example to "-std=c++11", committed r421.
Original comment by vsap...@gmail.com
on 3 Jan 2013 at 9:18
Original issue reported on code.google.com by
supercilious.dude@gmail.com
on 22 Aug 2012 at 11:07