Add isStrict argument to DirectEval

Summary: Direct calls to eval should inherit the strictness of the function they are defined in. We already have that information available in `ESTreeIRGen-expr.cpp`, so this diff is simply a matter of plumbing that information all the way through to the runtime. Introduce a new argument to the bytecode operation `DirectEval`, and use this argument in `Interpreter::caseDirectEval`. Reviewed By: tmikov Differential Revision: D41860786 fbshipit-source-id: 0faa33e520ccc86d58e280e8242352782583ff28
facebook · Mar 29, 2023 · f6b56d3 · f6b56d3
1 parent 4c18518
commit f6b56d3
Show file tree

Hide file tree

Showing 30 changed files with 75 additions and 37 deletions.
diff --git a/include/hermes/BCGen/HBC/BytecodeList.def b/include/hermes/BCGen/HBC/BytecodeList.def
@@ -522,7 +522,8 @@ DEFINE_OPCODE_1(Catch, Reg8)
 /// ES6 18.2.1.1 PerformEval(Arg2, evalRealm, strictCaller=true, direct=true)
 /// Arg1 is the destination of the return value.
 /// Arg2 is the value to eval.
-DEFINE_OPCODE_2(DirectEval, Reg8, Reg8)
+/// Arg3 is a boolean which is true if the eval should be performed in strict mode.
+DEFINE_OPCODE_3(DirectEval, Reg8, Reg8, UInt8)
 
 /// Throw an exception.
 /// throw Arg1;

diff --git a/include/hermes/BCGen/HBC/BytecodeVersion.h b/include/hermes/BCGen/HBC/BytecodeVersion.h
@@ -19,8 +19,8 @@ namespace hermes {
 namespace hbc {
 
 // Bytecode version generated by this version of the compiler.
-// Updated: Mar 07, 2023
-const static uint32_t BYTECODE_VERSION = 94;
+// Updated: Mar 22, 2023
+const static uint32_t BYTECODE_VERSION = 95;
 
 } // namespace hbc
 } // namespace hermes

diff --git a/include/hermes/IR/IRBuilder.h b/include/hermes/IR/IRBuilder.h
@@ -440,7 +440,7 @@ class IRBuilder {
       Value *value,
       UnaryOperatorInst::OpKind opKind);
 
-  DirectEvalInst *createDirectEvalInst(Value *operand);
+  DirectEvalInst *createDirectEvalInst(Value *operand, LiteralBool *isStrict);
 
   SwitchInst *createSwitchInst(
       Value *input,

diff --git a/include/hermes/IR/Instrs.def b/include/hermes/IR/Instrs.def
@@ -52,7 +52,6 @@ DEF_VALUE(MovInst, SingleOperandInst)
 DEF_VALUE(ImplicitMovInst, Instruction)
 DEF_VALUE(CoerceThisNSInst, SingleOperandInst)
 DEF_VALUE(UnaryOperatorInst, SingleOperandInst)
-DEF_VALUE(DirectEvalInst, SingleOperandInst)
 #ifdef INCLUDE_HBC_BACKEND
 DEF_VALUE(HBCLoadConstInst, SingleOperandInst)
 DEF_VALUE(HBCLoadParamInst, SingleOperandInst)
@@ -90,6 +89,7 @@ DEF_VALUE(AllocObjectLiteralInst, Instruction)
 DEF_VALUE(CreateArgumentsInst, Instruction)
 DEF_VALUE(CatchInst, Instruction)
 DEF_VALUE(DebuggerInst, Instruction)
+DEF_VALUE(DirectEvalInst, Instruction)
 DEF_VALUE(CreateRegExpInst, Instruction)
 DEF_VALUE(TryEndInst, Instruction)
 DEF_VALUE(GetNewTargetInst, Instruction)

diff --git a/include/hermes/IR/Instrs.h b/include/hermes/IR/Instrs.h
@@ -2822,17 +2822,23 @@ class SaveAndYieldInst : public TerminatorInst {
   }
 };
 
