Bug 141028 - Crash in JSC::DFG::StackLayoutPhase::run
: Crash in JSC::DFG::StackLayoutPhase::run
Status: RESOLVED INVALID
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore
: 528+ (Nightly build)
: Unspecified Unspecified
: P2 Critical
Assigned To: Nobody
: InRadar
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2015-01-29 01:38 PST by Han Choongwoo
Modified: 2015-04-08 15:10 PDT (History)
7 users (show)

See Also:


Attachments
Patch (1.48 KB, patch)
2015-01-29 01:40 PST, Han Choongwoo
no flags Details | Formatted Diff | Diff
Patch (4.19 KB, patch)
2015-02-02 17:36 PST, Han Choongwoo
fpizlo: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.