Ignore:
Timestamp:
Dec 15, 2020, 4:33:34 PM (5 years ago)
Author:
Alexey Shvayka
Message:

Non-enumerable property fails to shadow inherited enumerable property from for-in
​https://bugs.webkit.org/show_bug.cgi?id=38970

Reviewed by Keith Miller.

JSTests:

  • stress/arguments-bizarre-behaviour-disable-enumerability.js:
  • stress/for-in-redefine-enumerable.js: Added.
  • stress/for-in-shadow-non-enumerable.js: Added.
  • test262/expectations.yaml: Mark 4 test cases as passing.

Source/JavaScriptCore:

While for/in was initially specified with notion of "shadowing", it wasn't clarified
until ES5 that Enumerable attributes are ignored when determining if a property
has already been processed. Recently, for/in spec was expanded [1] to pin down common
case enumeration as it's currently implemented by V8 and SpiderMonkey.

Since keeping track of DontEnum properties is a massive slowdown for uncached runs
(with any data structure used), this patch simply adds Enumerable check to
has_{indexed,structure,generic}_property bytecode ops and does renaming chores.

Common code is now shared between HasIndexedProperty (emitted for 0 in arr) and
HasEnumerableIndexedProperty DFG nodes via passing different slow path ops rather
than having OpInfo with PropertySlot::InternalMethodType, which is a nice refactor.

While this change aligns common case for/in enumeration with the spec and other
engines, it also introduces a few observable discrepancies from V8 and SpiderMonkey,
which are permitted by the spec [2]:
a) properties that have been redefined as DontEnum within loop body are skipped,

which matches the spec [3] and seems like expected behavior;

b) "shadowing" is broken if a DontEnum property of already visited object is

added / deleted / redefined within loop body, which (pretty much) never happens.

This patch introduces a new invariant: all properties getOwn*PropertyNames() returns
in DontEnumPropertiesMode::Exclude should be reported as Enumerable by
getOwnPropertySlot(). JSCallbackObject and RuntimeArray are fixed to follow it.

for/in and Object.keys microbenchmarks are neutral. This change does not affect
JSPropertyNameEnumerator caching, nor fast paths of its bytecodes.

[1]: ​https://github.com/tc39/ecma262/pull/1791
[2]: ​https://tc39.es/ecma262/#sec-enumerate-object-properties (last paragraph)
[3]: ​https://tc39.es/ecma262/#sec-%foriniteratorprototype%.next (step 7.b.iii)

  • API/JSCallbackObjectFunctions.h:

(JSC::JSCallbackObject<Parent>::getOwnPropertySlot):

  • API/tests/testapi.c:
  • API/tests/testapiScripts/testapi.js:
  • bytecode/BytecodeList.rb:
  • bytecode/BytecodeUseDef.cpp:

(JSC::computeUsesForBytecodeIndexImpl):
(JSC::computeDefsForBytecodeIndexImpl):

  • bytecode/CodeBlock.cpp:

(JSC::CodeBlock::finishCreation):

  • bytecode/Opcode.h:
  • bytecompiler/BytecodeGenerator.cpp:

(JSC::BytecodeGenerator::emitHasEnumerableIndexedProperty):
(JSC::BytecodeGenerator::emitHasEnumerableStructureProperty):
(JSC::BytecodeGenerator::emitHasEnumerableProperty):
(JSC::BytecodeGenerator::emitHasGenericProperty): Deleted.
(JSC::BytecodeGenerator::emitHasIndexedProperty): Deleted.
(JSC::BytecodeGenerator::emitHasStructureProperty): Deleted.

  • bytecompiler/BytecodeGenerator.h:
  • bytecompiler/NodesCodegen.cpp:

(JSC::ForInNode::emitBytecode):

  • dfg/DFGAbstractInterpreterInlines.h:

(JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):

  • dfg/DFGByteCodeParser.cpp:

(JSC::DFG::ByteCodeParser::parseBlock):

  • dfg/DFGCapabilities.cpp:

(JSC::DFG::capabilityLevel):

  • dfg/DFGClobberize.h:

(JSC::DFG::clobberize):

  • dfg/DFGDoesGC.cpp:

(JSC::DFG::doesGC):

  • dfg/DFGFixupPhase.cpp:

(JSC::DFG::FixupPhase::fixupNode):
(JSC::DFG::FixupPhase::convertToHasIndexedProperty):

  • dfg/DFGNode.h:

(JSC::DFG::Node::hasArrayMode):
(JSC::DFG::Node::hasInternalMethodType const): Deleted.
(JSC::DFG::Node::internalMethodType const): Deleted.
(JSC::DFG::Node::setInternalMethodType): Deleted.

  • dfg/DFGNodeType.h:
  • dfg/DFGOperations.cpp:

(JSC::DFG::JSC_DEFINE_JIT_OPERATION):

  • dfg/DFGOperations.h:
  • dfg/DFGPredictionPropagationPhase.cpp:
  • dfg/DFGSSALoweringPhase.cpp:

(JSC::DFG::SSALoweringPhase::handleNode):

  • dfg/DFGSafeToExecute.h:

(JSC::DFG::safeToExecute):

  • dfg/DFGSpeculativeJIT.cpp:

(JSC::DFG::SpeculativeJIT::compileHasEnumerableProperty):
(JSC::DFG::SpeculativeJIT::compileHasEnumerableStructureProperty):
(JSC::DFG::SpeculativeJIT::compileHasIndexedProperty):
(JSC::DFG::SpeculativeJIT::compileHasGenericProperty): Deleted.
(JSC::DFG::SpeculativeJIT::compileHasStructureProperty): Deleted.

  • dfg/DFGSpeculativeJIT.h:
  • dfg/DFGSpeculativeJIT32_64.cpp:

(JSC::DFG::SpeculativeJIT::compile):

  • dfg/DFGSpeculativeJIT64.cpp:

(JSC::DFG::SpeculativeJIT::compile):

  • ftl/FTLCapabilities.cpp:

(JSC::FTL::canCompile):

  • ftl/FTLLowerDFGToB3.cpp:

(JSC::FTL::DFG::LowerDFGToB3::compileNode):
(JSC::FTL::DFG::LowerDFGToB3::compileHasIndexedProperty):
(JSC::FTL::DFG::LowerDFGToB3::compileHasEnumerableProperty):
(JSC::FTL::DFG::LowerDFGToB3::compileHasEnumerableStructureProperty):
(JSC::FTL::DFG::LowerDFGToB3::compileHasGenericProperty): Deleted.
(JSC::FTL::DFG::LowerDFGToB3::compileHasStructureProperty): Deleted.

  • jit/JIT.cpp:

(JSC::JIT::privateCompileMainPass):
(JSC::JIT::privateCompileSlowCases):

  • jit/JIT.h:
  • jit/JITOpcodes.cpp:

(JSC::JIT::emit_op_has_enumerable_structure_property):
(JSC::JIT::emit_op_has_enumerable_indexed_property):
(JSC::JIT::emitSlow_op_has_enumerable_indexed_property):
(JSC::JIT::emit_op_has_structure_property): Deleted.
(JSC::JIT::emit_op_has_indexed_property): Deleted.
(JSC::JIT::emitSlow_op_has_indexed_property): Deleted.

  • jit/JITOpcodes32_64.cpp:

(JSC::JIT::emit_op_has_enumerable_structure_property):
(JSC::JIT::emit_op_has_enumerable_indexed_property):
(JSC::JIT::emitSlow_op_has_enumerable_indexed_property):
(JSC::JIT::emit_op_has_structure_property): Deleted.
(JSC::JIT::emit_op_has_indexed_property): Deleted.
(JSC::JIT::emitSlow_op_has_indexed_property): Deleted.

  • jit/JITOperations.cpp:

(JSC::JSC_DEFINE_JIT_OPERATION):

  • jit/JITOperations.h:
  • llint/LowLevelInterpreter.asm:
  • llint/LowLevelInterpreter64.asm:
  • runtime/CommonSlowPaths.cpp:

(JSC::JSC_DEFINE_COMMON_SLOW_PATH):

  • runtime/CommonSlowPaths.h:
  • runtime/JSObject.cpp:

(JSC::JSObject::hasProperty const):
(JSC::JSObject::hasEnumerableProperty const):
(JSC::JSObject::hasPropertyGeneric const): Deleted.

  • runtime/JSObject.h:

Source/WebCore:

Report RuntimeArray indices as Enumerable.

Test: platform/mac/fast/dom/wrapper-classes-objc.html

  • bridge/runtime_array.cpp:

(JSC::RuntimeArray::getOwnPropertySlot):
(JSC::RuntimeArray::getOwnPropertySlotByIndex):

LayoutTests:

  • platform/mac/fast/dom/wrapper-classes-objc-expected.txt:
  • platform/mac/fast/dom/wrapper-classes-objc.html:
