Bug 1473272: Don't treat classes with default constructors as self-hosted code. r=tcampbell
☠☠ backed out by 8229a612b15e ☠ ☠
authorJim Blandy <jimb@mozilla.com>
Sat, 28 Jul 2018 20:26:46 -0700
changeset 432438 2d615f488d9f
parent 432437 ed3c662bb21d
child 432439 3a51308cf76a
push id106738
push usertcampbell@mozilla.com
push dateMon, 20 Aug 2018 18:57:09 +0000
treeherdermozilla-inbound@2d615f488d9f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstcampbell
bugs1473272
milestone63.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1473272: Don't treat classes with default constructors as self-hosted code. r=tcampbell
js/src/builtin/Classes.js
js/src/jit-test/tests/class/bug1473272-default-constructors.js
js/src/jit-test/tests/debug/class-default-constructor-01.js
js/src/vm/Interpreter.cpp
js/src/vm/JSScript.cpp
js/src/vm/JSScript.h
--- a/js/src/builtin/Classes.js
+++ b/js/src/builtin/Classes.js
@@ -1,17 +1,15 @@
 // Give a builtin constructor that we can use as the default. When we give
 // it to our newly made class, we will be sure to set it up with the correct name
 // and .prototype, so that everything works properly.
 
+// The template classes should have all their code on a single line, so that
+// bytecodes' line offsets within the script don't contribute spurious offsets
+// when we transplant them into the real class definition. Punting on column numbers.
+
 var DefaultDerivedClassConstructor =
-    class extends null {
-        constructor(...args) {
-            super(...allowContentIter(args));
-        }
-    };
+    class extends null { constructor(...args) { super(...allowContentIter(args)); } };
 MakeDefaultConstructor(DefaultDerivedClassConstructor);
 
 var DefaultBaseClassConstructor =
