Macro expansion in #include

[GoogleCodeExporter]

What steps will reproduce the problem?  Give the *exact* arguments passed
to include-what-you-use, and attach the input source file that gives the
problem (minimal test-cases are much appreciated!)

consider main.cpp :
#define CSTDLIB <cstdlib>
#include CSTDLIB

then try to compile :
> include-what-you-use main.cpp

What is the expected output?
>main.cpp should add these lines:
>
>main.cpp should remove these lines:
>- #include <cstdlib>  // lines 2-2
>
>The full include-list for main.cpp:
>---

What do you see instead?
>iwyu_lexer_utils.cc:84: Assertion failed: false && "Unexpected token being 
#included"
>Stack dump:
>0.      main.cpp:2:2: current parser token 'include'
>Aborted

What version of the product are you using? On what operating system?
svn trunk r349

Please provide any additional information below.
For instance this prevents analyzing any code depending on Boost because of 
boost/config.hpp being included
http://www.boost.org/doc/libs/1_49_0/boost/config.hpp

Original issue reported on code.google.com by chatelet...@gmail.com on 31 Mar 2012 at 9:51

[GoogleCodeExporter]

Sorry for the late reply -- I've moved on from clang-work (in fact, from C++ in 
general), so I'm afraid I won't be very helpful for solving problems like this.

That said, I could have sworn there was a test for computed includes in 
tests/badinc.cc.  Do those tests pass for you?  If so, I wonder what is 
different here.  Maybe you can run iwyu in the debugger to try to figure out 
what's going on.

Original comment by csilv...@gmail.com on 20 Apr 2012 at 1:35

[GoogleCodeExporter]

I'm attaching output from a failed test and svn info. If there's anything more 
I can do, please let me know.

Original comment by rene.m...@gmail.com on 20 Apr 2012 at 11:49

Attachments:

• test.log
• svn.log

[GoogleCodeExporter]

Aha! -- the tests *do* fail.  So something has changed in clang that is causing 
iwyu to crash now in the face of computed includes.  It's hopefully an easy 
fix, but as I mentioned I'm not on iwyu anymore and won't be able to take a 
look at it.

On the wiki page (http://code.google.com/p/include-what-you-use/w/list) there 
are some tips for developers, including how to run tests in the debugger.  
Maybe that will turn up what the issue is, and an easy fix.  Good luck!

Original comment by csilv...@gmail.com on 24 Apr 2012 at 6:03

[GoogleCodeExporter]

Looks like this issue is caused by Clang change introduced r153527. After this 
change for code like
#include MACRO
GetIncludeNameAsTyped() receives location which points to MACRO, not to the 
string the MACRO expands to.

Currently I am trying to understand if it is Clang's fault that it returns such 
a location or such behavior is correct and we should expand macros in IWYU 
ourselves.

Original comment by vsap...@gmail.com on 9 May 2012 at 4:20

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

Here is a patch which fixes this issue. I'd like to check myself if it helps to 
analyze Boost, to explain what was the cause of this bug, and to write in more 
details why I've fixed the issue the way I've fixed it (i.e. which alternatives 
I've discarded), probably I'm still missing the better way to fix bug.

But patch is actually finished and I think it is ready for review. During 
review please pay attention to comments and naming of variables/functions - I 
am not entirely satisfied with them.

Original comment by vsap...@gmail.com on 23 May 2012 at 10:59

Attachments:

• computed_include_issue_67.patch

[GoogleCodeExporter]

Nice work!

After applying the patch, though, 54 out of 62 tests fail with seg faults.

I built and ran tests on Ubuntu Ubuntu 10.10 x86_64 with;
 IWYU rev. 351
 Clang and LLVM rev. 157379
 GCC: g++ (Ubuntu/Linaro 4.5.1-7ubuntu2) 4.5.1

Which Clang version did you develop the patch for?

I tried to get a backtrace from GDB;

--
#0  0x000000000042b26d in __gnu_cxx::new_allocator<clang::SourceLocation>::const
ruct (this=0x174e6d0, __p=0x1fc, __val=...)
    at /usr/include/c++/4.5/ext/new_allocator.h:105
#1  0x000000000042b2ca in std::deque<clang::SourceLocation, std::allocator<clang
::SourceLocation> >::_M_push_back_aux (this=0x174e6d0, __t=...)
    at /usr/include/c++/4.5/bits/deque.tcc:376
#2  0x000000000041fbb3 in std::deque<clang::SourceLocation, std::allocator<clang
::SourceLocation> >::push_back (this=0x174e6d0, __x=...)
    at /usr/include/c++/4.5/bits/stl_deque.h:1293
#3  0x000000000041769d in std::stack<clang::SourceLocation, std::deque<clang::So
urceLocation, std::allocator<clang::SourceLocation> > >::push (this=0x174e6d0,
    __x=...) at /usr/include/c++/4.5/bits/stl_stack.h:183
#4  0x00000000005155b4 in include_what_you_use::IwyuPreprocessorInfo::InclusionD
irective (this=0x174e360, hash_loc=..., include_token=..., filename=...,
    is_angled=false, file=0x1768890, last_include_token_loc=...,
    search_path=..., relative_path=...) at iwyu_preprocessor.cc:606
#5  0x0000000000d2f827 in clang::Preprocessor::HandleIncludeDirective (
    this=0x1707100, HashLoc=..., IncludeTok=..., LookupFrom=0x0,
    isImport=false) at PPDirectives.cpp:1357
#6  0x0000000000d2d248 in clang::Preprocessor::HandleDirective (
    this=0x1707100, Result=...) at PPDirectives.cpp:649
