LibJS: Correct behaviour of direct vs. indirect eval

eval only has direct access to the local scope when accessed through
the name eval. This includes locals named eval, because of course it
does.

.prettierignore

@@ -1 +1,3 @@
 Base/home/anon/Source/js
+Userland/Libraries/LibJS/Tests/eval-aliasing.js
+

Userland/Libraries/LibJS/AST.cpp

@@ -220,6 +220,11 @@ Value CallExpression::execute(Interpreter& interpreter, GlobalObject& global_obj
         }
     }
 
+    if (!is<NewExpression>(*this) && is<Identifier>(*m_callee) && static_cast<Identifier const&>(*m_callee).string() == vm.names.eval.as_string() && &callee.as_function() == global_object.eval_function()) {
+        auto script_value = arguments.size() == 0 ? js_undefined() : arguments[0];
+        return perform_eval(script_value, global_object, vm.in_strict_mode() ? CallerMode::Strict : CallerMode::NonStrict, EvalMode::Direct);
+    }
+
     vm.call_frame().current_node = interpreter.current_node();
     Object* new_object = nullptr;
     Value result;

Userland/Libraries/LibJS/Parser.cpp

@@ -226,11 +226,15 @@ Associativity Parser::operator_associativity(TokenType type) const
     }
 }
 
