Bug 140033

Summary: Crash in operationNewFunction when scrolling on Google+
Product: WebKit Reporter: Michael Saboff <msaboff>
Component: JavaScriptCoreAssignee: Michael Saboff <msaboff>
Status: RESOLVED FIXED    
Severity: Normal Keywords: InRadar
Priority: P2    
Version: 312.x   
Hardware: All   
OS: All   
URL: http://plus.google.com
Bug Depends on:    
Bug Blocks: 140145    
Attachments:
Description Flags
Patch
none
Updated patch. oliver: review+

Description Michael Saboff 2015-01-01 15:54:25 PST
From rdar://problem/19327455:

To reproduce, open http://plus.google.com (logged in), scroll down to the bottom and up to the top again. Repeat this a couple of times (~10) and you'll get a crash.

Here's the crash stack with ToT WebKit (r177633):

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   com.apple.JavaScriptCore      	0x0000000114aa2846 operationNewFunction + 86
1   ???                           	0x000053c3be412de1 0 + 92100175670753
2   ???                           	0x000053c3be5b8544 0 + 92100177397060
3   ???                           	0x000053c3be32addf 0 + 92100174720479
4   ???                           	0x000053c3bdd7de29 0 + 92100168769065
5   ???                           	0x000053c3be3412ab 0 + 92100174811819
6   ???                           	0x000053c3be3510b7 0 + 92100174876855
7   ???                           	0x000053c3be5beb83 0 + 92100177423235
8   ???                           	0x000053c3be2d2c0f 0 + 92100174359567
9   ???                           	0x000053c3be3163bf 0 + 92100174635967
10  ???                           	0x000053c3be297fe4 0 + 92100174118884
11  ???                           	0x000053c3bdcc8c9a 0 + 92100168027290
12  ???                           	0x000053c3be25f18a 0 + 92100173885834
13  ???                           	0x000053c3be781384 0 + 92100179268484
14  ???                           	0x000053c3bdc1538e 0 + 92100167291790
15  ???                           	0x000053c3be66f0b3 0 + 92100178145459
16  com.apple.JavaScriptCore      	0x0000000114da4068 vmEntryToJavaScript + 326
17  com.apple.JavaScriptCore      	0x0000000114d163e9 JSC::JITCode::execute(JSC::VM*, JSC::ProtoCallFrame*) + 169
18  com.apple.JavaScriptCore      	0x000000011496d765 JSC::Interpreter::executeCall(JSC::ExecState*, JSC::JSObject*, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) + 485
19  com.apple.JavaScriptCore      	0x000000011496d56e JSC::call(JSC::ExecState*, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) + 62
20  com.apple.JavaScriptCore      	0x0000000114a9e31a JSC::boundFunctionCall(JSC::ExecState*) + 586
21  ???                           	0x000053c3bdc01034 0 + 92100167209012
22  ???                           	0x000053c3bdcd6a7f 0 + 92100168084095
23  ???                           	0x000053c3bdc451b8 0 + 92100167487928
24  ???                           	0x000053c3bdc4d2a2 0 + 92100167520930
25  ???                           	0x000053c3bdc01a3a 0 + 92100167211578
26  ???                           	0x000053c3bdc79fea 0 + 92100167704554
27  ???                           	0x000053c3be62a8c8 0 + 92100177864904
28  com.apple.JavaScriptCore      	0x0000000114da9938 llint_entry + 22208
29  ???                           	0x000053c3fdc00440 0 + 92101240947776
30  com.apple.JavaScriptCore      	0x0000000114da9938 llint_entry + 22208
31  com.apple.JavaScriptCore      	0x0000000114da4068 vmEntryToJavaScript + 326
32  com.apple.JavaScriptCore      	0x0000000114d163e9 JSC::JITCode::execute(JSC::VM*, JSC::ProtoCallFrame*) + 169
33  com.apple.JavaScriptCore      	0x000000011496d765 JSC::Interpreter::executeCall(JSC::ExecState*, JSC::JSObject*, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) + 485
34  com.apple.JavaScriptCore      	0x000000011496d56e JSC::call(JSC::ExecState*, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) + 62
35  com.apple.JavaScriptCore      	0x0000000114a9e31a JSC::boundFunctionCall(JSC::ExecState*) + 586
36  ???                           	0x000053c3bdc01034 0 + 92100167209012
37  ???                           	0x000053c3bdcd6a7f 0 + 92100168084095
38  ???                           	0x000053c3be26c40d 0 + 92100173939725
39  ???                           	0x000053c3bdcbe367 0 + 92100167983975
40  com.apple.JavaScriptCore      	0x0000000114da4068 vmEntryToJavaScript + 326
41  com.apple.JavaScriptCore      	0x0000000114d163e9 JSC::JITCode::execute(JSC::VM*, JSC::ProtoCallFrame*) + 169
42  com.apple.JavaScriptCore      	0x000000011496d765 JSC::Interpreter::executeCall(JSC::ExecState*, JSC::JSObject*, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) + 485
43  com.apple.JavaScriptCore      	0x000000011496d56e JSC::call(JSC::ExecState*, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) + 62
44  com.apple.JavaScriptCore      	0x0000000114a9e31a JSC::boundFunctionCall(JSC::ExecState*) + 586
45  ???                           	0x000053c3bdc01034 0 + 92100167209012
46  ???                           	0x000053c3bdcd6a7f 0 + 92100168084095
47  ???                           	0x000053c3be650bdc 0 + 92100178021340
48  com.apple.JavaScriptCore      	0x0000000114da99a3 llint_entry + 22315
49  com.apple.JavaScriptCore      	0x0000000114da4068 vmEntryToJavaScript + 326
50  com.apple.JavaScriptCore      	0x0000000114d163e9 JSC::JITCode::execute(JSC::VM*, JSC::ProtoCallFrame*) + 169
51  com.apple.JavaScriptCore      	0x000000011496d765 JSC::Interpreter::executeCall(JSC::ExecState*, JSC::JSObject*, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) + 485
52  com.apple.JavaScriptCore      	0x000000011496d56e JSC::call(JSC::ExecState*, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) + 62
53  com.apple.JavaScriptCore      	0x0000000114a9e31a JSC::boundFunctionCall(JSC::ExecState*) + 586
54  ???                           	0x000053c3bdc01034 0 + 92100167209012
55  ???                           	0x000053c3be7e34b2 0 + 92100179670194
56  ???                           	0x000053c3be6cb80e 0 + 92100178524174
57  com.apple.JavaScriptCore      	0x0000000114da4068 vmEntryToJavaScript + 326
58  com.apple.JavaScriptCore      	0x0000000114d163e9 JSC::JITCode::execute(JSC::VM*, JSC::ProtoCallFrame*) + 169
59  com.apple.JavaScriptCore      	0x000000011496d765 JSC::Interpreter::executeCall(JSC::ExecState*, JSC::JSObject*, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) + 485
60  com.apple.JavaScriptCore      	0x000000011496d56e JSC::call(JSC::ExecState*, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) + 62
61  com.apple.JavaScriptCore      	0x0000000114a9e31a JSC::boundFunctionCall(JSC::ExecState*) + 586
62  ???                           	0x000053c3bdc01034 0 + 92100167209012
63  ???                           	0x000053c3be806c3f 0 + 92100179815487
64  ???                           	0x000053c3bdc79fea 0 + 92100167704554
65  ???                           	0x000053c3bdcc14ff 0 + 92100167996671
66  ???                           	0x000053c3bddb4e61 0 + 92100168994401
67  ???                           	0x000053c3bddc3e35 0 + 92100169055797
68  ???                           	0x000053c3bdc84464 0 + 92100167746660
69  ???                           	0x000053c3bddb63ef 0 + 92100168999919
70  com.apple.JavaScriptCore      	0x0000000114da4068 vmEntryToJavaScript + 326
71  com.apple.JavaScriptCore      	0x0000000114d163e9 JSC::JITCode::execute(JSC::VM*, JSC::ProtoCallFrame*) + 169
72  com.apple.JavaScriptCore      	0x000000011496d765 JSC::Interpreter::executeCall(JSC::ExecState*, JSC::JSObject*, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) + 485
73  com.apple.JavaScriptCore      	0x000000011496d56e JSC::call(JSC::ExecState*, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) + 62
74  com.apple.JavaScriptCore      	0x0000000114a9e31a JSC::boundFunctionCall(JSC::ExecState*) + 586
75  com.apple.JavaScriptCore      	0x0000000114da4201 vmEntryToNative + 332
76  com.apple.JavaScriptCore      	0x000000011496d7ac JSC::Interpreter::executeCall(JSC::ExecState*, JSC::JSObject*, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) + 556
77  com.apple.JavaScriptCore      	0x0000000114b2541f JSC::call(JSC::ExecState*, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&, JSC::JSValue*) + 63
78  com.apple.WebCore             	0x0000000115196f73 WebCore::JSEventListener::handleEvent(WebCore::ScriptExecutionContext*, WebCore::Event*) + 1027
79  com.apple.WebCore             	0x0000000115196a1c WebCore::EventTarget::fireEventListeners(WebCore::Event*, WebCore::EventTargetData*, WTF::Vector<WebCore::RegisteredEventListener, 1ul, WTF::CrashOnOverflow>&) + 652
80  com.apple.WebCore             	0x0000000115075e3f WebCore::EventTarget::fireEventListeners(WebCore::Event*) + 239
81  com.apple.WebCore             	0x0000000115194405 WebCore::EventTarget::dispatchEvent(WTF::PassRefPtr<WebCore::Event>) + 85
82  com.apple.WebCore             	0x0000000115194357 WebCore::XMLHttpRequestProgressEventThrottle::dispatchEvent(WTF::PassRefPtr<WebCore::Event>) + 199
83  com.apple.WebCore             	0x0000000115194258 WebCore::XMLHttpRequestProgressEventThrottle::dispatchReadyStateChangeEvent(WTF::PassRefPtr<WebCore::Event>, WebCore::ProgressEventAction) + 56
84  com.apple.WebCore             	0x0000000115194048 WebCore::XMLHttpRequest::callReadyStateChangeListener() + 168
85  com.apple.WebCore             	0x00000001151ba420 WebCore::XMLHttpRequest::didFinishLoading(unsigned long, double) + 368
86  com.apple.WebCore             	0x0000000115124c6a WebCore::CachedResource::checkNotify() + 170
87  com.apple.WebCore             	0x000000011541ef53 WebCore::CachedRawResource::finishLoading(WebCore::SharedBuffer*) + 227
88  com.apple.WebCore             	0x0000000115124b2c WebCore::SubresourceLoader::didFinishLoading(double) + 92
89  com.apple.WebKit              	0x000000011431fbe0 WebKit::WebResourceLoader::didReceiveWebResourceLoaderMessage(IPC::Connection*, IPC::MessageDecoder&) + 652
90  com.apple.WebKit              	0x00000001141b0d54 IPC::Connection::dispatchMessage(std::__1::unique_ptr<IPC::MessageDecoder, std::__1::default_delete<IPC::MessageDecoder> >) + 94
91  com.apple.WebKit              	0x00000001141b2eee IPC::Connection::dispatchOneMessage() + 114
92  com.apple.JavaScriptCore      	0x0000000114e67902 WTF::RunLoop::performWork() + 850
93  com.apple.JavaScriptCore      	0x0000000114e67e22 WTF::RunLoop::performWork(void*) + 34
94  com.apple.CoreFoundation      	0x00007fff8eb5d681 __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ + 17
95  com.apple.CoreFoundation      	0x00007fff8eb4f8dc __CFRunLoopDoSources0 + 476
96  com.apple.CoreFoundation      	0x00007fff8eb4ee3f __CFRunLoopRun + 927
97  com.apple.CoreFoundation      	0x00007fff8eb4e858 CFRunLoopRunSpecific + 296
98  com.apple.HIToolbox           	0x00007fff9189b90f RunCurrentEventLoopInMode + 235
99  com.apple.HIToolbox           	0x00007fff9189b68a ReceiveNextEventCommon + 431
100 com.apple.HIToolbox           	0x00007fff9189b4cb _BlockUntilNextEventMatchingListInModeWithFilter + 71
101 com.apple.AppKit              	0x00007fff98f1486d _DPSNextEvent + 964
102 com.apple.AppKit              	0x00007fff98f14020 -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] + 194
103 com.apple.AppKit              	0x00007fff98f07ee3 -[NSApplication run] + 594
104 com.apple.AppKit              	0x00007fff98ef3254 NSApplicationMain + 1832
105 libxpc.dylib                  	0x00007fff93d4def2 _xpc_objc_main + 793
106 libxpc.dylib                  	0x00007fff93d4fa9d xpc_main + 490
107 com.apple.WebKit.WebContent.Development	0x000000010d099620 0x10d098000 + 5664
108 libdyld.dylib                 	0x00007fff92f885c9 start + 1
Comment 1 Michael Saboff 2015-01-01 16:07:37 PST
Consider a function that creates another function that is never used.  The DFG will dead code eliminate the creation of the inner function.  If the scope register is DCE as well AND we OSR exit in the middle of the outer function to a path where we create the inner function, there won't be a valid scope needed to create the inner function and we crash.

