Bug 141028

Summary: Crash in JSC::DFG::StackLayoutPhase::run
Product: WebKit Reporter: Han Choongwoo <cwhan.tunz>
Component: JavaScriptCoreAssignee: Nobody <webkit-unassigned>
Status: RESOLVED INVALID    
Severity: Critical CC: fpizlo, ggaren, mark.lam, msaboff, oliver, ossy, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch fpizlo: review-

Description Han Choongwoo 2015-01-29 01:38:12 PST
---
function f(arguments) {
arguments;
f.apply(null, ['']);
}
f()
---

This code crash. (also in safari)

I think 'arguments' is problem.
it may think 'arguments' as the origin 'arguments' object.

I found it with afl-fuzz.

ASSERTION FAILED: usesArguments()

(gdb) bt
#0  0x00007ffff73d9399 in WTFCrash () at /development/tunz/javascript/webkit/Source/WTF/wtf/Assertions.cpp:321
#1  0x00007ffff6d249fd in JSC::CodeBlock::argumentsRegister (this=0x7ffff7fbcb40)
    at /development/tunz/javascript/webkit/Source/JavaScriptCore/bytecode/CodeBlock.h:344
#2  0x00007ffff6e27d75 in JSC::DFG::Graph::argumentsRegisterFor (this=0x7ffffffef2e0, inlineCallFrame=0x7ffff7f9b5f0)
    at /development/tunz/javascript/webkit/Source/JavaScriptCore/dfg/DFGGraph.h:415
#3  0x00007ffff6ff9880 in JSC::DFG::StackLayoutPhase::run (this=0x7ffffffeed50)
    at /development/tunz/javascript/webkit/Source/JavaScriptCore/dfg/DFGStackLayoutPhase.cpp:112
#4  0x00007ffff6ffa8a2 in JSC::DFG::runAndLog<JSC::DFG::StackLayoutPhase> (phase=...)
    at /development/tunz/javascript/webkit/Source/JavaScriptCore/dfg/DFGPhase.h:77
#5  0x00007ffff6ffa742 in JSC::DFG::runPhase<JSC::DFG::StackLayoutPhase> (graph=...)
    at /development/tunz/javascript/webkit/Source/JavaScriptCore/dfg/DFGPhase.h:87
#6  0x00007ffff6ff8d66 in JSC::DFG::performStackLayout (graph=...)
    at /development/tunz/javascript/webkit/Source/JavaScriptCore/dfg/DFGStackLayoutPhase.cpp:272
#7  0x00007ffff6f4ee5b in JSC::DFG::Plan::compileInThreadImpl (this=0x7ffff7fbc6c0, longLivedState=...)
    at /development/tunz/javascript/webkit/Source/JavaScriptCore/dfg/DFGPlan.cpp:295
#8  0x00007ffff6f4e652 in JSC::DFG::Plan::compileInThread (this=0x7ffff7fbc6c0, longLivedState=..., threadData=0x0)
    at /development/tunz/javascript/webkit/Source/JavaScriptCore/dfg/DFGPlan.cpp:164
#9  0x00007ffff6e9dcb2 in JSC::DFG::compileImpl (vm=..., codeBlock=0x7ffff7fbc900, profiledDFGCodeBlock=0x0,
    mode=JSC::DFG::DFGMode, osrEntryBytecodeIndex=0, mustHandleValues=..., callback=...)
    at /development/tunz/javascript/webkit/Source/JavaScriptCore/dfg/DFGDriver.cpp:111
#10 0x00007ffff6e9ddce in JSC::DFG::compile (vm=..., codeBlock=0x7ffff7fbc900, profiledDFGCodeBlock=0x0,
    mode=JSC::DFG::DFGMode, osrEntryBytecodeIndex=0, mustHandleValues=..., passedCallback=...)
    at /development/tunz/javascript/webkit/Source/JavaScriptCore/dfg/DFGDriver.cpp:131
#11 0x00007ffff70eb83e in JSC::operationOptimize (exec=0x7ffffffefc90, bytecodeIndex=0)
    at /development/tunz/javascript/webkit/Source/JavaScriptCore/jit/JITOperations.cpp:1196
Comment 1 Han Choongwoo 2015-01-29 01:40:17 PST
Created attachment 245614 [details]
Patch
Comment 2 Radar WebKit Bug Importer 2015-02-02 15:51:40 PST
<rdar://problem/19692411>
Comment 3 Han Choongwoo 2015-02-02 17:36:53 PST
Created attachment 245915 [details]
Patch
Comment 4 Filip Pizlo 2015-04-08 14:20:06 PDT
Comment on attachment 245915 [details]
Patch

We removed this usesArguments() stuff.
Comment 5 Geoffrey Garen 2015-04-08 15:05:25 PDT
Comment on attachment 245915 [details]
Patch

Should we still take these additional regression tests, since they presumably cover something that was not covered before?
Comment 6 Filip Pizlo 2015-04-08 15:10:09 PDT
(In reply to comment #5)
> Comment on attachment 245915 [details]
> Patch
> 
> Should we still take these additional regression tests, since they
> presumably cover something that was not covered before?

Nah.

Previously, if you said "arguments" then the whole compiler - every compiler in every tier - would flip into this alternate reality world.  So we were steadily converging towards having two versions of every test: one that said "arguments" and one that didn't.

This isn't true anymore.  Saying "arguments" only changes how the arguments themselves are accessed but after bytecode generation, none of the tiers really care.

So, having tests for recursion using apply where you say "arguments" isn't really useful.  There's nothing special about that anymore.