Changeset 212692 in webkit for trunk/Source/JavaScriptCore/interpreter/Interpreter.cpp

Timestamp:

Feb 20, 2017, 5:51:05 PM (8 years ago)

Author:

[email protected]

Message:

[Re-landing] CachedCall should let GC know to keep its arguments alive.
​https://bugs.webkit.org/show_bug.cgi?id=168567
<rdar://problem/30475767>

Reviewed by Saam Barati.

Source/JavaScriptCore:

We fix this by having CachedCall use a MarkedArgumentBuffer to store its
arguments instead of a Vector.

Also declared CachedCall, MarkedArgumentBuffer, and ProtoCallFrame as
WTF_FORBID_HEAP_ALLOCATION because they rely on being stack allocated for
correctness.

Update: the original patch has a bug in MarkedArgumentBuffer::expandCapacity()
where it was copying and calling addMarkSet() on values in m_buffer beyond m_size
(up to m_capacity). As a result, depending on the pre-existing values in
m_inlineBuffer, this may result in a computed Heap pointer that is wrong, and
subsequently, manifest as a crash. This is likely to be the cause of the PLT
regression.

I don't have a new test for this fix because the issue relies on sufficiently bad
values randomly showing up in m_inlineBuffer when we do an ensureCapacity() which
calls expandCapacity().

• interpreter/CachedCall.h:

(JSC::CachedCall::CachedCall):
(JSC::CachedCall::call):
(JSC::CachedCall::clearArguments):
(JSC::CachedCall::appendArgument):
(JSC::CachedCall::setArgument): Deleted.

• interpreter/CallFrame.h:

(JSC::ExecState::emptyList):

• interpreter/Interpreter.cpp:

(JSC::Interpreter::prepareForRepeatCall):

• interpreter/Interpreter.h:
• interpreter/ProtoCallFrame.h:
• runtime/ArgList.cpp:

(JSC::MarkedArgumentBuffer::slowEnsureCapacity):
(JSC::MarkedArgumentBuffer::expandCapacity):
(JSC::MarkedArgumentBuffer::slowAppend):

• runtime/ArgList.h:

(JSC::MarkedArgumentBuffer::append):
(JSC::MarkedArgumentBuffer::ensureCapacity):

• runtime/StringPrototype.cpp:

(JSC::replaceUsingRegExpSearch):

• runtime/VM.cpp:

(JSC::VM::VM):

• runtime/VM.h:

Source/WTF:

Added a WTF_FORBID_HEAP_ALLOCATION that will cause a compilation failure if
a class declared with it is malloced.

While this doesn't prevent that class declared WTF_FORBID_HEAP_ALLOCATION from
being embedded in another class that is heap allocated, it does at minimum
document the intent and gives the users of this class a chance to do the
right thing.

• WTF.xcodeproj/project.pbxproj:
• wtf/ForbidHeapAllocation.h: Added.

File:

• trunk/Source/JavaScriptCore/interpreter/Interpreter.cpp (modified) (3 diffs)

trunk/Source/JavaScriptCore/interpreter/Interpreter.cpp

@@ -1 +1 @@
 /*
- * Copyright (C) 2008-2010, 2012-2016 Apple Inc. All rights reserved.
+ * Copyright (C) 2008-2017 Apple Inc. All rights reserved.
  * Copyright (C) 2008 Cameron Zwarich <[email protected]>
  *
@@ -1006 +1006 @@
 }
 
-CallFrameClosure Interpreter::prepareForRepeatCall(FunctionExecutable* functionExecutable, CallFrame* callFrame, ProtoCallFrame* protoCallFrame, JSFunction* function, int argumentCountIncludingThis, JSScope* scope, JSValue* args)
+CallFrameClosure Interpreter::prepareForRepeatCall(FunctionExecutable* functionExecutable, CallFrame* callFrame, ProtoCallFrame* protoCallFrame, JSFunction* function, int argumentCountIncludingThis, JSScope* scope, const ArgList& args)
 {
     VM& vm = *scope->vm();
@@ -1026 +1026 @@
     size_t argsCount = argumentCountIncludingThis;
 
-    protoCallFrame->init(newCodeBlock, function, jsUndefined(), argsCount, args);
+    protoCallFrame->init(newCodeBlock, function, jsUndefined(), argsCount, args.data());
     // Return the successful closure:
     CallFrameClosure result = { callFrame, protoCallFrame, function, functionExecutable, &vm, scope, newCodeBlock->numParameters(), argumentCountIncludingThis };