Diff
Modified: trunk/JSTests/ChangeLog (239625 => 239626)
--- trunk/JSTests/ChangeLog 2019-01-04 19:45:09 UTC (rev 239625)
+++ trunk/JSTests/ChangeLog 2019-01-04 19:52:26 UTC (rev 239626)
@@ -1,3 +1,14 @@
+2019-01-04 Tadeu Zagallo <[email protected]>
+
+ Baseline version of get_by_id may corrupt metadata
+ https://bugs.webkit.org/show_bug.cgi?id=193085
+ <rdar://problem/23453006>
+
+ Reviewed by Saam Barati.
+
+ * stress/get-by-id-change-mode.js: Added.
+ (forEach):
+
2019-01-02 Yusuke Suzuki <[email protected]>
[JSC] Optimize Object.prototype.toString
Added: trunk/JSTests/stress/get-by-id-change-mode.js (0 => 239626)
--- trunk/JSTests/stress/get-by-id-change-mode.js (rev 0)
+++ trunk/JSTests/stress/get-by-id-change-mode.js 2019-01-04 19:52:26 UTC (rev 239626)
@@ -0,0 +1,12 @@
+//@ runDefault("--useConcurrentJIT=0")
+
+forEach({ length: 5 }, function() {
+ for (var i = 0; i < 10; i++) {
+ forEach([1], function() {});
+ }
+});
+
+function forEach(a, b) {
+ for (var c = 0; c < a.length; c++)
+ b();
+}
Modified: trunk/Source/_javascript_Core/ChangeLog (239625 => 239626)
--- trunk/Source/_javascript_Core/ChangeLog 2019-01-04 19:45:09 UTC (rev 239625)
+++ trunk/Source/_javascript_Core/ChangeLog 2019-01-04 19:52:26 UTC (rev 239626)
@@ -1,3 +1,43 @@
+2019-01-04 Tadeu Zagallo <[email protected]>
+
+ Baseline version of get_by_id may corrupt metadata
+ https://bugs.webkit.org/show_bug.cgi?id=193085
+ <rdar://problem/23453006>
+
+ Reviewed by Saam Barati.
+
+ The Baseline version of get_by_id unconditionally calls `emitArrayProfilingSiteForBytecodeIndexWithCell`
+ if the property is `length`. However, since the bytecode rewrite, get_by_id only has an ArrayProfile entry
+ in the metadata if its mode is `GetByIdMode::ArrayLength`. That might result in one of two bad things:
+ 1) get_by_id's mode is not ArrayLength, and a duplicate, out-of-line ArrayProfile entry will be created by
+ `CodeBlock::getOrAddArrayProfile`.
+ 2) get_by_id's mode *is* ArrayLength and we generate the array profiling code pointing to the ArrayProfile
+ that lives in the metadata table. This works fine as long as get_by_id does not change modes. If that happens,
+ the JIT code will write into the metadata table, overwriting the 'GetByIdModeMetadata` for another mode.
+
+ Add a check to the Baseline version of get_by_id so that we only do the ArrayProfiling if the get_by_id's
+ mode is ArrayLength
+
+ * bytecode/BytecodeList.rb:
+ * bytecode/CodeBlock.cpp:
+ (JSC::CodeBlock::getArrayProfile):
+ (JSC::CodeBlock::addArrayProfile): Deleted.
+ (JSC::CodeBlock::getOrAddArrayProfile): Deleted.
+ * bytecode/CodeBlock.h:
+ (JSC::CodeBlock::numberOfArrayProfiles const): Deleted.
+ (JSC::CodeBlock::arrayProfiles): Deleted.
+ * bytecode/CodeBlockInlines.h:
+ (JSC::CodeBlock::forEachArrayProfile):
+ * jit/JIT.h:
+ * jit/JITInlines.h:
+ (JSC::JIT::emitArrayProfilingSiteForBytecodeIndexWithCell): Deleted.
+ * jit/JITPropertyAccess.cpp:
+ (JSC::JIT::emit_op_get_by_id):
+ * jit/JITPropertyAccess32_64.cpp:
+ (JSC::JIT::emit_op_get_by_id):
+ * llint/LLIntSlowPaths.cpp:
+ (JSC::LLInt::LLINT_SLOW_PATH_DECL):
+
2019-01-02 Yusuke Suzuki <[email protected]>
[JSC] Optimize Object.prototype.toString
Modified: trunk/Source/_javascript_Core/bytecode/BytecodeList.rb (239625 => 239626)
--- trunk/Source/_javascript_Core/bytecode/BytecodeList.rb 2019-01-04 19:45:09 UTC (rev 239625)
+++ trunk/Source/_javascript_Core/bytecode/BytecodeList.rb 2019-01-04 19:52:26 UTC (rev 239626)
@@ -413,8 +413,8 @@
},
metadata: {
mode: GetByIdMode,
+ hitCountForLLIntCaching: unsigned,
modeMetadata: GetByIdModeMetadata,
- hitCountForLLIntCaching: unsigned,
profile: ValueProfile,
}
@@ -473,8 +473,8 @@
oldStructure: StructureID,
offset: unsigned,
newStructure: StructureID,
+ flags: PutByIdFlags,
structureChain: WriteBarrierBase[StructureChain],
- flags: PutByIdFlags,
},
metadata_initializers: {
flags: :flags
Modified: trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp (239625 => 239626)
--- trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp 2019-01-04 19:45:09 UTC (rev 239625)
+++ trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp 2019-01-04 19:52:26 UTC (rev 239626)
@@ -2498,16 +2498,11 @@
return &metadata.modeMetadata.arrayLengthMode.arrayProfile;
break;
}
-
default:
break;
}
- for (auto& m_arrayProfile : m_arrayProfiles) {
- if (m_arrayProfile.bytecodeOffset() == bytecodeOffset)
- return &m_arrayProfile;
- }
- return 0;
+ return nullptr;
}
ArrayProfile* CodeBlock::getArrayProfile(unsigned bytecodeOffset)
@@ -2516,33 +2511,6 @@
return getArrayProfile(locker, bytecodeOffset);
}
-ArrayProfile* CodeBlock::addArrayProfile(const ConcurrentJSLocker&, unsigned bytecodeOffset)
-{
- m_arrayProfiles.append(ArrayProfile(bytecodeOffset));
- return &m_arrayProfiles.last();
-}
-
-ArrayProfile* CodeBlock::addArrayProfile(unsigned bytecodeOffset)
-{
- ConcurrentJSLocker locker(m_lock);
- return addArrayProfile(locker, bytecodeOffset);
-}
-
-ArrayProfile* CodeBlock::getOrAddArrayProfile(const ConcurrentJSLocker& locker, unsigned bytecodeOffset)
-{
- ArrayProfile* result = getArrayProfile(locker, bytecodeOffset);
- if (result)
- return result;
- return addArrayProfile(locker, bytecodeOffset);
-}
-
-ArrayProfile* CodeBlock::getOrAddArrayProfile(unsigned bytecodeOffset)
-{
- ConcurrentJSLocker locker(m_lock);
- return getOrAddArrayProfile(locker, bytecodeOffset);
-}
-
-
#if ENABLE(DFG_JIT)
Vector<CodeOrigin, 0, UnsafeVectorOverflow>& CodeBlock::codeOrigins()
{
Modified: trunk/Source/_javascript_Core/bytecode/CodeBlock.h (239625 => 239626)
--- trunk/Source/_javascript_Core/bytecode/CodeBlock.h 2019-01-04 19:45:09 UTC (rev 239625)
+++ trunk/Source/_javascript_Core/bytecode/CodeBlock.h 2019-01-04 19:52:26 UTC (rev 239626)
@@ -464,14 +464,8 @@
bool couldTakeSpecialFastCase(InstructionStream::Offset bytecodeOffset);
- unsigned numberOfArrayProfiles() const { return m_arrayProfiles.size(); }
- const ArrayProfileVector& arrayProfiles() { return m_arrayProfiles; }
- ArrayProfile* addArrayProfile(const ConcurrentJSLocker&, unsigned bytecodeOffset);
- ArrayProfile* addArrayProfile(unsigned bytecodeOffset);
ArrayProfile* getArrayProfile(const ConcurrentJSLocker&, unsigned bytecodeOffset);
ArrayProfile* getArrayProfile(unsigned bytecodeOffset);
- ArrayProfile* getOrAddArrayProfile(const ConcurrentJSLocker&, unsigned bytecodeOffset);
- ArrayProfile* getOrAddArrayProfile(unsigned bytecodeOffset);
// Exception handling support
@@ -987,7 +981,6 @@
RefCountedArray<ValueProfile> m_argumentValueProfiles;
Vector<std::unique_ptr<ValueProfileAndOperandBuffer>> m_catchProfiles;
SegmentedVector<RareCaseProfile, 8> m_rareCaseProfiles;
- ArrayProfileVector m_arrayProfiles;
// Constant Pool
COMPILE_ASSERT(sizeof(Register) == sizeof(WriteBarrier<Unknown>), Register_must_be_same_size_as_WriteBarrier_Unknown);
Modified: trunk/Source/_javascript_Core/bytecode/CodeBlockInlines.h (239625 => 239626)
--- trunk/Source/_javascript_Core/bytecode/CodeBlockInlines.h 2019-01-04 19:45:09 UTC (rev 239625)
+++ trunk/Source/_javascript_Core/bytecode/CodeBlockInlines.h 2019-01-04 19:52:26 UTC (rev 239626)
@@ -51,9 +51,6 @@
template<typename Functor>
void CodeBlock::forEachArrayProfile(const Functor& func)
{
- for (auto& arrayProfile : m_arrayProfiles)
- func(arrayProfile);
-
if (m_metadata) {
m_metadata->forEach<OpGetById>([&] (auto& metadata) {
if (metadata.mode == GetByIdMode::ArrayLength)
Modified: trunk/Source/_javascript_Core/jit/JIT.h (239625 => 239626)
--- trunk/Source/_javascript_Core/jit/JIT.h 2019-01-04 19:45:09 UTC (rev 239625)
+++ trunk/Source/_javascript_Core/jit/JIT.h 2019-01-04 19:52:26 UTC (rev 239626)
@@ -365,7 +365,6 @@
emitValueProfilingSiteIfProfiledOpcode(Op bytecode);
void emitArrayProfilingSiteWithCell(RegisterID cell, RegisterID indexingType, ArrayProfile*);
- void emitArrayProfilingSiteForBytecodeIndexWithCell(RegisterID cell, RegisterID indexingType, unsigned bytecodeIndex);
void emitArrayProfileStoreToHoleSpecialCase(ArrayProfile*);
void emitArrayProfileOutOfBoundsSpecialCase(ArrayProfile*);
Modified: trunk/Source/_javascript_Core/jit/JITInlines.h (239625 => 239626)
--- trunk/Source/_javascript_Core/jit/JITInlines.h 2019-01-04 19:45:09 UTC (rev 239625)
+++ trunk/Source/_javascript_Core/jit/JITInlines.h 2019-01-04 19:52:26 UTC (rev 239626)
@@ -354,11 +354,6 @@
load8(Address(cell, JSCell::indexingTypeAndMiscOffset()), indexingType);
}
-inline void JIT::emitArrayProfilingSiteForBytecodeIndexWithCell(RegisterID cell, RegisterID indexingType, unsigned bytecodeIndex)
-{
- emitArrayProfilingSiteWithCell(cell, indexingType, m_codeBlock->getOrAddArrayProfile(bytecodeIndex));
-}
-
inline void JIT::emitArrayProfileStoreToHoleSpecialCase(ArrayProfile* arrayProfile)
{
store8(TrustedImm32(1), arrayProfile->addressOfMayStoreToHole());
Modified: trunk/Source/_javascript_Core/jit/JITPropertyAccess.cpp (239625 => 239626)
--- trunk/Source/_javascript_Core/jit/JITPropertyAccess.cpp 2019-01-04 19:45:09 UTC (rev 239625)
+++ trunk/Source/_javascript_Core/jit/JITPropertyAccess.cpp 2019-01-04 19:52:26 UTC (rev 239626)
@@ -572,6 +572,7 @@
void JIT::emit_op_get_by_id(const Instruction* currentInstruction)
{
auto bytecode = currentInstruction->as<OpGetById>();
+ auto& metadata = bytecode.metadata(m_codeBlock);
int resultVReg = bytecode.dst.offset();
int baseVReg = bytecode.base.offset();
const Identifier* ident = &(m_codeBlock->identifier(bytecode.property));
@@ -580,8 +581,11 @@
emitJumpSlowCaseIfNotJSCell(regT0, baseVReg);
- if (*ident == m_vm->propertyNames->length && shouldEmitProfiling())
- emitArrayProfilingSiteForBytecodeIndexWithCell(regT0, regT1, m_bytecodeOffset);
+ if (*ident == m_vm->propertyNames->length && shouldEmitProfiling()) {
+ Jump notArrayLengthMode = branch8(NotEqual, AbsoluteAddress(&metadata.mode), TrustedImm32(static_cast<uint8_t>(GetByIdMode::ArrayLength)));
+ emitArrayProfilingSiteWithCell(regT0, regT1, &metadata.modeMetadata.arrayLengthMode.arrayProfile);
+ notArrayLengthMode.link(this);
+ }
JITGetByIdGenerator gen(
m_codeBlock, CodeOrigin(m_bytecodeOffset), CallSiteIndex(m_bytecodeOffset), RegisterSet::stubUnavailableRegisters(),
Modified: trunk/Source/_javascript_Core/jit/JITPropertyAccess32_64.cpp (239625 => 239626)
--- trunk/Source/_javascript_Core/jit/JITPropertyAccess32_64.cpp 2019-01-04 19:45:09 UTC (rev 239625)
+++ trunk/Source/_javascript_Core/jit/JITPropertyAccess32_64.cpp 2019-01-04 19:52:26 UTC (rev 239626)
@@ -571,6 +571,7 @@
void JIT::emit_op_get_by_id(const Instruction* currentInstruction)
{
auto bytecode = currentInstruction->as<OpGetById>();
+ auto& metadata = bytecode.metadata(m_codeBlock);
int dst = bytecode.dst.offset();
int base = bytecode.base.offset();
const Identifier* ident = &(m_codeBlock->identifier(bytecode.property));
@@ -578,8 +579,11 @@
emitLoad(base, regT1, regT0);
emitJumpSlowCaseIfNotJSCell(base, regT1);
- if (*ident == m_vm->propertyNames->length && shouldEmitProfiling())
- emitArrayProfilingSiteForBytecodeIndexWithCell(regT0, regT2, m_bytecodeOffset);
+ if (*ident == m_vm->propertyNames->length && shouldEmitProfiling()) {
+ Jump notArrayLengthMode = branch8(NotEqual, AbsoluteAddress(&metadata.mode), TrustedImm32(static_cast<uint8_t>(GetByIdMode::ArrayLength)));
+ emitArrayProfilingSiteWithCell(regT0, regT2, &metadata.modeMetadata.arrayLengthMode.arrayProfile);
+ notArrayLengthMode.link(this);
+ }
JITGetByIdGenerator gen(
m_codeBlock, CodeOrigin(m_bytecodeOffset), CallSiteIndex(currentInstruction), RegisterSet::stubUnavailableRegisters(),
Modified: trunk/Source/_javascript_Core/llint/LLIntSlowPaths.cpp (239625 => 239626)
--- trunk/Source/_javascript_Core/llint/LLIntSlowPaths.cpp 2019-01-04 19:45:09 UTC (rev 239625)
+++ trunk/Source/_javascript_Core/llint/LLIntSlowPaths.cpp 2019-01-04 19:52:26 UTC (rev 239626)
@@ -825,6 +825,7 @@
&& isJSArray(baseValue)
&& ident == vm.propertyNames->length) {
metadata.mode = GetByIdMode::ArrayLength;
+ new (&metadata.modeMetadata.arrayLengthMode.arrayProfile) ArrayProfile(codeBlock->bytecodeOffset(pc));
metadata.modeMetadata.arrayLengthMode.arrayProfile.observeStructure(baseValue.asCell()->structure(vm));
// Prevent the prototype cache from ever happening.
