Bug 129969

Summary: 32-bit x86 handleUncaughtException returns to wrong location after a stack overflow
Product: WebKit Reporter: Mark Lam <mark.lam>
Component: JavaScriptCoreAssignee: Mark Lam <mark.lam>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, fpizlo, ggaren, mhahnenberg, mmirman, msaboff, oliver, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 129318    
Attachments:
Description Flags
the patch. none

Description Mark Lam 2014-03-08 08:15:44 PST
I haven't isolated the root cause yet, but here's what I'm seeing while running stress/recurse-infinitely-on-getter.js test with the JIT enable (it runs fine with only the LLINT):

Stack trace 1: before handleUncaughtException returns:

(lldb) bt 15
* thread #1: tid = 0xaadf21, 0x006164eb JavaScriptCore`handleUncaughtException + 39, queue = 'com.apple.main-thread', stop reason = instruction step into
  * frame #0: 0x006164eb JavaScriptCore`handleUncaughtException + 39
    frame #1: 0xbfc1fa10
    frame #2: 0x0048ccca JavaScriptCore`JSC::Interpreter::executeCall(this=0x03839b20, callFrame=0xbfc1fe58, function=0x01f8dc10, callType=CallTypeJS, callData=0xbfc1fbe0, thisValue=JSValue at 0xbfc1fb14, args=0xbfc1fbc0) + 1482 at Interpreter.cpp:994
    frame #3: 0x000f816d JavaScriptCore`JSC::call(exec=0xbfc1fe58, functionObject=JSValue at 0xbfc1fb84, callType=CallTypeJS, callData=0xbfc1fbe0, thisValue=JSValue at 0xbfc1fb94, args=0xbfc1fbc0) + 253 at CallData.cpp:39
    frame #4: 0x004112e9 JavaScriptCore`JSC::callGetter(exec=0xbfc1fe58, base=JSValue at 0xbfc1fc14, getterSetter=JSValue at 0xbfc1fc1c) + 313 at GetterSetter.cpp:61
    frame #5: 0x007548f2 JavaScriptCore`JSC::PropertySlot::functionGetter(this=0xbfc1fdb8, exec=0xbfc1fe58) const + 194 at PropertySlot.cpp:32
    frame #6: 0x0006e59f JavaScriptCore`JSC::PropertySlot::getValue(this=0xbfc1fdb8, exec=0xbfc1fe58, propertyName=PropertyName at 0xbfc1fce8) const + 239 at JSObject.h:1564
    frame #7: 0x00092f15 JavaScriptCore`JSC::JSValue::get(this=0xbfc1fdd8, exec=0xbfc1fe58, propertyName=PropertyName at 0xbfc1fd58, slot=0xbfc1fdb8) const + 357 at JSCJSValueInlines.h:670
    frame #8: 0x004c1240 JavaScriptCore`operationGetByIdBuildList(exec=0xbfc1fe58, stubInfo=0x01a33120, base=-21441347712, uid=0x0383aa60) + 256 at JITOperations.cpp:140
    frame #9: 0x01f649b1
    frame #10: 0x00615dc4 JavaScriptCore`callToJavaScript + 292
    frame #11: 0x004afd60 JavaScriptCore`JSC::JITCode::execute(this=0x01a33740, vm=0x022b5600, protoCallFrame=0xbfc20010) + 64 at JITCode.cpp:47
    frame #12: 0x0048ccca JavaScriptCore`JSC::Interpreter::executeCall(this=0x03839b20, callFrame=0xbfc203f8, function=0x01f8dc10, callType=CallTypeJS, callData=0xbfc20180, thisValue=JSValue at 0xbfc200b4, args=0xbfc20160) + 1482 at Interpreter.cpp:994
    frame #13: 0x000f816d JavaScriptCore`JSC::call(exec=0xbfc203f8, functionObject=JSValue at 0xbfc20124, callType=CallTypeJS, callData=0xbfc20180, thisValue=JSValue at 0xbfc20134, args=0xbfc20160) + 253 at CallData.cpp:39
    frame #14: 0x004112e9 JavaScriptCore`JSC::callGetter(exec=0xbfc203f8, base=JSValue at 0xbfc201b4, getterSetter=JSValue at 0xbfc201bc) + 313 at GetterSetter.cpp:61

Stack trace 2: after handleUncaughtException returns (by stepping thru the "retl" instruction in handleUncaughtException):

(lldb) 
Process 25830 stopped
* thread #1: tid = 0xaadf21, 0x006164f5 JavaScriptCore`handleUncaughtException + 49, queue = 'com.apple.main-thread', stop reason = instruction step into
    frame #0: 0x006164f5 JavaScriptCore`handleUncaughtException + 49
JavaScriptCore`handleUncaughtException + 49:
-> 0x6164f5:  retl   

JavaScriptCore`llint_op_enter:
   0x6164f6:  movl   %esp, %ecx
   0x6164f8:  andl   $0xf, %ecx
   0x6164fb:  testl  %ecx, %ecx

(lldb) 
Process 25830 stopped
* thread #1: tid = 0xaadf21, 0x0005ff2f JavaScriptCore`JSC::Heap::heap(cell=0x01f8dc10) + 31 at Heap.h:398, queue = 'com.apple.main-thread', stop reason = instruction step into
    frame #0: 0x0005ff2f JavaScriptCore`JSC::Heap::heap(cell=0x01f8dc10) + 31 at Heap.h:398
   395  
   396      inline Heap* Heap::heap(const JSCell* cell)
   397      {
-> 398          return MarkedBlock::blockFor(cell)->heap();
   399      }
   400  
   401      inline Heap* Heap::heap(const JSValue v)

(lldb) bt 15
* thread #1: tid = 0xaadf21, 0x0005ff2f JavaScriptCore`JSC::Heap::heap(cell=0x01f8dc10) + 31 at Heap.h:398, queue = 'com.apple.main-thread', stop reason = instruction step into
  * frame #0: 0x0005ff2f JavaScriptCore`JSC::Heap::heap(cell=0x01f8dc10) + 31 at Heap.h:398
    frame #1: 0x0006ec80 JavaScriptCore`JSC::JSCell::classInfo(this=0x00aab93c) const + 64 at JSDestructibleObject.h:36

(lldb) 

The stack pointer appears to be messed up somewhere in the process of getting to handleUncaughtExceptions.
Comment 1 Mark Lam 2014-03-08 08:19:17 PST
<rdar://problem/16270160>
Comment 2 Mark Lam 2014-03-08 08:35:32 PST
Looks like the 32-bit version of handleUncaughtException isn't handling an edge case for stack overflows that the 64-bit version does.  Now testing the solution.
Comment 3 Mark Lam 2014-03-08 08:50:04 PST
Created attachment 226215 [details]
the patch.
Comment 4 Geoffrey Garen 2014-03-08 10:45:18 PST
Comment on attachment 226215 [details]
the patch.

r=me
Comment 5 WebKit Commit Bot 2014-03-08 11:17:14 PST
Comment on attachment 226215 [details]
the patch.

Clearing flags on attachment: 226215

Committed r165334: <http://trac.webkit.org/changeset/165334>
Comment 6 WebKit Commit Bot 2014-03-08 11:17:17 PST
All reviewed patches have been landed.  Closing bug.