Since the DFG determined that the scope is dead, the inner function must have been proven dead.  Therefore, we can fix this in the baseline generated code by checking the scope before creating the function.  If the scope is undefined, return undefined instead of creating a new function.
Comment 2 Michael Saboff 2015-01-01 16:29:17 PST
Created attachment 243874 [details]
Patch
Comment 3 Oliver Hunt 2015-01-01 18:20:24 PST
Comment on attachment 243874 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=243874&action=review

> Source/JavaScriptCore/ChangeLog:11
> +        exit to a path where that funciton gets created, check the scope register value

function

> Source/JavaScriptCore/jit/JITOpcodes.cpp:1071
> +    notUndefinedScope = branch64(NotEqual, regT0, TrustedImm64(JSValue::encode(jsUndefined())));
> +    store64(TrustedImm64(JSValue::encode(jsUndefined())), Address(callFrameRegister, sizeof(Register) * dst));
>  #else
>      emitLoadPayload(currentInstruction[2].u.operand, regT0);
> +    notUndefinedScope = branch32(NotEqual, tagFor(dst), TrustedImm32(JSValue::UndefinedTag));
> +    emitStore(dst, jsUndefined());
>  #endif

i thought we have macro functions for checking and set undefined
Comment 4 Michael Saboff 2015-01-01 19:02:32 PST
Created attachment 243879 [details]
Updated patch.

