Skip to content

Commit b1cb079

Browse files
boingoingboingoing
committed
Class members which capture super via dot (super.constructor) end up getting the wrong value
We were using the wrong bytecode register when emitting LdHomeObjProto in the case of a lambda capturing super via a dot reference. Consider this Javascript: ```javascript class A { } class B extends A { foo() { const _s = () => super.constructor; console.log(_s().name); } } ``` The member function foo correctly stashes this and super but the lambda needs to load them from the environment and it was loading them into the same register. Here's the incorrect bytecode: ``` Function _s ( (#1.5), #6) (In0) (size: 8 [8]) 5 locals (2 temps from R3), 1 inline cache Constant Table: ======== ===== R1 LdRoot Line 13: super.constructor Col 26: ^ 0000 ProfiledLdEnvSlot R3 = [1][2] <0> 0006 ProfiledLdEnvSlot R3 = [1][3] <1> 000c LdHomeObjProto R4 R3 0011 ProfiledLdSuperFld R0 = R4(this=R3). #0 <0> 0019 Br x:001e ( 2) 001c LdUndef R0 ``` With the fix in this PR, here's the bytecode we get for _s: ``` Function _s ( (#1.5), #6) (In0) (size: 8 [8]) 6 locals (3 temps from R3), 1 inline cache Constant Table: ======== ===== R1 LdRoot Line 56: super.constructor Col 26: ^ 0000 ProfiledLdEnvSlot R3 = [1][2] <0> 0006 ProfiledLdEnvSlot R4 = [1][3] <1> 000c LdHomeObjProto R5 R3 0011 ProfiledLdSuperFld R0 = R5(this=R4). #0 <0> 0019 Br x:001e ( 2) 001c LdUndef R0 ```
1 parent 9c289b8 commit b1cb079

File tree

3 files changed

+105
-24
lines changed

3 files changed

+105
-24
lines changed

‎lib/Runtime/ByteCode/ByteCodeEmitter.cpp

