Bug 1473272 - Don't treat classes with default constructors as self-hosted code. r=tcampbell
authorJim Blandy <jimb@mozilla.com>
Sat, 28 Jul 2018 20:26:46 -0700
changeset 432959 714f1a873154e3435a212c8ef62196fd93613bbf
parent 432958 bb407121dfdedbea6672e59419ad74d1b436ab71
child 432960 8aed27360bd2a4efb25ae8a0f1d989892a1be1c7
push id34493
push usercbrindusan@mozilla.com
push dateThu, 23 Aug 2018 03:45:14 +0000
treeherdermozilla-central@5f1db4de173d [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,24 @@
+// 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 frame of new Error().stack.split('\n')) {
+        function lineNumber(frame) {
+            return +frame.match(/(\d+):\d+$/)[1];
+        }
+
+        if (frame.startsWith("Z@"))
+            assertEq(lineNumber(frame), 4);
+        if (frame.startsWith("Y@"))
+            assertEq(lineNumber(frame), 5);
+        if (frame.startsWith("X@"))
+            assertEq(lineNumber(frame), 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
@@ -304,18 +304,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