WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
119860
CVE-2013-5195
Crash during exception unwinding
https://bugs.webkit.org/show_bug.cgi?id=119860
Summary
Crash during exception unwinding
Oliver Hunt
Reported
2013-08-15 13:36:48 PDT
Friendly test case: function g() { (eval("-7") = 0); } for (;;) try { g() } catch(e) {} 1 0x10b9c4680 WTFCrash 2 0x10b74402f JSC::JSActivation* JSC::jsCast<JSC::JSActivation*>(JSC::JSValue) 3 0x10b73f190 JSC::Interpreter::unwindCallFrame(JSC::StackIterator&, JSC::JSValue) 4 0x10b740362 JSC::Interpreter::throwException(JSC::ExecState*&, JSC::JSValue&, unsigned int) 5 0x10b76124f JSC::genericThrow(JSC::VM*, JSC::ExecState*, JSC::JSValue, unsigned int) 6 0x10b7614a6 JSC::jitThrow(JSC::VM*, JSC::ExecState*, JSC::JSValue, JSC::ReturnAddressPtr) 7 0x10b782146 cti_vm_throw 8 0x10b7830c0 jscGeneratedNativeCode 9 0x10b75eb07 JSC::JITCode::execute(JSC::JSStack*, JSC::ExecState*, JSC::VM*) 10 0x10b741cc9 JSC::Interpreter::execute(JSC::ProgramExecutable*, JSC::ExecState*, JSC::JSObject*) 11 0x10b5486f1 JSC::evaluate(JSC::ExecState*, JSC::SourceCode const&, JSC::JSValue, JSC::JSValue*) 12 0x10b40b167 runWithScripts(GlobalObject*, WTF::Vector<Script, 0ul, WTF::CrashOnOverflow> const&, bool) 13 0x10b40a85c jscmain(int, char**) 14 0x10b40a6be main 15 0x7fff8def75fd start
Attachments
Patch
(10.70 KB, patch)
2013-08-16 16:25 PDT
,
Oliver Hunt
no flags
Details
Formatted Diff
Diff
Patch
(11.43 KB, patch)
2013-08-16 16:31 PDT
,
Oliver Hunt
no flags
Details
Formatted Diff
Diff
Patch
(13.21 KB, patch)
2013-08-19 12:19 PDT
,
Oliver Hunt
fpizlo
: review+
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Oliver Hunt
Comment 1
2013-08-15 16:48:46 PDT
As far as i can make out OSR exit is overwriting the activation register with undefined, but not the arguments register
Radar WebKit Bug Importer
Comment 2
2013-08-15 16:49:20 PDT
<
rdar://problem/14752184
>
Oliver Hunt
Comment 3
2013-08-16 16:25:12 PDT
Created
attachment 208963
[details]
Patch
Filip Pizlo
Comment 4
2013-08-16 16:27:08 PDT
Comment on
attachment 208963
[details]
Patch Why doesn't this have changes to DFGInPlaceAbstractState, and probably elsewhere, to remove Throw/ThrowReferenceError from the list of block terminals and add Unreachable to that same list?
Oliver Hunt
Comment 5
2013-08-16 16:31:00 PDT
Created
attachment 208964
[details]
Patch
Filip Pizlo
Comment 6
2013-08-16 18:26:21 PDT
Comment on
attachment 208964
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=208964&action=review
Please change the comment in DFGAbstractInterpreter.h that mentions Throw as a terminal. Please change the comment in DFGFlushLivenessAnalysis that mentions Throw as a terminal. Ditto in DFGLivenessAnalysis.
> Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:1497 > + case Unreachable: > + break;
Since Throw/ThrowReferenceError do setIsValid(false) and presumably all other uses of Unreachable will also be dominated by something that always exits/terminates, shouldn't this be RELEASE_ASSERT_NOT_REACHED()?
> Source/JavaScriptCore/dfg/DFGClobberize.h:114 > + case Unreachable:
Since this is a control construct, shouldn't it write(SideState)?
> Source/JavaScriptCore/dfg/DFGInPlaceAbstractState.cpp:447 > + case Unreachable: > + return false;
Why isn't this in the above case block, along with the cfaBranchDirection assertion?
> Source/JavaScriptCore/dfg/DFGNodeType.h:252 > macro(Throw, NodeMustGenerate) \ > macro(ThrowReferenceError, NodeMustGenerate) \
Shouldn't you be removing these from the block that has the "block terminals" comment? Wouldn't they be more appropriate in another block of node types with a comment like "these always exit but aren't terminals to allow liveness hooks after them"?
> Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp:4649 > + case Unreachable: > + // We should not ever reach this code > + m_jit.breakpoint(); > + noResult(node); > + break;
Shouldn't this be a RELEASE_ASSERT_NOT_REACHED() because the *compiler* shouldn't even reach this node? (Ditto in the 32-bit compiler.)
Oliver Hunt
Comment 7
2013-08-19 12:19:49 PDT
Created
attachment 209107
[details]
Patch
WebKit Commit Bot
Comment 8
2013-08-19 12:22:14 PDT
Attachment 209107
[details]
did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast/js/dfg-activation-register-overwritten-in-throw-expected.txt', u'LayoutTests/fast/js/dfg-activation-register-overwritten-in-throw.html', u'LayoutTests/fast/js/script-tests/dfg-activation-register-overwritten-in-throw.js', u'Source/JavaScriptCore/ChangeLog', u'Source/JavaScriptCore/dfg/DFGAbstractInterpreter.h', u'Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h', u'Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp', u'Source/JavaScriptCore/dfg/DFGClobberize.h', u'Source/JavaScriptCore/dfg/DFGFixupPhase.cpp', u'Source/JavaScriptCore/dfg/DFGFlushLivenessAnalysisPhase.cpp', u'Source/JavaScriptCore/dfg/DFGInPlaceAbstractState.cpp', u'Source/JavaScriptCore/dfg/DFGNode.h', u'Source/JavaScriptCore/dfg/DFGNodeType.h', u'Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp', u'Source/JavaScriptCore/dfg/DFGSafeToExecute.h', u'Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp', u'Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp']" exit_code: 1 Source/JavaScriptCore/dfg/DFGNodeType.h:246: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Total errors found: 1 in 18 files If any of these errors are false positives, please file a bug against check-webkit-style.
Filip Pizlo
Comment 9
2013-08-19 12:38:02 PDT
Comment on
attachment 209107
[details]
Patch sweet
Oliver Hunt
Comment 10
2013-08-19 12:39:09 PDT
Committed
r154290
: <
http://trac.webkit.org/changeset/154290
>
Oliver Hunt
Comment 11
2013-08-20 14:39:17 PDT
***
Bug 119902
has been marked as a duplicate of this bug. ***
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug