Bug 45457

Summary: [Qt] QtTestBrowser is crashing on www.index.hu
Product: WebKit Reporter: Balazs Kelemen <kbalazs>
Component: JavaScriptCoreAssignee: Nobody <webkit-unassigned>
Status: RESOLVED DUPLICATE    
Severity: Critical CC: abecsi, barraclough, ggaren, kling, loki, oliver, ossy, tonikitoo, zherczeg
Priority: P1 Keywords: Qt, QtTriaged
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Linux   
Bug Depends on: 47920    
Bug Blocks:    
Attachments:
Description Flags
backtrace in debug
none
backtrace in release
none
Reduced testcase
none
valgrind output with www.index.hu
none
valgrind output with Kling's reduced test case none

Description Balazs Kelemen 2010-09-09 07:53:28 PDT
Both in debug and release build. MiniBrowser is crashing as well. I have tried the js regression tests in debug but those are ok. Release js tests and layouttests are tested on the build-bot and those are green.
Comment 1 Balazs Kelemen 2010-09-09 07:55:15 PDT
Created attachment 67030 [details]
backtrace in debug
Comment 2 Balazs Kelemen 2010-09-09 07:55:50 PDT
Created attachment 67031 [details]
backtrace in release
Comment 3 Csaba Osztrogonác 2010-09-10 03:57:51 PDT
I tried index.hu in QtTestBrowser few days ago and it worked.
It should be a new regression, I'm going to find the culprit commit.
Comment 4 Csaba Osztrogonác 2010-09-10 05:55:42 PDT
(In reply to comment #3)
> I tried index.hu in QtTestBrowser few days ago and it worked.
> It should be a new regression, I'm going to find the culprit commit.

I tried a lot of revisions. (r63000 - ToT) It seems it isn't a new regression,
but a very old real bug revealed by the newest version of index.hu website.

Additionally I tried with QtTestBrowser on Windows and it works without crashes.
Comment 5 Csaba Osztrogonác 2010-09-13 02:23:16 PDT
(In reply to comment #4)
Unfortunately QtTestBrowser crashes on Windows now. :S
(http://trac.webkit.org/changeset/67368)

Additionally I tried an interpreter build and it works correctly with index.hu.
Comment 6 Gavin Barraclough 2010-09-13 22:08:42 PDT
Nothing obvious stands out from the backtrace alone.  Looks very much like String.substring is being passed a bad this value from JIT code, but looks like there's insufficient information here to be able to determine why.
Comment 7 Balazs Kelemen 2010-10-02 07:25:28 PDT
Zoltan, I believe you have the knowledge to fix this.
Could you take a try? Maybe SVG can wait a little bit ;)
Comment 8 Andreas Kling 2010-10-18 10:42:26 PDT
Created attachment 71057 [details]
Reduced testcase

Reduced the crash to 3 lines of JavaScript.
Comment 9 Andreas Kling 2010-10-18 10:47:08 PDT
Also relevant: it hits an assertion:

CallFrame.h, line 71: ASSERT(scopeChain()->globalData);

#0  0x00007ffff579211e in JSC::ExecState::globalData (this=0x7fffdc77f0c0) at ../../../JavaScriptCore/interpreter/CallFrame.h:71
#1  0x00007ffff5832a60 in JSC::ExecState::hadException (this=0x7fffdc77f0c0) at ../../../JavaScriptCore/interpreter/CallFrame.h:83
#2  0x00007ffff6802849 in JSC::callDefaultValueFunction (exec=0x7fffdc77f0c0, object=0x7fffdc702b40, propertyName=...) at ../../../JavaScriptCore/runtime/JSObject.cpp:260
#3  0x00007ffff680299c in JSC::JSObject::defaultValue (this=0x7fffdc702b40, exec=0x7fffdc77f0c0, hint=JSC::PreferString) at ../../../JavaScriptCore/runtime/JSObject.cpp:279
#4  0x00007ffff5793351 in JSC::JSObject::toPrimitive (this=0x7fffdc702b40, exec=0x7fffdc77f0c0, preferredType=JSC::PreferString)
    at ../../../JavaScriptCore/runtime/JSObject.h:644
#5  0x00007ffff68038ba in JSC::JSObject::toString (this=0x7fffdc702b40, exec=0x7fffdc77f0c0) at ../../../JavaScriptCore/runtime/JSObject.cpp:483
#6  0x00007ffff6833d5e in JSC::JSValue::toThisString(JSC::ExecState*) const () from /home/kling/src/webkit/WebKitBuild/Debug/bin/../lib/libQtWebKit.so.4
#7  0x00007ffff6831bba in JSC::stringProtoFuncSubstring (exec=0x7fffdc77f0c0) at ../../../JavaScriptCore/runtime/StringPrototype.cpp:800
Comment 10 Gavin Barraclough 2010-10-18 13:51:01 PDT
Nice work regressing!
The regression crashes for me on this machine, will try to put a couple of cycles into spotting what's going on here.
Comment 11 Andreas Kling 2010-10-19 00:33:32 PDT
Running the test through 'jsc' yields some interesting valgrind output:

==19191== Conditional jump or move depends on uninitialised value(s)
==19191==    at 0x428401: JSC::BytecodeGenerator::emitOpcode(JSC::OpcodeID) (BytecodeGenerator.cpp:678)
==19191==    by 0x42741E: JSC::BytecodeGenerator::BytecodeGenerator(JSC::EvalNode*, JSC::Debugger const*, JSC::ScopeChain const&, WTF::HashMap<WTF::RefPtr<WTF::StringImpl>, JSC::SymbolTableEntry, JSC::IdentifierRepHash, WTF::HashTraits<WTF::RefPtr<WTF::StringImpl> >, JSC::SymbolTableIndexHashTraits>*, JSC::EvalCodeBlock*) (BytecodeGenerator.cpp:485)
==19191==    by 0x463896: JSC::EvalExecutable::compileInternal(JSC::ExecState*, JSC::ScopeChainNode*) (Executable.cpp:111)
==19191==    by 0x49683F: JSC::EvalExecutable::compile(JSC::ExecState*, JSC::ScopeChainNode*) (Executable.h:207)
==19191==    by 0x532B3E: JSC::EvalCodeCache::get(JSC::ExecState*, bool, JSC::UString const&, JSC::ScopeChainNode*, JSC::JSValue&) (EvalCodeCache.h:55)
==19191==    by 0x52D91F: JSC::Interpreter::callEval(JSC::ExecState*, JSC::RegisterFile*, JSC::Register*, int, int, JSC::JSValue&) (Interpreter.cpp:410)
==19191==    by 0x56142A: cti_op_call_eval (JITStubs.cpp:3236)
==19191==    by 0x5568E9: JSC::JITThunks::tryCacheGetByID(JSC::ExecState*, JSC::CodeBlock*, JSC::ReturnAddressPtr, JSC::JSValue, JSC::Identifier const&, JSC::PropertySlot const&, JSC::StructureStubInfo*) (JITStubs.cpp:999)
==19191==    by 0x532443: JSC::JITCode::execute(JSC::RegisterFile*, JSC::ExecState*, JSC::JSGlobalData*, JSC::JSValue*) (JITCode.h:77)
==19191==    by 0x52F0CD: JSC::Interpreter::execute(JSC::ProgramExecutable*, JSC::ExecState*, JSC::ScopeChainNode*, JSC::JSObject*, JSC::JSValue*) (Interpreter.cpp:746)
==19191==    by 0x45FF49: JSC::evaluate(JSC::ExecState*, JSC::ScopeChain&, JSC::SourceCode const&, JSC::JSValue) (Completion.cpp:63)
==19191==    by 0x406971: runWithScripts(GlobalObject*, WTF::Vector<Script, 0ul> const&, bool) (jsc.cpp:391)

==19191== Invalid read of size 8
==19191==    at 0x49184E: JSC::RegisterFile::end() const (RegisterFile.h:118)
==19191==    by 0x491B96: JSC::ExecState::init(JSC::CodeBlock*, JSC::Instruction*, JSC::ScopeChainNode*, JSC::ExecState*, int, JSC::JSObject*) (CallFrame.h:121)
==19191==    by 0x52F8C8: JSC::Interpreter::executeCall(JSC::ExecState*, JSC::JSObject*, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&, JSC::JSValue*) (Interpreter.cpp:842)
==19191==    by 0x585723: JSC::call(JSC::ExecState*, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) (CallData.cpp:38)
==19191==    by 0x498F2B: JSC::callDefaultValueFunction(JSC::ExecState*, JSC::JSObject const*, JSC::Identifier const&) (JSObject.cpp:258)
==19191==    by 0x4990CF: JSC::JSObject::defaultValue(JSC::ExecState*, JSC::PreferredPrimitiveType) const (JSObject.cpp:279)
==19191==    by 0x40B0E6: JSC::JSObject::toPrimitive(JSC::ExecState*, JSC::PreferredPrimitiveType) const (JSObject.h:644)
==19191==    by 0x499FED: JSC::JSObject::toString(JSC::ExecState*) const (JSObject.cpp:483)
==19191==    by 0x4D2CE1: JSC::JSValue::toThisString(JSC::ExecState*) const (JSObject.h:759)
==19191==    by 0x4D0305: JSC::stringProtoFuncSubstring(JSC::ExecState*) (StringPrototype.cpp:800)
==19191==    by 0x392891A9: ???
==19191==    by 0x532443: JSC::JITCode::execute(JSC::RegisterFile*, JSC::ExecState*, JSC::JSGlobalData*, JSC::JSValue*) (JITCode.h:77)
==19191==  Address 0x6d9b9f0 is 0 bytes after a block of size 48 alloc'd
==19191==    at 0x4C285D8: malloc (vg_replace_malloc.c:236)
==19191==    by 0x4108FB: WTF::fastMalloc(unsigned long) (FastMalloc.cpp:250)
==19191==    by 0x407891: WTF::FastAllocBase::operator new(unsigned long) (FastAllocBase.h:121)
==19191==    by 0x491A00: JSC::ScopeChain::ScopeChain(JSC::JSObject*, JSC::JSGlobalData*, JSC::JSGlobalObject*, JSC::JSObject*) (ScopeChain.h:168)
==19191==    by 0x48C110: JSC::JSGlobalObject::init(JSC::JSObject*) (JSGlobalObject.cpp:132)
==19191==    by 0x40C20D: JSC::JSGlobalObject::JSGlobalObject() (JSGlobalObject.h:171)
==19191==    by 0x404C00: GlobalObject::GlobalObject(WTF::Vector<JSC::UString, 0ul> const&) (jsc.cpp:152)
==19191==    by 0x40754C: jscmain(int, char**, JSC::JSGlobalData*) (jsc.cpp:528)
==19191==    by 0x4065EE: main (jsc.cpp:348)

It finally dies with this assertion:
ASSERTION FAILED: callerFrame == noCaller() || callerFrame->removeHostCallFrameFlag()->registerFile()->end() >= this
(../../../JavaScriptCore/interpreter/CallFrame.h:121 void JSC::ExecState::init(JSC::CodeBlock*, JSC::Instruction*, JSC::ScopeChainNode*, JSC::CallFrame*, int, JSC::JSObject*))
Comment 12 Oliver Hunt 2010-10-19 11:26:05 PDT
(In reply to comment #11)
> Running the test through 'jsc' yields some interesting valgrind output:
> 
> ==19191== Conditional jump or move depends on uninitialised value(s)
> ==19191==    at 0x428401: JSC::BytecodeGenerator::emitOpcode(JSC::OpcodeID) (BytecodeGenerator.cpp:678)

I looked at this briefly and it looks like m_lastOpcodePosition is only initialised in one constructor.  If you put up a patch to fix this i'll review.

The ProgramNode constructor has this

    , m_lastOpcodeID(op_end)
#ifndef NDEBUG
    , m_lastOpcodePosition(0)
#endif

But the other versions don't have the
#ifndef NDEBUG
    , m_lastOpcodePosition(0)
#endif

bit.

Adding that should get valgrind past this uninitialised var so we can see if it picks up anything else.
Comment 13 Antonio Gomes 2010-10-19 12:00:24 PDT
Maybe we should update the bug title now with something more realistic :-)
Comment 14 Oliver Hunt 2010-10-19 12:10:51 PDT
stab stab stabitty

This is the same as bug 41454

The registerfile is shrunk below the size of a contained frame.
Comment 15 Csaba Osztrogonác 2010-10-19 12:15:44 PDT
(In reply to comment #12)
> (In reply to comment #11)
> > Running the test through 'jsc' yields some interesting valgrind output:
> > 
> > ==19191== Conditional jump or move depends on uninitialised value(s)
> > ==19191==    at 0x428401: JSC::BytecodeGenerator::emitOpcode(JSC::OpcodeID) (BytecodeGenerator.cpp:678)
> 
> I looked at this briefly and it looks like m_lastOpcodePosition is only initialised in one constructor.  If you put up a patch to fix this i'll review.
> 
> The ProgramNode constructor has this
> 
>     , m_lastOpcodeID(op_end)
> #ifndef NDEBUG
>     , m_lastOpcodePosition(0)
> #endif
> 
> But the other versions don't have the
> #ifndef NDEBUG
>     , m_lastOpcodePosition(0)
> #endif
> 
> bit.
> 
> Adding that should get valgrind past this uninitialised var so we can see if it picks up anything else.

I tried it locally, but I got similar crash and backtrace.
But these uninitialized members are real bugs. I'll upload
a patch to fix them soon.
Comment 16 Oliver Hunt 2010-10-19 12:23:45 PDT
(In reply to comment #15)
> (In reply to comment #12)
> > (In reply to comment #11)
> > > Running the test through 'jsc' yields some interesting valgrind output:
> > > 
> > > ==19191== Conditional jump or move depends on uninitialised value(s)
> > > ==19191==    at 0x428401: JSC::BytecodeGenerator::emitOpcode(JSC::OpcodeID) (BytecodeGenerator.cpp:678)
> > 
> > I looked at this briefly and it looks like m_lastOpcodePosition is only initialised in one constructor.  If you put up a patch to fix this i'll review.
> > 
> > The ProgramNode constructor has this
> > 
> >     , m_lastOpcodeID(op_end)
> > #ifndef NDEBUG
> >     , m_lastOpcodePosition(0)
> > #endif
> > 
> > But the other versions don't have the
> > #ifndef NDEBUG
> >     , m_lastOpcodePosition(0)
> > #endif
> > 
> > bit.
> > 
> > Adding that should get valgrind past this uninitialised var so we can see if it picks up anything else.
> 
> I tried it locally, but I got similar crash and backtrace.
> But these uninitialized members are real bugs. I'll upload
> a patch to fix them soon.

Oh sorry, I wasn't clear enough -- i knew these weren't the bug, but i figured silencing valgrind warnings == win.
Comment 17 Gavin Barraclough 2010-10-19 12:27:23 PDT
Looks like this is a dupe of https://bugs.webkit.org/show_bug.cgi?id=41948
Comment 18 Oliver Hunt 2010-10-19 12:36:35 PDT
(In reply to comment #17)
> Looks like this is a dupe of https://bugs.webkit.org/show_bug.cgi?id=41948

yup, i couldn't find the actual bug tracking the fix (I've now retitled that bug to represent what it actually is)
Comment 19 Csaba Osztrogonác 2010-10-19 12:50:45 PDT
Created attachment 71194 [details]
valgrind output with www.index.hu

I ran valgrind after I fixed member initialization locally.
Comment 20 Csaba Osztrogonác 2010-10-19 12:55:54 PDT
Created attachment 71195 [details]
valgrind output with Kling's reduced test case
Comment 21 Andreas Kling 2010-11-22 20:16:13 PST

*** This bug has been marked as a duplicate of bug 41948 ***