#7  0x0000000000d1a131 in clang::Lexer::LexTokenInternal (this=0x1766b90,
    Result=...) at Lexer.cpp:3192
---Type <return> to continue, or q <return> to quit---
#8  0x00000000004f5160 in clang::Lexer::Lex (this=0x1766b90, Result=...)
    at /home/kimgr/dev/llvm-2/llvm/tools/clang/tools/include-what-you-use/../../include/clang/Lex/Lexer.h:147
#9  0x0000000000555286 in clang::Preprocessor::Lex (this=0x1707100, Result=...)
    at /home/kimgr/dev/llvm-2/llvm/tools/clang/lib/Frontend/../../include/clang/Lex/Preprocessor.h:606
#10 0x0000000000d1826e in clang::Lexer::LexTokenInternal (this=0x1768870,
    Result=...) at Lexer.cpp:2586
#11 0x00000000004f5160 in clang::Lexer::Lex (this=0x1768870, Result=...)
    at /home/kimgr/dev/llvm-2/llvm/tools/clang/tools/include-what-you-use/../../include/clang/Lex/Lexer.h:147
#12 0x0000000000555286 in clang::Preprocessor::Lex (this=0x1707100, Result=...)
    at /home/kimgr/dev/llvm-2/llvm/tools/clang/lib/Frontend/../../include/clang/Lex/Preprocessor.h:606
#13 0x00000000006ff0c3 in clang::Parser::ConsumeToken (this=0x1765be0)
    at /home/kimgr/dev/llvm-2/llvm/tools/clang/lib/Parse/../../include/clang/Parse/Parser.h:313
#14 0x00000000006f721f in clang::Parser::ExpectAndConsumeSemi (this=0x1765be0,
    DiagID=1052) at Parser.cpp:188
#15 0x0000000000708764 in clang::Parser::ParseDeclGroup (this=0x1765be0,
    DS=..., Context=0, AllowFunctionDefinitions=false, DeclEnd=0x7fffffffe0c0,
    FRI=0x0) at ParseDecl.cpp:1316
#16 0x0000000000707b65 in clang::Parser::ParseSimpleDeclaration (
---Type <return> to continue, or q <return> to quit---
    this=0x1765be0, Stmts=..., Context=0, DeclEnd=..., attrs=...,
    RequireSemi=true, FRI=0x0) at ParseDecl.cpp:1057
#17 0x000000000070796d in clang::Parser::ParseDeclaration (this=0x1765be0,
    Stmts=..., Context=0, DeclEnd=..., attrs=...) at ParseDecl.cpp:1010
#18 0x00000000006f939d in clang::Parser::ParseExternalDeclaration (
    this=0x1765be0, attrs=..., DS=0x0) at Parser.cpp:671
#19 0x00000000006f8d42 in clang::Parser::ParseTopLevelDecl (this=0x1765be0,
    Result=...) at Parser.cpp:551
#20 0x00000000006f58af in clang::ParseAST (S=..., PrintStats=false,
    SkipFunctionBodies=false) at ParseAST.cpp:87
#21 0x0000000000550cf0 in clang::ASTFrontendAction::ExecuteAction (
    this=0x16fd6f0) at FrontendAction.cpp:416
#22 0x0000000000550925 in clang::FrontendAction::Execute (this=0x16fd6f0)
    at FrontendAction.cpp:335
#23 0x00000000005249b6 in clang::CompilerInstance::ExecuteAction (
    this=0x1701c10, Act=...) at CompilerInstance.cpp:677
#24 0x0000000000406af7 in main (argc=6, argv=0x7fffffffe438) at iwyu.cc:3706
--

It looks like heap corruption at first glance. I don't know my way around the 
GNU C++ libraries or tools, so I might be missing something vital. Please post 
back if I can provide more detail.

- Kim

Original comment by kim.gras...@gmail.com on 24 May 2012 at 7:42

[GoogleCodeExporter]

I've checked patch with LLVM + Clang at r157339. I'll update and try again.

Which tests don't fail with applied patch? Without applied patch only badinc.cc 
fails, correct?

Original comment by vsap...@gmail.com on 24 May 2012 at 12:35

[GoogleCodeExporter]

I think it's something to do with the toolset used to build clang + iwyu -- 
I've used GCC to build, so I think clang is using GCC's headers.

I've narrowed it down to the fact that FileChanged_EnterFile does not add 
something that passes as IsBuiltinOrCommandLineFile, but FileChanged_ExitToFile 
does not make the same check, so the stack is popped at exit even if nothing 
has been pushed at entry.

So, presumably, your toolset/headers don't trigger this built-in condition, and 
your new stack becomes consistent.

Attached is a log of output from;

$ ./run_iwyu_tests.py &> results.txt

I've worked around by adding a IsBuiltinOrCommandLineFile check in 
FileChanged_ExitToFile, which makes the segfaults go away, but unfortunately 
tests fail on badinc.cc again... I haven't had time to dig further.

- Kim

Original comment by kim.gras...@gmail.com on 24 May 2012 at 1:18

Attachments:

• results.txt

[GoogleCodeExporter]

The idea was to push include filename location every time we encounter #include 
directive (InclusionDirective). After that we include file (EnterFile, 
ExitToFile) or skip file (FileSkipped) if it has header guards and was included 
earlier.

I've assumed that all EnterFile, FileSkipped callbacks are performed for actual 
#includes, except the first EnterFile. Probably this assumption is wrong, need 
to check it in debugger.

Original comment by vsap...@gmail.com on 24 May 2012 at 2:08

[GoogleCodeExporter]

I added debug prints to show the sequence of Enter/Inclusion/Exit/Rename 
(whatever that last one means...), and ran the simplest test case I knew -- 
array.cc:

--
$ ../../../../Debug+Asserts/bin/include-what-you-use -Xiwyu --verbose=0 -I . 
tests/array.cc

EnterFile, loc: tests/array.cc
EnterFile, loc: <built-in>
RenameFile, loc: <built-in>
EnterFile, loc: <built-in>
ExitFile, loc: <built-in>
ExitFile, loc: tests/array.cc
InclusionDirective, including: tests/direct.h
EnterFile, loc: tests/direct.h
InclusionDirective, including: tests/indirect.h
EnterFile, loc: tests/indirect.h
ExitFile, loc: tests/direct.h
ExitFile, loc: tests/array.cc
--

