Bug 1680848 - Don't allow cloning to change non-synactic flag. r=jandem

Assert that uses of `CloneAndExecuteScript` have a consistent NonSyntactic
flag. These APIs still perform cloning for the cross-realm cases. The shell
functions need small tweaks to support this invariant.

Standalone functions are not cloned and do no update their environment so if
they had Syntactic global scope they will always continue to, and otherwise
will stick in non-syntactic mode.

Differential Revision: https://phabricator.services.mozilla.com/D98825

dom/base/JSExecutionContext.cpp

@@ -152,6 +152,12 @@ nsresult JSExecutionContext::InternalCompile(
   MOZ_ASSERT(!mScript);
 
   if (mScopeChain.length() != 0) {
+    // Serialized bytecode should not be mixed with non-syntactic mode.
+    // Currently, mScopeChain is only used by nsNPAPIPlugin which does not
+    // support bytecode caching. This will all be removed in Bug 1689348.
+    MOZ_ASSERT(!mEncodeBytecode);
+    MOZ_ASSERT(mExpectScopeChain);
+
     aCompileOptions.setNonSyntacticScope(true);
   }
 
@@ -270,7 +276,8 @@ nsresult JSExecutionContext::ExecScript() {
 
   MOZ_ASSERT(mScript);
 
-  if (!JS_ExecuteScript(mCx, mScopeChain, mScript)) {
+  if (!(mScopeChain.empty() ? JS_ExecuteScript(mCx, mScript)
+                            : JS_ExecuteScript(mCx, mScopeChain, mScript))) {
     mSkip = true;
     mRv = EvaluationExceptionToNSResult(mCx);
     return mRv;
@@ -303,7 +310,9 @@ nsresult JSExecutionContext::ExecScript(
   MOZ_ASSERT(mScript);
   MOZ_ASSERT(mWantsReturnValue);
 
-  if (!JS_ExecuteScript(mCx, mScopeChain, mScript, aRetValue)) {
+  if (!(mScopeChain.empty()
+            ? JS_ExecuteScript(mCx, mScript, aRetValue)
+            : JS_ExecuteScript(mCx, mScopeChain, mScript, aRetValue))) {
     mSkip = true;
     mRv = EvaluationExceptionToNSResult(mCx);
     return mRv;

js/src/builtin/Eval.cpp

@@ -390,7 +390,7 @@ static bool ExecuteInExtensibleLexicalEnvironment(JSContext* cx,
 
   RootedScript script(cx, scriptArg);
   if (script->realm() != cx->realm()) {
-    script = CloneGlobalScript(cx, ScopeKind::NonSyntactic, script);
+    script = CloneGlobalScript(cx, script);
     if (!script) {
       return false;
     }

js/src/shell/js.cpp

@@ -2430,7 +2430,9 @@ static bool Evaluate(JSContext* cx, unsigned argc, Value* vp) {
       }
     }
 
-    if (!JS_ExecuteScript(cx, envChain, script, args.rval())) {
+    if (!(envChain.empty()
+              ? JS_ExecuteScript(cx, script, args.rval())
+              : JS_ExecuteScript(cx, envChain, script, args.rval()))) {
       if (catchTermination && !JS_IsExceptionPending(cx)) {
         JSAutoRealm ar1(cx, callerGlobal);
         JSString* str = JS_NewStringCopyZ(cx, "terminated");

js/src/vm/CompilationAndEvaluation.cpp

@@ -320,9 +320,17 @@ class FunctionCompiler {
 
     RootedObject enclosingEnv(cx_);
     RootedScope enclosingScope(cx_);
-    if (!CreateNonSyntacticEnvironmentChain(cx_, envChain, &enclosingEnv,
-                                            &enclosingScope)) {
-      return nullptr;
+    if (envChain.empty()) {
+      // A compiled function has a burned-in environment chain, so if no exotic
+      // environment was requested, we can use the global lexical environment
+      // directly and not need to worry about any potential non-syntactic scope.
+      enclosingEnv.set(&cx_->global()->lexicalEnvironment());
+      enclosingScope.set(&cx_->global()->emptyGlobalScope());
+    } else {
+      if (!CreateNonSyntacticEnvironmentChain(cx_, envChain, &enclosingEnv,
+                                              &enclosingScope)) {
+        return nullptr;
+      }
     }
 
     cx_->check(enclosingEnv);
@@ -418,27 +426,22 @@ MOZ_NEVER_INLINE static bool ExecuteScript(JSContext* cx, HandleObject envChain,
   AssertHeapIsIdle();
   CHECK_THREAD(cx);
   cx->check(envChain, script);
-  MOZ_ASSERT_IF(!IsGlobalLexicalEnvironment(envChain),
-                script->hasNonSyntacticScope());
+
+  if (!IsGlobalLexicalEnvironment(envChain)) {
+    MOZ_RELEASE_ASSERT(script->hasNonSyntacticScope());
+  }
+
   return Execute(cx, script, envChain, rval);
 }
 
 static bool ExecuteScript(JSContext* cx, HandleObjectVector envChain,
-                          HandleScript scriptArg, MutableHandleValue rval) {
+                          HandleScript script, MutableHandleValue rval) {
   RootedObject env(cx);
   RootedScope dummy(cx);
   if (!CreateNonSyntacticEnvironmentChain(cx, envChain, &env, &dummy)) {
     return false;
   }
 
-  RootedScript script(cx, scriptArg);
-  if (!script->hasNonSyntacticScope() && !IsGlobalLexicalEnvironment(env)) {
-    script = CloneGlobalScript(cx, ScopeKind::NonSyntactic, script);
-    if (!script) {
-      return false;
-    }
-  }
-
   return ExecuteScript(cx, env, script, rval);
 }
 
@@ -475,7 +478,7 @@ JS_PUBLIC_API bool JS::CloneAndExecuteScript(JSContext* cx,
   RootedScript script(cx, scriptArg);
   RootedObject globalLexical(cx, &cx->global()->lexicalEnvironment());
   if (script->realm() != cx->realm()) {
-    script = CloneGlobalScript(cx, ScopeKind::Global, script);
+    script = CloneGlobalScript(cx, script);
     if (!script) {
       return false;
     }
@@ -488,9 +491,10 @@ JS_PUBLIC_API bool JS::CloneAndExecuteScript(JSContext* cx,
                                              HandleScript scriptArg,
                                              JS::MutableHandleValue rval) {
   CHECK_THREAD(cx);
+  MOZ_RELEASE_ASSERT(scriptArg->hasNonSyntacticScope());
   RootedScript script(cx, scriptArg);
   if (script->realm() != cx->realm()) {
-    script = CloneGlobalScript(cx, ScopeKind::NonSyntactic, script);
+    script = CloneGlobalScript(cx, script);
     if (!script) {
       return false;
     }

js/src/vm/EnvironmentObject.cpp

@@ -876,46 +876,42 @@ bool js::CreateNonSyntacticEnvironmentChain(JSContext* cx,
                                             HandleObjectVector envChain,
                                             MutableHandleObject env,
                                             MutableHandleScope scope) {
+  // Callers are responsible for segregating the NonSyntactic case from simple
+  // compilation cases.
+  MOZ_RELEASE_ASSERT(!envChain.empty());
+
   RootedObject globalLexical(cx, &cx->global()->lexicalEnvironment());
   if (!CreateObjectsForEnvironmentChain(cx, envChain, globalLexical, env)) {
     return false;
   }
 
-  if (!envChain.empty()) {
-    scope.set(GlobalScope::createEmpty(cx, ScopeKind::NonSyntactic));
-    if (!scope) {
-      return false;
-    }
-
-    // The XPConnect subscript loader, which may pass in its own
-    // environments to load scripts in, expects the environment chain to
-    // be the holder of "var" declarations. In SpiderMonkey, such objects
-    // are called "qualified varobjs", the "qualified" part meaning the
-    // declaration was qualified by "var". There is only sadness.
-    //
-    // See JSObject::isQualifiedVarObj.
-    if (!JSObject::setQualifiedVarObj(cx, env)) {
-      return false;
-    }
-
-    // Also get a non-syntactic lexical environment to capture 'let' and
-    // 'const' bindings. To persist lexical bindings, we have a 1-1
-    // mapping with the final unwrapped environment object (the
-    // environment that stores the 'var' bindings) and the lexical
-    // environment.
-    //
-    // TODOshu: disallow the subscript loader from using non-distinguished
-    // objects as dynamic scopes.
-    env.set(ObjectRealm::get(env).getOrCreateNonSyntacticLexicalEnvironment(
-        cx, env));
-    if (!env) {
-      return false;
-    }
-  } else {
-    scope.set(&cx->global()->emptyGlobalScope());
+  scope.set(GlobalScope::createEmpty(cx, ScopeKind::NonSyntactic));
+  if (!scope) {
+    return false;
   }
 
-  return true;
+  // The XPConnect subscript loader, which may pass in its own
+  // environments to load scripts in, expects the environment chain to
+  // be the holder of "var" declarations. In SpiderMonkey, such objects
+  // are called "qualified varobjs", the "qualified" part meaning the
+  // declaration was qualified by "var". There is only sadness.
+  //
+  // See JSObject::isQualifiedVarObj.
+  if (!JSObject::setQualifiedVarObj(cx, env)) {
+    return false;
+  }
+
+  // Also get a non-syntactic lexical environment to capture 'let' and
+  // 'const' bindings. To persist lexical bindings, we have a 1-1
+  // mapping with the final unwrapped environment object (the
+  // environment that stores the 'var' bindings) and the lexical
+  // environment.
+  //
+  // TODOshu: disallow the subscript loader from using non-distinguished
+  // objects as dynamic scopes.
+  env.set(
+      ObjectRealm::get(env).getOrCreateNonSyntacticLexicalEnvironment(cx, env));
+  return !!env;
 }
 
 /*****************************************************************************/

js/src/vm/JSFunction.cpp

@@ -2233,22 +2233,6 @@ JSFunction* js::CloneFunctionAndScript(JSContext* cx, HandleFunction fun,
   clone->initScript(nullptr);
   clone->initEnvironment(enclosingEnv);
 
-  /*
-   * Across compartments or if we have to introduce a non-syntactic scope we
-   * have to clone the script for interpreted functions. Cross-compartment
-   * cloning only happens via JSAPI (JS::CloneFunctionObject) which
-   * dynamically ensures that 'script' has no enclosing lexical scope (only
-   * the global scope or other non-lexical scope).
-   */
-#ifdef DEBUG
-  RootedObject terminatingEnv(cx, enclosingEnv);
-  while (IsSyntacticEnvironment(terminatingEnv)) {
-    terminatingEnv = terminatingEnv->enclosingEnvironment();
-  }
-  MOZ_ASSERT_IF(!terminatingEnv->is<GlobalObject>(),
-                newScope->hasOnChain(ScopeKind::NonSyntactic));
-#endif
-
   RootedScript script(cx, fun->nonLazyScript());
   MOZ_ASSERT(script->realm() == fun->realm());
   MOZ_ASSERT(cx->compartment() == clone->compartment(),

js/src/vm/JSScript.cpp

@@ -4555,10 +4555,10 @@ static JSScript* CopyScriptImpl(JSContext* cx, HandleScript src,
   return dst;
 }
 
-JSScript* js::CloneGlobalScript(JSContext* cx, ScopeKind scopeKind,
-                                HandleScript src) {
-  MOZ_ASSERT(scopeKind == ScopeKind::Global ||
-             scopeKind == ScopeKind::NonSyntactic);
+JSScript* js::CloneGlobalScript(JSContext* cx, HandleScript src) {
+  MOZ_ASSERT(src->realm() != cx->realm(),
+             "js::CloneGlobalScript should only be used for for realm "
+             "mismatches. Otherwise just share the script directly.");
 
   Rooted<ScriptSourceObject*> sourceObject(cx, src->sourceObject());
   if (cx->compartment() != sourceObject->compartment()) {
@@ -4571,7 +4571,7 @@ JSScript* js::CloneGlobalScript(JSContext* cx, ScopeKind scopeKind,
   MOZ_ASSERT(src->bodyScopeIndex() == GCThingIndex::outermostScopeIndex());
   Rooted<GCVector<Scope*>> scopes(cx, GCVector<Scope*>(cx));
   Rooted<GlobalScope*> original(cx, &src->bodyScope()->as<GlobalScope>());
-  GlobalScope* clone = GlobalScope::clone(cx, original, scopeKind);
+  GlobalScope* clone = GlobalScope::clone(cx, original);
   if (!clone || !scopes.append(clone)) {
     return nullptr;
   }
@@ -4597,6 +4597,10 @@ JSScript* js::CloneScriptIntoFunction(JSContext* cx, HandleScope enclosingScope,
                                       HandleFunction fun, HandleScript src,
                                       Handle<ScriptSourceObject*> sourceObject,
                                       SourceExtent* maybeClassExtent) {
+  MOZ_ASSERT(src->realm() != cx->realm(),
+             "js::CloneScriptIntoFunction should only be used for for realm "
+             "mismatches. Otherwise just share the script directly.");
+
   // We are either delazifying a self-hosted lazy function or the function
   // should be in an inactive state.
   MOZ_ASSERT(fun->isIncomplete() || fun->hasSelfHostedLazyScript());

js/src/vm/JSScript.h

@@ -2477,8 +2477,7 @@ JSScript* CloneScriptIntoFunction(JSContext* cx, HandleScope enclosingScope,
                                   Handle<ScriptSourceObject*> sourceObject,
                                   SourceExtent* maybeClassExtent = nullptr);
 
-JSScript* CloneGlobalScript(JSContext* cx, ScopeKind scopeKind,
-                            HandleScript src);
+JSScript* CloneGlobalScript(JSContext* cx, HandleScript src);
 
 bool CheckCompileOptionsMatch(const JS::ReadOnlyCompileOptions& options,
                               js::ImmutableScriptFlags flags,

js/src/vm/Scope.cpp

@@ -1197,15 +1197,15 @@ GlobalScope* GlobalScope::createWithData(
 }
 
 /* static */
-GlobalScope* GlobalScope::clone(JSContext* cx, Handle<GlobalScope*> scope,
-                                ScopeKind kind) {
+GlobalScope* GlobalScope::clone(JSContext* cx, Handle<GlobalScope*> scope) {
   Rooted<RuntimeData*> dataOriginal(cx, &scope->as<GlobalScope>().data());
   Rooted<UniquePtr<RuntimeData>> dataClone(
       cx, CopyScopeData<GlobalScope>(cx, dataOriginal));
   if (!dataClone) {
     return nullptr;
   }
-  return Scope::create<GlobalScope>(cx, kind, nullptr, nullptr, &dataClone);
+  return Scope::create<GlobalScope>(cx, scope->kind(), nullptr, nullptr,
+                                    &dataClone);
 }
 
 template <XDRMode mode>

js/src/vm/Scope.h

@@ -885,8 +885,7 @@ class GlobalScope : public Scope {
     return create(cx, kind, nullptr);
   }
 
-  static GlobalScope* clone(JSContext* cx, Handle<GlobalScope*> scope,
-                            ScopeKind kind);
+  static GlobalScope* clone(JSContext* cx, Handle<GlobalScope*> scope);
 
   template <XDRMode mode>
   static XDRResult XDR(XDRState<mode>* xdr, ScopeKind kind,