From 94b5fa37cfe866aa41082e2997fa38f143102784 Mon Sep 17 00:00:00 2001
From: Jan de Mooij <jdemooij@mozilla.com>
Date: Tue, 30 May 2023 07:46:32 +0000
Subject: [PATCH] Bug 1832582 part 4 - Add JOF_USES_ENV to JSOp::Arguments and
 JSOp::Generator. r=iain a=pascalc

This shouldn't affect behavior because for `JSOp::Arguments` this replaces a similar check
in `WarpOracle`, and generators have their own environment object causing us to return
true from `ScriptUsesEnvironmentChain`.

Differential Revision: https://phabricator.services.mozilla.com/D178653
---
 js/src/jit/WarpBuilder.cpp | 7 +++++++
 js/src/jit/WarpOracle.cpp  | 6 +-----
 js/src/vm/Opcodes.h        | 4 ++--
 3 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/js/src/jit/WarpBuilder.cpp b/js/src/jit/WarpBuilder.cpp
index c313080ad9acd..fce8defafbbe8 100644
--- a/js/src/jit/WarpBuilder.cpp
+++ b/js/src/jit/WarpBuilder.cpp
@@ -1557,6 +1557,7 @@ bool WarpBuilder::build_Arguments(BytecodeLocation loc) {
   auto* snapshot = getOpSnapshot<WarpArguments>(loc);
   MOZ_ASSERT(info().needsArgsObj());
   MOZ_ASSERT(snapshot);
+  MOZ_ASSERT(usesEnvironmentChain());
 
   ArgumentsObject* templateObj = snapshot->templateObj();
   MDefinition* env = current->environmentChain();
@@ -1836,6 +1837,8 @@ MConstant* WarpBuilder::globalLexicalEnvConstant() {
 }
 
 bool WarpBuilder::build_GetName(BytecodeLocation loc) {
+  MOZ_ASSERT(usesEnvironmentChain());
+
   MDefinition* env = current->environmentChain();
   return buildIC(loc, CacheKind::GetName, {env});
 }
@@ -1865,6 +1868,8 @@ bool WarpBuilder::build_GetGName(BytecodeLocation loc) {
 }
 
 bool WarpBuilder::build_BindName(BytecodeLocation loc) {
+  MOZ_ASSERT(usesEnvironmentChain());
+
   MDefinition* env = current->environmentChain();
   return buildIC(loc, CacheKind::BindName, {env});
 }
@@ -2226,6 +2231,8 @@ bool WarpBuilder::build_CheckThisReinit(BytecodeLocation loc) {
 }
 
 bool WarpBuilder::build_Generator(BytecodeLocation loc) {
+  MOZ_ASSERT(usesEnvironmentChain());
+
   MDefinition* callee = getCallee();
   MDefinition* environmentChain = current->environmentChain();
   MDefinition* argsObj = info().needsArgsObj() ? current->argumentsObject()
diff --git a/js/src/jit/WarpOracle.cpp b/js/src/jit/WarpOracle.cpp
index 9af2f2508dc02..68cea660bf097 100644
--- a/js/src/jit/WarpOracle.cpp
+++ b/js/src/jit/WarpOracle.cpp
@@ -256,11 +256,7 @@ ICEntry& WarpScriptOracle::getICEntryAndFallback(BytecodeLocation loc,
 
 WarpEnvironment WarpScriptOracle::createEnvironment() {
   // Don't do anything if the script doesn't use the environment chain.
-  // Always make an environment chain if the script needs an arguments object
-  // because ArgumentsObject construction requires the environment chain to be
-  // passed in.
-  if (!script_->jitScript()->usesEnvironmentChain() &&
-      !script_->needsArgsObj()) {
+  if (!script_->jitScript()->usesEnvironmentChain()) {
     return WarpEnvironment(NoEnvironment());
   }
 
diff --git a/js/src/vm/Opcodes.h b/js/src/vm/Opcodes.h
index 40fa176db6896..d26ea4de39998 100644
--- a/js/src/vm/Opcodes.h
+++ b/js/src/vm/Opcodes.h
@@ -2005,7 +2005,7 @@
      *   Operands:
      *   Stack: => gen
      */ \
-    MACRO(Generator, generator, NULL, 1, 0, 1, JOF_BYTE) \
+    MACRO(Generator, generator, NULL, 1, 0, 1, JOF_BYTE|JOF_USES_ENV) \
     /*
      * Suspend the current generator and return to the caller.
      *
@@ -3378,7 +3378,7 @@
      *   Operands:
      *   Stack: => arguments
      */ \
-    MACRO(Arguments, arguments, NULL, 1, 0, 1, JOF_BYTE) \
+    MACRO(Arguments, arguments, NULL, 1, 0, 1, JOF_BYTE|JOF_USES_ENV) \
     /*
      * Create and push the rest parameter array for current function call.
      *