-    class {
-        constructor() { }
-    };
+    class { constructor() { } };
 MakeDefaultConstructor(DefaultBaseClassConstructor);
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/class/bug1473272-default-constructors.js
@@ -0,0 +1,20 @@
+// Test the source location info in a derived-class default constructor.
+
+function W() { test(); }
+class Z extends W {}  // line 4
+class Y extends Z {}  // line 5
+
+class X extends Y {}  // line 7
+
+function test() {
+    for (let line of new Error().stack.split('\n')) {
+        if (line.startsWith("Z@"))
+            assertEq(line.split(":")[1], "4");
+        if (line.startsWith("Y@"))
+            assertEq(line.split(":")[1], "5");
+        if (line.startsWith("X@"))
+            assertEq(line.split(":")[1], "7");
+    }
+}
+
+new X;
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/debug/class-default-constructor-01.js
@@ -0,0 +1,34 @@
+// We should be able to retrieve the script of a class's default constructor.
+
+var g = newGlobal();
+var dbg = new Debugger;
+var gDO = dbg.addDebuggee(g);
+
+// Class definitions go in the global's lexical environment, so we can't use
+// getOwnPropertyDescriptor or g.X to retrieve their constructor.
+//
+// Derived clasess use a different script from the self-hosted compartment, so
+// check those too.
+gDO.executeInGlobal(`   // 1729
+  class X {};           // 1730
+                        // 1731
+                        // 1732
+  class Y extends X {}; // 1733
+`, { lineNumber: 1729 });
+
+function check(name, text, startLine) {
+  print(`checking ${name}`);
+  var desc = gDO.executeInGlobal(name).return;
+  assertEq(desc.class, 'Function');
+  assertEq(desc.name, name);
+  var script = desc.script;
+  assertEq(script instanceof Debugger.Script, true,
+           "default constructor's script should be available");
+  assertEq(script.startLine, startLine,
+           "default constructor's starting line should be set");
+  var source = script.source;
+  assertEq(source.text.substr(script.sourceStart, script.sourceLength), text);
+}
+
+check('X', 'class X {}', 1730);
+check('Y', 'class Y extends X {}', 1733);
--- a/js/src/vm/Interpreter.cpp
+++ b/js/src/vm/Interpreter.cpp
@@ -303,18 +303,21 @@ js::MakeDefaultConstructor(JSContext* cx
     // Create the script now, as the source span needs to be overridden for
     // toString. Calling toString on a class constructor must not return the
     // source for just the constructor function.
     JSScript *ctorScript = JSFunction::getOrCreateScript(cx, ctor);
     if (!ctorScript)
         return nullptr;
     uint32_t classStartOffset = GetSrcNoteOffset(classNote, 0);
     uint32_t classEndOffset = GetSrcNoteOffset(classNote, 1);
-    ctorScript->setDefaultClassConstructorSpan(script->sourceObject(), classStartOffset,
-                                               classEndOffset);
+    unsigned column;
+    unsigned line = PCToLineNumber(script, pc, &column);
+    ctorScript->setDefaultClassConstructorSpan(script->sourceObject(),
+                                               classStartOffset, classEndOffset,
+                                               line, column);
 
     return ctor;
 }
 
 bool
 js::ReportIsNotFunction(JSContext* cx, HandleValue v, int numToSkip, MaybeConstruct construct)
 {
     unsigned error = construct ? JSMSG_NOT_CONSTRUCTOR : JSMSG_NOT_FUNCTION;
--- a/js/src/vm/JSScript.cpp
+++ b/js/src/vm/JSScript.cpp
@@ -997,22 +997,30 @@ js::XDRLazyScript(XDRState<XDR_DECODE>*,
 void
 JSScript::setSourceObject(JSObject* object)
 {
     MOZ_ASSERT(compartment() == object->compartment());
     sourceObject_ = object;
 }
 
 void
-JSScript::setDefaultClassConstructorSpan(JSObject* sourceObject, uint32_t start, uint32_t end)
+JSScript::setDefaultClassConstructorSpan(JSObject* sourceObject, uint32_t start, uint32_t end,
+                                         unsigned line, unsigned column)
 {
     MOZ_ASSERT(isDefaultClassConstructor());
     setSourceObject(sourceObject);
     toStringStart_ = start;
     toStringEnd_ = end;
+    sourceStart_ = start;
+    sourceEnd_ = end;
+    lineno_ = line;
+    column_ = column;
+    // Since this script has been changed to point into the user's source, we
+    // can clear its self-hosted flag, allowing Debugger to see it.
+    bitFields_.selfHosted_ = false;
 }
 
 js::ScriptSourceObject&
 JSScript::scriptSourceUnwrap() const {
     // This may be called off the main thread. It's OK not to expose the source
     // object here as it doesn't escape.
     return UncheckedUnwrapWithoutExpose(sourceObject())->as<ScriptSourceObject>();
 }
--- a/js/src/vm/JSScript.h
+++ b/js/src/vm/JSScript.h
@@ -1721,17 +1721,18 @@ class JSScript : public js::gc::TenuredC
     void setSourceObject(JSObject* object);
     JSObject* sourceObject() const {
         return sourceObject_;
     }
     js::ScriptSourceObject& scriptSourceUnwrap() const;
     js::ScriptSource* scriptSource() const;
     js::ScriptSource* maybeForwardedScriptSource() const;
 
-    void setDefaultClassConstructorSpan(JSObject* sourceObject, uint32_t start, uint32_t end);
+    void setDefaultClassConstructorSpan(JSObject* sourceObject, uint32_t start, uint32_t end,
+                                        unsigned line, unsigned column);
 
     bool mutedErrors() const { return scriptSource()->mutedErrors(); }
     const char* filename() const { return scriptSource()->filename(); }
     const char* maybeForwardedFilename() const { return maybeForwardedScriptSource()->filename(); }
 
 #ifdef MOZ_VTUNE
     uint32_t vtuneMethodID() const { return vtuneMethodId_; }
 #endif