Bug 135155

Summary: ASSERTION FAILED: info.spillFormat() & DataFormatJS in JSC::DFG::SpeculativeJIT::fillSpeculateCell
Product: WebKit Reporter: Renata Hodovan <rhodovan.u-szeged>
Component: JavaScriptCoreAssignee: Filip Pizlo <fpizlo>
Status: RESOLVED FIXED    
Severity: Normal CC: fpizlo, llango.u-szeged, mhahnenberg, oliver, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 116980    
Attachments:
Description Flags
Test case
none
the patch oliver: review+

Description Renata Hodovan 2014-07-22 02:29:51 PDT
Created attachment 235280 [details]
Test case

Release assert was hit in DFGSpeculativeJIT with the following script:

function run() {
    for (var t = 1; 1 <= 2; t++) {
        t.length = function() {
            var foo = iv.charCodeAt(foo, undefined);
        };
    }
}

run();


The test was run on Ubuntu 13.10, x86_64.


The related backtrace:

ASSERTION FAILED: info.spillFormat() & DataFormatJS
../../Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp(1022) : JSC::GPRReg JSC::DFG::SpeculativeJIT::fillSpeculateCell(JSC::DFG::Edge)
1   0x7ffff73b4662 /home/reni/data/REPOS/webkit_sec/WebKitBuild/Debug/lib/libjavascriptcoregtk-3.0.so.0(WTFCrash+0x1e) [0x7ffff73b4662]
2   0x7ffff70643c7 /home/reni/data/REPOS/webkit_sec/WebKitBuild/Debug/lib/libjavascriptcoregtk-3.0.so.0(_ZN3JSC3DFG14SpeculativeJIT17fillSpeculateCellENS0_4EdgeE+0x24f) [0x7ffff70643c7]
3   0x7ffff704c695 /home/reni/data/REPOS/webkit_sec/WebKitBuild/Debug/lib/libjavascriptcoregtk-3.0.so.0(_ZN3JSC3DFG20SpeculateCellOperand3gprEv+0x71) [0x7ffff704c695]
4   0x7ffff7041382 /home/reni/data/REPOS/webkit_sec/WebKitBuild/Debug/lib/libjavascriptcoregtk-3.0.so.0(_ZN3JSC3DFG14SpeculativeJIT19compileStoreBarrierEPNS0_4NodeE+0xa6) [0x7ffff7041382]
5   0x7ffff7077cc9 /home/reni/data/REPOS/webkit_sec/WebKitBuild/Debug/lib/libjavascriptcoregtk-3.0.so.0(_ZN3JSC3DFG14SpeculativeJIT7compileEPNS0_4NodeE+0xf771) [0x7ffff7077cc9]
6   0x7ffff702f4d5 /home/reni/data/REPOS/webkit_sec/WebKitBuild/Debug/lib/libjavascriptcoregtk-3.0.so.0(_ZN3JSC3DFG14SpeculativeJIT19compileCurrentBlockEv+0x613) [0x7ffff702f4d5]
7   0x7ffff702fa8a /home/reni/data/REPOS/webkit_sec/WebKitBuild/Debug/lib/libjavascriptcoregtk-3.0.so.0(_ZN3JSC3DFG14SpeculativeJIT7compileEv+0x98) [0x7ffff702fa8a]
8   0x7ffff6fc43a2 /home/reni/data/REPOS/webkit_sec/WebKitBuild/Debug/lib/libjavascriptcoregtk-3.0.so.0(_ZN3JSC3DFG11JITCompiler11compileBodyEv+0x26) [0x7ffff6fc43a2]
9   0x7ffff6fc5b4e /home/reni/data/REPOS/webkit_sec/WebKitBuild/Debug/lib/libjavascriptcoregtk-3.0.so.0(_ZN3JSC3DFG11JITCompiler15compileFunctionEv+0x19a) [0x7ffff6fc5b4e]
10  0x7ffff7018d35 /home/reni/data/REPOS/webkit_sec/WebKitBuild/Debug/lib/libjavascriptcoregtk-3.0.so.0(_ZN3JSC3DFG4Plan19compileInThreadImplERNS0_14LongLivedStateE+0x5af) [0x7ffff7018d35]
11  0x7ffff7018514 /home/reni/data/REPOS/webkit_sec/WebKitBuild/Debug/lib/libjavascriptcoregtk-3.0.so.0(_ZN3JSC3DFG4Plan15compileInThreadERNS0_14LongLivedStateEPNS0_10ThreadDataE+0x148) [0x7ffff7018514]
12  0x7ffff6f95e36 /home/reni/data/REPOS/webkit_sec/WebKitBuild/Debug/lib/libjavascriptcoregtk-3.0.so.0(+0xab8e36) [0x7ffff6f95e36]
13  0x7ffff6f95ecd /home/reni/data/REPOS/webkit_sec/WebKitBuild/Debug/lib/libjavascriptcoregtk-3.0.so.0(_ZN3JSC3DFG7compileERNS_2VMEPNS_9CodeBlockES4_NS0_15CompilationModeEjRKNS_8OperandsINS_7JSValueENS_18OperandValueTraitsIS7_EEEEN3WTF10PassRefPtrINS_27DeferredCompilationCallbackEEE+0x6a) [0x7ffff6f95ecd]
14  0x7ffff715b80a /home/reni/data/REPOS/webkit_sec/WebKitBuild/Debug/lib/libjavascriptcoregtk-3.0.so.0(+0xc7e80a) [0x7ffff715b80a]
15  0x7ffff2914d66 [0x7ffff2914d66]

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff73b4667 in WTFCrash () at ../../Source/WTF/wtf/Assertions.cpp:329
329	    *(int *)(uintptr_t)0xbbadbeef = 0;
(gdb) bt
#0  0x00007ffff73b4667 in WTFCrash () at ../../Source/WTF/wtf/Assertions.cpp:329
#1  0x00007ffff70643c7 in JSC::DFG::SpeculativeJIT::fillSpeculateCell (this=0x68e8d0, edge=...)
    at ../../Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp:1022
