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

GoogleCodeExporter commented 9 years ago

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:

GoogleCodeExporter commented 9 years ago

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

Added labels: ****
Removed labels: ****

GoogleCodeExporter commented 9 years ago

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

Added labels: ****
Removed labels: ****

GoogleCodeExporter commented 9 years ago

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

Added labels: ****
Removed labels: ****

GoogleCodeExporter commented 9 years ago

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
Added labels: ****
Removed labels: ****

HaveF / include-what-you-use

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