-NonnullRefPtr<Program> Parser::parse_program()
+NonnullRefPtr<Program> Parser::parse_program(bool starts_in_strict_mode)
 {
     auto rule_start = push_start();
     ScopePusher scope(*this, ScopePusher::Var | ScopePusher::Let | ScopePusher::Function);
     auto program = adopt_ref(*new Program({ m_filename, rule_start.position(), position() }));
+    if (starts_in_strict_mode) {
+        program->set_strict_mode();
+        m_state.strict_mode = true;
+    }
 
     bool first = true;
     while (!done()) {

Userland/Libraries/LibJS/Parser.h

@@ -37,7 +37,7 @@ class Parser {
 public:
     explicit Parser(Lexer lexer);
 
-    NonnullRefPtr<Program> parse_program();
+    NonnullRefPtr<Program> parse_program(bool starts_in_strict_mode = false);
 
     template<typename FunctionNodeType>
     NonnullRefPtr<FunctionNodeType> parse_function_node(u8 parse_options = FunctionNodeParseOptions::CheckForFunctionAndName);

Userland/Libraries/LibJS/Runtime/AbstractOperations.cpp

@@ -7,12 +7,16 @@
 
 #include <AK/Function.h>
 #include <AK/Result.h>
+#include <AK/TemporaryChange.h>
+#include <LibJS/Interpreter.h>
+#include <LibJS/Parser.h>
 #include <LibJS/Runtime/AbstractOperations.h>
 #include <LibJS/Runtime/BoundFunction.h>
 #include <LibJS/Runtime/DeclarativeEnvironmentRecord.h>
 #include <LibJS/Runtime/ErrorTypes.h>
 #include <LibJS/Runtime/Function.h>
 #include <LibJS/Runtime/FunctionEnvironmentRecord.h>
+#include <LibJS/Runtime/GlobalEnvironmentRecord.h>
 #include <LibJS/Runtime/GlobalObject.h>
 #include <LibJS/Runtime/Object.h>
 #include <LibJS/Runtime/ObjectEnvironmentRecord.h>
@@ -198,4 +202,30 @@ Object* get_super_constructor(VM& vm)
     return super_constructor;
 }
 
+// 19.2.1.1 PerformEval ( x, callerRealm, strictCaller, direct ), https://tc39.es/ecma262/#sec-performeval
+Value perform_eval(Value x, GlobalObject& caller_realm, CallerMode strict_caller, EvalMode direct)
+{
+    VERIFY(direct == EvalMode::Direct || strict_caller == CallerMode::NonStrict);
+    if (!x.is_string())
+        return x;
+
+    auto& vm = caller_realm.vm();
+    auto& code_string = x.as_string();
+    Parser parser { Lexer { code_string.string() } };
+    auto program = parser.parse_program(strict_caller == CallerMode::Strict);
+
+    if (parser.has_errors()) {
+        auto& error = parser.errors()[0];
+        vm.throw_exception<SyntaxError>(caller_realm, error.to_string());
+        return {};
+    }
+
+    auto& interpreter = vm.interpreter();
+    if (direct == EvalMode::Direct)
+        return interpreter.execute_statement(caller_realm, program).value_or(js_undefined());
+
+    TemporaryChange scope_change(vm.call_frame().lexical_environment, static_cast<EnvironmentRecord*>(&caller_realm.environment_record()));
+    return interpreter.execute_statement(caller_realm, program).value_or(js_undefined());
+}
+
 }

Userland/Libraries/LibJS/Runtime/AbstractOperations.h

@@ -25,6 +25,16 @@ Function* species_constructor(GlobalObject&, Object const&, Function& default_co
 GlobalObject* get_function_realm(GlobalObject&, Function const&);
 Object* get_prototype_from_constructor(GlobalObject&, Function const& constructor, Object* (GlobalObject::*intrinsic_default_prototype)());
 
+enum class CallerMode {
+    Strict,
+    NonStrict
+};
+enum class EvalMode {
+    Direct,
+    Indirect
+};
+Value perform_eval(Value, GlobalObject&, CallerMode, EvalMode);
+
 // 10.1.13 OrdinaryCreateFromConstructor ( constructor, intrinsicDefaultProto [ , internalSlotsList ] ), https://tc39.es/ecma262/#sec-ordinarycreatefromconstructor
 template<typename T, typename... Args>
 T* ordinary_create_from_constructor(GlobalObject& global_object, Function const& constructor, Object* (GlobalObject::*intrinsic_default_prototype)(), Args&&... args)

Userland/Libraries/LibJS/Runtime/GlobalObject.cpp

@@ -8,13 +8,13 @@
 #include <AK/CharacterTypes.h>
 #include <AK/Hex.h>
 #include <AK/Platform.h>
-#include <AK/TemporaryChange.h>
 #include <AK/Utf8View.h>
 #include <LibJS/Console.h>
 #include <LibJS/Heap/DeferGC.h>
 #include <LibJS/Interpreter.h>
 #include <LibJS/Lexer.h>
 #include <LibJS/Parser.h>
+#include <LibJS/Runtime/AbstractOperations.h>
 #include <LibJS/Runtime/AggregateErrorConstructor.h>
 #include <LibJS/Runtime/AggregateErrorPrototype.h>
 #include <LibJS/Runtime/ArrayBufferConstructor.h>
@@ -140,6 +140,8 @@ void GlobalObject::initialize_global_object()
     define_native_function(vm.names.parseFloat, parse_float, 1, attr);
     define_native_function(vm.names.parseInt, parse_int, 2, attr);
     define_native_function(vm.names.eval, eval, 1, attr);
+    m_eval_function = &get_without_side_effects(vm.names.eval).as_function();
+
     define_native_function(vm.names.encodeURI, encode_uri, 1, attr);
     define_native_function(vm.names.decodeURI, decode_uri, 1, attr);
     define_native_function(vm.names.encodeURIComponent, encode_uri_component, 1, attr);
@@ -223,6 +225,8 @@ void GlobalObject::visit_edges(Visitor& visitor)
     visitor.visit(m_##snake_name##_prototype);
     JS_ENUMERATE_ITERATOR_PROTOTYPES
 #undef __JS_ENUMERATE
+
+    visitor.visit(m_eval_function);
 }
 
 JS_DEFINE_NATIVE_FUNCTION(GlobalObject::gc)
@@ -335,23 +339,7 @@ JS_DEFINE_NATIVE_FUNCTION(GlobalObject::parse_int)
 // 19.2.1 eval ( x ), https://tc39.es/ecma262/#sec-eval-x
 JS_DEFINE_NATIVE_FUNCTION(GlobalObject::eval)
 {
-    if (!vm.argument(0).is_string())
-        return vm.argument(0);
-    auto& code_string = vm.argument(0).as_string();
-    JS::Parser parser { JS::Lexer { code_string.string() } };
-    auto program = parser.parse_program();
-
-    if (parser.has_errors()) {
-        auto& error = parser.errors()[0];
-        vm.throw_exception<SyntaxError>(global_object, error.to_string());
-        return {};
-    }
-
-    auto& caller_frame = vm.call_stack().at(vm.call_stack().size() - 2);
-    TemporaryChange scope_change(vm.call_frame().lexical_environment, caller_frame->lexical_environment);
-
-    auto& interpreter = vm.interpreter();
-    return interpreter.execute_statement(global_object, program).value_or(js_undefined());
+    return perform_eval(vm.argument(0), global_object, CallerMode::NonStrict, EvalMode::Indirect);
 }
 
 // 19.2.6.1.1 Encode ( string, unescapedSet ), https://tc39.es/ecma262/#sec-encode

Userland/Libraries/LibJS/Runtime/GlobalObject.h

@@ -36,6 +36,8 @@ public:
     // Not included in JS_ENUMERATE_NATIVE_OBJECTS due to missing distinct constructor
     GeneratorObjectPrototype* generator_object_prototype() { return m_generator_object_prototype; }
 
+    Function* eval_function() const { return m_eval_function; }
+
 #define __JS_ENUMERATE(ClassName, snake_name, PrototypeName, ConstructorName, ArrayType) \
     ConstructorName* snake_name##_constructor() { return m_##snake_name##_constructor; } \
     Object* snake_name##_prototype() { return m_##snake_name##_prototype; }
@@ -95,6 +97,8 @@ private:
     Object* m_##snake_name##_prototype { nullptr };
     JS_ENUMERATE_ITERATOR_PROTOTYPES
 #undef __JS_ENUMERATE
+
+    Function* m_eval_function;
 };
 
 template<typename ConstructorType>

Userland/Libraries/LibJS/Tests/eval-aliasing.js

@@ -0,0 +1,15 @@
+test("variable named 'eval' pointing to another function calls that function", function () {
+    var testValue = "inner";
+    // This breaks prettier as it considers this to be a parse error
+    // before even trying to do any linting
+    var eval = () => {
+        return "wat";
+    };
+    expect(eval("testValue")).toEqual("wat");
+});
+
+test("variable named 'eval' pointing to real eval works as a direct eval", function () {
+    var testValue = "inner";
+    var eval = globalThis.eval;
+    expect(eval("testValue")).toEqual("inner");
+});

Userland/Libraries/LibJS/Tests/eval-basic.js

@@ -30,3 +30,46 @@ test("returns 1st argument unless 1st argument is a string", () => {
     var stringObject = new String("1 + 2");
     expect(eval(stringObject)).toBe(stringObject);
 });
+
+// These eval scope tests use function expressions due to bug #8198
+var testValue = "outer";
+test("eval only touches locals if direct use", function () {
+    var testValue = "inner";
+    expect(globalThis.eval("testValue")).toEqual("outer");
+});
+
+test("alias to eval works as a global eval", function () {
+    var testValue = "inner";
+    var eval1 = globalThis.eval;
+    expect(eval1("testValue")).toEqual("outer");
+});
+
+test("eval evaluates all args", function () {
+    var i = 0;
+    expect(eval("testValue", i++, i++, i++)).toEqual("outer");
+    expect(i).toEqual(3);
+});
+
+test("eval tests for exceptions", function () {
+    var i = 0;
+    expect(function () {
+        eval("testValue", i++, i++, j, i++);
+    }).toThrowWithMessage(ReferenceError, "'j' is not defined");
+    expect(i).toEqual(2);
+});
+
+test("direct eval inherits non-strict evaluation", function () {
+    expect(eval("01")).toEqual(1);
+});
+
+test("direct eval inherits strict evaluation", function () {
+    "use strict";
+    expect(() => {
+        eval("01");
+    }).toThrowWithMessage(SyntaxError, "Unprefixed octal number not allowed in strict mode");
+});
+
+test("global eval evaluates as non-strict", function () {
+    "use strict";
+    expect(globalThis.eval("01"));
+});