#2  0x00007ffff704c695 in JSC::DFG::SpeculateCellOperand::gpr (this=0x7fffffffb4b0) at ../../Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h:3064
#3  0x00007ffff7041382 in JSC::DFG::SpeculativeJIT::compileStoreBarrier (this=0x68e8d0, node=0x7fffb08a0e80)
    at ../../Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:5359
#4  0x00007ffff7077cc9 in JSC::DFG::SpeculativeJIT::compile (this=0x68e8d0, node=0x7fffb08a0e80)
    at ../../Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp:4687
#5  0x00007ffff702f4d5 in JSC::DFG::SpeculativeJIT::compileCurrentBlock (this=0x68e8d0) at ../../Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1453
#6  0x00007ffff702fa8a in JSC::DFG::SpeculativeJIT::compile (this=0x68e8d0) at ../../Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1565
#7  0x00007ffff6fc43a2 in JSC::DFG::JITCompiler::compileBody (this=0x7fffffffbc40) at ../../Source/JavaScriptCore/dfg/DFGJITCompiler.cpp:113
#8  0x00007ffff6fc5b4e in JSC::DFG::JITCompiler::compileFunction (this=0x7fffffffbc40) at ../../Source/JavaScriptCore/dfg/DFGJITCompiler.cpp:347
#9  0x00007ffff7018d35 in JSC::DFG::Plan::compileInThreadImpl (this=0x68a7b0, longLivedState=...) at ../../Source/JavaScriptCore/dfg/DFGPlan.cpp:290
#10 0x00007ffff7018514 in JSC::DFG::Plan::compileInThread (this=0x68a7b0, longLivedState=..., threadData=0x0)
    at ../../Source/JavaScriptCore/dfg/DFGPlan.cpp:159