Lines changed: 17 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -10833,44 +10833,37 @@ void Emit(ParseNode *pnode, ByteCodeGenerator *byteCodeGenerator, FuncInfo *func
1083310833
case knopDot:
1083410834
{
1083510835
Emit(pnode->AsParseNodeBin()->pnode1, byteCodeGenerator, funcInfo, false);
10836-
funcInfo->ReleaseLoc(pnode->AsParseNodeBin()->pnode1);
10837-
funcInfo->AcquireLoc(pnode);
10838-
Js::PropertyId propertyId = pnode->AsParseNodeBin()->pnode2->AsParseNodeName()->PropertyIdFromNameNode();
1083910836

1084010837
Js::RegSlot callObjLocation = pnode->AsParseNodeBin()->pnode1->location;
1084110838
Js::RegSlot protoLocation = callObjLocation;
1084210839

10840+
if (ByteCodeGenerator::IsSuper(pnode->AsParseNodeBin()->pnode1))
10841+
{
10842+
Emit(pnode->AsParseNodeSuperReference()->pnodeThis, byteCodeGenerator, funcInfo, false);
10843+
protoLocation = byteCodeGenerator->EmitLdObjProto(Js::OpCode::LdHomeObjProto, callObjLocation, funcInfo);
10844+
funcInfo->ReleaseLoc(pnode->AsParseNodeSuperReference()->pnodeThis);
10845+
}
10846+
10847+
funcInfo->ReleaseLoc(pnode->AsParseNodeBin()->pnode1);
10848+
funcInfo->AcquireLoc(pnode);
10849+
Js::PropertyId propertyId = pnode->AsParseNodeBin()->pnode2->AsParseNodeName()->PropertyIdFromNameNode();
10850+
uint cacheId = funcInfo->FindOrAddInlineCacheId(protoLocation, propertyId, false, false);
10851+
1084310852
if (propertyId == Js::PropertyIds::length)
1084410853
{
10845-
uint cacheId = funcInfo->FindOrAddInlineCacheId(protoLocation, propertyId, false, false);
1084610854
byteCodeGenerator->Writer()->PatchableProperty(Js::OpCode::LdLen_A, pnode->location, protoLocation, cacheId);
1084710855
}
1084810856
else if (pnode->IsCallApplyTargetLoad())
1084910857
{
10850-
if (ByteCodeGenerator::IsSuper(pnode->AsParseNodeBin()->pnode1))
10851-
{
10852-
Emit(pnode->AsParseNodeSuperReference()->pnodeThis, byteCodeGenerator, funcInfo, false);
10853-
protoLocation = byteCodeGenerator->EmitLdObjProto(Js::OpCode::LdHomeObjProto, callObjLocation, funcInfo);
10854-
funcInfo->ReleaseLoc(pnode->AsParseNodeSuperReference()->pnodeThis);
10855-
}
10856-
uint cacheId = funcInfo->FindOrAddInlineCacheId(protoLocation, propertyId, false, false);
1085710858
byteCodeGenerator->Writer()->PatchableProperty(Js::OpCode::LdFldForCallApplyTarget, pnode->location, protoLocation, cacheId);
1085810859
}
10860+
else if (ByteCodeGenerator::IsSuper(pnode->AsParseNodeBin()->pnode1))
10861+
{
10862+
byteCodeGenerator->Writer()->PatchablePropertyWithThisPtr(Js::OpCode::LdSuperFld, pnode->location, protoLocation, pnode->AsParseNodeSuperReference()->pnodeThis->location, cacheId, isConstructorCall);
10863+
}
1085910864
else
1086010865
{
10861-
if (ByteCodeGenerator::IsSuper(pnode->AsParseNodeBin()->pnode1))
10862-
{
10863-
Emit(pnode->AsParseNodeSuperReference()->pnodeThis, byteCodeGenerator, funcInfo, false);
10864-
protoLocation = byteCodeGenerator->EmitLdObjProto(Js::OpCode::LdHomeObjProto, callObjLocation, funcInfo);
10865-
funcInfo->ReleaseLoc(pnode->AsParseNodeSuperReference()->pnodeThis);
10866-
uint cacheId = funcInfo->FindOrAddInlineCacheId(protoLocation, propertyId, false, false);
10867-
byteCodeGenerator->Writer()->PatchablePropertyWithThisPtr(Js::OpCode::LdSuperFld, pnode->location, protoLocation, pnode->AsParseNodeSuperReference()->pnodeThis->location, cacheId, isConstructorCall);
10868-
}
10869-
else
10870-
{
10871-
uint cacheId = funcInfo->FindOrAddInlineCacheId(callObjLocation, propertyId, false, false);
10872-
byteCodeGenerator->Writer()->PatchableProperty(Js::OpCode::LdFld, pnode->location, callObjLocation, cacheId, isConstructorCall);
10873-
}
10866+
byteCodeGenerator->Writer()->PatchableProperty(Js::OpCode::LdFld, pnode->location, callObjLocation, cacheId, isConstructorCall);
1087410867
}
1087510868

1087610869
break;

‎test/es6/rlexe.xml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1505,4 +1505,10 @@
15051505
<compile-flags>-args summary -endargs</compile-flags>
15061506
</default>
15071507
</test>
1508+
<test>
1509+
<default>
1510+
<files>super_bugs.js</files>
1511+
<compile-flags>-args summary -endargs</compile-flags>
1512+
</default>
1513+
</test>
15081514
</regress-exe>

‎test/es6/super_bugs.js

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
//-------------------------------------------------------------------------------------------------------
2+
// Copyright (C) Microsoft. All rights reserved.
3+
// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
4+
//-------------------------------------------------------------------------------------------------------
5+
6+
WScript.LoadScriptFile("..\\UnitTestFramework\\UnitTestFramework.js");
7+
8+
var tests = [
9+
{
10+
name: "Class member captures super as index reference via a lambda",
11+
body: function ()
12+
{
13+
class Base {}
14+
class Derived extends Base {
15+
test() {
16+
const _s = name => super[name];
17+
return _s('constructor').name;
18+
}
19+
}
20+
21+
assert.areEqual("Base", new Derived().test());
22+
}
23+
},
24+
{
25+
name: "Class member captures super as dot reference via a lambda",
26+
body: function ()
27+
{
28+
class Base {}
29+
class Derived extends Base {
30+
test() {
31+
const _s = () => super.constructor;
32+
return _s().name;
33+
}
34+
}
35+
36+
assert.areEqual("Base", new Derived().test());
37+
}
38+
},
39+
{
40+
name: "Class member captures super as dot reference via object with a getter",
41+
body: function ()
42+
{
43+
class Base {}
44+
class Derived extends Base {
45+
test() {
46+
const _super = Object.create(null, {
47+
constructor: {
48+
get: () => super.constructor
49+
}
50+
});
51+
52+
return _super.constructor.name;
53+
}
54+
}
55+
56+
assert.areEqual("Base", new Derived().test());
57+
}
58+
},
59+
{
60+
name: "Class member captures super in lambda via getter from outer super reference",
61+
body: function ()
62+
{
63+
class Base {}
64+
class Derived extends Base {
65+
test() {
66+
const con = super.constructor;
67+
const prop2 = {
68+
constructor: {
69+
get: () => con
70+
}
71+
};
72+
const _super2 = Object.create(null, prop2);
73+
return _super2.constructor.name;
74+
}
75+
}
76+
77+
assert.areEqual("Base", new Derived().test());
78+
}
79+
},
80+
];
81+
82+
testRunner.runTests(tests, { verbose: WScript.Arguments[0] != "summary" });

0 commit comments

Comments
 (0)