-class DirectEvalInst : public SingleOperandInst {
+class DirectEvalInst : public Instruction {
   DirectEvalInst(const DirectEvalInst &) = delete;
   void operator=(const DirectEvalInst &) = delete;
 
  public:
-  explicit DirectEvalInst(Value *value)
-      : SingleOperandInst(ValueKind::DirectEvalInstKind, value) {}
+  enum { CodeStringIdx, IsStrictIdx };
+
+  explicit DirectEvalInst(Value *codeString, LiteralBool *isStrict)
+      : Instruction(ValueKind::DirectEvalInstKind) {
+    setType(Type::createAnyType());
+    pushOperand(codeString);
+    pushOperand(isStrict);
+  }
   explicit DirectEvalInst(
       const DirectEvalInst *src,
       llvh::ArrayRef<Value *> operands)
-      : SingleOperandInst(src, operands) {}
+      : Instruction(src, operands) {}
 
   SideEffectKind getSideEffect() const {
     return SideEffectKind::Unknown;
@@ -2842,6 +2848,14 @@ class DirectEvalInst : public SingleOperandInst {
     return {};
   }
 
+  Value *getCodeString() const {
+    return getOperand(CodeStringIdx);
+  }
+
+  bool getIsStrict() const {
+    return cast<LiteralBool>(getOperand(IsStrictIdx))->getValue();
+  }
+
   static bool classof(const Value *V) {
     return kindIsA(V->getKind(), ValueKind::DirectEvalInstKind);
   }

diff --git a/include/hermes/VM/JSLib.h b/include/hermes/VM/JSLib.h
@@ -58,6 +58,7 @@ CallResult<HermesValue> evalInEnvironment(
     Handle<Environment> environment,
     const ScopeChain &scopeChain,
     Handle<> thisArg,
+    bool isStrict,
     bool singleFunction);
 
 /// If the target CJS module is not initialized, execute it.

diff --git a/lib/BCGen/HBC/ISel.cpp b/lib/BCGen/HBC/ISel.cpp
@@ -325,8 +325,9 @@ void HBCISel::generateSingleOperandInst(
 
 void HBCISel::generateDirectEvalInst(DirectEvalInst *Inst, BasicBlock *next) {
   auto dst = encodeValue(Inst);
-  auto src = encodeValue(Inst->getSingleOperand());
-  BCFGen_->emitDirectEval(dst, src);
+  auto src = encodeValue(Inst->getCodeString());
+  auto isStrict = Inst->getIsStrict();
+  BCFGen_->emitDirectEval(dst, src, isStrict);
 }
 
 void HBCISel::generateAddEmptyStringInst(

diff --git a/lib/BCGen/HBC/Passes.cpp b/lib/BCGen/HBC/Passes.cpp
@@ -208,6 +208,11 @@ bool LoadConstants::operandMustBeLiteral(Instruction *Inst, unsigned opIndex) {
       opIndex == ThrowIfHasRestrictedGlobalPropertyInst::PropertyIdx) {
     return true;
   }
+  // DirectEvalInst's isStrict is a boolean constant.
+  if (llvh::isa<DirectEvalInst>(Inst) &&
+      opIndex == DirectEvalInst::IsStrictIdx) {
+    return true;
+  }
 
   return false;
 }

diff --git a/lib/IR/IRBuilder.cpp b/lib/IR/IRBuilder.cpp
@@ -819,8 +819,10 @@ SwitchImmInst *IRBuilder::createSwitchImmInst(
   return inst;
 }
 
-DirectEvalInst *IRBuilder::createDirectEvalInst(Value *operand) {
-  auto *inst = new DirectEvalInst(operand);
+DirectEvalInst *IRBuilder::createDirectEvalInst(
+    Value *operand,
+    LiteralBool *isStrict) {
+  auto *inst = new DirectEvalInst(operand, isStrict);
   insert(inst);
   return inst;
 }

diff --git a/lib/IRGen/ESTreeIRGen-expr.cpp b/lib/IRGen/ESTreeIRGen-expr.cpp
@@ -787,7 +787,9 @@ Value *ESTreeIRGen::genCallEvalExpr(ESTree::CallExpressionNode *call) {
         call->getSourceRange(), "Extra eval() arguments are ignored");
   }
 
-  return Builder.createDirectEvalInst(args[0]);
+  bool isStrict = Builder.getInsertionBlock()->getParent()->isStrictMode();
+  return Builder.createDirectEvalInst(
+      args[0], Builder.getLiteralBool(isStrict));
 }
 
 /// Convert a property key node to its JavaScript string representation.

diff --git a/lib/VM/Debugger/Debugger.cpp b/lib/VM/Debugger/Debugger.cpp
@@ -1168,6 +1168,7 @@ HermesValue Debugger::evalInFrame(
         Handle<Environment>::vmcast(runtime_, env),
         chain,
         Handle<>(&frameInfo->frame->getThisArgRef()),
+        false,
         singleFunction);
   }
 

diff --git a/lib/VM/Interpreter-slowpaths.cpp b/lib/VM/Interpreter-slowpaths.cpp
@@ -37,6 +37,7 @@ ExecutionStatus Interpreter::caseDirectEval(
     const Inst *ip) {
   auto *result = &O1REG(DirectEval);
   auto *input = &O2REG(DirectEval);
+  bool isStrict = ip->iDirectEval.op3;
 
   GCScopeMarkerRAII gcMarker{runtime};
 
@@ -82,7 +83,11 @@ ExecutionStatus Interpreter::caseDirectEval(
   scopeChain.scopes.emplace_back();
 
   auto cr = vm::directEval(
-      runtime, Handle<StringPrimitive>::vmcast(input), scopeChain, false);
+      runtime,
+      Handle<StringPrimitive>::vmcast(input),
+      scopeChain,
+      isStrict,
+      false);
   if (cr == ExecutionStatus::EXCEPTION)
     return ExecutionStatus::EXCEPTION;
 

diff --git a/lib/VM/JSLib/JSLibInternal.cpp b/lib/VM/JSLib/JSLibInternal.cpp
@@ -444,7 +444,8 @@ CallResult<HermesValue> createDynamicFunction(
   builder->appendStringPrim(body);
   builder->appendASCIIRef(functionFooter);
 
-  auto evalRes = directEval(runtime, builder->getStringPrimitive(), {}, true);
+  auto evalRes =
+      directEval(runtime, builder->getStringPrimitive(), {}, false, true);
   if (evalRes == ExecutionStatus::EXCEPTION) {
     return ExecutionStatus::EXCEPTION;
   }

diff --git a/lib/VM/JSLib/JSLibInternal.h b/lib/VM/JSLib/JSLibInternal.h
@@ -444,7 +444,8 @@ CallResult<HermesValue> directEval(
     Runtime &runtime,
     Handle<StringPrimitive> str,
     const ScopeChain &scopeChain,
-    bool singleFunction = false);
+    bool isStrict,
+    bool singleFunction);
 
 /// ES10 23.1.1.2 AddEntriesFromIterable
 /// Calls a callback with each pair of [key, value] from an iterable.

diff --git a/lib/VM/JSLib/eval.cpp b/lib/VM/JSLib/eval.cpp
@@ -31,6 +31,7 @@ CallResult<HermesValue> evalInEnvironment(
     Handle<Environment> environment,
     const ScopeChain &scopeChain,
     Handle<> thisArg,
+    bool isStrict,
     bool singleFunction) {
 #ifdef HERMESVM_LEAN
   return runtime.raiseEvalUnsupported(utf8code);
@@ -40,7 +41,7 @@ CallResult<HermesValue> evalInEnvironment(
   }
 
   hbc::CompileFlags compileFlags;
-  compileFlags.strict = false;
+  compileFlags.strict = isStrict;
   compileFlags.includeLibHermes = false;
   compileFlags.verifyIR = runtime.verifyEvalIR;
   compileFlags.emitAsyncBreakCheck = runtime.asyncBreakCheckInEval;
@@ -102,6 +103,7 @@ CallResult<HermesValue> directEval(
     Runtime &runtime,
     Handle<StringPrimitive> str,
     const ScopeChain &scopeChain,
+    bool isStrict,
     bool singleFunction) {
   // Convert the code into UTF8.
   std::string code;
@@ -119,6 +121,7 @@ CallResult<HermesValue> directEval(
       Runtime::makeNullHandle<Environment>(),
       scopeChain,
       runtime.getGlobal(),
+      isStrict,
       singleFunction);
 }
 
@@ -129,7 +132,8 @@ CallResult<HermesValue> eval(void *, Runtime &runtime, NativeArgs args) {
     return args.getArg(0);
   }
 
-  return directEval(runtime, args.dyncastArg<StringPrimitive>(0), {}, false);
+  return directEval(
+      runtime, args.dyncastArg<StringPrimitive>(0), {}, false, false);
 }
 
 } // namespace vm

diff --git a/test/BCGen/HBC/async-function.js b/test/BCGen/HBC/async-function.js
@@ -24,7 +24,7 @@ var simpleAsyncFE = async function () {
 // Auto-generated content below. Please do not modify manually.
 
 // CHECK:Bytecode File Information:
-// CHECK-NEXT:  Bytecode version number: 94
+// CHECK-NEXT:  Bytecode version number: 95
 // CHECK-NEXT:  Source hash: {{.*}}
 // CHECK-NEXT:  Function count: 10
 // CHECK-NEXT:  String count: 11

diff --git a/test/BCGen/HBC/callbuiltin.js b/test/BCGen/HBC/callbuiltin.js
@@ -93,7 +93,7 @@ print(foo({a: 10, b: 20, lastKey:30, 5:6}))
 // CHKRA-NEXT:function_end
 
 // CHKBC:Bytecode File Information:
-// CHKBC-NEXT:  Bytecode version number: 94
+// CHKBC-NEXT:  Bytecode version number: 95
 // CHKBC-NEXT:  Source hash: {{.*}}
 // CHKBC-NEXT:  Function count: 4
 // CHKBC-NEXT:  String count: 13

diff --git a/test/BCGen/HBC/calln.js b/test/BCGen/HBC/calln.js
@@ -111,7 +111,7 @@ function foo5(f) { f(1, 2, 3, 4); }
 // LRA-NEXT:function_end
 
 // BCGEN:Bytecode File Information:
-// BCGEN-NEXT:  Bytecode version number: 94
+// BCGEN-NEXT:  Bytecode version number: 95
 // BCGEN-NEXT:  Source hash: {{.*}}
 // BCGEN-NEXT:  Function count: 6
 // BCGEN-NEXT:  String count: 6

diff --git a/test/BCGen/HBC/es6/generator.js b/test/BCGen/HBC/es6/generator.js
@@ -22,7 +22,7 @@ function *args() {
 // Auto-generated content below. Please do not modify manually.
 
 // CHECK:Bytecode File Information:
-// CHECK-NEXT:  Bytecode version number: 94
+// CHECK-NEXT:  Bytecode version number: 95
 // CHECK-NEXT:  Source hash: {{.*}}
 // CHECK-NEXT:  Function count: 5
 // CHECK-NEXT:  String count: 8

diff --git a/test/BCGen/HBC/lower-globalobject.js b/test/BCGen/HBC/lower-globalobject.js
@@ -37,7 +37,7 @@ function foo() {
 // CHKRA-NEXT:function_end
 
 // CHKBC:Bytecode File Information:
-// CHKBC-NEXT:  Bytecode version number: 94
+// CHKBC-NEXT:  Bytecode version number: 95
 // CHKBC-NEXT:  Source hash: {{.*}}
 // CHKBC-NEXT:  Function count: 2
 // CHKBC-NEXT:  String count: 3

diff --git a/test/IRGen/es6/block-scoping-top-level-scope.js b/test/IRGen/es6/block-scoping-top-level-scope.js
@@ -53,7 +53,7 @@ function StrictHasParamExprs() {
 // Auto-generated content below. Please do not modify manually.
 
 // BS:Bytecode File Information:
-// BS-NEXT:  Bytecode version number: 94
+// BS-NEXT:  Bytecode version number: 95
 // BS-NEXT:  Source hash: {{.*}}
 // BS-NEXT:  Function count: 13
 // BS-NEXT:  String count: 11
@@ -444,7 +444,7 @@ function StrictHasParamExprs() {
 // BS-NEXT:  0x00d8  end of debug string table
 
 // NOBS:Bytecode File Information:
-// NOBS-NEXT:  Bytecode version number: 94
+// NOBS-NEXT:  Bytecode version number: 95
 // NOBS-NEXT:  Source hash: {{.*}}
 // NOBS-NEXT:  Function count: 13
 // NOBS-NEXT:  String count: 11

diff --git a/test/IRGen/eval.js b/test/IRGen/eval.js
@@ -42,7 +42,7 @@ function baz() {
 // CHECK-NEXT:S{foo#0#1()#2} = []
 // CHECK-NEXT:%BB0:
 // CHECK-NEXT:  %0 = CreateScopeInst %S{foo#0#1()#2}
-// CHECK-NEXT:  %1 = DirectEvalInst "1 + 1" : string
+// CHECK-NEXT:  %1 = DirectEvalInst "1 + 1" : string, false : boolean
 // CHECK-NEXT:  %2 = ReturnInst %1
 // CHECK-NEXT:%BB1:
 // CHECK-NEXT:  %3 = ReturnInst undefined : undefined
@@ -55,7 +55,7 @@ function baz() {
 // CHECK-NEXT:  %1 = TryLoadGlobalPropertyInst globalObject : object, "Math" : string
 // CHECK-NEXT:  %2 = LoadPropertyInst globalObject : object, "foo" : string
 // CHECK-NEXT:  %3 = CallInst %2, undefined : undefined
-// CHECK-NEXT:  %4 = DirectEvalInst "2 + 2" : string
+// CHECK-NEXT:  %4 = DirectEvalInst "2 + 2" : string, false : boolean
 // CHECK-NEXT:  %5 = ReturnInst %4
 // CHECK-NEXT:%BB1:
 // CHECK-NEXT:  %6 = ReturnInst undefined : undefined

diff --git a/test/IRGen/hbc_object_literals.js b/test/IRGen/hbc_object_literals.js
@@ -118,7 +118,7 @@ var obj4 = {
 // IRGEN-NEXT:function_end
 
 // BCGEN:Bytecode File Information:
-// BCGEN-NEXT:  Bytecode version number: 94
+// BCGEN-NEXT:  Bytecode version number: 95
 // BCGEN-NEXT:  Source hash: {{.*}}
 // BCGEN-NEXT:  Function count: 2
 // BCGEN-NEXT:  String count: 24

diff --git a/test/Optimizer/cjs/cjs-require.js b/test/Optimizer/cjs/cjs-require.js
@@ -21,7 +21,7 @@ exports.bar = bar;
 // Auto-generated content below. Please do not modify manually.
 
 // CHKOPT:Bytecode File Information:
-// CHKOPT-NEXT:  Bytecode version number: 94
+// CHKOPT-NEXT:  Bytecode version number: 95
 // CHKOPT-NEXT:  Source hash: {{.*}}
 // CHKOPT-NEXT:  Function count: 7
 // CHKOPT-NEXT:  String count: 7
@@ -158,7 +158,7 @@ exports.bar = bar;
 // CHKOPT-NEXT:  0x0000  end of debug string table
 
 // CHKDBG:Bytecode File Information:
-// CHKDBG-NEXT:  Bytecode version number: 94
+// CHKDBG-NEXT:  Bytecode version number: 95
 // CHKDBG-NEXT:  Source hash: {{.*}}
 // CHKDBG-NEXT:  Function count: 7
 // CHKDBG-NEXT:  String count: 7

diff --git a/test/hermes/not-a-function.js.bcdefault.txt b/test/hermes/not-a-function.js.bcdefault.txt
@@ -8,7 +8,7 @@
 // Auto-generated content below. Please do not modify manually.
 
 // CHK-BCDEFAULT:Bytecode File Information:
-// CHK-BCDEFAULT-NEXT:  Bytecode version number: 94
+// CHK-BCDEFAULT-NEXT:  Bytecode version number: 95
 // CHK-BCDEFAULT-NEXT:  Source hash: {{.*}}
 // CHK-BCDEFAULT-NEXT:  Function count: 18
 // CHK-BCDEFAULT-NEXT:  String count: 19

diff --git a/test/hermes/not-a-function.js.bcg.txt b/test/hermes/not-a-function.js.bcg.txt
@@ -8,7 +8,7 @@
 // Auto-generated content below. Please do not modify manually.
 
 // CHK-BCG:Bytecode File Information:
-// CHK-BCG-NEXT:  Bytecode version number: 94
+// CHK-BCG-NEXT:  Bytecode version number: 95
 // CHK-BCG-NEXT:  Source hash: {{.*}}
 // CHK-BCG-NEXT:  Function count: 18
 // CHK-BCG-NEXT:  String count: 19

diff --git a/test/hermes/not-a-function.js.bcg0.txt b/test/hermes/not-a-function.js.bcg0.txt
@@ -8,7 +8,7 @@
 // Auto-generated content below. Please do not modify manually.
 
 // CHK-BCG0:Bytecode File Information:
-// CHK-BCG0-NEXT:  Bytecode version number: 94
+// CHK-BCG0-NEXT:  Bytecode version number: 95
 // CHK-BCG0-NEXT:  Source hash: {{.*}}
 // CHK-BCG0-NEXT:  Function count: 18
 // CHK-BCG0-NEXT:  String count: 19

diff --git a/test/hermes/not-a-function.js.bcg1.txt b/test/hermes/not-a-function.js.bcg1.txt
@@ -8,7 +8,7 @@
 // Auto-generated content below. Please do not modify manually.
 
 // CHK-BCG1:Bytecode File Information:
-// CHK-BCG1-NEXT:  Bytecode version number: 94
+// CHK-BCG1-NEXT:  Bytecode version number: 95
 // CHK-BCG1-NEXT:  Source hash: {{.*}}
 // CHK-BCG1-NEXT:  Function count: 18
 // CHK-BCG1-NEXT:  String count: 19

diff --git a/test/hermes/not-a-function.js.bcg2.txt b/test/hermes/not-a-function.js.bcg2.txt
@@ -8,7 +8,7 @@
 // Auto-generated content below. Please do not modify manually.
 
 // CHK-BCG2:Bytecode File Information:
-// CHK-BCG2-NEXT:  Bytecode version number: 94
+// CHK-BCG2-NEXT:  Bytecode version number: 95
 // CHK-BCG2-NEXT:  Source hash: {{.*}}
 // CHK-BCG2-NEXT:  Function count: 18
 // CHK-BCG2-NEXT:  String count: 19

diff --git a/test/hermes/not-a-function.js.bcg3.txt b/test/hermes/not-a-function.js.bcg3.txt
@@ -8,7 +8,7 @@
 // Auto-generated content below. Please do not modify manually.
 
 // CHK-BCG3:Bytecode File Information:
-// CHK-BCG3-NEXT:  Bytecode version number: 94
+// CHK-BCG3-NEXT:  Bytecode version number: 95
 // CHK-BCG3-NEXT:  Source hash: {{.*}}
 // CHK-BCG3-NEXT:  Function count: 18
 // CHK-BCG3-NEXT:  String count: 19