From ffa0c4d8646c4975ef2f738e3e54a84240d34ce4 Mon Sep 17 00:00:00 2001 From: Brian Smith Date: Fri, 7 Jul 2023 17:38:09 -0500 Subject: [PATCH 1/7] No Issue - Fix debug builds on ARM Mac. mach_override used by the IO Poisoner used in debug mode is not support on ARM. https://bugzilla.mozilla.org/show_bug.cgi?id=1658385 --- xpcom/build/PoisonIOInterposerMac.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/xpcom/build/PoisonIOInterposerMac.cpp b/xpcom/build/PoisonIOInterposerMac.cpp index d76b728ade..d76d6af415 100644 --- a/xpcom/build/PoisonIOInterposerMac.cpp +++ b/xpcom/build/PoisonIOInterposerMac.cpp @@ -4,7 +4,9 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ #include "PoisonIOInterposer.h" +#ifndef __aarch64__ #include "mach_override.h" +#endif #include "mozilla/ArrayUtils.h" #include "mozilla/Assertions.h" @@ -360,9 +362,11 @@ InitPoisonIOInterposer() if (!d->Function) { continue; } +#ifndef __aarch64__ DebugOnly t = mach_override_ptr(d->Function, d->Wrapper, &d->Buffer); MOZ_ASSERT(t == err_none); +#endif } } From eb2cca7242cbc8cee10d7188b2b297d4e5f06d0d Mon Sep 17 00:00:00 2001 From: Brian Smith Date: Fri, 7 Jul 2023 17:46:55 -0500 Subject: [PATCH 2/7] Issue #2268 - Fix Mac packaging by making the individual parts configurable. Add --with-macbundle-entitlement= to specify alternate entitlements or "none" Add --with-macbundle-type=hybrid to use the old DMG format. --- build/moz.configure/old.configure | 2 ++ old-configure.in | 21 +++++++++++++++++++ python/mozbuild/mozpack/dmg.py | 35 ++++++++++++++++++++++++------- 3 files changed, 50 insertions(+), 8 deletions(-) diff --git a/build/moz.configure/old.configure b/build/moz.configure/old.configure index 03ba868dee..527e3afd50 100644 --- a/build/moz.configure/old.configure +++ b/build/moz.configure/old.configure @@ -282,7 +282,9 @@ def old_configure_options(*options): '--with-ios-sdk', '--with-jitreport-granularity', '--with-macbundlename-prefix', + '--with-macbundle-entitlement', '--with-macbundle-identity', + '--with-macbundle-type', '--with-macos-private-frameworks', '--with-macos-sdk', '--with-nspr-cflags', diff --git a/old-configure.in b/old-configure.in index 17d3e833ec..ee34db960e 100644 --- a/old-configure.in +++ b/old-configure.in @@ -5010,6 +5010,16 @@ fi AC_DEFINE_UNQUOTED(MOZ_MACBUNDLE_ID,$MOZ_MACBUNDLE_ID) AC_SUBST(MOZ_MACBUNDLE_ID) +dnl ======================================================== +dnl = Mac bundle codesign entitlements +dnl ======================================================== +MOZ_ARG_WITH_STRING(macbundle-entitlement, +[ --with-macbundle-entitlement=entitlementname + Entitlements to add to the Mac application bundle], +[ MOZ_MACBUNDLE_ENTITLEMENT="$withval"]) + +AC_SUBST(MOZ_MACBUNDLE_ENTITLEMENT) + dnl ======================================================== dnl = Mac bundle codesign identity dnl ======================================================== @@ -5020,6 +5030,17 @@ MOZ_ARG_WITH_STRING(macbundle-identity, AC_SUBST(MOZ_MACBUNDLE_IDENTITY) +dnl ======================================================== +dnl = Mac bundle DMG type +dnl ======================================================== +MOZ_ARG_WITH_STRING(macbundle-type, +[ --with-macbundle-type=hybrid + Method to use to create the DMG], +[ MOZ_MACBUNDLE_TYPE="$withval"]) + +AC_SUBST(MOZ_MACBUNDLE_TYPE) + + dnl ======================================================== dnl = Child Process Name for IPC dnl ======================================================== diff --git a/python/mozbuild/mozpack/dmg.py b/python/mozbuild/mozpack/dmg.py index b231f731b8..505cd8729e 100644 --- a/python/mozbuild/mozpack/dmg.py +++ b/python/mozbuild/mozpack/dmg.py @@ -44,19 +44,25 @@ def set_folder_icon(dir): def create_dmg_from_staged(stagedir, output_dmg, tmpdir, volume_name): 'Given a prepared directory stagedir, produce a DMG at output_dmg.' + import buildconfig if not is_linux: # Running on OS X hybrid = os.path.join(tmpdir, 'hybrid.dmg') - subprocess.check_call(['hdiutil', 'create', - '-fs', 'HFS+', - '-volname', volume_name, - '-srcfolder', stagedir, - '-ov', hybrid]) + hdiutiloptions = ['create', '-fs', 'HFS+', + '-volname', volume_name, + '-srcfolder', stagedir, + '-ov', hybrid] + if buildconfig.substs['MOZ_MACBUNDLE_TYPE'] == 'hybrid': + hdiutiloptions = ['makehybrid', '-hfs', + '-hfs-volume-name', volume_name, + '-hfs-openfolder', stagedir, + '-ov', stagedir, + '-o', hybrid] + subprocess.check_call(['hdiutil'] + hdiutiloptions) subprocess.check_call(['hdiutil', 'convert', '-format', 'UDBZ', '-imagekey', 'bzip2-level=9', '-ov', hybrid, '-o', output_dmg]) else: - import buildconfig uncompressed = os.path.join(tmpdir, 'uncompressed.dmg') subprocess.check_call([ buildconfig.substs['GENISOIMAGE'], @@ -123,15 +129,28 @@ def create_dmg(source_directory, output_dmg, volume_name, extra_files): identity = buildconfig.substs['MOZ_MACBUNDLE_IDENTITY'] if identity != '': dylibs = [] + entitlements = [] appbundle = os.path.join(stagedir, buildconfig.substs['MOZ_MACBUNDLE_NAME']) # If the -bin file is in Resources add it to the dylibs as well resourcebin = os.path.join(appbundle, 'Contents/Resources/' + buildconfig.substs['MOZ_APP_NAME'] + '-bin') if os.path.isfile(resourcebin): dylibs.append(resourcebin) # Create a list of dylibs in Contents/Resources that won't get signed by --deep - for root, dirnames, filenames in os.walk('Contents/Resources/'): + for root, dirnames, filenames in os.walk(os.path.join(appbundle,'Contents/Resources/')): for filename in fnmatch.filter(filenames, '*.dylib'): dylibs.append(os.path.join(root, filename)) + # Select default entitlements based on whether debug is enabled entitlement = os.path.abspath(os.path.join(os.getcwd(), '../../platform/security/mac/production.entitlements.xml')) - subprocess.check_call(['codesign', '--deep', '--timestamp', '--options', 'runtime', '--entitlements', entitlement, '-s', identity] + dylibs + [appbundle]) + if buildconfig.substs['MOZ_DEBUG']: + entitlement = os.path.abspath(os.path.join(os.getcwd(), '../../platform/security/mac/developer.entitlements.xml')) + # Check to see if the entitlements are disabled or overrided by mozconfig + entitlementname = buildconfig.substs['MOZ_MACBUNDLE_ENTITLEMENT'] + if entitlementname != '': + if os.path.isfile(entitlementname): + entitlement = entitlementname + if entitlementname != 'none': + if os.path.isfile(entitlement): + entitlements = ['--entitlements', entitlement] + # Call the codesign tool + subprocess.check_call(['codesign', '--deep', '--timestamp', '--options', 'runtime'] + entitlements + [ '-s', identity] + dylibs + [appbundle]) create_dmg_from_staged(stagedir, output_dmg, tmpdir, volume_name) From efde4d468e99b46c52c6a8dc292278ca1908b892 Mon Sep 17 00:00:00 2001 From: Brian Smith Date: Wed, 12 Jul 2023 14:24:28 -0500 Subject: [PATCH 3/7] Issue #2255 - Add support for Maybe https://bugzilla.mozilla.org/show_bug.cgi?id=1620568 Make Maybe::emplace() work when T is const https://bugzilla.mozilla.org/show_bug.cgi?id=1335780 --- mfbt/Maybe.h | 86 +++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 85 insertions(+), 1 deletion(-) diff --git a/mfbt/Maybe.h b/mfbt/Maybe.h index bc123b047f..6c0fbe7ba2 100644 --- a/mfbt/Maybe.h +++ b/mfbt/Maybe.h @@ -83,7 +83,15 @@ template class Maybe { bool mIsSome; - AlignedStorage2 mStorage; + + // To support |Maybe| we give |mStorage| the type |T| with any + // const-ness removed. That allows us to |emplace()| an object into + // |mStorage|. Since we treat the contained object as having type |T| + // everywhere else (both internally, and when exposed via public methods) the + // contained object is still treated as const once stored since |const| is + // part of |T|'s type signature. + typedef typename RemoveCV::Type StorageType; + AlignedStorage2 mStorage; public: typedef T ValueType; @@ -453,6 +461,72 @@ public: } }; +template +class Maybe { + public: + constexpr Maybe() = default; + constexpr MOZ_IMPLICIT Maybe(Nothing) {} + + void emplace(T& aRef) { mValue = &aRef; } + + /* Methods that check whether this Maybe contains a value */ + explicit operator bool() const { return isSome(); } + bool isSome() const { return mValue; } + bool isNothing() const { return !mValue; } + + T& ref() const { + MOZ_DIAGNOSTIC_ASSERT(isSome()); + return *mValue; + } + + T* operator->() const { return &ref(); } + T& operator*() const { return ref(); } + + // Deliberately not defining value and ptr accessors, as these may be + // confusing on a reference-typed Maybe. + + // XXX Should we define refOr? + + void reset() { mValue = nullptr; } + + template + Maybe& apply(Func&& aFunc) { + if (isSome()) { + std::forward(aFunc)(ref()); + } + return *this; + } + + template + const Maybe& apply(Func&& aFunc) const { + if (isSome()) { + std::forward(aFunc)(ref()); + } + return *this; + } + + template + auto map(Func&& aFunc) { + Maybe(aFunc)(ref()))> val; + if (isSome()) { + val.emplace(std::forward(aFunc)(ref())); + } + return val; + } + + template + auto map(Func&& aFunc) const { + Maybe(aFunc)(ref()))> val; + if (isSome()) { + val.emplace(std::forward(aFunc)(ref())); + } + return val; + } + + private: + T* mValue = nullptr; +}; + /* * Some() creates a Maybe value containing the provided T value. If T has a * move constructor, it's used to make this as efficient as possible. @@ -474,6 +548,13 @@ Some(T&& aValue) return value; } +template +Maybe SomeRef(T& aValue) { + Maybe value; + value.emplace(aValue); + return value; +} + template Maybe::Type>::Type> ToMaybe(T* aPtr) @@ -492,6 +573,9 @@ ToMaybe(T* aPtr) template bool operator==(const Maybe& aLHS, const Maybe& aRHS) { + static_assert(!std::is_reference::value, + "operator== is not defined for Maybe, compare values or " + "addresses explicitly instead"); if (aLHS.isNothing() != aRHS.isNothing()) { return false; } From 1f5eaee10132d2f7686b40d72fce0bc085718ec5 Mon Sep 17 00:00:00 2001 From: Brian Smith Date: Wed, 12 Jul 2023 14:25:06 -0500 Subject: [PATCH 4/7] Issue #2255 - Use Maybe<> in the Performance API. --- dom/performance/Performance.cpp | 30 ++++++++++++++++-------------- dom/performance/Performance.h | 6 +++--- 2 files changed, 19 insertions(+), 17 deletions(-) diff --git a/dom/performance/Performance.cpp b/dom/performance/Performance.cpp index 9a1d134626..0217b20153 100755 --- a/dom/performance/Performance.cpp +++ b/dom/performance/Performance.cpp @@ -413,16 +413,16 @@ DOMHighResTimeStamp Performance::ConvertNameToTimestamp(const nsAString& aName, DOMHighResTimeStamp Performance::ResolveEndTimeForMeasure( const Optional& aEndMark, - const PerformanceMeasureOptions* aOptions, + const Maybe& aOptions, ErrorResult& aRv) { DOMHighResTimeStamp endTime; if (aEndMark.WasPassed()) { endTime = ConvertMarkToTimestampWithString(aEndMark.Value(), aRv); - } else if (aOptions != nullptr && aOptions->mEnd.WasPassed()) { + } else if (aOptions && aOptions->mEnd.WasPassed()) { endTime = ConvertMarkToTimestamp(ResolveTimestampAttribute::End, aOptions->mEnd.Value(), aRv); - } else if (aOptions != nullptr && aOptions->mStart.WasPassed() && + } else if (aOptions && aOptions->mStart.WasPassed() && aOptions->mDuration.WasPassed()) { const DOMHighResTimeStamp start = ConvertMarkToTimestamp( ResolveTimestampAttribute::Start, aOptions->mStart.Value(), aRv); @@ -447,16 +447,16 @@ Performance::ResolveEndTimeForMeasure( DOMHighResTimeStamp Performance::ResolveStartTimeForMeasure( - const nsAString* aStartMark, - const PerformanceMeasureOptions* aOptions, + const Maybe& aStartMark, + const Maybe& aOptions, ErrorResult& aRv) { DOMHighResTimeStamp startTime; - if (aOptions != nullptr && aOptions->mStart.WasPassed()) { + if (aOptions && aOptions->mStart.WasPassed()) { startTime = ConvertMarkToTimestamp(ResolveTimestampAttribute::Start, aOptions->mStart.Value(), aRv); - } else if (aOptions != nullptr && aOptions->mDuration.WasPassed() && + } else if (aOptions && aOptions->mDuration.WasPassed() && aOptions->mEnd.WasPassed()) { const DOMHighResTimeStamp duration = ConvertMarkToTimestampWithDOMHighResTimeStamp( @@ -474,7 +474,7 @@ Performance::ResolveStartTimeForMeasure( } startTime = end - duration; - } else if (aStartMark != nullptr) { + } else if (aStartMark) { startTime = ConvertMarkToTimestampWithString(*aStartMark, aRv); } else { startTime = 0; @@ -496,13 +496,14 @@ Performance::Measure(JSContext* aCx, return nullptr; } - const PerformanceMeasureOptions* options = nullptr; + // Maybe is more readable than using the union type directly. + Maybe options; if (aStartOrMeasureOptions.IsPerformanceMeasureOptions()) { - options = &aStartOrMeasureOptions.GetAsPerformanceMeasureOptions(); + options.emplace(aStartOrMeasureOptions.GetAsPerformanceMeasureOptions()); } const bool isOptionsNotEmpty = - (options != nullptr) && + options.isSome() && (!options->mDetail.isUndefined() || options->mStart.WasPassed() || options->mEnd.WasPassed() || options->mDuration.WasPassed()); if (isOptionsNotEmpty) { @@ -529,9 +530,10 @@ Performance::Measure(JSContext* aCx, return nullptr; } - const nsAString* startMark = nullptr; + // Convert to Maybe for consistency with options. + Maybe startMark; if (aStartOrMeasureOptions.IsString()) { - startMark = &aStartOrMeasureOptions.GetAsString(); + startMark.emplace(aStartOrMeasureOptions.GetAsString()); } const DOMHighResTimeStamp startTime = ResolveStartTimeForMeasure(startMark, options, aRv); @@ -540,7 +542,7 @@ Performance::Measure(JSContext* aCx, } JS::Rooted detail(aCx); - if (options != nullptr && !options->mDetail.isNullOrUndefined()) { + if (options && !options->mDetail.isNullOrUndefined()) { StructuredSerializeOptions serializeOptions; JS::Rooted valueToClone(aCx, options->mDetail); nsContentUtils::StructuredClone(aCx, GetParentObject(), valueToClone, diff --git a/dom/performance/Performance.h b/dom/performance/Performance.h index 2108e1e394..d84cf06765 100644 --- a/dom/performance/Performance.h +++ b/dom/performance/Performance.h @@ -195,11 +195,11 @@ private: DOMHighResTimeStamp ResolveEndTimeForMeasure( const Optional& aEndMark, - const PerformanceMeasureOptions* aOptions, + const Maybe& aOptions, ErrorResult& aRv); DOMHighResTimeStamp ResolveStartTimeForMeasure( - const nsAString* aStartMark, - const PerformanceMeasureOptions* aOptions, + const Maybe& aStartMark, + const Maybe& aOptions, ErrorResult& aRv); }; From 323007ec3dda095994a874a0c35e85694f938de2 Mon Sep 17 00:00:00 2001 From: Brian Smith Date: Wed, 12 Jul 2023 17:52:59 -0500 Subject: [PATCH 5/7] Issue #2255 - Fix build bustage on Linux. Need #include to ensure std::forward is available. --- mfbt/Maybe.h | 1 + 1 file changed, 1 insertion(+) diff --git a/mfbt/Maybe.h b/mfbt/Maybe.h index 6c0fbe7ba2..cc02528f92 100644 --- a/mfbt/Maybe.h +++ b/mfbt/Maybe.h @@ -16,6 +16,7 @@ #include // for placement new #include +#include namespace mozilla { From 3eb7729d04564b94c13e376782842dd969ff3650 Mon Sep 17 00:00:00 2001 From: Martok Date: Thu, 13 Jul 2023 02:37:07 +0200 Subject: [PATCH 6/7] Issue #2271 - Separate cloning of native and interpreted functions Separate code paths make it easier to follow and specialize than a single one-size-fits-all function. Based-on: m-c 1405766, 1411954 --- js/src/jsapi.cpp | 23 +++------- js/src/jsfun.cpp | 94 ++++++++++++++++++++++++++------------- js/src/jsfun.h | 6 +++ js/src/vm/Interpreter.cpp | 8 +++- js/src/vm/SelfHosting.cpp | 38 +++++++++------- js/src/wasm/AsmJS.cpp | 4 +- js/src/wasm/AsmJS.h | 3 ++ 7 files changed, 108 insertions(+), 68 deletions(-) diff --git a/js/src/jsapi.cpp b/js/src/jsapi.cpp index 9e5853b454..0e29f02176 100644 --- a/js/src/jsapi.cpp +++ b/js/src/jsapi.cpp @@ -3530,9 +3530,6 @@ CreateNonSyntacticEnvironmentChain(JSContext* cx, AutoObjectVector& envChain, static bool IsFunctionCloneable(HandleFunction fun) { - if (!fun->isInterpreted()) - return true; - // If a function was compiled with non-global syntactic environments on // the environment chain, we could have baked in EnvironmentCoordinates // into the script. We cannot clone it without breaking the compiler's @@ -3570,6 +3567,11 @@ CloneFunctionObject(JSContext* cx, HandleObject funobj, HandleObject env, Handle return nullptr; } + if (fun->isNative()) { + JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, JSMSG_CANT_CLONE_OBJECT); + return nullptr; + } + if (!IsFunctionCloneable(fun)) { JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, JSMSG_BAD_CLONE_FUNOBJ_SCOPE); return nullptr; @@ -3580,21 +3582,6 @@ CloneFunctionObject(JSContext* cx, HandleObject funobj, HandleObject env, Handle return nullptr; } - if (IsAsmJSModule(fun)) { - JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, JSMSG_CANT_CLONE_OBJECT); - return nullptr; - } - - if (IsWrappedAsyncFunction(fun)) { - JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, JSMSG_CANT_CLONE_OBJECT); - return nullptr; - } - - if (IsWrappedAsyncGenerator(fun)) { - JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, JSMSG_CANT_CLONE_OBJECT); - return nullptr; - } - if (CanReuseScriptForClone(cx->compartment(), fun, env)) { // If the script is to be reused, either the script can already handle // non-syntactic scopes, or there is only the standard global lexical diff --git a/js/src/jsfun.cpp b/js/src/jsfun.cpp index 67df78c2f1..bd133b822e 100644 --- a/js/src/jsfun.cpp +++ b/js/src/jsfun.cpp @@ -50,6 +50,7 @@ #include "vm/StringBuffer.h" #include "vm/WrapperObject.h" #include "vm/Xdr.h" +#include "wasm/AsmJS.h" #include "jsscriptinlines.h" @@ -2026,6 +2027,8 @@ bool js::CanReuseScriptForClone(JSCompartment* compartment, HandleFunction fun, HandleObject newParent) { + MOZ_ASSERT(fun->isInterpreted()); + if (compartment != fun->compartment() || fun->isSingleton() || ObjectGroup::useSingletonForClone(fun)) @@ -2044,12 +2047,11 @@ js::CanReuseScriptForClone(JSCompartment* compartment, HandleFunction fun, if (IsSyntacticEnvironment(newParent)) return true; - // We need to clone the script if we're interpreted and not already marked - // as having a non-syntactic scope. If we're lazy, go ahead and clone the - // script; see the big comment at the end of CopyScriptInternal for the - // explanation of what's going on there. - return !fun->isInterpreted() || - (fun->hasScript() && fun->nonLazyScript()->hasNonSyntacticScope()); + // We need to clone the script if we're not already marked as having a + // non-syntactic scope. If we're lazy, go ahead and clone the script; see + // the big comment at the end of CopyScriptInternal for the explanation of + // what's going on there. + return fun->hasScript() && fun->nonLazyScript()->hasNonSyntacticScope(); } static inline JSFunction* @@ -2096,6 +2098,7 @@ js::CloneFunctionReuseScript(JSContext* cx, HandleFunction fun, HandleObject enc HandleObject proto /* = nullptr */) { MOZ_ASSERT(NewFunctionEnvironmentIsWellFormed(cx, enclosingEnv)); + MOZ_ASSERT(fun->isInterpreted()); MOZ_ASSERT(!fun->isBoundFunction()); MOZ_ASSERT(CanReuseScriptForClone(cx->compartment(), fun, enclosingEnv)); @@ -2106,13 +2109,12 @@ js::CloneFunctionReuseScript(JSContext* cx, HandleFunction fun, HandleObject enc if (fun->hasScript()) { clone->initScript(fun->nonLazyScript()); clone->initEnvironment(enclosingEnv); - } else if (fun->isInterpretedLazy()) { + } else { + MOZ_ASSERT(fun->isInterpretedLazy()); MOZ_ASSERT(fun->compartment() == clone->compartment()); LazyScript* lazy = fun->lazyScriptOrNull(); clone->initLazyScript(lazy); clone->initEnvironment(enclosingEnv); - } else { - clone->initNative(fun->native(), fun->jitInfo()); } /* @@ -2130,25 +2132,19 @@ js::CloneFunctionAndScript(JSContext* cx, HandleFunction fun, HandleObject enclo HandleObject proto /* = nullptr */) { MOZ_ASSERT(NewFunctionEnvironmentIsWellFormed(cx, enclosingEnv)); + MOZ_ASSERT(fun->isInterpreted()); MOZ_ASSERT(!fun->isBoundFunction()); - JSScript::AutoDelazify funScript(cx); - if (fun->isInterpreted()) { - funScript = fun; - if (!funScript) - return nullptr; - } + JSScript::AutoDelazify funScript(cx, fun); + if (!funScript) + return nullptr; RootedFunction clone(cx, NewFunctionClone(cx, fun, SingletonObject, allocKind, proto)); if (!clone) return nullptr; - if (fun->hasScript()) { - clone->initScript(nullptr); - clone->initEnvironment(enclosingEnv); - } else { - clone->initNative(fun->native(), fun->jitInfo()); - } + clone->initScript(nullptr); + clone->initEnvironment(enclosingEnv); /* * Across compartments or if we have to introduce a non-syntactic scope we @@ -2165,21 +2161,57 @@ js::CloneFunctionAndScript(JSContext* cx, HandleFunction fun, HandleObject enclo newScope->hasOnChain(ScopeKind::NonSyntactic)); #endif - if (clone->isInterpreted()) { - RootedScript script(cx, fun->nonLazyScript()); - MOZ_ASSERT(script->compartment() == fun->compartment()); - MOZ_ASSERT(cx->compartment() == clone->compartment(), - "Otherwise we could relazify clone below!"); + RootedScript script(cx, fun->nonLazyScript()); + MOZ_ASSERT(script->compartment() == fun->compartment()); + MOZ_ASSERT(cx->compartment() == clone->compartment(), + "Otherwise we could relazify clone below!"); - RootedScript clonedScript(cx, CloneScriptIntoFunction(cx, newScope, clone, script)); - if (!clonedScript) - return nullptr; - Debugger::onNewScript(cx, clonedScript); - } + RootedScript clonedScript(cx, CloneScriptIntoFunction(cx, newScope, clone, script)); + if (!clonedScript) + return nullptr; + Debugger::onNewScript(cx, clonedScript); return clone; } +JSFunction* +js::CloneAsmJSModuleFunction(JSContext* cx, HandleFunction fun) +{ + MOZ_ASSERT(fun->isNative()); + MOZ_ASSERT(IsAsmJSModule(fun)); + MOZ_ASSERT(fun->isExtended()); + MOZ_ASSERT(cx->compartment() == fun->compartment()); + + JSFunction* clone = NewFunctionClone(cx, fun, GenericObject, AllocKind::FUNCTION_EXTENDED, + /* proto = */ nullptr); + if (!clone) + return nullptr; + + MOZ_ASSERT(fun->native() == InstantiateAsmJS); + MOZ_ASSERT(!fun->jitInfo()); + clone->initNative(InstantiateAsmJS, nullptr); + + clone->setGroup(fun->group()); + return clone; +} + +JSFunction* +js::CloneSelfHostingIntrinsic(JSContext* cx, HandleFunction fun) +{ + MOZ_ASSERT(fun->isNative()); + MOZ_ASSERT(fun->compartment()->isSelfHosting); + MOZ_ASSERT(!fun->isExtended()); + MOZ_ASSERT(cx->compartment() != fun->compartment()); + + JSFunction* clone = NewFunctionClone(cx, fun, SingletonObject, AllocKind::FUNCTION, + /* proto = */ nullptr); + if (!clone) + return nullptr; + + clone->initNative(fun->native(), fun->jitInfo()); + return clone; +} + /* * Return an atom for use as the name of a builtin method with the given * property id. diff --git a/js/src/jsfun.h b/js/src/jsfun.h index 1833aaeea5..49aac71654 100644 --- a/js/src/jsfun.h +++ b/js/src/jsfun.h @@ -824,6 +824,12 @@ CloneFunctionAndScript(JSContext* cx, HandleFunction fun, HandleObject parent, gc::AllocKind kind = gc::AllocKind::FUNCTION, HandleObject proto = nullptr); +extern JSFunction* +CloneAsmJSModuleFunction(JSContext* cx, HandleFunction fun); + +extern JSFunction* +CloneSelfHostingIntrinsic(JSContext* cx, HandleFunction fun); + } // namespace js inline js::FunctionExtended* diff --git a/js/src/vm/Interpreter.cpp b/js/src/vm/Interpreter.cpp index d7c1b8e84a..3515a9336b 100644 --- a/js/src/vm/Interpreter.cpp +++ b/js/src/vm/Interpreter.cpp @@ -4302,7 +4302,13 @@ js::Lambda(JSContext* cx, HandleFunction fun, HandleObject parent) { MOZ_ASSERT(!fun->isArrow()); - RootedObject clone(cx, CloneFunctionObjectIfNotSingleton(cx, fun, parent)); + JSFunction* clone; + if (fun->isNative()) { + MOZ_ASSERT(IsAsmJSModule(fun)); + clone = CloneAsmJSModuleFunction(cx, fun); + } else { + clone = CloneFunctionObjectIfNotSingleton(cx, fun, parent); + } if (!clone) return nullptr; diff --git a/js/src/vm/SelfHosting.cpp b/js/src/vm/SelfHosting.cpp index 357e151bde..c1276f5a43 100644 --- a/js/src/vm/SelfHosting.cpp +++ b/js/src/vm/SelfHosting.cpp @@ -2909,23 +2909,29 @@ CloneObject(JSContext* cx, HandleNativeObject selfHostedObject) RootedObject clone(cx); if (selfHostedObject->is()) { RootedFunction selfHostedFunction(cx, &selfHostedObject->as()); - bool hasName = selfHostedFunction->explicitName() != nullptr; + 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(); - MOZ_ASSERT(!CanReuseScriptForClone(cx->compartment(), selfHostedFunction, cx->global())); - Rooted globalLexical(cx, &cx->global()->lexicalEnvironment()); - RootedScope emptyGlobalScope(cx, &cx->global()->emptyGlobalScope()); - 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())); + // 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(); + + Handle global = cx->global(); + Rooted globalLexical(cx, &global->lexicalEnvironment()); + RootedScope emptyGlobalScope(cx, &global->emptyGlobalScope()); + MOZ_ASSERT(!CanReuseScriptForClone(cx->compartment(), selfHostedFunction, global)); + 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())); + } + } else { + clone = CloneSelfHostingIntrinsic(cx, selfHostedFunction); } } else if (selfHostedObject->is()) { RegExpObject& reobj = selfHostedObject->as(); diff --git a/js/src/wasm/AsmJS.cpp b/js/src/wasm/AsmJS.cpp index a56d4a3830..534fc5f699 100644 --- a/js/src/wasm/AsmJS.cpp +++ b/js/src/wasm/AsmJS.cpp @@ -8100,8 +8100,8 @@ AsmJSModuleFunctionToModule(JSFunction* fun) } // Implements the semantics of an asm.js module function that has been successfully validated. -static bool -InstantiateAsmJS(JSContext* cx, unsigned argc, JS::Value* vp) +bool +js::InstantiateAsmJS(JSContext* cx, unsigned argc, JS::Value* vp) { CallArgs args = CallArgsFromVp(argc, vp); diff --git a/js/src/wasm/AsmJS.h b/js/src/wasm/AsmJS.h index a38b204a8b..296617c79b 100644 --- a/js/src/wasm/AsmJS.h +++ b/js/src/wasm/AsmJS.h @@ -56,6 +56,9 @@ IsAsmJSFunction(JSFunction* fun); extern bool IsAsmJSStrictModeModuleOrFunction(JSFunction* fun); +extern bool +InstantiateAsmJS(JSContext* cx, unsigned argc, JS::Value* vp); + // asm.js testing natives: extern bool From 76052fcda88a4c0d0c916ffd84abbbb2ee614c5b Mon Sep 17 00:00:00 2001 From: Martok Date: Thu, 13 Jul 2023 02:47:15 +0200 Subject: [PATCH 7/7] 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);