clang::FullSourceLoc::getSpellingLoc() const: Assertion 'isValid()' failed

[GoogleCodeExporter]

This assertion shows up a bunch when running on firefox (or just spidermonkey) 
on linux x86-64 but I'm not quite sure how to distill a reduced testcase.

Invocation:

/home/ehren/llvm/llvm/Debug+Asserts/bin/include-what-you-use -o jsanalyze.o -c  
-I./../../dist/system_wrappers_js -include 
/home/ehren/llvm/mozilla-central/js/src/config/gcc_hidden.h 
-DOSTYPE=\"Linux2.6.27.41-170.2.117.fc10\" -DOSARCH=Linux -DEXPORT_JS_API 
-D__STDC_LIMIT_MACROS -DJS_HAS_CTYPES -DDLL_PREFIX=\"lib\" -DDLL_SUFFIX=\".so\" 
-Ictypes/libffi/include -I.  -I/home/ehren/llvm/mozilla-central/js/src -I. 
-I./../../dist/include -I./../../dist/include/nsprpub  
-I/home/ehren/llvm/mozilla-central/objdir/dist/include/nspr   
-I/home/ehren/llvm/mozilla-central/js/src 
-I/home/ehren/llvm/mozilla-central/js/src/assembler 
-I/home/ehren/llvm/mozilla-central/js/src/yarr  -fPIC  -fno-rtti 
-fno-exceptions -Wall -Wpointer-arith -Woverloaded-virtual -Wsynth 
-Wno-ctor-dtor-privacy -Wno-non-virtual-dtor -Wcast-align -Wno-invalid-offsetof 
-Wno-variadic-macros -Werror=return-type -pedantic -Wno-long-long 
-fno-strict-aliasing -pthread -pipe  -DDEBUG -D_DEBUG -DTRACING -g 
-DUSE_SYSTEM_MALLOC=1 -DENABLE_ASSEMBLER=1 -DENABLE_JIT=1   -DMOZILLA_CLIENT 
-include ./js-confdefs.h -MD -MF .deps/jsanalyze.pp 
/home/ehren/llvm/mozilla-central/js/src/jsanalyze.cpp

From the backtrace it looks like this concerns include-what-you-use warnings 
that would should up with IWYU_VERBOSE=7. 

Eg the verbose output before the crash is:

Ignoring full use of js::analyze::Script 
(/home/ehren/llvm/mozilla-central/js/src/jsanalyze.cpp:655:35): #including dfn 
from "jsanalyze.h"
Noting fwd-decl use of js::analyze::Bytecode 
(/home/ehren/llvm/mozilla-central/js/src/jsanalyze.cpp:659:37) is satisfied by 
dfn in /home/ehren/llvm/mozilla-central/js/src/jsanalyze.h:56:8
Ignoring fwd-decl use of js::analyze::Bytecode 
(/home/ehren/llvm/mozilla-central/js/src/jsanalyze.cpp:659:37): have definition 
at /home/ehren/llvm/mozilla-central/js/src/jsanalyze.h:56:8
Ignoring full use of js::analyze::Script 
(/home/ehren/llvm/mozilla-central/js/src/jsanalyze.cpp:659:47): #including dfn 
from "jsanalyze.h"
Ignoring full use of js::analyze::Bytecode 
(/home/ehren/llvm/mozilla-central/js/src/jsanalyze.cpp:659:28): #including dfn 
from "jsanalyze.h"
Ignoring full use of js::analyze::Script 
(/home/ehren/llvm/mozilla-central/js/src/jsanalyze.cpp:661:21): #including dfn 
from "jsanalyze.h"
Ignoring full use of js::analyze::Bytecode 
(/home/ehren/llvm/mozilla-central/js/src/jsanalyze.cpp:666:28): #including dfn 
from "jsanalyze.h"
Ignoring full use of js::analyze::Script 
(/home/ehren/llvm/mozilla-central/js/src/jsanalyze.cpp:671:16): #including dfn 
from "jsanalyze.h"
include-what-you-use: SourceLocation.cpp:84: clang::FullSourceLoc 
clang::FullSourceLoc::getSpellingLoc() const: Assertion `isValid()' failed.
Stack dump:
0.      <eof> parser at end of file

When I apply the attached patch I get:

Invalid location: warning:  is defined in <jsanalyze.h>, which isn't directly 
#included.

and then a bunch of correct output (except for the angle brackets but that's 
another issue):

/home/ehren/llvm/mozilla-central/js/src/jsanalyze.cpp:55:5: warning: 
JS_FinishArenaPool is defined in <jsarena.h>, which isn't directly #included.
/home/ehren/llvm/mozilla-central/js/src/jsanalyze.cpp:60:12: warning: 
JSArenaPool needs a declaration, but does not provide or directly #include one.
/home/ehren/llvm/mozilla-central/js/src/jsanalyze.cpp:63:5: warning: 
JS_ARENA_ALLOCATE is defined in <jsarena.h>, which isn't directly #included.

etc

I guess if something like this patch is accepted I could also clean up the 
warning in iwyu_output.cc:GetWarningMessage to say something like 

warning: anonymous decl defined in "blah.h"

instead of just printing a space whenever the location is bad.

In any case, there might be deeper issues though...

eg from looking at the frame for GetWarningMsg:

#5  0x00000000004dbe20 in GetWarningMsg (use=@0x1f71e50) at iwyu_output.cc:1308
        warning = {static npos = 18446744073709551615,
  _M_dataplus = {<std::allocator<char>> = {<__gnu_cxx::new_allocator<char>> = {<No data fields>}, <No data fields>},
    _M_p = 0x1f90658 "/home/ehren/llvm/mozilla-central/js/src/jsanalyze.cpp:653:49: warning: JSScript is defined in <jsscript.h>, which isn't directly #included.\n"}}
        spelling_loc = {<clang::SourceLocation> = {ID = 22006}, SrcMgr = 0x1249150}
        instantiation_loc = {<clang::SourceLocation> = {ID = 33095856}, SrcMgr = 0x7fffffffd4ff}
        warning = {static npos = 18446744073709551615,
  _M_dataplus = {<std::allocator<char>> = {<__gnu_cxx::new_allocator<char>> = {<No data fields>}, <No data fields>}, _M_p = 0x7fffffffd4b0 "\020\t�\001"}}

it seems like the proper warning should be:

/home/ehren/llvm/mozilla-central/js/src/jsanalyze.cpp:653:49: warning: JSScript 
is defined in <jsscript.h>, which isn't directly #included.

Not sure why we end up with an invalid spelling loc in this case. Also not sure 
where __gnu_cxx::new_allocator<char>> comes into play. The source code line 
that's causing trouble is jsanalyze.cpp:653 which is a JS_ASSERT that resolves 
to RegularFunctionCall("message", __FILE__, __LINE__) so I wonder if it's 
something screwy with the __FILE__ directives.

Perhaps a follow up issue is needed? I'm not sure if this would affect 
correctness.

(note, same issue affects GetInstantiationLoc so I rolled both fixes into one)

Original issue reported on code.google.com by Ehren.M on 15 May 2011 at 8:56

Attachments:

• patch-isvalid.diff
• backtrace.txt

[GoogleCodeExporter]

I think your patch may be papering over a real problem, which I'd rather get at 
here.  With the patch, you get
   Invalid location: warning:  is defined in <jsanalyze.h>, which isn't directly #included.

You suggest that this might be an 'anonymous decl', but I suspect it's not: I 
suspect we're making a OneUse object with NULL values for everything, somehow.  
(Then the question is not 'why is the spelling loc 0', but 'why is the 
instantiation loc not-0' :-) )

One way to test this is in gdb, to 'print use' (in the GetWarningMsg frame).  
What does it show?

Original comment by csilv...@gmail.com on 15 May 2011 at 10:32

[GoogleCodeExporter]

Here's p use:
$1 = (const include_what_you_use::OneUse &) @0x1f72ec0: {symbol_name_ = {static 
npos = 18446744073709551615,
    _M_dataplus = {<std::allocator<char>> = {<__gnu_cxx::new_allocator<char>> = {<No data fields>}, <No data fields>}, _M_p = 0x3759cf9018 ""}}, short_symbol_name_ = {
    static npos = 18446744073709551615, _M_dataplus = {<std::allocator<char>> = {<__gnu_cxx::new_allocator<char>> = {<No data fields>}, <No data fields>},
      _M_p = 0x3759cf9018 ""}}, decl_ = 0x0, decl_filepath_ = {static npos = 18446744073709551615,
    _M_dataplus = {<std::allocator<char>> = {<__gnu_cxx::new_allocator<char>> = {<No data fields>}, <No data fields>}, _M_p = 0x1f7a348 "<jsanalyze.h>"}}, use_loc_ = {
    ID = 0}, use_kind_ = include_what_you_use::OneUse::kFullUse, in_cxx_method_body_ = false, comment_ = {static npos = 18446744073709551615,
    _M_dataplus = {<std::allocator<char>> = {<__gnu_cxx::new_allocator<char>> = {<No data fields>}, <No data fields>}, _M_p = 0x3759cf9018 ""}},
  public_headers_ = {<std::_Vector_base<std::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::basic_string<char, std::char_traits<char>, std::allocator<char> > > >> = {
      _M_impl = {<std::allocator<std::basic_string<char, std::char_traits<char>, std::allocator<char> > >> = {<__gnu_cxx::new_allocator<std::basic_string<char, std::char_traits<char>, std::allocator<char> > >> = {<No data fields>}, <No data fields>}, _M_start = 0x0, _M_finish = 0x0, _M_end_of_storage = 0x0}}, <No data fields>},
  suggested_header_ = {static npos = 18446744073709551615,
    _M_dataplus = {<std::allocator<char>> = {<__gnu_cxx::new_allocator<char>> = {<No data fields>}, <No data fields>}, _M_p = 0x1f79528 "<jsanalyze.h>"}},
  ignore_use_ = false, is_iwyu_violation_ = true}

Original comment by Ehren.M on 15 May 2011 at 10:48

[GoogleCodeExporter]

I think this was a problem with __FILE__ but it looks like it's been fixed 
somehow. 

When I run the same testcase now with the latest rev. of clang and iwyu with 
IWYU_VERBOSE=7 I don't actually get any lines matching "is defined in 
<jsscript.h>, which isn't directly #included." though so I couldn't say exactly 
why there's no more failure.

However, when you compiled 

const char *file = __FILE__;

it used to produce verbose output like 

[ Use macro   ] file.cpp:1:20:  (from Invalid location)

now you get

[ Use macro   ] file.cpp:1:20: __FILE__ (from Invalid location)

...probably the reason there's no more crash (although it's odd that I don't 
see any mention of __FILE__ in the verbose output from the original 
spidermonkey tescase)

Original comment by Ehren.M on 3 Jun 2011 at 1:15

[GoogleCodeExporter]

Hmm, so it sounds like the bug has magically fixed itself?  I'm happy to let it 
lie, in that case, and will close this Fixed (there's no "MagicallyFixed" 
status :-) ).  If I misunderstood, feel free to reopen the bug.

Original comment by csilv...@gmail.com on 3 Jun 2011 at 8:37

• Changed state: Fixed