Skip to content

Commit c7ecad7

Browse files
shvaikaleshshvaikalesh
committed
super should not depend on __proto__
https://bugs.webkit.org/show_bug.cgi?id=157972 Reviewed by Saam Barati. JSTests: * microbenchmarks/object-get-prototype-of-primitive.js: Added. * stress/class-syntax-derived-default-constructor.js: * stress/get-prototype-of.js: Added. * stress/super-property-access.js: * test262/expectations.yaml: Mark 4 test cases as passing. Source/JavaScriptCore: Before this change, both super() call [1] and super.property [2] relied on Object.prototype.__proto__ to acquire super base, which was observable and incorrect if __proto__ gets removed. This patch introduces get_prototype_of bytecode, ensuring returned values are profiled so the op can be wired to existing DFG and FTL implementations. In order to avoid performance regression w/o DFG (__proto__ is optimized via IntrinsicGetterAccessCase), fast paths for LLInt and baseline JIT are added (64-bit only), utilizing OverridesGetPrototypeOutOfLine type info flag. This change aligns JSC with V8 and SpiderMonkey, progressing microbenchmarks/ super-get-by-{id,val}-with-this-monomorphic.js by 7-10%. SixSpeed is neutral. Also, extracts JSValue::getPrototype() method to avoid code duplication and utilizes it in objectConstructorGetPrototypeOf(), advancing provided microbenchmark by 40%. [1]: https://tc39.es/ecma262/#sec-getsuperconstructor (step 5) [2]: https://tc39.es/ecma262/#sec-getsuperbase (step 5) * builtins/BuiltinNames.h: * bytecode/BytecodeIntrinsicRegistry.h: * bytecode/BytecodeList.rb: * bytecode/BytecodeUseDef.cpp: (JSC::computeUsesForBytecodeIndexImpl): (JSC::computeDefsForBytecodeIndexImpl): * bytecode/CodeBlock.cpp: (JSC::CodeBlock::finishCreation): * bytecode/Opcode.h: * bytecompiler/BytecodeGenerator.cpp: (JSC::BytecodeGenerator::emitGetPrototypeOf): * bytecompiler/BytecodeGenerator.h: * bytecompiler/NodesCodegen.cpp: (JSC::emitSuperBaseForCallee): (JSC::emitGetSuperFunctionForConstruct): (JSC::BytecodeIntrinsicNode::emit_intrinsic_getPrototypeOf): * dfg/DFGAbstractInterpreterInlines.h: (JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects): * dfg/DFGByteCodeParser.cpp: (JSC::DFG::ByteCodeParser::handleIntrinsicGetter): (JSC::DFG::ByteCodeParser::parseBlock): * dfg/DFGCapabilities.cpp: (JSC::DFG::capabilityLevel): * dfg/DFGOperations.cpp: * jit/IntrinsicEmitter.cpp: (JSC::IntrinsicGetterAccessCase::canEmitIntrinsicGetter): * jit/JIT.cpp: (JSC::JIT::privateCompileMainPass): (JSC::JIT::privateCompileSlowCases): * jit/JIT.h: * jit/JITOpcodes.cpp: (JSC::JIT::emit_op_get_prototype_of): * llint/LowLevelInterpreter.asm: * llint/LowLevelInterpreter64.asm: * runtime/CommonSlowPaths.cpp: (JSC::SLOW_PATH_DECL): * runtime/CommonSlowPaths.h: * runtime/JSCJSValue.h: * runtime/JSCJSValueInlines.h: (JSC::JSValue::getPrototype const): * runtime/JSGlobalObjectFunctions.cpp: (JSC::globalFuncProtoGetter): * runtime/JSObject.cpp: (JSC::JSObject::calculatedClassName): * runtime/JSObject.h: (JSC::JSObject::getPrototype): * runtime/JSObjectInlines.h: (JSC::JSObject::canPerformFastPutInlineExcludingProto): (JSC::JSObject::getPropertySlot): (JSC::JSObject::getNonIndexPropertySlot): * runtime/JSProxy.h: * runtime/JSTypeInfo.h: (JSC::TypeInfo::overridesGetPrototype const): * runtime/ObjectConstructor.cpp: (JSC::objectConstructorGetPrototypeOf): * runtime/ProxyObject.h: * runtime/Structure.h: * runtime/Structure.cpp: (JSC::Structure::validateFlags): Source/WebCore: No new tests, no behavior change. * bindings/scripts/CodeGeneratorJS.pm: (GenerateHeader): Set OverridesGetPrototype structure flag for CustomGetPrototype IDL attribute. Canonical link: https://commits.webkit.org/225973@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@263035 268f45cc-cd09-0410-ab3c-d52691b4dbfc
1 parent a9cd8be commit c7ecad7