#11 0x00007ffff6f95e36 in JSC::DFG::compileImpl (vm=..., codeBlock=0x68c010, profiledDFGCodeBlock=0x0, mode=JSC::DFG::DFGMode, 
    osrEntryBytecodeIndex=8, mustHandleValues=..., callback=...) at ../../Source/JavaScriptCore/dfg/DFGDriver.cpp:104
#12 0x00007ffff6f95ecd in JSC::DFG::compile (vm=..., codeBlock=0x68c010, profiledDFGCodeBlock=0x0, mode=JSC::DFG::DFGMode, osrEntryBytecodeIndex=8, 
    mustHandleValues=..., passedCallback=...) at ../../Source/JavaScriptCore/dfg/DFGDriver.cpp:124
#13 0x00007ffff715b80a in JSC::operationOptimize (exec=0x7fffffffcc40, bytecodeIndex=8) at ../../Source/JavaScriptCore/jit/JITOperations.cpp:1195
#14 0x00007ffff2914d66 in ?? ()
#15 0x0000000000654490 in ?? ()
#16 0x00007fffb08c53f0 in ?? ()
#17 0xffff0000000005d5 in ?? ()
#18 0xffff0000000005d6 in ?? ()
#19 0x00007fffffffcc90 in ?? ()
#20 0x00007ffff739e0c8 in llint_entry () from /home/reni/data/REPOS/webkit_sec/WebKitBuild/Debug/lib/libjavascriptcoregtk-3.0.so.0
Backtrace stopped: frame did not save the PC
Comment 1 Radar WebKit Bug Importer 2014-07-22 10:06:06 PDT
<rdar://problem/17763909>
Comment 2 Filip Pizlo 2014-07-22 11:03:58 PDT
This is not a security bug.  In the future, please don't put compiler release asserts into the security component.  It is misleading.
Comment 3 Filip Pizlo 2014-07-22 11:07:32 PDT
Created attachment 235299 [details]
the patch
Comment 4 Geoffrey Garen 2014-07-22 11:19:28 PDT
Comment on attachment 235299 [details]
the patch

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

Does the 32_64 path need this change too? Maybe not because 32_64 already has an explicit int path?

> Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp:1028
>          RELEASE_ASSERT(info.spillFormat() & DataFormatJS);

This ASSERT seems redundant now, since the clause above checks the same condition.
Comment 5 Filip Pizlo 2014-07-22 11:24:21 PDT
(In reply to comment #4)
> (From update of attachment 235299 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=235299&action=review
> 
> Does the 32_64 path need this change too? Maybe not because 32_64 already has an explicit int path?
> 
> > Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp:1028
> >          RELEASE_ASSERT(info.spillFormat() & DataFormatJS);
> 
> This ASSERT seems redundant now, since the clause above checks the same condition.

Yup, removed.
Comment 6 Filip Pizlo 2014-07-22 12:48:27 PDT
Landed in http://trac.webkit.org/changeset/171354
Comment 7 Renata Hodovan 2014-07-22 14:36:14 PDT
(In reply to comment #2)
> This is not a security bug.  In the future, please don't put compiler release asserts into the security component.  It is misleading.

I'm sorry. It seems that two weeks of vacation had bad effect on my memory. I remembered that release asserts should be reported as security in WebKit too. Next time I'll be more careful.
Comment 8 Filip Pizlo 2014-07-22 14:44:53 PDT
(In reply to comment #7)
> (In reply to comment #2)
> > This is not a security bug.  In the future, please don't put compiler release asserts into the security component.  It is misleading.
> 
> I'm sorry. It seems that two weeks of vacation had bad effect on my memory. I remembered that release asserts should be reported as security in WebKit too. Next time I'll be more careful.

No problem.  I believe that there is an ASSERT_WITH_SECURITY_IMPLICATIONS macro somewhere, which is meant to help disambiguate.

Release assertions in JSC are almost never security-related.