ES6 constructor returns class instead of object
eebbing opened this issue · 10 comments
The code example below should log the numbers from 0 up to 500. Instead, the code breaks after about 100 iterations. The issue is that after about 100 iterations the constructor of class B returns the class itself instead of an instance of B.
The fact that this issue only occurs after multiple iterations suggests to me that the bug is somewhere in the JIT engine.
By the way, the error disappears if the method body of A.toB() is replaced by return new B(this);.
I tested the following code in Microsoft Edge 38.14393.0.0 (Microsoft EdgeHTML 14.14393)
var Test = {};
class A {
constructor(foo) { this.foo = foo; }
toB() { return new Test.B(this); }
}
class B {
constructor(bar) { this.bar = bar; }
}
Test.B = B;
for (let i=0; i<500; i++)
{
const a = new A(i);
const b = a.toB();
try
{
console.log(b.bar.foo);
}
catch (e)
{
console.log(e); // e.description -> "Unable to get property 'foo' of undefined or null reference"
console.log(b); // b -> class B { ... }
break;
}
}
For what it's worth, I decided to look a little bit further into it, and was able to reproduce the error with the latest source code. I ran the script below via ch.exe (x64 debug build) with the following arguments:
ch.exe -forceNative -MaxinterpretCount:10 -MaxSimpleJITRunCount:10 E:\path-to\test.js
The script is:
var Test = {};
class A {
constructor(foo) { this.foo = foo; }
toB() { return new Test.B(this); }
}
class B {
constructor(bar) { this.bar = bar; }
}
Test.B = B;
for (let i=0; i<20; i++)
{
const a = new A(i);
const b = a.toB();
try
{
WScript.Echo(b.bar.foo);
}
catch (e)
{
WScript.Echo(e);
WScript.Echo(b);
break;
}
}
The output of the script is:
0
1
2
3
4
5
6
7
8
9
TypeError: Unable to get property 'foo' of undefined or null reference
class B {
constructor(bar) { this.bar = bar; }
}
Awesome! Thanks @eebbing, having a simple repro that we can run with ch.exe makes it a lot easier to debug. We'll look into this soon.
not sure if this helps (hopefully, it might), but this testcase can be further simplified by running ../../Build/VcBuild/bin/x86_debug/ch.exe -trace:backend -off:backend:1,3 -dump:irbuilder -dump:lowerer -off:inline -off:simplejit -mic:3 tt.js
to only compile Function A.prototype.toB ( (#1.2), #3)
s9.var = StartCall 2 (0x2).i32 #000f
arg2(s10)<4>.var = ArgOut_A s2[LikelyCanBeTaggedValue_Object].var!, s9.var! #0012
s11<s15>[UninitializedObject].var = NewScObjectNoCtor 0xXXXXXXXX (FunctionObject [B (#1.3), #4]).var #0015 Bailout: #0015 (BailOutFailedCtorGuardCheck)
arg1(s12)<0>.var = ArgOut_A s11<s15>[UninitializedObject].var, arg2(s10)<4>.var! #0015
s0<s16>[UninitializedObject].var = CallIFixed 0xXXXXXXXX (FunctionObject [B (#1.3), #4]).var, arg1(s12)<0>.var! #0015
NewScObjectNoCtor (which relies on JavascriptOperators::NewScObjectCommon) might return the c-tor function back in the case below (this might be why func B is observed instead of an object)
if (functionInfo->IsClassConstructor() && !isBaseClassConstructorNewScObject)
{
Both Inliner and Lower use returnNewScObj flag to decide which object needs to be returned: the one returned by NewScObjectNoCtor or s0<s16>[UninitializedObject].var = CallIFixed 0xXXXXXXXX.
In this case, s0<s16>[UninitializedObject].var = CallIFixed 0xXXXXXXXX seems to construct the correct object (via NewScObjectNoCtorFull) but we still return s11?
IR after GlobOpt
-----------------------------------------------------------------------------
************ IR after GlobOpt (FullJit) ************
-----------------------------------------------------------------------------
Function A.prototype.toB ( (#1.2), #3) Instr Count:18
FunctionEntry #
BLOCK 0: Out(1)
$L3: #
s1[Object].var = Ld_A 0xXXXXXXXX (GlobalObject)[Object].var #
s2.var = ArgIn_A prm1<16>.var! #0000
s2[LikelyCanBeTaggedValue_Object].var = StrictLdThis s2.var! #0002
Line 5: return new Test.B(this); }
Col 13: ^
StatementBoundary #0 #0005
s3<s13>[LikelyCanBeTaggedValue_Object].var = LdRootFld s6(s1[Object]->Test)<1,m,~-,s?,s?>[LikelyCanBeTaggedValue_Object].var! #0005
CheckFixedFld s7(s3<s13>[LikelyCanBeTaggedValue_Object]->B)<0,m=,++,s13!,s14+,{B(0)=}>.var! #000b Bailout: #000b (BailOutFailedFixedFieldTypeCheck)
s9.var = StartCall 2 (0x2).i32 #000f
arg2(s10)<4>.var = ArgOut_A s2[LikelyCanBeTaggedValue_Object].var!, s9.var! #0012
s11<s15>[UninitializedObject].var = NewScObjectNoCtor 0xXXXXXXXX (FunctionObject [B (#1.3), #4]).var #0015 Bailout: #0015 (BailOutFailedCtorGuardCheck)
arg1(s12)<0>.var = ArgOut_A s11<s15>[UninitializedObject].var, arg2(s10)<4>.var! #0015
s0<s16>[UninitializedObject].var = CallIFixed 0xXXXXXXXX (FunctionObject [B (#1.3), #4]).var, arg1(s12)<0>.var! #0015
Line 5: }
Col 38: ^
StatementBoundary #1 #0024
StatementBoundary #-1 #0024
Ret s11<s15>[UninitializedObject].var! #0024
BLOCK 1: In(0)
@eebbing @ianwjhalliday @Krovatkin thanks for participating - could you help review the PR pls?
Fix is merged.
As of today with the current Fast Ring version of Edge (40.15025.1000), I am still seeing this issue, though the behavior changed slightly: In my test-case the problem reproduces as soon as I open the developer tools. It does not happen anymore when the tools stay closed. Once they are open, the constructor does not return the instance but the class. Is this expected? Is there a different version of Chakra running once the dev tools are open?
Tried both (long and short) test cases above as JS code in HTML files, opened through Node http-server in Edge 40.15026.1000.0 with F12 opened. Couldn't repro. Do you have a repro you can share? Thanks!
I'm a colleague of @yGuy . We have prepared a repro that works against our public URLs: https://gist.github.com/michabaur/2a627a3e7a5703dbd682b6ef3274e5dc. Just extract the two files, open test.html, and then open the dev tools. For us, this breaks after a few seconds in Edge 40.15025.1000
@michabaur @yGuy thanks for the test case.
Fix is merged in release/1.4
What Edge version/release will contain this fix?