Bug 1488385: Default class constructors are no longer self-hosted once they're cloned. r=tcampbell
authorJim Blandy <jimb@mozilla.com>
Sat, 13 Oct 2018 01:56:53 +0000
changeset 499779 c317490b6e780bca51c5a2342e4ac9bd8913fb25
parent 499778 b2593fa8b1bd907c0685dc2c5d6b9f266ab564c5
child 499780 0cf18ffee1c0318be8dbf89ef1aecedb8243da0f
push id1864
push userffxbld-merge
push dateMon, 03 Dec 2018 15:51:40 +0000
treeherdermozilla-release@f040763d99ad [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstcampbell
bugs1488385
milestone64.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 1488385: Default class constructors are no longer self-hosted once they're cloned. r=tcampbell When we clone the script of a default class constructor (derived or base), we need to clear its SELF_HOSTED flag, since such functions are supposed to behave just like ordinary functions from user code: their frames should appear on the stack, and so on. However, the SELF_HOSTED flag must remain set until the script is actually cloned from the original, to satisfy the (quite reasonable) assertion that only SELF_HOSTED functions get their scripts cloned from the self-hosted compartment. Depends on D8620 Differential Revision: https://phabricator.services.mozilla.com/D8621
js/src/jit-test/tests/class/bug1488385.js
js/src/vm/Interpreter.cpp
js/src/vm/JSFunction.h
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/class/bug1488385.js
@@ -0,0 +1,13 @@
+// Default class constructors should no longer be marked as self-hosted
+// functions. They should be munged to appear in every respect as if they
+// originated with the class definition.
+
+load(libdir + 'asserts.js');
+
+function f() {
+    return f.caller.p;
+}
+
+// Since default constructors are strict mode code, this should get:
+// TypeError: access to strict mode caller function is censored
+assertThrowsInstanceOf(() => new class extends f {}, TypeError);
--- a/js/src/vm/Interpreter.cpp
+++ b/js/src/vm/Interpreter.cpp
@@ -301,23 +301,31 @@ js::MakeDefaultConstructor(JSContext* cx
                                                           proto, TenuredObject, &ctor))
     {
         return nullptr;
     }
 
     ctor->setIsConstructor();
     ctor->setIsClassConstructor();
 
-    // 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.
+    // Create the script now, so we can fix up its source span below.
     JSScript *ctorScript = JSFunction::getOrCreateScript(cx, ctor);
     if (!ctorScript) {
         return nullptr;
     }
+
+    // This function's frames are fine to expose to JS; it should not be treated
+    // as an opaque self-hosted builtin. But the script cloning code naturally
+    // expects to be applied to self-hosted functions, so do the clone first,
+    // and clear this afterwards.
+    ctor->clearIsSelfHosted();
+
+    // Override the source span needs for toString. Calling toString on a class
+    // constructor should return the class declaration, not the source for the
+    // (self-hosted) constructor function.
     uint32_t classStartOffset = GetSrcNoteOffset(classNote, 0);
     uint32_t classEndOffset = GetSrcNoteOffset(classNote, 1);
     unsigned column;
     unsigned line = PCToLineNumber(script, pc, &column);
     ctorScript->setDefaultClassConstructorSpan(script->sourceObject(),
                                                classStartOffset, classEndOffset,
                                                line, column);
 
--- a/js/src/vm/JSFunction.h
+++ b/js/src/vm/JSFunction.h
@@ -64,18 +64,20 @@ class JSFunction : public js::NativeObje
                                        name was guessed for it anyway. See
                                        atom_ for more info about this flag. */
         HAS_BOUND_FUNCTION_NAME_PREFIX = 0x0020, /* bound functions reuse the HAS_GUESSED_ATOM
                                                     flag to track if atom_ already contains the
                                                     "bound " function name prefix */
         LAMBDA           = 0x0040,  /* function comes from a FunctionExpression, ArrowFunction, or
                                        Function() call (not a FunctionDeclaration or nonstandard
                                        function-statement) */
-        SELF_HOSTED      = 0x0080,  /* function is self-hosted builtin and must not be
-                                       decompilable nor constructible. */
+        SELF_HOSTED      = 0x0080,  /* On an interpreted function, indicates a self-hosted builtin,
+                                       which must not be decompilable nor constructible. On a native
+                                       function, indicates an 'intrinsic', intended for use from
+                                       self-hosted code only. */
         HAS_INFERRED_NAME = 0x0100, /* function had no explicit name, but a name was
                                        set by SetFunctionName at compile time or
                                        SetFunctionNameIfNoOwnName at runtime. See
                                        atom_ for more info about this flag. */
         INTERPRETED_LAZY = 0x0200,  /* function is interpreted but doesn't have a script yet */
         RESOLVED_LENGTH  = 0x0400,  /* f.length has been resolved (see fun_resolve). */
         RESOLVED_NAME    = 0x0800,  /* f.name has been resolved (see fun_resolve). */
         NEW_SCRIPT_CLEARED  = 0x1000, /* For a function used as an interpreted constructor, whether
@@ -106,16 +108,21 @@ class JSFunction : public js::NativeObje
         INTERPRETED_SETTER = INTERPRETED | SETTER_KIND,
         INTERPRETED_LAMBDA = INTERPRETED | LAMBDA | CONSTRUCTOR,
         INTERPRETED_LAMBDA_ARROW = INTERPRETED | LAMBDA | ARROW_KIND,
         INTERPRETED_LAMBDA_GENERATOR_OR_ASYNC = INTERPRETED | LAMBDA,
         INTERPRETED_NORMAL = INTERPRETED | CONSTRUCTOR,
         INTERPRETED_GENERATOR_OR_ASYNC = INTERPRETED,
         NO_XDR_FLAGS = RESOLVED_LENGTH | RESOLVED_NAME,
 
+        /* Flags preserved when cloning a function.
+           (Exception: js::MakeDefaultConstructor produces default constructors for ECMAScript
+           classes by cloning self-hosted functions, and then clearing their SELF_HOSTED bit,
+           setting their CONSTRUCTOR bit, and otherwise munging them to look like they originated
+           with the class definition.) */
         STABLE_ACROSS_CLONES = CONSTRUCTOR | LAMBDA | SELF_HOSTED | FUNCTION_KIND_MASK
     };
 
     static_assert((INTERPRETED | INTERPRETED_LAZY) == js::JS_FUNCTION_INTERPRETED_BITS,
                   "jsfriendapi.h's JSFunction::INTERPRETED-alike is wrong");
     static_assert(((FunctionKindLimit - 1) << FUNCTION_KIND_SHIFT) <= FUNCTION_KIND_MASK,
                   "FunctionKind doesn't fit into flags_");
 
@@ -343,16 +350,20 @@ class JSFunction : public js::NativeObje
 
     void setIsClassConstructor() {
         MOZ_ASSERT(!isClassConstructor());
         MOZ_ASSERT(isConstructor());
 
         setKind(ClassConstructor);
     }
 
+    void clearIsSelfHosted() {
+        flags_ &= ~SELF_HOSTED;
+    }
+
     // Can be called multiple times by the parser.
     void setArgCount(uint16_t nargs) {
         this->nargs_ = nargs;
     }
 
     void setIsBoundFunction() {
         MOZ_ASSERT(!isBoundFunction());
         flags_ |= BOUND_FUN;