Location:
trunk/Source/JavaScriptCore/bytecompiler
Files:
3 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp

    r270870 r270874  
    43664366}
    43674367
    4368 RegisterID* BytecodeGenerator::emitHasGenericProperty(RegisterID* dst, RegisterID* base, RegisterID* propertyName)
    4369 {
    4370     OpHasGenericProperty::emit(this, dst, base, propertyName);
    4371     return dst;
    4372 }
    4373 
    4374 RegisterID* BytecodeGenerator::emitHasIndexedProperty(RegisterID* dst, RegisterID* base, RegisterID* propertyName)
    4375 {
    4376     OpHasIndexedProperty::emit(this, dst, base, propertyName);
    4377     return dst;
    4378 }
    4379 
    4380 RegisterID* BytecodeGenerator::emitHasStructureProperty(RegisterID* dst, RegisterID* base, RegisterID* propertyName, RegisterID* enumerator)
    4381 {
    4382     OpHasStructureProperty::emit(this, dst, base, propertyName, enumerator);
     4368RegisterID* BytecodeGenerator::emitHasEnumerableIndexedProperty(RegisterID* dst, RegisterID* base, RegisterID* propertyName)
     4369{
     4370    OpHasEnumerableIndexedProperty::emit(this, dst, base, propertyName);
     4371    return dst;
     4372}
     4373
     4374RegisterID* BytecodeGenerator::emitHasEnumerableStructureProperty(RegisterID* dst, RegisterID* base, RegisterID* propertyName, RegisterID* enumerator)
     4375{
     4376    OpHasEnumerableStructureProperty::emit(this, dst, base, propertyName, enumerator);
     4377    return dst;
     4378}
     4379
     4380RegisterID* BytecodeGenerator::emitHasEnumerableProperty(RegisterID* dst, RegisterID* base, RegisterID* propertyName)
     4381{
     4382    OpHasEnumerableProperty::emit(this, dst, base, propertyName);
    43834383    return dst;
    43844384}

  • trunk/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h

    r270870 r270874  
    903903        void emitCheckTraps();
    904904
    905         RegisterID* emitHasIndexedProperty(RegisterID* dst, RegisterID* base, RegisterID* propertyName);
    906         RegisterID* emitHasStructureProperty(RegisterID* dst, RegisterID* base, RegisterID* propertyName, RegisterID* enumerator);
     905        RegisterID* emitHasEnumerableIndexedProperty(RegisterID* dst, RegisterID* base, RegisterID* propertyName);
     906        RegisterID* emitHasEnumerableStructureProperty(RegisterID* dst, RegisterID* base, RegisterID* propertyName, RegisterID* enumerator);
     907        RegisterID* emitHasEnumerableProperty(RegisterID* dst, RegisterID* base, RegisterID* propertyName);
    907908        RegisterID* emitHasOwnStructureProperty(RegisterID* dst, RegisterID* base, RegisterID* propertyName, RegisterID* enumerator);
    908         RegisterID* emitHasGenericProperty(RegisterID* dst, RegisterID* base, RegisterID* propertyName);
    909909        RegisterID* emitGetPropertyEnumerator(RegisterID* dst, RegisterID* base);
    910910        RegisterID* emitGetEnumerableLength(RegisterID* dst, RegisterID* base);

  • trunk/Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp

    r269939 r270874  
    39633963        RefPtr<RegisterID> result = generator.emitEqualityOp<OpLess>(generator.newTemporary(), i.get(), length.get());
    39643964        generator.emitJumpIfFalse(result.get(), loopEnd.get());
    3965         generator.emitHasIndexedProperty(result.get(), base.get(), i.get());
     3965        generator.emitHasEnumerableIndexedProperty(result.get(), base.get(), i.get());
    39663966        generator.emitJumpIfFalse(result.get(), *scope->continueTarget());
    39673967
    … …  
    40044004        RefPtr<RegisterID> result = generator.emitIsNull(generator.newTemporary(), propertyName.get());
    40054005        generator.emitJumpIfTrue(result.get(), loopEnd.get());
    4006         generator.emitHasStructureProperty(result.get(), base.get(), propertyName.get(), enumerator.get());
     4006        generator.emitHasEnumerableStructureProperty(result.get(), base.get(), propertyName.get(), enumerator.get());
    40074007        generator.emitJumpIfFalse(result.get(), *scope->continueTarget());
    40084008
    … …  
    40464046        generator.emitJumpIfTrue(result.get(), loopEnd.get());
    40474047
    4048         generator.emitHasGenericProperty(result.get(), base.get(), propertyName.get());
     4048        generator.emitHasEnumerableProperty(result.get(), base.get(), propertyName.get());
    40494049        generator.emitJumpIfFalse(result.get(), *scope->continueTarget());
    40504050
Note: See TracChangeset for help on using the changeset viewer.