Skip to content
Permalink
Browse files
put_to_scope/get_from_scope should not cache lexical scopes when expe…
…cting a global object

https://bugs.webkit.org/show_bug.cgi?id=182549
<rdar://problem/36189995>

Reviewed by Saam Barati.

JSTests:

* stress/var-injection-cache-invalidation.js: Added.
(allocateLotsOfThings):
(test):

Source/JavaScriptCore:

Previously, the llint/baseline caching for put_to_scope and
get_from_scope would cache lexical environments when the
varInjectionWatchpoint had been fired for global properties. Code
in the DFG does not follow this same assumption so we could
potentially return the wrong result. Additionally, the baseline
would write barrier the global object rather than the lexical
enviroment object. This patch makes it so that we do not cache
anything other than the global object for when the resolve type is
GlobalPropertyWithVarInjectionChecks or GlobalProperty.

* assembler/MacroAssembler.cpp:
(JSC::MacroAssembler::jitAssert):
* assembler/MacroAssembler.h:
* jit/JITPropertyAccess.cpp:
(JSC::JIT::emit_op_get_from_scope):
(JSC::JIT::emit_op_put_to_scope):
* runtime/CommonSlowPaths.h:
(JSC::CommonSlowPaths::tryCachePutToScopeGlobal):
(JSC::CommonSlowPaths::tryCacheGetFromScopeGlobal):
* runtime/Options.h:

Canonical link: https://commits.webkit.org/198344@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@228193 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
kmiller68 committed Feb 6, 2018
1 parent 72b9f2a commit 58322b7d05f300bb9e257e191df07ccd62db9a0b
@@ -1,3 +1,15 @@
2018-02-06 Keith Miller <keith_miller@apple.com>

put_to_scope/get_from_scope should not cache lexical scopes when expecting a global object
https://bugs.webkit.org/show_bug.cgi?id=182549
<rdar://problem/36189995>

Reviewed by Saam Barati.

* stress/var-injection-cache-invalidation.js: Added.
(allocateLotsOfThings):
(test):

2018-02-03 Yusuke Suzuki <utatane.tea@gmail.com>

Unreviewed, follow up for test262 update
@@ -0,0 +1,19 @@
a = 0;

function allocateLotsOfThings(array) {
for (let i = 0; i < 1e4; i++)
array[i] = { next: array[Math.floor(i / 2)] };
}

function test() {
a = 5;
for (var i = 0; i < 1e3; i++) {
allocateLotsOfThings([]);
edenGC();
eval("var a = new Int32Array(100);");
}
}
noInline(test);
noDFG(test);

test();
@@ -1,3 +1,32 @@
2018-02-06 Keith Miller <keith_miller@apple.com>

put_to_scope/get_from_scope should not cache lexical scopes when expecting a global object
https://bugs.webkit.org/show_bug.cgi?id=182549
<rdar://problem/36189995>

Reviewed by Saam Barati.

Previously, the llint/baseline caching for put_to_scope and
get_from_scope would cache lexical environments when the
varInjectionWatchpoint had been fired for global properties. Code
in the DFG does not follow this same assumption so we could
potentially return the wrong result. Additionally, the baseline
would write barrier the global object rather than the lexical
enviroment object. This patch makes it so that we do not cache
anything other than the global object for when the resolve type is
GlobalPropertyWithVarInjectionChecks or GlobalProperty.

* assembler/MacroAssembler.cpp:
(JSC::MacroAssembler::jitAssert):
* assembler/MacroAssembler.h:
* jit/JITPropertyAccess.cpp:
(JSC::JIT::emit_op_get_from_scope):
(JSC::JIT::emit_op_put_to_scope):
* runtime/CommonSlowPaths.h:
(JSC::CommonSlowPaths::tryCachePutToScopeGlobal):
(JSC::CommonSlowPaths::tryCacheGetFromScopeGlobal):
* runtime/Options.h:

2018-01-28 Filip Pizlo <fpizlo@apple.com>

Global objects should be able to use TLCs to allocate from different blocks from each other
@@ -28,13 +28,24 @@

#if ENABLE(ASSEMBLER)

#include "Options.h"
#include "ProbeContext.h"
#include <wtf/PrintStream.h>
#include <wtf/ScopedLambda.h>

