New issue
Advanced search Search tips

Issue 653407 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 1
Type: Bug



Sign in to add a comment

'this is undefined' in a subclass super call.

Reported by aari...@gmail.com, Oct 6 2016

Issue description

UserAgent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/55.0.2881.5 Safari/537.36

Steps to reproduce the problem:
1. Open the JS console.
2. Enter:
class superClass { 
  constructor () {} 
}
class subClass extends superClass { 
  constructor () { 
    super(); 
  } 
}
for (var i = 0; i < 100000; i++) 
  new subClass();

What is the expected behavior?
No errors.

What went wrong?
When the subclass calls super() it errors 'this is undefined'.

Did this work before? Yes Works in the stable branch currently, besides that, not sure when in Canary specifically this stopped working.

Chrome version: 55.0.2881.5  Channel: canary
OS Version: 10.0
Flash Version: 

I'm not sure why this happens--nothing should be changing. I assume it's an internal issue, potentially with memory/object counts. On my computer, it happens when 'i' reaches 11569 every single time, in this example, but in my project where I found this bug, it's a much lower object count.
 
ss+(2016-10-06+at+01.39.53).png
3.7 KB View Download
Components: -Blink Blink>JavaScript
I cannot reproduce this but sending to the v8 team for investigation.

Comment 2 by aari...@gmail.com, Oct 7 2016

Thanks. An update to my initial post, the object count in my own project is consistent as well, it crashes on the 11th construction every time.

I did find a workaround (unfortunately, it's both ugly and probably much less efficient):

new (subclass.bind({}))(<...args>);

This fixes it by providing it an empty object each time it's created, so it never has a chance to have an undefined 'this'.

Let me know if there's anything I can do to help reproduce.
I can reproduce in Canary.
Cc: adamk@chromium.org
Components: -Blink>JavaScript Blink>JavaScript>Language
Labels: -Pri-2 Proj-Ignition Pri-1
Owner: rmcilroy@chromium.org
Status: Assigned (was: Unconfirmed)
I can reproduce this with Ignition turned on. With Ignition turned off, the error does not reproduce.
Cc: mvstan...@chromium.org machenb...@chromium.org
machenbach/mvstanton: A good case for a correctness fuzzer btw.
Cc: bmeu...@chromium.org jarin@chromium.org
Looks like deoptimizer?
Cc: rmcilroy@chromium.org leszeks@chromium.org mythria@chromium.org
Owner: ----
Status: Available (was: Assigned)
I'm going to be OOO for two weeks after today and don't have time to look into this before I leave. Adding Mythi and Leszek to have a look. 

Comment 8 by adamk@chromium.org, Oct 7 2016

Components: -Blink>JavaScript>Language Blink>JavaScript>Compiler
For those reproducing, this only hits with --ignition-staging (not --ignition), since it's OSR.
Owner: mythria@chromium.org
Status: Started (was: Available)

Comment 10 by mythria@google.com, Oct 17 2016

I can also reproduce this with --ignition --turbo with a slightly modified code to trigger the optimization from ignition to turbofan. Still investigating the problem.

class superClass { 
  constructor () {} 
}
class subClass extends superClass { 
  constructor () { 
    super(); 
  } 
}

f() {
 new subClass();
}

f(); // We need this to collect feedback. Otherwise it doesn't trigger the problem.
%OptimizeFunctionOnNextCall(f)
f();

Comment 11 by mythria@google.com, Oct 17 2016

When we don't collect feedback for f(), SubClass doesn't get inlined into f(), so we don't see this problem. Only optimizing SubClass is not a problem too. It could be related to a bug in BytecodeGraphBuilder when combined with inlining.
Project Member

Comment 12 by bugdroid1@chromium.org, Oct 18 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/cad36659b1dc230773eb6d2a985fa8b9a12a4d86

commit cad36659b1dc230773eb6d2a985fa8b9a12a4d86
Author: mythria <mythria@chromium.org>
Date: Tue Oct 18 11:47:35 2016

[turbofan] When inlining JSCallConstruct receiver should be set to the hole.

When inlining JSCallConstruct in turbofan, receiver is initialized to model
the behaviour of constructor. When an implicit receiver is not required the
receiver value should be set to the hole value instead of undefined value.
When initializing the receiver via super calls, we check that the receiver
is the hole value.

BUG= chromium:653407 

Review-Url: https://codereview.chromium.org/2424123002
Cr-Commit-Position: refs/heads/master@{#40396}

[modify] https://crrev.com/cad36659b1dc230773eb6d2a985fa8b9a12a4d86/src/compiler/js-inlining.cc
[add] https://crrev.com/cad36659b1dc230773eb6d2a985fa8b9a12a4d86/test/mjsunit/regress/regress-653407.js

Status: Fixed (was: Started)
Both the original and modified test cases work for me locally with this patch. Marking it as fixed. Please re-open if it does not fix the original issue.

Sign in to add a comment