Bug 1546232 - Simplify value in extended slot of self-hosted functions. r=anba
authorTooru Fujisawa <arai_a@mac.com>
Wed, 08 May 2019 16:51:21 +0000
changeset 534981 bd2ad6987449e5be5014d0e35ccbe599159c9e82
parent 534980 0da5ad89c60ef6b5652edc179f16ba2cf7ce8499
child 534982 a51c08b24de339dea7f16b9f8a6f37acf0c0ed6c
push id2082
push userffxbld-merge
push dateMon, 01 Jul 2019 08:34:18 +0000
treeherdermozilla-release@2fb19d0466d2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersanba
bugs1546232
milestone68.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 1546232 - Simplify value in extended slot of self-hosted functions. r=anba Differential Revision: https://phabricator.services.mozilla.com/D28769
js/src/builtin/SelfHostingDefines.h
js/src/frontend/Parser.cpp
js/src/jit-test/tests/auto-regress/bug1546232-2.js
js/src/jit-test/tests/auto-regress/bug1546232.js
js/src/vm/JSFunction.cpp
js/src/vm/SelfHosting.cpp
js/src/vm/SelfHosting.h
--- a/js/src/builtin/SelfHostingDefines.h
+++ b/js/src/builtin/SelfHostingDefines.h
@@ -62,21 +62,16 @@
 #define PROP_DESC_VALUE_INDEX 1
 #define PROP_DESC_GETTER_INDEX 1
 #define PROP_DESC_SETTER_INDEX 2
 
 // The extended slot in which the self-hosted name for self-hosted builtins is
 // stored.
 #define LAZY_FUNCTION_NAME_SLOT 0
 
-// The extended slot which contains a boolean value that indicates whether
-// that the canonical name of the self-hosted builtins is set in self-hosted
-// global. This slot is used only in debug build.
-#define HAS_SELFHOSTED_CANONICAL_NAME_SLOT 0
-
 // Stores the length for bound functions, so the .length property doesn't need
 // to be resolved eagerly.
 #define BOUND_FUN_LENGTH_SLOT 1
 
 #define ITERATOR_SLOT_TARGET 0
 // Used for collection iterators.
 #define ITERATOR_SLOT_RANGE 1
 // Used for list, i.e. Array and String, iterators.
--- a/js/src/frontend/Parser.cpp
+++ b/js/src/frontend/Parser.cpp
@@ -1955,19 +1955,16 @@ JSFunction* AllocNewFunction(JSContext* 
                              bool isSelfHosting /* = false */,
                              bool inFunctionBox /* = false */) {
   MOZ_ASSERT_IF(kind == FunctionSyntaxKind::Statement, atom != nullptr);
 
   RootedFunction fun(cx);
 
   gc::AllocKind allocKind = gc::AllocKind::FUNCTION;
   JSFunction::Flags flags;
-#ifdef DEBUG
-  bool isGlobalSelfHostedBuiltin = false;
-#endif
   switch (kind) {
     case FunctionSyntaxKind::Expression:
       flags = (generatorKind == GeneratorKind::NotGenerator &&
                        asyncKind == FunctionAsyncKind::SyncFunction
                    ? JSFunction::INTERPRETED_LAMBDA
                    : JSFunction::INTERPRETED_LAMBDA_GENERATOR_OR_ASYNC);
       break;
     case FunctionSyntaxKind::Arrow:
@@ -1988,41 +1985,32 @@ JSFunction* AllocNewFunction(JSContext* 
       allocKind = gc::AllocKind::FUNCTION_EXTENDED;
       break;
     case FunctionSyntaxKind::Setter:
       flags = JSFunction::INTERPRETED_SETTER;
       allocKind = gc::AllocKind::FUNCTION_EXTENDED;
       break;
     default:
       MOZ_ASSERT(kind == FunctionSyntaxKind::Statement);
-#ifdef DEBUG
       if (isSelfHosting && !inFunctionBox) {
-        isGlobalSelfHostedBuiltin = true;
         allocKind = gc::AllocKind::FUNCTION_EXTENDED;
       }
-#endif
       flags = (generatorKind == GeneratorKind::NotGenerator &&
                        asyncKind == FunctionAsyncKind::SyncFunction
                    ? JSFunction::INTERPRETED_NORMAL
                    : JSFunction::INTERPRETED_GENERATOR_OR_ASYNC);
   }
 
   fun = NewFunctionWithProto(cx, nullptr, 0, flags, nullptr, atom, proto,
                              allocKind, TenuredObject);
   if (!fun) {
     return nullptr;
   }
   if (isSelfHosting) {
     fun->setIsSelfHostedBuiltin();
-#ifdef DEBUG
-    if (isGlobalSelfHostedBuiltin) {
-      fun->setExtendedSlot(HAS_SELFHOSTED_CANONICAL_NAME_SLOT,
-                           BooleanValue(false));
-    }
-#endif
   }
   return fun;
 }
 
 JSFunction* ParserBase::newFunction(HandleAtom atom, FunctionSyntaxKind kind,
                                     GeneratorKind generatorKind,
                                     FunctionAsyncKind asyncKind,
                                     HandleObject proto /* = nullptr */) {
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/auto-regress/bug1546232-2.js
@@ -0,0 +1,8 @@
+var g = newGlobal();
+g.eval(`
+Array.from(function*() {
+    yield 1;
+    relazifyFunctions();
+    yield 1;
+}());
+`);
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/auto-regress/bug1546232.js
@@ -0,0 +1,4 @@
+var g = newGlobal();
+g.Int8Array.from([]);
+relazifyFunctions();
+g.eval("[].values()");
--- a/js/src/vm/JSFunction.cpp
+++ b/js/src/vm/JSFunction.cpp
@@ -1215,18 +1215,17 @@ const Class JSFunction::class_ = {js_Fun
 const Class* const js::FunctionClassPtr = &JSFunction::class_;
 
 bool JSFunction::isDerivedClassConstructor() {
   bool derived;
   if (isInterpretedLazy()) {
     // There is only one plausible lazy self-hosted derived
     // constructor.
     if (isSelfHostedBuiltin()) {
-      JSAtom* name =
-          &getExtendedSlot(LAZY_FUNCTION_NAME_SLOT).toString()->asAtom();
+      JSAtom* name = GetSelfHostedFunctionName(this);
 
       // This function is called from places without access to a
       // JSContext. Trace some plumbing to get what we want.
       derived = name == compartment()
                             ->runtimeFromAnyThread()
                             ->commonNames->DefaultDerivedClassConstructor;
     } else {
       derived = lazyScript()->isDerivedClassConstructor();
@@ -1681,18 +1680,17 @@ bool JSFunction::createScriptForLazilyIn
       }
     }
 
     return true;
   }
 
   /* Lazily cloned self-hosted script. */
   MOZ_ASSERT(fun->isSelfHostedBuiltin());
-  RootedAtom funAtom(
-      cx, &fun->getExtendedSlot(LAZY_FUNCTION_NAME_SLOT).toString()->asAtom());
+  RootedAtom funAtom(cx, GetSelfHostedFunctionName(fun));
   if (!funAtom) {
     return false;
   }
   Rooted<PropertyName*> funName(cx, funAtom->asPropertyName());
   return cx->runtime()->cloneSelfHostedFunctionScript(cx, funName, fun);
 }
 
 void JSFunction::maybeRelazify(JSRuntime* rt) {
@@ -1729,36 +1727,34 @@ void JSFunction::maybeRelazify(JSRuntime
   }
 
   // Don't relazify functions with JIT code.
   if (!u.scripted.s.script_->isRelazifiable()) {
     return;
   }
 
   // To delazify self-hosted builtins we need the name of the function
-  // to clone. This name is stored in the first extended slot. Since
-  // that slot is sometimes also used for other purposes, make sure it
-  // contains a string.
+  // to clone. This name is stored in the first extended slot.
   if (isSelfHostedBuiltin() &&
       (!isExtended() || !getExtendedSlot(LAZY_FUNCTION_NAME_SLOT).isString())) {
     return;
   }
 
   JSScript* script = nonLazyScript();
 
   flags_ &= ~INTERPRETED;
   flags_ |= INTERPRETED_LAZY;
   LazyScript* lazy = script->maybeLazyScript();
   u.scripted.s.lazy_ = lazy;
   if (lazy) {
     MOZ_ASSERT(!isSelfHostedBuiltin());
   } else {
     MOZ_ASSERT(isSelfHostedBuiltin());
     MOZ_ASSERT(isExtended());
-    MOZ_ASSERT(getExtendedSlot(LAZY_FUNCTION_NAME_SLOT).toString()->isAtom());
+    MOZ_ASSERT(GetSelfHostedFunctionName(this));
   }
 
   realm->scheduleDelazificationForDebugger();
 }
 
 // ES2018 draft rev 2aea8f3e617b49df06414eb062ab44fad87661d3
 // 19.2.1.1.1 CreateDynamicFunction( constructor, newTarget, kind, args )
 static bool CreateDynamicFunction(JSContext* cx, const CallArgs& args,
--- a/js/src/vm/SelfHosting.cpp
+++ b/js/src/vm/SelfHosting.cpp
@@ -908,32 +908,52 @@ bool js::intrinsic_NewRegExpStringIterat
   if (!obj) {
     return false;
   }
 
   args.rval().setObject(*obj);
   return true;
 }
 
+JSAtom* js::GetSelfHostedFunctionName(JSFunction* fun) {
+  Value name = fun->getExtendedSlot(LAZY_FUNCTION_NAME_SLOT);
+  if (!name.isString()) {
+    return nullptr;
+  }
+  return &name.toString()->asAtom();
+}
+
+static void SetSelfHostedFunctionName(JSFunction* fun, JSAtom* name) {
+  fun->setExtendedSlot(LAZY_FUNCTION_NAME_SLOT, StringValue(name));
+}
+
 static bool intrinsic_SetCanonicalName(JSContext* cx, unsigned argc,
                                        Value* vp) {
   CallArgs args = CallArgsFromVp(argc, vp);
   MOZ_ASSERT(args.length() == 2);
 
   RootedFunction fun(cx, &args[0].toObject().as<JSFunction>());
   MOZ_ASSERT(fun->isSelfHostedBuiltin());
   JSAtom* atom = AtomizeString(cx, args[1].toString());
   if (!atom) {
     return false;
   }
 
+  // _SetCanonicalName can only be called on top-level function declarations.
+  MOZ_ASSERT(fun->kind() == JSFunction::NormalFunction);
+  MOZ_ASSERT(!fun->isLambda());
+
+  // It's an error to call _SetCanonicalName multiple times.
+  MOZ_ASSERT(!GetSelfHostedFunctionName(fun));
+
+  // Set the lazy function name so we can later retrieve the script from the
+  // self-hosting global.
+  SetSelfHostedFunctionName(fun, fun->explicitName());
   fun->setAtom(atom);
-#ifdef DEBUG
-  fun->setExtendedSlot(HAS_SELFHOSTED_CANONICAL_NAME_SLOT, BooleanValue(true));
-#endif
+
   args.rval().setUndefined();
   return true;
 }
 
 static bool intrinsic_GeneratorObjectIsClosed(JSContext* cx, unsigned argc,
                                               Value* vp) {
   CallArgs args = CallArgsFromVp(argc, vp);
   MOZ_ASSERT(args.length() == 1);
@@ -3267,43 +3287,46 @@ static JSObject* CloneObject(JSContext* 
     }
   }
 #endif
 
   RootedObject clone(cx);
   if (selfHostedObject->is<JSFunction>()) {
     RootedFunction selfHostedFunction(cx, &selfHostedObject->as<JSFunction>());
     if (selfHostedFunction->isInterpreted()) {
-      bool hasName = selfHostedFunction->explicitName() != nullptr;
-
       // Arrow functions use the first extended slot for their lexical |this|
-      // value.
-      MOZ_ASSERT(!selfHostedFunction->isArrow());
-      js::gc::AllocKind kind = hasName ? gc::AllocKind::FUNCTION_EXTENDED
-                                       : selfHostedFunction->getAllocKind();
+      // value. And methods use the first extended slot for their home-object.
+      // We only expect to see normal functions here.
+      MOZ_ASSERT(selfHostedFunction->kind() == JSFunction::NormalFunction);
+      js::gc::AllocKind kind = selfHostedFunction->getAllocKind();
 
       Handle<GlobalObject*> global = cx->global();
       Rooted<LexicalEnvironmentObject*> globalLexical(
           cx, &global->lexicalEnvironment());
       RootedScope emptyGlobalScope(cx, &global->emptyGlobalScope());
       Rooted<ScriptSourceObject*> sourceObject(
           cx, SelfHostingScriptSourceObject(cx));
       if (!sourceObject) {
         return nullptr;
       }
       MOZ_ASSERT(
           !CanReuseScriptForClone(cx->realm(), selfHostedFunction, global));
       clone = CloneFunctionAndScript(cx, selfHostedFunction, globalLexical,
                                      emptyGlobalScope, sourceObject, kind);
       // To be able to re-lazify the cloned function, its name in the
-      // self-hosting compartment has to be stored on the clone.
-      if (clone && hasName) {
-        Value nameVal = StringValue(selfHostedFunction->explicitName());
-        clone->as<JSFunction>().setExtendedSlot(LAZY_FUNCTION_NAME_SLOT,
-                                                nameVal);
+      // self-hosting compartment has to be stored on the clone. Re-lazification
+      // is only possible if this isn't a function expression.
+      if (clone && !selfHostedFunction->isLambda()) {
+        // If |_SetCanonicalName| was called on the function, the self-hosted
+        // name is stored in the extended slot.
+        JSAtom* name = GetSelfHostedFunctionName(selfHostedFunction);
+        if (!name) {
+          name = selfHostedFunction->explicitName();
+        }
+        SetSelfHostedFunctionName(&clone->as<JSFunction>(), name);
       }
     } else {
       clone = CloneSelfHostingIntrinsic(cx, selfHostedFunction);
     }
   } else if (selfHostedObject->is<RegExpObject>()) {
     RegExpObject& reobj = selfHostedObject->as<RegExpObject>();
     RootedAtom source(cx, reobj.getSource());
     MOZ_ASSERT(source->isPermanentAtom());
@@ -3391,30 +3414,28 @@ bool JSRuntime::createLazySelfHostedFunc
   JSFunction* selfHostedFun = getUnclonedSelfHostedFunction(cx, selfHostedName);
   if (!selfHostedFun) {
     return false;
   }
 
   if (!selfHostedFun->isClassConstructor() &&
       !selfHostedFun->hasGuessedAtom() &&
       selfHostedFun->explicitName() != selfHostedName) {
-    MOZ_ASSERT(
-        selfHostedFun->getExtendedSlot(HAS_SELFHOSTED_CANONICAL_NAME_SLOT)
-            .toBoolean());
+    MOZ_ASSERT(GetSelfHostedFunctionName(selfHostedFun) == selfHostedName);
     funName = selfHostedFun->explicitName();
   }
 
   fun.set(NewScriptedFunction(cx, nargs, JSFunction::INTERPRETED_LAZY, funName,
                               proto, gc::AllocKind::FUNCTION_EXTENDED,
                               newKind));
   if (!fun) {
     return false;
   }
   fun->setIsSelfHostedBuiltin();
-  fun->setExtendedSlot(LAZY_FUNCTION_NAME_SLOT, StringValue(selfHostedName));
+  SetSelfHostedFunctionName(fun, selfHostedName);
   return true;
 }
 
 bool JSRuntime::cloneSelfHostedFunctionScript(JSContext* cx,
                                               HandlePropertyName name,
                                               HandleFunction targetFun) {
   RootedFunction sourceFun(cx, getUnclonedSelfHostedFunction(cx, name));
   if (!sourceFun) {
@@ -3500,34 +3521,25 @@ bool JSRuntime::cloneSelfHostedValue(JSC
   return CloneValue(cx, selfHostedValue, vp);
 }
 
 void JSRuntime::assertSelfHostedFunctionHasCanonicalName(
     JSContext* cx, HandlePropertyName name) {
 #ifdef DEBUG
   JSFunction* selfHostedFun = getUnclonedSelfHostedFunction(cx, name);
   MOZ_ASSERT(selfHostedFun);
-  MOZ_ASSERT(selfHostedFun->getExtendedSlot(HAS_SELFHOSTED_CANONICAL_NAME_SLOT)
-                 .toBoolean());
+  MOZ_ASSERT(GetSelfHostedFunctionName(selfHostedFun) == name);
 #endif
 }
 
 bool js::IsSelfHostedFunctionWithName(JSFunction* fun, JSAtom* name) {
   return fun->isSelfHostedBuiltin() && fun->isExtended() &&
          GetSelfHostedFunctionName(fun) == name;
 }
 
-JSAtom* js::GetSelfHostedFunctionName(JSFunction* fun) {
-  Value name = fun->getExtendedSlot(LAZY_FUNCTION_NAME_SLOT);
-  if (!name.isString()) {
-    return nullptr;
-  }
-  return &name.toString()->asAtom();
-}
-
 static_assert(
     JSString::MAX_LENGTH <= INT32_MAX,
     "StringIteratorNext in builtin/String.js assumes the stored index "
     "into the string is an Int32Value");
 
 static_assert(JSString::MAX_LENGTH == MAX_STRING_LENGTH,
               "JSString::MAX_LENGTH matches self-hosted constant for maximum "
               "string length");
--- a/js/src/vm/SelfHosting.h
+++ b/js/src/vm/SelfHosting.h
@@ -15,16 +15,29 @@
 namespace js {
 
 /*
  * Check whether the given JSFunction is a self-hosted function whose
  * self-hosted name is the given name.
  */
 bool IsSelfHostedFunctionWithName(JSFunction* fun, JSAtom* name);
 
+/*
+ * Returns the name of the function's binding in the self-hosted global.
+ *
+ * This returns a non-null value only when:
+ *   * This is a top level function declaration in the self-hosted global.
+ *   * And either:
+ *     * This function is not cloned and `_SetCanonicalName` has been called to
+ *       set a different function name.
+ *     * This function is cloned.
+ *
+ * For functions not cloned and not `_SetCanonicalName`ed, use
+ * `fun->explicitName()` instead.
+ */
 JSAtom* GetSelfHostedFunctionName(JSFunction* fun);
 
 bool IsCallSelfHostedNonGenericMethod(NativeImpl impl);
 
 bool ReportIncompatibleSelfHostedMethod(JSContext* cx, const CallArgs& args);
 
 /* Get the compile options used when compiling self hosted code. */
 void FillSelfHostingCompileOptions(JS::CompileOptions& options);