Bug 1667258 - Remove support for RealmCreationOptions::cloneSingletons. r=jandem

Script object-literal singletons are only generated for top-level run-once
scripts which both XDR and cloning no longer need to support. As a result,
the `cloneSingletons` mechanism is no longer needed and can be removed.

We can simplify the Interpreter and JITs handling of JSOp::Object to no
longer worry about cloneSingletons as a result. They also lets us remove the
`setSingletonsAsValues` code since we no longer have realm-wide poison bits.

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

js/public/RealmOptions.h

@@ -129,12 +129,6 @@ class JS_PUBLIC_API RealmCreationOptions {
     return *this;
   }
 
-  bool cloneSingletons() const { return cloneSingletons_; }
-  RealmCreationOptions& setCloneSingletons(bool flag) {
-    cloneSingletons_ = flag;
-    return *this;
-  }
-
   // Determines whether 1) the global Atomic property is defined and atomic
   // operations are supported, and 2) whether shared-memory operations are
   // supported.
@@ -272,7 +266,6 @@ class JS_PUBLIC_API RealmCreationOptions {
   bool invisibleToDebugger_ = false;
   bool mergeable_ = false;
   bool preserveJitCode_ = false;
-  bool cloneSingletons_ = false;
   bool sharedMemoryAndAtomics_ = false;
   bool defineSharedArrayBufferConstructor_ = true;
   bool coopAndCoep_ = false;
@@ -338,12 +331,6 @@ class JS_PUBLIC_API RealmBehaviors {
     Mode mode_;
   };
 
-  bool getSingletonsAsTemplates() const { return singletonsAsTemplates_; }
-  RealmBehaviors& setSingletonsAsValues() {
-    singletonsAsTemplates_ = false;
-    return *this;
-  }
-
   // A Realm can stop being "live" in all the ways that matter before its global
   // is actually GCed.  Consumers that tear down parts of a Realm or its global
   // before that point should set isNonLive accordingly.
@@ -357,11 +344,6 @@ class JS_PUBLIC_API RealmBehaviors {
   bool discardSource_ = false;
   bool disableLazyParsing_ = false;
   bool clampAndJitterTime_ = true;
-
-  // To XDR singletons, we need to ensure that all singletons are all used as
-  // templates, by making JSOP_OBJECT return a clone of the JSScript
-  // singleton, instead of returning the value which is baked in the JSScript.
-  bool singletonsAsTemplates_ = true;
   bool isNonLive_ = false;
 };
 

js/src/jit/BaselineCodeGen.cpp

@@ -913,6 +913,8 @@ static gc::Cell* GetScriptGCThing(JSScript* script, jsbytecode* pc,
       return script->getAtom(pc);
     case ScriptGCThingType::RegExp:
       return script->getRegExp(pc);
+    case ScriptGCThingType::Object:
+      return script->getObject(pc);
     case ScriptGCThingType::Function:
       return script->getFunction(pc);
     case ScriptGCThingType::Scope:
@@ -957,6 +959,7 @@ void BaselineInterpreterCodeGen::loadScriptGCThing(ScriptGCThingType type,
       masm.xorPtr(Imm32(2), dest);
       break;
     case ScriptGCThingType::RegExp:
+    case ScriptGCThingType::Object:
     case ScriptGCThingType::Function:
       // No-op because GCCellPtr tag bits are zero for objects.
       static_assert(uintptr_t(TraceKind::Object) == 0,
@@ -2602,38 +2605,18 @@ bool BaselineInterpreterCodeGen::emit_Symbol() {
   return true;
 }
 
-JSObject* BaselineCompilerHandler::maybeNoCloneSingletonObject() {
-  Realm* realm = script()->realm();
-  if (realm->creationOptions().cloneSingletons()) {
-    return nullptr;
-  }
-
-  realm->behaviors().setSingletonsAsValues();
-  return script()->getObject(pc());
+template <>
+bool BaselineCompilerCodeGen::emit_Object() {
+  frame.push(ObjectValue(*handler.script()->getObject(handler.pc())));
+  return true;
 }
 
-template <typename Handler>
-bool BaselineCodeGen<Handler>::emit_Object() {
-  // If we know we don't have to clone the object literal, just push it
-  // directly. Note that the interpreter always does the VM call; that's fine
-  // because this op is only used in run-once code.
-  if (JSObject* obj = handler.maybeNoCloneSingletonObject()) {
-    frame.push(ObjectValue(*obj));
-    return true;
-  }
-
-  prepareVMCall();
-
-  pushBytecodePCArg();
-  pushScriptArg();
-
-  using Fn = JSObject* (*)(JSContext*, HandleScript, jsbytecode*);
-  if (!callVM<Fn, SingletonObjectLiteralOperation>()) {
-    return false;
-  }
-
-  // Box and push return value.
-  masm.tagValue(JSVAL_TYPE_OBJECT, ReturnReg, R0);
+template <>
+bool BaselineInterpreterCodeGen::emit_Object() {
+  Register scratch1 = R0.scratchReg();
+  Register scratch2 = R1.scratchReg();
+  loadScriptGCThing(ScriptGCThingType::Object, scratch1, scratch2);
+  masm.tagValue(JSVAL_TYPE_OBJECT, scratch1, R0);
   frame.push(R0);
   return true;
 }

js/src/jit/BaselineCodeGen.h

@@ -18,7 +18,7 @@ namespace js {
 
 namespace jit {
 
-enum class ScriptGCThingType { Atom, RegExp, Function, Scope, BigInt };
+enum class ScriptGCThingType { Atom, RegExp, Object, Function, Scope, BigInt };
 
 // Base class for BaselineCompiler and BaselineInterpreterGenerator. The Handler
 // template is a class storing fields/methods that are interpreter or compiler
@@ -359,8 +359,6 @@ class BaselineCompilerHandler {
     static constexpr size_t NumSlotsLimit = 128;
     return script()->nslots() > NumSlotsLimit;
   }
-
-  JSObject* maybeNoCloneSingletonObject();
 };
 
 using BaselineCompilerCodeGen = BaselineCodeGen<BaselineCompilerHandler>;
@@ -482,8 +480,6 @@ class BaselineInterpreterHandler {
   // The interpreter doesn't know the number of slots statically so we always
   // include them.
   bool mustIncludeSlotsInStackCheck() const { return true; }
-
-  JSObject* maybeNoCloneSingletonObject() { return nullptr; }
 };
 
 using BaselineInterpreterCodeGen = BaselineCodeGen<BaselineInterpreterHandler>;

js/src/jit/CodeGenerator.cpp

@@ -3865,13 +3865,6 @@ void CodeGenerator::visitTableSwitchV(LTableSwitchV* ins) {
   emitTableSwitchDispatch(mir, index, ToRegisterOrInvalid(ins->tempPointer()));
 }
 
-void CodeGenerator::visitCloneLiteral(LCloneLiteral* lir) {
-  pushArg(ToRegister(lir->getObjectLiteral()));
-
-  using Fn = JSObject* (*)(JSContext*, HandleObject);
-  callVM<Fn, DeepCloneObjectLiteral>(lir);
-}
-
 void CodeGenerator::visitParameter(LParameter* lir) {}
 
 void CodeGenerator::visitCallee(LCallee* lir) {

js/src/jit/CompileWrappers.cpp

@@ -207,24 +207,11 @@ bool CompileRealm::hasAllocationMetadataBuilder() {
   return realm()->hasAllocationMetadataBuilder();
 }
 
-// Note: This function is thread-safe because setSingletonAsValue sets a boolean
-// variable to false, and this boolean variable has no way to be resetted to
-// true. So even if there is a concurrent write, this concurrent write will
-// always have the same value.  If there is a concurrent read, then we will
-// clone a singleton instead of using the value which is baked in the JSScript,
-// and this would be an unfortunate allocation, but this will not change the
-// semantics of the JavaScript code which is executed.
-void CompileRealm::setSingletonsAsValues() {
-  realm()->behaviors().setSingletonsAsValues();
-}
-
 JitCompileOptions::JitCompileOptions()
-    : cloneSingletons_(false),
-      profilerSlowAssertionsEnabled_(false),
+    : profilerSlowAssertionsEnabled_(false),
       offThreadCompilationAvailable_(false) {}
 
 JitCompileOptions::JitCompileOptions(JSContext* cx) {
-  cloneSingletons_ = cx->realm()->creationOptions().cloneSingletons();
   profilerSlowAssertionsEnabled_ =
       cx->runtime()->geckoProfiler().enabled() &&
       cx->runtime()->geckoProfiler().slowAssertionsEnabled();

js/src/jit/CompileWrappers.h

@@ -109,9 +109,6 @@ class CompileRealm {
   const uint32_t* addressOfGlobalWriteBarriered();
 
   bool hasAllocationMetadataBuilder();
-
-  // Mirror RealmOptions.
-  void setSingletonsAsValues();
 };
 
 class JitCompileOptions {
@@ -119,8 +116,6 @@ class JitCompileOptions {
   JitCompileOptions();
   explicit JitCompileOptions(JSContext* cx);
 
-  bool cloneSingletons() const { return cloneSingletons_; }
-
   bool profilerSlowAssertionsEnabled() const {
     return profilerSlowAssertionsEnabled_;
   }
@@ -134,7 +129,6 @@ class JitCompileOptions {
 #endif
 
  private:
-  bool cloneSingletons_;
   bool profilerSlowAssertionsEnabled_;
   bool offThreadCompilationAvailable_;
 #ifdef DEBUG

js/src/jit/IonBuilder.cpp

@@ -11648,15 +11648,6 @@ AbortReasonOr<Ok> IonBuilder::jsop_regexp(RegExpObject* reobj) {
 }
 
 AbortReasonOr<Ok> IonBuilder::jsop_object(JSObject* obj) {
-  if (mirGen_.options.cloneSingletons()) {
-    MCloneLiteral* clone =
-        MCloneLiteral::New(alloc(), constant(ObjectValue(*obj)));
-    current->add(clone);
-    current->push(clone);
-    return resumeAfter(clone);
-  }
-
-  realm->setSingletonsAsValues();
   pushConstant(ObjectValue(*obj));
   return Ok();
 }

js/src/jit/Lowering.cpp

@@ -43,16 +43,6 @@ LBoxAllocation LIRGenerator::useBoxAtStart(MDefinition* mir,
   return useBox(mir, policy, /* useAtStart = */ true);
 }
 
-void LIRGenerator::visitCloneLiteral(MCloneLiteral* ins) {
-  MOZ_ASSERT(ins->type() == MIRType::Object);
-  MOZ_ASSERT(ins->input()->type() == MIRType::Object);
-
-  LCloneLiteral* lir =
-      new (alloc()) LCloneLiteral(useRegisterAtStart(ins->input()));
-  defineReturn(lir, ins);
-  assignSafepoint(lir, ins);
-}
-
 void LIRGenerator::visitParameter(MParameter* param) {
   ptrdiff_t offset;
   if (param->index() == MParameter::THIS_SLOT) {

js/src/jit/MIR.h

@@ -1679,19 +1679,6 @@ class MWasmFloatConstant : public MNullaryInstruction {
 #endif
 };
 
-// Deep clone a constant JSObject.
-class MCloneLiteral : public MUnaryInstruction, public ObjectPolicy<0>::Data {
- protected:
-  explicit MCloneLiteral(MDefinition* obj)
-      : MUnaryInstruction(classOpcode, obj) {
-    setResultType(MIRType::Object);
-  }
-
- public:
-  INSTRUCTION_HEADER(CloneLiteral)
-  TRIVIAL_NEW_WRAPPERS
-};
-
 class MParameter : public MNullaryInstruction {
   int32_t index_;
 

js/src/jit/VMFunctionList-inl.h

@@ -108,7 +108,6 @@ namespace jit {
   _(DebugLeaveThenRecreateLexicalEnv,                                          \
     js::jit::DebugLeaveThenRecreateLexicalEnv)                                 \
   _(Debug_CheckSelfHosted, js::Debug_CheckSelfHosted)                          \
-  _(DeepCloneObjectLiteral, js::DeepCloneObjectLiteral)                        \
   _(DefFunOperation, js::DefFunOperation)                                      \
   _(DefLexicalOperation, js::DefLexicalOperation)                              \
   _(DefVarOperation, js::DefVarOperation)                                      \
@@ -243,7 +242,6 @@ namespace jit {
   _(SetObjectElementWithReceiver, js::SetObjectElementWithReceiver)            \
   _(SetProperty, js::jit::SetProperty)                                         \
   _(SetPropertySuper, js::SetPropertySuper)                                    \
-  _(SingletonObjectLiteralOperation, js::SingletonObjectLiteralOperation)      \
   _(StartDynamicModuleImport, js::StartDynamicModuleImport)                    \
   _(StrictlyEqual, js::jit::StrictlyEqual<js::jit::EqualityKind::Equal>)       \
   _(StrictlyNotEqual, js::jit::StrictlyEqual<js::jit::EqualityKind::NotEqual>) \

js/src/jit/WarpBuilder.cpp

@@ -2324,15 +2324,6 @@ bool WarpBuilder::build_Object(BytecodeLocation loc) {
   JSObject* obj = loc.getObject(script_);
   MConstant* objConst = constant(ObjectValue(*obj));
 
-  if (mirGen().options.cloneSingletons()) {
-    auto* clone = MCloneLiteral::New(alloc(), objConst);
-    current->add(clone);
-    current->push(clone);
-    return resumeAfter(clone, loc);
-  }
-
-  // WarpOracle called realm->setSingletonsAsValues() so we can just push the
-  // object here.
   current->push(objConst);
   return true;
 }

js/src/jit/WarpOracle.cpp

@@ -394,13 +394,6 @@ AbortReasonOr<WarpScriptSnapshot*> WarpScriptOracle::createScriptSnapshot() {
         break;
       }
 
-      case JSOp::Object: {
-        if (!mirGen_.options.cloneSingletons()) {
-          cx_->realm()->behaviors().setSingletonsAsValues();
-        }
-        break;
-      }
-
       case JSOp::GetImport: {
         PropertyName* name = loc.getPropertyName(script_);
         if (!AddWarpGetImport(alloc_, opSnapshots, offset, script_, name)) {
@@ -714,6 +707,7 @@ AbortReasonOr<WarpScriptSnapshot*> WarpScriptOracle::createScriptSnapshot() {
       case JSOp::InitHiddenElemGetter:
       case JSOp::InitHiddenElemSetter:
       case JSOp::NewTarget:
+      case JSOp::Object:
       case JSOp::CheckIsObj:
       case JSOp::CheckObjCoercible:
       case JSOp::FunWithProto:

js/src/jit/shared/LIR-shared.h

@@ -222,22 +222,6 @@ class LNurseryObject : public LInstructionHelper<1, 0, 0> {
   MNurseryObject* mir() const { return mir_->toNurseryObject(); }
 };
 
-// Clone an object literal such as we are not modifying the object contained in
-// the sources.
-class LCloneLiteral : public LCallInstructionHelper<1, 1, 0> {
- public:
-  LIR_HEADER(CloneLiteral)
-
-  explicit LCloneLiteral(const LAllocation& obj)
-      : LCallInstructionHelper(classOpcode) {
-    setOperand(0, obj);
-  }
-
-  const LAllocation* getObjectLiteral() { return getOperand(0); }
-
-  MCloneLiteral* mir() const { return mir_->toCloneLiteral(); }
-};
-
 // Formal argument for a function, returning a box. Formal arguments are
 // initially read from the stack.
 class LParameter : public LInstructionHelper<BOX_PIECES, 0, 0> {

js/src/shell/js.cpp

@@ -6364,13 +6364,6 @@ static bool NewGlobal(JSContext* cx, unsigned argc, Value* vp) {
       creationOptions.setInvisibleToDebugger(v.toBoolean());
     }
 
-    if (!JS_GetProperty(cx, opts, "cloneSingletons", &v)) {
-      return false;
-    }
-    if (v.isBoolean()) {
-      creationOptions.setCloneSingletons(v.toBoolean());
-    }
-
     if (!JS_GetProperty(cx, opts, "sameZoneAs", &v)) {
       return false;
     }
@@ -8845,10 +8838,6 @@ JS_FN_HELP("rateMyCacheIR", RateMyCacheIR, 0, 0,
 "         as the given object.\n"
 "      newCompartment: If true, the global will always be created in a new\n"
 "         compartment and zone.\n"
-"      cloneSingletons: If true, always clone the objects baked into\n"
-"         scripts, even if it's a top-level script that will only run once\n"
-"         (defaults to using them directly in scripts that will only run\n"
-"         once).\n"
 "      invisibleToDebugger: If true, the global will be invisible to the\n"
 "         debugger (default false)\n"
 "      disableLazyParsing: If true, don't create lazy scripts for functions\n"

js/src/vm/Interpreter.cpp

@@ -3545,11 +3545,7 @@ static MOZ_NEVER_INLINE JS_HAZ_JSNATIVE_CALLER bool Interpret(JSContext* cx,
 
     CASE(Object) {
       MOZ_ASSERT(script->treatAsRunOnce());
-      JSObject* obj = SingletonObjectLiteralOperation(cx, script, REGS.pc);
-      if (!obj) {
-        goto error;
-      }
-      PUSH_OBJECT(*obj);
+      PUSH_OBJECT(*script->getObject(REGS.pc));
     }
     END_CASE(Object)
 
@@ -4945,20 +4941,6 @@ bool js::DefFunOperation(JSContext* cx, HandleScript script,
   return PutProperty(cx, parent, id, rval, script->strict());
 }
 
-JSObject* js::SingletonObjectLiteralOperation(JSContext* cx,
-                                              HandleScript script,
-                                              jsbytecode* pc) {
-  MOZ_ASSERT(JSOp(*pc) == JSOp::Object);
-
-  RootedObject obj(cx, script->getObject(pc));
-  if (cx->realm()->creationOptions().cloneSingletons()) {
-    return DeepCloneObjectLiteral(cx, obj);
-  }
-
-  cx->realm()->behaviors().setSingletonsAsValues();
-  return obj;
-}
-
 JSObject* js::ImportMetaOperation(JSContext* cx, HandleScript script) {
   RootedObject module(cx, GetModuleObjectForScript(script));
   MOZ_ASSERT(module);

js/src/vm/JSObject.cpp

@@ -1229,8 +1229,7 @@ static bool DeepCloneValue(JSContext* cx, Value* vp) {
 
 JSObject* js::DeepCloneObjectLiteral(JSContext* cx, HandleObject obj) {
   /* NB: Keep this in sync with XDRObjectLiteral. */
-  MOZ_ASSERT_IF(obj->isSingleton(),
-                cx->realm()->behaviors().getSingletonsAsTemplates());
+  MOZ_ASSERT(!obj->isSingleton());
   MOZ_ASSERT(obj->is<PlainObject>() || obj->is<ArrayObject>());
 
   if (obj->is<ArrayObject>()) {