From 76052fcda88a4c0d0c916ffd84abbbb2ee614c5b Mon Sep 17 00:00:00 2001 From: Martok Date: Thu, 13 Jul 2023 02:47:15 +0200 Subject: [PATCH] Issue #2271 - Use declared names of self-hosted functions for cloning Ensure that cloning a self-hosted function always has access to the declared name from the source and that it doesn't get lost on successive clones. This is done by storing the declared name in an extended slot on rename, and cloning it with the function. Based-on: m-c 1546232 --- js/src/builtin/SelfHostingDefines.h | 5 --- js/src/frontend/Parser.cpp | 10 ----- js/src/jsfun.cpp | 12 +++-- js/src/vm/SelfHosting.cpp | 69 ++++++++++++++++++----------- js/src/vm/SelfHosting.h | 13 ++++++ 5 files changed, 62 insertions(+), 47 deletions(-) diff --git a/js/src/builtin/SelfHostingDefines.h b/js/src/builtin/SelfHostingDefines.h index 987351815c..9a93a2c795 100644 --- a/js/src/builtin/SelfHostingDefines.h +++ b/js/src/builtin/SelfHostingDefines.h @@ -52,11 +52,6 @@ // 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 diff --git a/js/src/frontend/Parser.cpp b/js/src/frontend/Parser.cpp index d617941503..a1b669bf91 100644 --- a/js/src/frontend/Parser.cpp +++ b/js/src/frontend/Parser.cpp @@ -2807,9 +2807,6 @@ Parser::newFunction(HandleAtom atom, FunctionSyntaxKind kind, gc::AllocKind allocKind = gc::AllocKind::FUNCTION; JSFunction::Flags flags; -#ifdef DEBUG - bool isGlobalSelfHostedBuiltin = false; -#endif switch (kind) { case FunctionSyntaxKind::Expression: flags = (generatorKind == NotGenerator && asyncKind == SyncFunction @@ -2846,12 +2843,9 @@ Parser::newFunction(HandleAtom atom, FunctionSyntaxKind kind, break; default: MOZ_ASSERT(kind == FunctionSyntaxKind::Statement); -#ifdef DEBUG if (options().selfHostingMode && !pc->isFunctionBox()) { - isGlobalSelfHostedBuiltin = true; allocKind = gc::AllocKind::FUNCTION_EXTENDED; } -#endif flags = (generatorKind == NotGenerator && asyncKind == SyncFunction ? JSFunction::INTERPRETED_NORMAL : JSFunction::INTERPRETED_GENERATOR_OR_ASYNC); @@ -2867,10 +2861,6 @@ Parser::newFunction(HandleAtom atom, FunctionSyntaxKind kind, return nullptr; if (options().selfHostingMode) { fun->setIsSelfHostedBuiltin(); -#ifdef DEBUG - if (isGlobalSelfHostedBuiltin) - fun->setExtendedSlot(HAS_SELFHOSTED_CANONICAL_NAME_SLOT, BooleanValue(false)); -#endif } return fun; } diff --git a/js/src/jsfun.cpp b/js/src/jsfun.cpp index bd133b822e..5255aebbfc 100644 --- a/js/src/jsfun.cpp +++ b/js/src/jsfun.cpp @@ -1251,7 +1251,7 @@ JSFunction::infallibleIsDefaultClassConstructor(JSContext* cx) const bool isDefault = false; if (isInterpretedLazy()) { - JSAtom* name = &getExtendedSlot(LAZY_FUNCTION_NAME_SLOT).toString()->asAtom(); + JSAtom* name = GetSelfHostedFunctionName(const_cast(this)); isDefault = name == cx->names().DefaultDerivedClassConstructor || name == cx->names().DefaultBaseClassConstructor; } else { @@ -1271,7 +1271,7 @@ JSFunction::isDerivedClassConstructor() // 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. @@ -1530,7 +1530,7 @@ JSFunction::createScriptForLazilyInterpretedFunction(JSContext* cx, HandleFuncti /* 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 funName(cx, funAtom->asPropertyName()); @@ -1569,9 +1569,7 @@ JSFunction::maybeRelazify(JSRuntime* rt) 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())) { @@ -1589,7 +1587,7 @@ JSFunction::maybeRelazify(JSRuntime* rt) } else { MOZ_ASSERT(isSelfHostedBuiltin()); MOZ_ASSERT(isExtended()); - MOZ_ASSERT(getExtendedSlot(LAZY_FUNCTION_NAME_SLOT).toString()->isAtom()); + MOZ_ASSERT(GetSelfHostedFunctionName(this)); } comp->scheduleDelazificationForDebugger(); diff --git a/js/src/vm/SelfHosting.cpp b/js/src/vm/SelfHosting.cpp index c1276f5a43..de497d02e1 100644 --- a/js/src/vm/SelfHosting.cpp +++ b/js/src/vm/SelfHosting.cpp @@ -903,6 +903,22 @@ intrinsic_NewRegExpStringIterator(JSContext* cx, unsigned argc, Value* vp) 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) { @@ -915,10 +931,18 @@ intrinsic_SetCanonicalName(JSContext* cx, unsigned argc, Value* vp) 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; } @@ -2910,13 +2934,11 @@ CloneObject(JSContext* cx, HandleNativeObject selfHostedObject) if (selfHostedObject->is()) { RootedFunction selfHostedFunction(cx, &selfHostedObject->as()); 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(); + // 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 global = cx->global(); Rooted globalLexical(cx, &global->lexicalEnvironment()); @@ -2925,10 +2947,16 @@ CloneObject(JSContext* cx, HandleNativeObject selfHostedObject) clone = CloneFunctionAndScript(cx, selfHostedFunction, globalLexical, emptyGlobalScope, 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) { - clone->as().setExtendedSlot(LAZY_FUNCTION_NAME_SLOT, - StringValue(selfHostedFunction->explicitName())); + // 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(), name); } } else { clone = CloneSelfHostingIntrinsic(cx, selfHostedFunction); @@ -3016,7 +3044,7 @@ JSRuntime::createLazySelfHostedFunctionClone(JSContext* cx, HandlePropertyName s 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(); } @@ -3025,7 +3053,7 @@ JSRuntime::createLazySelfHostedFunctionClone(JSContext* cx, HandlePropertyName s if (!fun) return false; fun->setIsSelfHostedBuiltin(); - fun->setExtendedSlot(LAZY_FUNCTION_NAME_SLOT, StringValue(selfHostedName)); + SetSelfHostedFunctionName(fun, selfHostedName); return true; } @@ -3111,7 +3139,7 @@ JSRuntime::assertSelfHostedFunctionHasCanonicalName(JSContext* cx, HandlePropert #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 } @@ -3133,15 +3161,6 @@ js::IsSelfHostedFunctionWithName(JSFunction* fun, JSAtom* name) return fun->isSelfHostedBuiltin() && 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"); diff --git a/js/src/vm/SelfHosting.h b/js/src/vm/SelfHosting.h index 6b1a03e851..04962f4445 100644 --- a/js/src/vm/SelfHosting.h +++ b/js/src/vm/SelfHosting.h @@ -22,6 +22,19 @@ namespace js { 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);