namespace JSC {

const double MacroAssembler::twoToThe32 = (double)0x100000000ull;

void MacroAssembler::jitAssert(const ScopedLambda<Jump(void)>& functor)
{
if (Options::enableJITDebugAssetions()) {
Jump passed = functor();
breakpoint();
passed.link(this);
}
}

#if ENABLE(MASM_PROBE)
static void stdFunctionCallback(Probe::Context& context)
{
@@ -61,6 +61,13 @@ namespace JSC { typedef MacroAssemblerX86_64 MacroAssemblerBase; };

#include "MacroAssemblerHelpers.h"

namespace WTF {

template<typename FunctionType>
class ScopedLambda;

} // namespace WTF

namespace JSC {

#if ENABLE(MASM_PROBE)
@@ -1884,6 +1891,9 @@ class MacroAssembler : public MacroAssemblerBase {
urshift32(src, trustedImm32ForShift(amount), dest);
}

// If the result jump is taken that means the assert passed.
void jitAssert(const WTF::ScopedLambda<Jump(void)>&);

#if ENABLE(MASM_PROBE)
// This function emits code to preserve the CPUState (e.g. registers),
// call a user supplied probe function, and restore the CPUState before
@@ -43,6 +43,7 @@
#include "ScopedArgumentsTable.h"
#include "SlowPathCall.h"
#include "StructureStubInfo.h"
#include <wtf/ScopedLambda.h>
#include <wtf/StringPrintStream.h>


@@ -857,12 +858,16 @@ void JIT::emit_op_get_from_scope(Instruction* currentInstruction)
switch (resolveType) {
case GlobalProperty:
case GlobalPropertyWithVarInjectionChecks: {
emitLoadWithStructureCheck(scope, structureSlot); // Structure check covers var injection.
emitLoadWithStructureCheck(scope, structureSlot); // Structure check covers var injection since we don't cache structures for anything but the GlobalObject. Additionally, resolve_scope handles checking for the var injection.
GPRReg base = regT0;
GPRReg result = regT0;
GPRReg offset = regT1;
GPRReg scratch = regT2;


jitAssert(scopedLambda<Jump(void)>([&] () -> Jump {
return branchPtr(Equal, base, TrustedImmPtr(m_codeBlock->globalObject()));
}));

load32(operandSlot, offset);
if (!ASSERT_DISABLED) {
Jump isOutOfLine = branch32(GreaterThanOrEqual, offset, TrustedImm32(firstOutOfLineOffset));
@@ -985,9 +990,13 @@ void JIT::emit_op_put_to_scope(Instruction* currentInstruction)
switch (resolveType) {
case GlobalProperty:
case GlobalPropertyWithVarInjectionChecks: {
emitLoadWithStructureCheck(scope, structureSlot); // Structure check covers var injection.
emitLoadWithStructureCheck(scope, structureSlot); // Structure check covers var injection since we don't cache structures for anything but the GlobalObject. Additionally, resolve_scope handles checking for the var injection.
emitGetVirtualRegister(value, regT2);


jitAssert(scopedLambda<Jump(void)>([&] () -> Jump {
return branchPtr(Equal, regT0, TrustedImmPtr(m_codeBlock->globalObject()));
}));

loadPtr(Address(regT0, JSObject::butterflyOffset()), regT0);
loadPtr(operandSlot, regT1);
negPtr(regT1);
@@ -138,8 +138,11 @@ inline void tryCachePutToScopeGlobal(
}

if (resolveType == GlobalProperty || resolveType == GlobalPropertyWithVarInjectionChecks) {
JSGlobalObject* globalObject = codeBlock->globalObject();
ASSERT(globalObject == scope || globalObject->varInjectionWatchpoint()->hasBeenInvalidated());
if (!slot.isCacheablePut()
|| slot.base() != scope
|| scope != globalObject
|| !scope->structure()->propertyAccessesAreCacheable())
return;

@@ -183,9 +186,11 @@ inline void tryCacheGetFromScopeGlobal(
}

// Covers implicit globals. Since they don't exist until they first execute, we didn't know how to cache them at compile time.
if (slot.isCacheableValue() && slot.slotBase() == scope && scope->structure()->propertyAccessesAreCacheable()) {
if (resolveType == GlobalProperty || resolveType == GlobalPropertyWithVarInjectionChecks) {
CodeBlock* codeBlock = exec->codeBlock();
if (resolveType == GlobalProperty || resolveType == GlobalPropertyWithVarInjectionChecks) {
CodeBlock* codeBlock = exec->codeBlock();
JSGlobalObject* globalObject = codeBlock->globalObject();
ASSERT(scope == globalObject || globalObject->varInjectionWatchpoint()->hasBeenInvalidated());
if (slot.isCacheableValue() && slot.slotBase() == scope && scope == globalObject && scope->structure()->propertyAccessesAreCacheable()) {
Structure* structure = scope->structure(vm);
{
ConcurrentJSLocker locker(codeBlock->m_lock);
@@ -250,6 +250,7 @@ constexpr bool enableAsyncIteration = false;
v(bool, b3AlwaysFailsBeforeLink, false, Normal, nullptr) \
v(bool, ftlCrashes, false, Normal, nullptr) /* fool-proof way of checking that you ended up in the FTL. ;-) */\
v(bool, clobberAllRegsInFTLICSlowPath, !ASSERT_DISABLED, Normal, nullptr) \
v(bool, enableJITDebugAssetions, !ASSERT_DISABLED, Normal, nullptr) \
v(bool, useAccessInlining, true, Normal, nullptr) \
v(unsigned, maxAccessVariantListSize, 8, Normal, nullptr) \
v(bool, usePolyvariantDevirtualization, true, Normal, nullptr) \

0 comments on commit 58322b7

Please sign in to comment.