#include <ostream> vs #include "ostream"

GoogleCodeExporter commented 9 years ago

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!)
1. Use iwyu on a file missing some system includes
2. Compiling using cmake (cf #26), but issue is not related

What is the expected output? What do you see instead?

This is what I get:

#include <stdlib.h>                     // for atoi, atof, EXIT_SUCCESS
#include <string>                       // for operator==
#include "new"                          // for operator delete[], etc
#include "ostream"                      // for operator<<

I'd like to get:

#include <stdlib.h>                     // for atoi, atof, EXIT_SUCCESS
#include <string>                       // for operator==
#include <new>                          // for operator delete[], etc
#include <ostream>                      // for operator<<

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

iwyu revision: 304
gcc 4.6.1 on a Linux Debian testing.

Please provide any additional information below.

I suspect it's related to some missing case in iwyu_include_picker.cc
I will come with a minimal test case if necessary.

Original issue reported on code.google.com by emmanuel...@gmail.com on 23 Sep 2011 at 5:49

GoogleCodeExporter commented 9 years ago

My guess is it is an issue with cmake actually, though it's hard to tell with 
the data given.  It's unlikely to be iwyu_include_picker.cc.

Assuming ostream is a newly added include, iwyu decides whether to use <> or "" 
based on whether the file is in a 'system' directory.  It gets the list of 
system directories from clang.  If you provide the full output of the iwyu run, 
it should say where ostream lives.  Then you'll need to figure out what 
arguments cmake provides to iwyu (if any) to see what the system directories 
are.  Otherwise it will be the default system directories, which you can easily 
find by running iwyu in the debugger, though it may be possible some other way, 
via clang tools.

If you can't figure it out, the most useful info for you to include would be 
the *full* output of the iwyu run, and the exact command you used to run iwyu.  
A minimal test case, as you suspect, would also be helpful, but not as helpful 
as the first two.

Original comment by csilv...@gmail.com on 23 Sep 2011 at 2:43

GoogleCodeExporter commented 9 years ago

Attached is the full command (output-command) and the verbose output of iwuy 
(output-verbose). Let me know if there is a way to get more information out.

Original comment by emmanuel...@gmail.com on 24 Sep 2011 at 6:37

Attachments:

GoogleCodeExporter commented 9 years ago

Thanks!  The -v flag you gave was passed to clang, not iwyu.  For iwyu-verbose 
output, you need to do
   -Xiwyu --v=6

--v=6 will be a *lot* of output.  It will also share a lot about the content of 
your file.  --v=3 is likely enough, if either of those two facts concerns you.

Original comment by csilv...@gmail.com on 26 Sep 2011 at 6:28

GoogleCodeExporter commented 9 years ago

No issue with sharing the content of the file (this is on an open source 
project). The v=3 does not gave me much: it's following some typedef, but I 
still don't get the 
operator new[] is defined in "new"

v=6 ended up on a segfault, not sure if it went all the way. I couldn't find 
the lines that appear on the v=3. Do you confirm that all v=3 output should 
appear on the v=6? (if it is, I can explore the segfault further).

Original comment by emmanuel...@gmail.com on 27 Sep 2011 at 5:38

Attachments:

GoogleCodeExporter commented 9 years ago

OK, --v=6 was enough to get the info I needed (even before the crash, which 
definitely shouldn't be happening, but may be a bug in clang that I wouldn't 
know how to fix).

Here's what iwyu says:
   Search path: /usr/include/c++/4.6 (user)

The question is, why?  I see this
   Search path: /usr/include/c++/4.6/x86_64-linux-gnu/ (system)
   Search path: /usr/include/c++/4.6/backward (system)
so it's getting the subdirs right.  And I also see
   Search path: /usr/include/c++/4.5 (system)
   Search path: /usr/include/c++/4.4 (system)
so it's just 4.6 that's affected.

Your previous verbose output (which proved useful in itself!) says this:

#include "..." search starts here:
#include <...> search starts here:
  [...]
 /usr/include/c++/4.6

So whatever is producing that (clang somewhere?) thinks this is a system 
directory. I don't know why we don't.

If you're up for debugging clang, the relevant function is 
ComputeHeaderSearchPaths in iwyu_globals.cc.  It would be useful to see if 
/usr/include/c++/4.6 is showing up in the system_dir_begin()/system_dir_end().  
It would also be useful, after this function exits, to print the result vector 
and see what it contains.  In particular, I notice this:
     const char* path_type_name
        = (it->path_type == HeaderSearchPath::kSystemPath ? "system" : "user");
I wonder if it->path_type is garbage for /usr/include/c++/4.6, due to memory 
corruption or some such.

Original comment by csilv...@gmail.com on 27 Sep 2011 at 5:52

GoogleCodeExporter commented 9 years ago

At the end of ComputeHeaderSearchPaths, in search_path_map I have:

[...]
/home/christop/OTB/trunk/OTB/Utilities/otbsiftfast -> 2
/home/christop/OTB/trunk/OTB/Utilities/otbsvm -> 2
/home/christop/OTB/trunk/OTB/Utilities/tinyXMLlib -> 2
/home/christop/opensource/llvm-bin-debug/bin/../lib/clang/3.0/include -> 1
/usr/include -> 1
/usr/include/c++/4.4 -> 1
/usr/include/c++/4.4/backward -> 1
/usr/include/c++/4.4/x86_64-linux-gnu/ -> 1
/usr/include/c++/4.5 -> 1
/usr/include/c++/4.5/backward -> 1
/usr/include/c++/4.5/x86_64-linux-gnu/ -> 1
/usr/include/c++/4.6 -> 2
/usr/include/c++/4.6/backward -> 1
/usr/include/c++/4.6/x86_64-linux-gnu/ -> 1
/usr/include/x86_64-linux-gnu -> 1
/usr/local/include -> 1

So it is not garbage for /usr/include/c++/4.6, but it is the wrong type (user). 
So it doesn't seem to be memory corruption.

I also attached the backtrace of the segfault as it may be related.

I'll try to update llvm/clang/iwuy and see if it's still happening.

Original comment by emmanuel...@gmail.com on 2 Oct 2011 at 5:11

Attachments:

output-bt

GoogleCodeExporter commented 9 years ago

Hmm, I think you'll have to run this through the debugger, then, to see what is 
going on in the routines that create this map.  We're just copying data from 
clang, so it's likely the issue is there somewhere, and not in iwyu.  But it's 
difficult to be sure.

Original comment by csilv...@gmail.com on 3 Oct 2011 at 3:57

GoogleCodeExporter commented 9 years ago

I've tried to reproduce this issue on Mac OS X 10.6.8, but unsuccessfully. I've 
received
OTB/Testing/Code/BasicFilters/otbFrostFilterNew.cxx should add these lines:
#include <stdlib.h>                     // for EXIT_SUCCESS
#include <iostream>                     // for operator<<, ostream, cout, endl
<skipped>

Though I haven't configured OTB properly and encountered en error
OTB/Utilities/ITK/Code/Common/itkWin32Header.h:23:10: fatal error: 
'itkConfigure.h' file not found

But I have prepared a small test case to reproduce segfault (please find 
attached). Seems like it is a problem in clang, but I don't know what is the 
best way to report AST-related bugs, especially how to reproduce them. iwyu 
r323, llvm/clang r141492

Original comment by vsap...@gmail.com on 8 Oct 2011 at 4:13

Attachments:

GoogleCodeExporter commented 9 years ago

I went through the debugger and the creation of the header path seems to be 
correct on the clang side. More precisely, at the end of 
InitHeaderSearch::Realize (in InitHeaderSearch.cpp), I do have 
/usr/include/c++/4.6 with the SrcMgr::C_System type.

Something is interpreting it as a user directory later on, but I don't know 
what. Two troubling facts:
- in the header list, "/usr/include/c++/4.6" is the first system directory 
after a bunch of user directories.
- the RemoveDuplicates happens to remove one user directory
so in the header list, "/usr/include/c++/4.6" ends up at a position that was 
previously occupied by a user directory, if something externally 'remembers' a 
number of user directory without being updated, that would explain it.

Original comment by emmanuel...@gmail.com on 9 Oct 2011 at 7:29

GoogleCodeExporter commented 9 years ago

I confirm that there is an issue with the system_dir_begin() iterator: in 
ComputeHeaderSearchPaths in iwuy_globals.cc I have:

(gdb) p (*header_search).SearchDirs[75]
$78 = (clang::DirectoryLookup &) @0x1acbed0: {u = {Dir = 0x1abb6d0, Map = 
0x1abb6d0}, 
  DirCharacteristic = 0, UserSupplied = true, LookupType = 0, IsIndexHeaderMap = 0}
(gdb) p (*header_search).SearchDirs[76]
$79 = (clang::DirectoryLookup &) @0x1acbee0: {u = {Dir = 0x1abb710, Map = 
0x1abb710}, 
  DirCharacteristic = 1, UserSupplied = false, LookupType = 0, IsIndexHeaderMap = 0}
(gdb) p (*header_search).SearchDirs[77]
$80 = (clang::DirectoryLookup &) @0x1acbef0: {u = {Dir = 0x1abb750, Map = 
0x1abb750}, 
  DirCharacteristic = 1, UserSupplied = false, LookupType = 0, IsIndexHeaderMap = 0}
(gdb) p header_search->system_dir_begin()
$81 = {_M_current = 0x1acbef0}

[76] is "/usr/include/c++/4.6" with DirCharacteristic = 1 and UserSupplied = 
false it's clearly a system header, but the system_dir_begin() is pointing to 
the next one. The SystemDirIdx in clang HeaderSearch is probably missing an 
update during the RemoveDuplicates.

Would it be possible to get iwuy to rely on the DirCharacteristic instead of 
the system_dir_begin?

I can submit a patch if you are ok with this change.

Original comment by emmanuel...@gmail.com on 9 Oct 2011 at 11:20

GoogleCodeExporter commented 9 years ago

For reference:
I filed a bug for clang http://llvm.org/bugs/show_bug.cgi?id=11097

Original comment by emmanuel...@gmail.com on 10 Oct 2011 at 3:04

GoogleCodeExporter commented 9 years ago

Nice! -- I agree that the best way to resolve this is to fix the bug in clang.  
I would rather do that than do a workaround in iwyu.

It may be possible for you to fix it yourself and supply a patch (and a 
testcase?) -- I know the clang folks are responsive to that, though I don't 
know how easy it is to fix.  It's unlikely to get fixed soon otherwise, I'm 
guessing.

In the meantime, is it possible for you to run iwyu without the duplicate 
include-path entries?  That would avoid triggering this bug, if I understand 
what's going on correctly.

Original comment by csilv...@gmail.com on 10 Oct 2011 at 6:25

GoogleCodeExporter commented 9 years ago

That's pretty easy to fix in clang, I've just posted a patch. I'll update with 
the revision number here once this is fixed in clang.

Original comment by emmanuel...@gmail.com on 10 Oct 2011 at 7:22

GoogleCodeExporter commented 9 years ago

Great, thanks much!

Original comment by csilv...@gmail.com on 10 Oct 2011 at 6:40

GoogleCodeExporter commented 9 years ago

Committed clang revision 141565.

Original comment by Chad.Ros...@gmail.com on 10 Oct 2011 at 6:49

GoogleCodeExporter commented 9 years ago

So my understanding is that fixing the bug in clang should cause the problem 
you've seen to go away.  Can you verify that it has?  If so, I'll close the bug.

Original comment by csilv...@gmail.com on 10 Oct 2011 at 9:03

GoogleCodeExporter commented 9 years ago

Yes, we can close the bug, the segfault with --v=6 is likely to be clang 
related as well.

Original comment by emmanuel...@gmail.com on 11 Oct 2011 at 5:15

GoogleCodeExporter commented 9 years ago

Original comment by csilv...@gmail.com on 11 Oct 2011 at 6:27

Changed state: Verified

GoogleCodeExporter commented 9 years ago

Original comment by csilv...@gmail.com on 11 Oct 2011 at 6:27

Changed state: Fixed

yodamaster / include-what-you-use

#include <ostream> vs #include "ostream" #60