Diff
Modified: trunk/JSTests/ChangeLog (228192 => 228193)
--- trunk/JSTests/ChangeLog 2018-02-06 22:36:00 UTC (rev 228192)
+++ trunk/JSTests/ChangeLog 2018-02-06 22:42:26 UTC (rev 228193)
@@ -1,3 +1,15 @@
+2018-02-06 Keith Miller <keith_mil...@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....@gmail.com>
Unreviewed, follow up for test262 update
Added: trunk/JSTests/stress/var-injection-cache-invalidation.js (0 => 228193)
--- trunk/JSTests/stress/var-injection-cache-invalidation.js (rev 0)
+++ trunk/JSTests/stress/var-injection-cache-invalidation.js 2018-02-06 22:42:26 UTC (rev 228193)
@@ -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();
Modified: trunk/Source/_javascript_Core/ChangeLog (228192 => 228193)
--- trunk/Source/_javascript_Core/ChangeLog 2018-02-06 22:36:00 UTC (rev 228192)
+++ trunk/Source/_javascript_Core/ChangeLog 2018-02-06 22:42:26 UTC (rev 228193)
@@ -1,3 +1,32 @@
+2018-02-06 Keith Miller <keith_mil...@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 <fpi...@apple.com>
Global objects should be able to use TLCs to allocate from different blocks from each other
Modified: trunk/Source/_javascript_Core/assembler/MacroAssembler.cpp (228192 => 228193)
--- trunk/Source/_javascript_Core/assembler/MacroAssembler.cpp 2018-02-06 22:36:00 UTC (rev 228192)
+++ trunk/Source/_javascript_Core/assembler/MacroAssembler.cpp 2018-02-06 22:42:26 UTC (rev 228193)
@@ -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)
{
Modified: trunk/Source/_javascript_Core/assembler/MacroAssembler.h (228192 => 228193)
--- trunk/Source/_javascript_Core/assembler/MacroAssembler.h 2018-02-06 22:36:00 UTC (rev 228192)
+++ trunk/Source/_javascript_Core/assembler/MacroAssembler.h 2018-02-06 22:42:26 UTC (rev 228193)
@@ -61,6 +61,13 @@
#include "MacroAssemblerHelpers.h"
+namespace WTF {
+
+template<typename FunctionType>
+class ScopedLambda;
+
+} // namespace WTF
+
namespace JSC {
#if ENABLE(MASM_PROBE)
@@ -1884,6 +1891,9 @@
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
Modified: trunk/Source/_javascript_Core/jit/JITPropertyAccess.cpp (228192 => 228193)
--- trunk/Source/_javascript_Core/jit/JITPropertyAccess.cpp 2018-02-06 22:36:00 UTC (rev 228192)
+++ trunk/Source/_javascript_Core/jit/JITPropertyAccess.cpp 2018-02-06 22:42:26 UTC (rev 228193)
@@ -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 @@
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 @@
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);
Modified: trunk/Source/_javascript_Core/runtime/CommonSlowPaths.h (228192 => 228193)
--- trunk/Source/_javascript_Core/runtime/CommonSlowPaths.h 2018-02-06 22:36:00 UTC (rev 228192)
+++ trunk/Source/_javascript_Core/runtime/CommonSlowPaths.h 2018-02-06 22:42:26 UTC (rev 228193)
@@ -138,8 +138,11 @@
}
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 @@
}
// 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);
Modified: trunk/Source/_javascript_Core/runtime/Options.h (228192 => 228193)
--- trunk/Source/_javascript_Core/runtime/Options.h 2018-02-06 22:36:00 UTC (rev 228192)
+++ trunk/Source/_javascript_Core/runtime/Options.h 2018-02-06 22:42:26 UTC (rev 228193)
@@ -250,6 +250,7 @@
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) \