This run also has a check in ExitToFile that only pops from 
include_filename_location_stack_ if !IsBuiltInOrCommandLineFile.

Hope that helps,
- Kim

Original comment by kim.gras...@gmail.com on 24 May 2012 at 7:44

[GoogleCodeExporter]

Thanks, Kim. Provided log is extremely helpful. I've checked that I perform the 
same steps (enter/exit/rename file), but don't crash when pop too much elements 
from include_filename_location_stack_. Will update the patch accordingly.

Original comment by vsap...@gmail.com on 24 May 2012 at 11:28

[GoogleCodeExporter]

I've updated handling include_filename_location_stack_, but I don't like the 
idea with stack anymore. I am afraid I cannot reliably keep track of filename 
location stack due to all unpredictable <built-in> EnterFile, ExitFile 
(unpredictable at least for me). Of course I can read Clang source code and 
figure out why and when each <built-in> EnterFile is called, but I believe we 
should rely on PPCallbacks API, which doesn't explain <built-in> stuff.

So I've replaced stack with a single SourceLocation variable and overwrite it 
when encounter inclusion directive. I don't like this approach very much, 
because it seems fragile. But I don't know how to pass data from 
InclusionDirective to FileChanged_EnterFile and guarantee that these callbacks 
correspond to each other.

Patch is attached, I've checked it against Clang at r158020.

Original comment by vsap...@gmail.com on 5 Jun 2012 at 11:14

Attachments:

• issue_67_no_loc_stack.patch

[GoogleCodeExporter]

With this patch I get a new failure in tests/badinc.cc:

  tests/badinc.cc:547: Unexpected diagnostic:
  I2_TemplateClass::~I2_TemplateClass<FOO> is defined in "tests/badinc-i2-inl.h", which isn't directly #included.

tests/badinc.cc:547 says:

  I2_TemplateClass<int>* ptr = new I2_TemplateClass<int>(42, "hi");  // NewExpr

and it makes no sense to me that that line should require the dtor to be 
defined.

Running the tests without your patch applied stops on the computed include, so 
it's hard to say whether this error would have occurred anyway. Attaching 
result logs from both runs for reference.

I think this patch is good, and that we should track the dtor problem as a 
separate issue.

Original comment by kim.gras...@gmail.com on 10 Jun 2012 at 2:52

Attachments:

• r158020.351.txt
• r158020.351.patch.txt

[GoogleCodeExporter]

I am pretty sure the failure in tests/badinc.cc doesn't depend on macros, so 
I've applied patch in r353. Still keep this issue open before tests run without 
failures.

Failure in tests/badinc.cc is mentioned in issue #65, 
http://code.google.com/p/include-what-you-use/issues/detail?id=65#c12. I've 
missed 'Unexpected diagnostic' error and noticed only Mac-specific 
<sys/errno.h> problem.

Original comment by vsap...@gmail.com on 12 Jun 2012 at 8:50

• Changed state: Started

[GoogleCodeExporter]

Here is my diagnosis and explanation to the patch.

Including macros was broken because GetIncludeNameAsTyped received macro 
expansion location and didn't see the filename to which macro was expanded. 
This problem is caused by Clang r153527 because "As an inclusion position for 
the included file, use the file location of the file where it was included but 
*after* the macro expansions".

I've resolved this issue by using location of the last token in inclusion 
directive provided in InclusionDirective preprocessor callback. But when macro 
is #define MACRO <stdio.h>, then the last token is closing angled bracket. 
That's why I take macro start spelling location to obtain the entire filename.

Original comment by vsap...@gmail.com on 12 Jun 2012 at 8:52

[GoogleCodeExporter]

Original comment by vsap...@gmail.com on 2 Sep 2012 at 5:51

• Changed state: Verified