> Comment on attachment 243874 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=243874&action=review
> 
> > Source/JavaScriptCore/ChangeLog:11
> > +        exit to a path where that funciton gets created, check the scope register value
> 
> function

Fixed.

> > Source/JavaScriptCore/jit/JITOpcodes.cpp:1071
> > +    notUndefinedScope = branch64(NotEqual, regT0, TrustedImm64(JSValue::encode(jsUndefined())));
> > +    store64(TrustedImm64(JSValue::encode(jsUndefined())), Address(callFrameRegister, sizeof(Register) * dst));
> >  #else
> >      emitLoadPayload(currentInstruction[2].u.operand, regT0);
> > +    notUndefinedScope = branch32(NotEqual, tagFor(dst), TrustedImm32(JSValue::UndefinedTag));
> > +    emitStore(dst, jsUndefined());
> >  #endif
> 
> i thought we have macro functions for checking and set undefined

I did find is / is not integer, but I couldn't find any "is undefined" macros.

I made a fix to the 32 bit case after running JSC tests.
Comment 5 Michael Saboff 2015-01-03 19:47:25 PST
Committed r177871: <http://trac.webkit.org/changeset/177871>
Comment 6 Geoffrey Garen 2015-01-06 11:17:05 PST
Comment on attachment 243879 [details]
Updated patch.

This approach is pretty crazy, and likely wrong. Our engine is architected to produce the correct values upon OSR exit -- not to recover from incorrect values after OSR exit.

You should rethink your solution, and figure out why the OSR exit machinery did not know that the scope register would be used at this point in the bytecode.