- Revision
- 284036
- Author
- sbar...@apple.com
- Date
- 2021-10-12 14:51:35 -0700 (Tue, 12 Oct 2021)
Log Message
Fix spec-correctness when inlining __proto__ intrinsic using get_by_id_with_this
https://bugs.webkit.org/show_bug.cgi?id=231559
Reviewed by Yusuke Suzuki.
My original fix in r283512 has some theoretical spec correctness issues.
I'm not sure if they can be materialized or not since we only use
get_by_id_with_this in very limited scenarios. However, this patch just
makes it so we call the getter instead of attempting to inline it
when using get_by_id_with_this.
* jit/AssemblyHelpers.cpp:
(JSC::AssemblyHelpers::emitLoadPrototypeWithoutCheck): Deleted.
* jit/AssemblyHelpers.h:
* jit/IntrinsicEmitter.cpp:
(JSC::IntrinsicGetterAccessCase::canEmitIntrinsicGetter):
(JSC::IntrinsicGetterAccessCase::emitIntrinsicGetter):
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (284035 => 284036)
--- trunk/Source/_javascript_Core/ChangeLog 2021-10-12 21:46:35 UTC (rev 284035)
+++ trunk/Source/_javascript_Core/ChangeLog 2021-10-12 21:51:35 UTC (rev 284036)
@@ -1,3 +1,23 @@
+2021-10-12 Saam Barati <sbar...@apple.com>
+
+ Fix spec-correctness when inlining __proto__ intrinsic using get_by_id_with_this
+ https://bugs.webkit.org/show_bug.cgi?id=231559
+
+ Reviewed by Yusuke Suzuki.
+
+ My original fix in r283512 has some theoretical spec correctness issues.
+ I'm not sure if they can be materialized or not since we only use
+ get_by_id_with_this in very limited scenarios. However, this patch just
+ makes it so we call the getter instead of attempting to inline it
+ when using get_by_id_with_this.
+
+ * jit/AssemblyHelpers.cpp:
+ (JSC::AssemblyHelpers::emitLoadPrototypeWithoutCheck): Deleted.
+ * jit/AssemblyHelpers.h:
+ * jit/IntrinsicEmitter.cpp:
+ (JSC::IntrinsicGetterAccessCase::canEmitIntrinsicGetter):
+ (JSC::IntrinsicGetterAccessCase::emitIntrinsicGetter):
+
2021-10-12 Alex Christensen <achristen...@webkit.org>
Use STL instead of WTF::get_if, WTF::Monostate, WTF::visit, and WTF::holds_alternative
Modified: trunk/Source/_javascript_Core/jit/AssemblyHelpers.cpp (284035 => 284036)
--- trunk/Source/_javascript_Core/jit/AssemblyHelpers.cpp 2021-10-12 21:46:35 UTC (rev 284035)
+++ trunk/Source/_javascript_Core/jit/AssemblyHelpers.cpp 2021-10-12 21:51:35 UTC (rev 284036)
@@ -435,36 +435,6 @@
hasMonoProto.link(this);
}
-void AssemblyHelpers::emitLoadPrototypeWithoutCheck(VM& vm, GPRReg objectGPR, JSValueRegs resultRegs, GPRReg scratchGPR, GPRReg scratch2GPR)
-{
- ASSERT(objectGPR != scratchGPR);
- ASSERT(objectGPR != scratch2GPR);
- ASSERT(resultRegs.payloadGPR() != scratchGPR);
- ASSERT(resultRegs.payloadGPR() != scratch2GPR);
-#if USE(JSVALUE32_64)
- ASSERT(resultRegs.tagGPR() != scratchGPR);
- ASSERT(resultRegs.tagGPR() != scratch2GPR);
-#endif
-
- emitLoadStructure(vm, objectGPR, scratchGPR, scratch2GPR);
-#if USE(JSVALUE64)
- loadValue(MacroAssembler::Address(scratchGPR, Structure::prototypeOffset()), JSValueRegs(scratch2GPR));
-#else
- load32(MacroAssembler::Address(scratchGPR, Structure::prototypeOffset() + TagOffset), scratch2GPR);
-#endif
- auto hasMonoProto = branchIfNotEmpty(scratch2GPR);
- loadValue(MacroAssembler::Address(objectGPR, offsetRelativeToBase(knownPolyProtoOffset)), resultRegs);
- auto done = jump();
- hasMonoProto.link(this);
-#if USE(JSVALUE64)
- move(scratch2GPR, resultRegs.payloadGPR());
-#else
- load32(MacroAssembler::Address(scratchGPR, Structure::prototypeOffset() + PayloadOffset), resultRegs.payloadGPR());
- move(scratch2GPR, resultRegs.tagGPR());
-#endif
- done.link(this);
-}
-
void AssemblyHelpers::makeSpaceOnStackForCCall()
{
unsigned stackOffset = WTF::roundUpToMultipleOf(stackAlignmentBytes(), maxFrameExtentForSlowPathCall);
Modified: trunk/Source/_javascript_Core/jit/AssemblyHelpers.h (284035 => 284036)
--- trunk/Source/_javascript_Core/jit/AssemblyHelpers.h 2021-10-12 21:46:35 UTC (rev 284035)
+++ trunk/Source/_javascript_Core/jit/AssemblyHelpers.h 2021-10-12 21:51:35 UTC (rev 284036)
@@ -1530,7 +1530,6 @@
void emitLoadStructure(VM&, RegisterID source, RegisterID dest, RegisterID scratch);
void emitLoadPrototype(VM&, GPRReg objectGPR, JSValueRegs resultRegs, GPRReg scratchGPR, JumpList& slowPath);
- void emitLoadPrototypeWithoutCheck(VM&, GPRReg objectGPR, JSValueRegs resultRegs, GPRReg scratchGPR, GPRReg scratch2GPR);
void emitStoreStructureWithTypeInfo(TrustedImmPtr structure, RegisterID dest, RegisterID)
{
Modified: trunk/Source/_javascript_Core/jit/IntrinsicEmitter.cpp (284035 => 284036)
--- trunk/Source/_javascript_Core/jit/IntrinsicEmitter.cpp 2021-10-12 21:46:35 UTC (rev 284035)
+++ trunk/Source/_javascript_Core/jit/IntrinsicEmitter.cpp 2021-10-12 21:51:35 UTC (rev 284036)
@@ -48,15 +48,18 @@
bool IntrinsicGetterAccessCase::canEmitIntrinsicGetter(StructureStubInfo& stubInfo, JSFunction* getter, Structure* structure)
{
+ // We aren't structure checking the this value, so we don't know:
+ // - For type array loads, that it's a typed array.
+ // - For __proto__ getter, that the incoming value is an object,
+ // and if it overrides getPrototype structure flags.
+ // So for these cases, it's simpler to just call the getter directly.
+ if (stubInfo.thisValueIsInThisGPR())
+ return false;
switch (getter->intrinsic()) {
case TypedArrayByteOffsetIntrinsic:
case TypedArrayByteLengthIntrinsic:
case TypedArrayLengthIntrinsic: {
- // We aren't structure checking the this value, so we don't know it's a typed array.
- if (stubInfo.thisValueIsInThisGPR())
- return false;
-
TypedArrayType type = structure->classInfo()->typedArrayStorageType;
if (!isTypedView(type))
@@ -137,21 +140,10 @@
}
case UnderscoreProtoIntrinsic: {
- StructureStubInfo& stubInfo = *state.stubInfo;
- if (stubInfo.thisValueIsInThisGPR()) {
- auto allocator = state.makeDefaultScratchAllocator(state.scratchGPR);
- GPRReg scratch2GPR = allocator.allocateScratchGPR();
- auto preservedState = allocator.preserveReusedRegistersByPushing(jit, ScratchRegisterAllocator::ExtraStackSpace::NoExtraSpace);
-
- jit.emitLoadPrototypeWithoutCheck(state.m_vm, state.u.thisGPR, valueRegs, state.scratchGPR, scratch2GPR);
-
- allocator.restoreReusedRegistersByPopping(jit, preservedState);
- } else {
- if (structure()->hasPolyProto())
- jit.loadValue(CCallHelpers::Address(baseGPR, offsetRelativeToBase(knownPolyProtoOffset)), valueRegs);
- else
- jit.moveValue(structure()->storedPrototype(), valueRegs);
- }
+ if (structure()->hasPolyProto())
+ jit.loadValue(CCallHelpers::Address(baseGPR, offsetRelativeToBase(knownPolyProtoOffset)), valueRegs);
+ else
+ jit.moveValue(structure()->storedPrototype(), valueRegs);
state.succeed();
return;
}
