From 5ec2d8f6dd3e67e8aa85002dbad152a614f92eeb Mon Sep 17 00:00:00 2001 From: Akrosh Gandhi Date: Thu, 1 Sep 2016 18:24:07 -0700 Subject: [PATCH] Address deref issue During the forward global optimizer pass, given a property store that causes an object layout to go from object-header-inlined to non-object-header-inlined, kill all type syms with object-header-inlined types to protect against aliasing. --- lib/Backend/GlobOptFields.cpp | 35 ++++++++++++++++++--------------- lib/Backend/Opnd.cpp | 37 ++++++++++++++++++++++++++++------- 2 files changed, 49 insertions(+), 23 deletions(-) diff --git a/lib/Backend/GlobOptFields.cpp b/lib/Backend/GlobOptFields.cpp index 6569f77dd06..daac8c36dca 100644 --- a/lib/Backend/GlobOptFields.cpp +++ b/lib/Backend/GlobOptFields.cpp @@ -2170,7 +2170,7 @@ GlobOpt::FinishOptPropOp(IR::Instr *instr, IR::PropertySymOpnd *opnd, BasicBlock isObjTypeSpecialized = ProcessPropOpInTypeCheckSeq(instr, opnd, block, updateExistingValue, emitsTypeCheckOut, changesTypeValueOut, &isObjTypeChecked); } - if (opnd == instr->GetDst() && this->objectTypeSyms && !isObjTypeChecked) + if (opnd == instr->GetDst() && this->objectTypeSyms) { if (block == nullptr) { @@ -2180,26 +2180,29 @@ GlobOpt::FinishOptPropOp(IR::Instr *instr, IR::PropertySymOpnd *opnd, BasicBlock // This is a property store that may change the layout of the object that it stores to. This means that // it may change any aliased object. Do two things to address this: // - Add all object types in this function to the set that may have had a property added. This will prevent - // final type optimization across this instruction. + // final type optimization across this instruction. (Only needed here for non-specialized stores.) // - Kill all type symbols that currently hold object-header-inlined types. Any of them may have their layout // changed by the addition of a property. SymID opndId = opnd->HasObjectTypeSym() ? opnd->GetObjectTypeSym()->m_id : -1; - if (block->globOptData.maybeWrittenTypeSyms == nullptr) - { - block->globOptData.maybeWrittenTypeSyms = JitAnew(this->alloc, BVSparse, this->alloc); - } - if (isObjTypeSpecialized) + if (!isObjTypeChecked) { - // The current object will be protected by a type check, unless no further accesses to it are - // protected by this access. - Assert(this->objectTypeSyms->Test(opndId)); - this->objectTypeSyms->Clear(opndId); - } - block->globOptData.maybeWrittenTypeSyms->Or(this->objectTypeSyms); - if (isObjTypeSpecialized) - { - this->objectTypeSyms->Set(opndId); + if (block->globOptData.maybeWrittenTypeSyms == nullptr) + { + block->globOptData.maybeWrittenTypeSyms = JitAnew(this->alloc, BVSparse, this->alloc); + } + if (isObjTypeSpecialized) + { + // The current object will be protected by a type check, unless no further accesses to it are + // protected by this access. + Assert(this->objectTypeSyms->Test(opndId)); + this->objectTypeSyms->Clear(opndId); + } + block->globOptData.maybeWrittenTypeSyms->Or(this->objectTypeSyms); + if (isObjTypeSpecialized) + { + this->objectTypeSyms->Set(opndId); + } } if (!isObjTypeSpecialized || opnd->ChangesObjectLayout()) diff --git a/lib/Backend/Opnd.cpp b/lib/Backend/Opnd.cpp index fe52c1be0ca..a20f684b998 100644 --- a/lib/Backend/Opnd.cpp +++ b/lib/Backend/Opnd.cpp @@ -846,20 +846,43 @@ PropertySymOpnd::IsObjectHeaderInlined() const bool PropertySymOpnd::ChangesObjectLayout() const { + Js::Type *cachedType = this->IsMono() ? this->GetType() : this->GetFirstEquivalentType(); + Js::Type *finalType = this->GetFinalType(); - if (finalType == nullptr || !Js::DynamicType::Is(finalType->GetTypeId())) + if (finalType && Js::DynamicType::Is(finalType->GetTypeId())) + { + // This is the case where final type opt may cause pro-active type transition to take place. + + Assert(cachedType && Js::DynamicType::Is(cachedType->GetTypeId())); + + Js::DynamicTypeHandler * cachedTypeHandler = (static_cast(cachedType))->GetTypeHandler(); + Js::DynamicTypeHandler * finalTypeHandler = (static_cast(finalType))->GetTypeHandler(); + + return cachedTypeHandler->GetInlineSlotCapacity() != finalTypeHandler->GetInlineSlotCapacity() || + cachedTypeHandler->GetOffsetOfInlineSlots() != finalTypeHandler->GetOffsetOfInlineSlots(); + } + + if (!this->HasInitialType()) { return false; } - Js::Type *cachedType = this->IsMono() ? this->GetType() : this->GetFirstEquivalentType(); - Assert(cachedType && Js::DynamicType::Is(cachedType->GetTypeId())); + Js::Type *initialType = this->GetInitialType(); + if (initialType && Js::DynamicType::Is(initialType->GetTypeId())) + { + // This is the case where the type transition actually occurs. (This is the only case that's detectable + // during the loop pre-pass, since final types are not in place yet.) - Js::DynamicTypeHandler * cachedTypeHandler = (static_cast(cachedType))->GetTypeHandler(); - Js::DynamicTypeHandler * finalTypeHandler = (static_cast(finalType))->GetTypeHandler(); + Assert(cachedType && Js::DynamicType::Is(cachedType->GetTypeId())); - return cachedTypeHandler->GetInlineSlotCapacity() != finalTypeHandler->GetInlineSlotCapacity() || - cachedTypeHandler->GetOffsetOfInlineSlots() != finalTypeHandler->GetOffsetOfInlineSlots(); + Js::DynamicTypeHandler * cachedTypeHandler = (static_cast(cachedType))->GetTypeHandler(); + Js::DynamicTypeHandler * initialTypeHandler = (static_cast(initialType))->GetTypeHandler(); + + return cachedTypeHandler->GetInlineSlotCapacity() != initialTypeHandler->GetInlineSlotCapacity() || + cachedTypeHandler->GetOffsetOfInlineSlots() != initialTypeHandler->GetOffsetOfInlineSlots(); + } + + return false; } void