44 files changed

Lines changed: 352 additions & 76 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎JSTests/ChangeLog‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,16 @@
1+
2020-06-15 Alexey Shvayka <shvaikalesh@gmail.com>
2+
3+
super should not depend on __proto__
4+
https://bugs.webkit.org/show_bug.cgi?id=157972
5+
6+
Reviewed by Saam Barati.
7+
8+
* microbenchmarks/object-get-prototype-of-primitive.js: Added.
9+
* stress/class-syntax-derived-default-constructor.js:
10+
* stress/get-prototype-of.js: Added.
11+
* stress/super-property-access.js:
12+
* test262/expectations.yaml: Mark 4 test cases as passing.
13+
114
2020-06-14 Yusuke Suzuki <ysuzuki@apple.com>
215

316
Add wasm regresion test for loop

‎JSTests/microbenchmarks/object-get-prototype-of-primitive.js‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
const sym = Symbol();
2+
function test() {
3+
Object.getPrototypeOf(false);
4+
Object.getPrototypeOf(-1);
5+
Object.getPrototypeOf("");
6+
Object.getPrototypeOf(sym);
7+
Object.getPrototypeOf(3n);
8+
}
9+
noInline(test);
10+
11+
for (let i = 0; i < 1e7; ++i)
12+
test();

‎JSTests/stress/class-syntax-derived-default-constructor.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
Object.defineProperty(Object.prototype, "__proto__", {get: undefined});
12

23
var A = class A { };
34
var B = class B extends A { };

‎JSTests/stress/get-prototype-of.js‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
function assert(x) {
2+
if (!x)
3+
throw Error("Bad");
4+
}
5+
6+
function shouldThrow(func, errorMessage) {
7+
var errorThrown = false;
8+
var error;
9+
try {
10+
func();
11+
} catch (e) {
12+
errorThrown = true;
13+
error = e;
14+
}
15+
if (!errorThrown)
16+
throw new Error("Not thrown");
17+
if (String(error) !== errorMessage)
18+
throw new Error(`Bad error: ${error}`);
19+
}
20+
21+
function makePolyProtoObject() {
22+
function foo() {
23+
class C {
24+
constructor() {
25+
this._field = 42;
26+
}
27+
}
28+
return new C();
29+
}
30+
for (let i = 0; i < 1000; ++i)
31+
foo();
32+
return foo();
33+
}
34+
35+
const getPrototypeOf = $vm.createBuiltin("(function (v) { return @getPrototypeOf(v); })");
36+
noInline(getPrototypeOf);
37+
38+
const proxyPrototype = {};
39+
const proxy = new Proxy({}, {
40+
getPrototypeOf: () => proxyPrototype,
41+
});
42+
43+
const polyProtoObject = makePolyProtoObject();
44+
45+
for (let i = 0; i < 1e4; ++i) {
46+
shouldThrow(() => getPrototypeOf(undefined), "TypeError: undefined is not an object");
47+
shouldThrow(() => getPrototypeOf(null), "TypeError: null is not an object");
48+
49+
assert(getPrototypeOf(true) === Boolean.prototype);
50+
assert(getPrototypeOf(1) === Number.prototype);
51+
assert(getPrototypeOf("foo") === String.prototype);
52+
assert(getPrototypeOf(Symbol()) === Symbol.prototype);
53+
assert(getPrototypeOf(10n) === BigInt.prototype);
54+
55+
assert(getPrototypeOf({}) === Object.prototype);
56+
assert(getPrototypeOf([]) === Array.prototype);
57+
assert(getPrototypeOf(() => {}) === Function.prototype);
58+
59+
assert(getPrototypeOf(Object.prototype) === null);
60+
assert(getPrototypeOf(Object.create(null)) === null);
61+
62+
assert(getPrototypeOf(proxy) === proxyPrototype);
63+
assert(getPrototypeOf(polyProtoObject) === polyProtoObject.constructor.prototype);
64+
}

‎JSTests/stress/super-property-access.js‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
Object.defineProperty(Object.prototype, "__proto__", {get: undefined});
2+
13
function assert(b, m = "Bad!") {
24
if (!b) {
35
throw new Error(m);

‎JSTests/test262/expectations.yaml‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2975,12 +2975,6 @@ test/language/expressions/prefix-increment/S11.4.4_A5_T5.js:
29752975
test/language/expressions/prefix-increment/S11.4.4_A6_T3.js:
29762976
default: 'Test262Error: Expected true but got false'
29772977
strict mode: 'Test262Error: Expected true but got false'
2978-
test/language/expressions/super/call-poisoned-underscore-proto.js:
2979-
default: 'Test262Error: should not be called'
2980-
strict mode: 'Test262Error: should not be called'
2981-
test/language/expressions/super/prop-poisoned-underscore-proto.js:
2982-
default: 'Test262Error: should not be called'
2983-
strict mode: 'Test262Error: should not be called'
29842978
test/language/expressions/yield/star-iterable.js:
29852979
default: 'Test262Error: First result `done` flag Expected SameValue(«false», «undefined») to be true'
29862980
strict mode: 'Test262Error: First result `done` flag Expected SameValue(«false», «undefined») to be true'

‎Source/JavaScriptCore/ChangeLog‎

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,90 @@
1+
2020-06-15 Alexey Shvayka <shvaikalesh@gmail.com>
2+
3+
super should not depend on __proto__
4+
https://bugs.webkit.org/show_bug.cgi?id=157972
5+
6+
Reviewed by Saam Barati.
7+
8+
Before this change, both super() call [1] and super.property [2] relied on
9+
Object.prototype.__proto__ to acquire super base, which was observable and
10+
incorrect if __proto__ gets removed.
11+
12+
This patch introduces get_prototype_of bytecode, ensuring returned values
13+
are profiled so the op can be wired to existing DFG and FTL implementations.
14+
In order to avoid performance regression w/o DFG (__proto__ is optimized via
15+
IntrinsicGetterAccessCase), fast paths for LLInt and baseline JIT are added
16+
(64-bit only), utilizing OverridesGetPrototypeOutOfLine type info flag.
17+
18+
This change aligns JSC with V8 and SpiderMonkey, progressing microbenchmarks/
19+
super-get-by-{id,val}-with-this-monomorphic.js by 7-10%. SixSpeed is neutral.
20+
21+
Also, extracts JSValue::getPrototype() method to avoid code duplication and
22+
utilizes it in objectConstructorGetPrototypeOf(), advancing provided
23+
microbenchmark by 40%.
24+
25+
[1]: https://tc39.es/ecma262/#sec-getsuperconstructor (step 5)
26+
[2]: https://tc39.es/ecma262/#sec-getsuperbase (step 5)
27+
28+
* builtins/BuiltinNames.h:
29+
* bytecode/BytecodeIntrinsicRegistry.h:
30+
* bytecode/BytecodeList.rb:
31+
* bytecode/BytecodeUseDef.cpp:
32+
(JSC::computeUsesForBytecodeIndexImpl):
33+
(JSC::computeDefsForBytecodeIndexImpl):
34+
* bytecode/CodeBlock.cpp:
35+
(JSC::CodeBlock::finishCreation):
36+
* bytecode/Opcode.h:
37+
* bytecompiler/BytecodeGenerator.cpp:
38+
(JSC::BytecodeGenerator::emitGetPrototypeOf):
39+
* bytecompiler/BytecodeGenerator.h:
40+
* bytecompiler/NodesCodegen.cpp:
41+
(JSC::emitSuperBaseForCallee):
42+
(JSC::emitGetSuperFunctionForConstruct):
43+
(JSC::BytecodeIntrinsicNode::emit_intrinsic_getPrototypeOf):
44+
* dfg/DFGAbstractInterpreterInlines.h:
45+
(JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
46+
* dfg/DFGByteCodeParser.cpp:
47+
(JSC::DFG::ByteCodeParser::handleIntrinsicGetter):
48+
(JSC::DFG::ByteCodeParser::parseBlock):
49+
* dfg/DFGCapabilities.cpp:
50+
(JSC::DFG::capabilityLevel):
51+
* dfg/DFGOperations.cpp:
52+
* jit/IntrinsicEmitter.cpp:
53+
(JSC::IntrinsicGetterAccessCase::canEmitIntrinsicGetter):
54+
* jit/JIT.cpp:
55+
(JSC::JIT::privateCompileMainPass):
56+
(JSC::JIT::privateCompileSlowCases):
57+
* jit/JIT.h:
58+
* jit/JITOpcodes.cpp:
59+
(JSC::JIT::emit_op_get_prototype_of):
60+
* llint/LowLevelInterpreter.asm:
61+
* llint/LowLevelInterpreter64.asm:
62+
* runtime/CommonSlowPaths.cpp:
63+
(JSC::SLOW_PATH_DECL):
64+
* runtime/CommonSlowPaths.h:
65+
* runtime/JSCJSValue.h:
66+
* runtime/JSCJSValueInlines.h:
67+
(JSC::JSValue::getPrototype const):
68+
* runtime/JSGlobalObjectFunctions.cpp:
69+
(JSC::globalFuncProtoGetter):
70+
* runtime/JSObject.cpp:
71+
(JSC::JSObject::calculatedClassName):
72+
* runtime/JSObject.h:
73+
(JSC::JSObject::getPrototype):
74+
* runtime/JSObjectInlines.h:
75+
(JSC::JSObject::canPerformFastPutInlineExcludingProto):
76+
(JSC::JSObject::getPropertySlot):
77+
(JSC::JSObject::getNonIndexPropertySlot):
78+
* runtime/JSProxy.h:
79+
* runtime/JSTypeInfo.h:
80+
(JSC::TypeInfo::overridesGetPrototype const):
81+
* runtime/ObjectConstructor.cpp:
82+
(JSC::objectConstructorGetPrototypeOf):
83+
* runtime/ProxyObject.h:
84+
* runtime/Structure.h:
85+
* runtime/Structure.cpp:
86+
(JSC::Structure::validateFlags):
87+
188
2020-06-13 Devin Rousso <drousso@apple.com>
289

390
Make `errors` an own property of `AggregateError` instead of a prototype accessor

‎Source/JavaScriptCore/builtins/BuiltinNames.h‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,6 @@ namespace JSC {
6565
macro(create) \
6666
macro(defineProperty) \
6767
macro(defaultPromiseThen) \
68-
macro(getPrototypeOf) \
6968
macro(getOwnPropertyNames) \
7069
macro(ownKeys) \
7170
macro(Set) \

‎Source/JavaScriptCore/bytecode/BytecodeIntrinsicRegistry.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ enum class LinkTimeConstant : int32_t;
4242
macro(argumentCount) \
4343
macro(getByIdDirect) \
4444
macro(getByIdDirectPrivate) \
45+
macro(getPrototypeOf) \
4546
macro(getPromiseInternalField) \
4647
macro(getGeneratorInternalField) \
4748
macro(getAsyncGeneratorInternalField) \

‎Source/JavaScriptCore/bytecode/BytecodeList.rb‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -498,6 +498,15 @@
498498
offset: unsigned,
499499
}
500500

501+
op :get_prototype_of,
502+
args: {
503+
dst: VirtualRegister,
504+
value: VirtualRegister,
505+
},
506+
metadata: {
507+
profile: ValueProfile,
508+
}
509+
501510
op :try_get_by_id,
502511
args: {
503512
dst: VirtualRegister,

0 commit comments

Comments
 (0)