Fixed call argument value snapshotting.

Since fd5e523f (0.9.7), njs has evaluated all call argument expressions
before emitting the frame and PUT_ARG instructions.  This fixed await
expressions in call arguments and preserved argument side effects before
callee validation, but left PUT_ARG reading argument values only after later
argument expressions had already run.

As a result, an earlier argument backed by a mutable variable slot could
observe a later mutation of the same slot.  For example, f(a, a = 2) passed
2 as both arguments instead of preserving the first value as 1.  The same
issue applied to later argument effects such as getters.

The fix is to preserve affected argument values in temporary registers where
appropriate.

This fixes #1059 issue on Github.

diff --git a/src/njs_generator.c b/src/njs_generator.c
index 89d08a11c706133efeeb7ae038027bc60b8888ef..1cd57df6e1d70aae8b7dd88f6725091f561e6173 100644 (file)
--- a/src/njs_generator.c
+++ b/src/njs_generator.c
@@ -470,6 +470,15 @@ static njs_int_t njs_generate_call_argument_expressions(njs_vm_t *vm,
     njs_generator_t *generator, njs_parser_node_t *node);
 static njs_int_t njs_generate_call_argument_expressions_after(njs_vm_t *vm,
     njs_generator_t *generator, njs_parser_node_t *node);
+static njs_bool_t njs_generate_argument_aliases_mutable_slot(
+    njs_parser_node_t *node);
+static njs_bool_t njs_generate_argument_index_is_mutable(njs_index_t index);
+static njs_bool_t njs_generate_argument_uses_index(njs_parser_node_t *node,
+    njs_index_t index);
+static njs_bool_t njs_generate_argument_may_observe_effect(
+    njs_vm_t *vm, njs_parser_node_t *node);
+static njs_bool_t njs_generate_argument_suffix_may_observe_effect(
+    njs_vm_t *vm, njs_parser_node_t *node);
 static njs_uint_t njs_generate_call_nargs(njs_parser_node_t *node);
 static njs_int_t njs_generate_capture_stable_value(njs_vm_t *vm,
     njs_generator_t *generator, njs_parser_node_t *node);
@@ -5461,10 +5470,21 @@ static njs_int_t
 njs_generate_call_argument_expressions_after(njs_vm_t *vm,
     njs_generator_t *generator, njs_parser_node_t *node)
 {
+    njs_int_t  ret;
+
     if (node->right == NULL) {
         return njs_generator_stack_pop(vm, generator, NULL);
     }
 
+    if (njs_generate_argument_aliases_mutable_slot(node->left)
+        && njs_generate_argument_suffix_may_observe_effect(vm, node->right))
+    {
+        ret = njs_generate_capture_stable_value(vm, generator, node->left);
+        if (njs_slow_path(ret != NJS_OK)) {
+            return ret;
+        }
+    }
+
     njs_generator_next(generator, njs_generate, node->right->left);
 
     return njs_generator_after(vm, generator,
@@ -5474,6 +5494,122 @@ njs_generate_call_argument_expressions_after(njs_vm_t *vm,
 }
 
 
+static njs_bool_t
+njs_generate_argument_aliases_mutable_slot(njs_parser_node_t *node)
+{
+    njs_index_t  index;
+
+    njs_assert(node != NULL);
+
+    if (node->temporary) {
+        return 0;
+    }
+
+    if (node->token_type == NJS_TOKEN_THIS) {
+        return 0;
+    }
+
+    index = node->index;
+
+    if (!njs_generate_argument_index_is_mutable(index)) {
+        return 0;
+    }
+
+    return njs_generate_argument_uses_index(node, index);
+}
+
+
+static njs_bool_t
+njs_generate_argument_index_is_mutable(njs_index_t index)
+{
+    njs_assert(index != NJS_INDEX_NONE && index != NJS_INDEX_ERROR);
+
+    if (njs_scope_index_type(index) == NJS_LEVEL_STATIC) {
+        return 0;
+    }
+
+    if (njs_scope_index_var(index) == NJS_VARIABLE_CONST) {
+        return 0;
+    }
+
+    return 1;
+}
+
+
+static njs_bool_t
+njs_generate_argument_uses_index(njs_parser_node_t *node, njs_index_t index)
+{
+    njs_assert(node != NULL);
+
+    if (node->index == index
+        && (node->token_type == NJS_TOKEN_NAME
+            || node->token_type == NJS_TOKEN_ARGUMENTS
+            || node->token_type == NJS_TOKEN_EVAL))
+    {
+        return 1;
+    }
+
+    if (node->left != NULL && node->left->index == index
+        && njs_generate_argument_uses_index(node->left, index))
+    {
+        return 1;
+    }
+
+    if (node->right != NULL && node->right->index == index
+        && njs_generate_argument_uses_index(node->right, index))
+    {
+        return 1;
+    }
+
+    return 0;
+}
+
+
+static njs_bool_t
+njs_generate_argument_may_observe_effect(njs_vm_t *vm, njs_parser_node_t *node)
+{
+    njs_assert(node != NULL);
+
+    switch (node->token_type) {
+    case NJS_TOKEN_NULL:
+    case NJS_TOKEN_NUMBER:
+    case NJS_TOKEN_TRUE:
+    case NJS_TOKEN_UNDEFINED:
+    case NJS_TOKEN_FALSE:
+    case NJS_TOKEN_STRING:
+    case NJS_TOKEN_ARGUMENTS:
+    case NJS_TOKEN_THIS:
+        return 0;
+
+    case NJS_TOKEN_NAME:
+    case NJS_TOKEN_EVAL:
+        /*
+         * if njs_variable_resolve(vm, node) == NULL the name reference is
+         * global, it may invoke global accessor and therefore may observe
+         * effect.
+         */
+        return njs_variable_resolve(vm, node) == NULL;
+
+    default:
+        return 1;
+    }
+}
+
+
+static njs_bool_t
+njs_generate_argument_suffix_may_observe_effect(njs_vm_t *vm,
+    njs_parser_node_t *node)
+{
+    for (; node != NULL; node = node->right) {
+        if (njs_generate_argument_may_observe_effect(vm, node->left)) {
+            return 1;
+        }
+    }
+
+    return 0;
+}
+
+
 static njs_uint_t
 njs_generate_call_nargs(njs_parser_node_t *node)
 {

diff --git a/src/test/njs_unit_test.c b/src/test/njs_unit_test.c
index 30b7ba1b648d2de8a788e64fa9d2aba5a44389f4..edbddd79b817abe1a6bf5c4cacb32f4a2d2cc204 100644 (file)
--- a/src/test/njs_unit_test.c
+++ b/src/test/njs_unit_test.c
@@ -11834,6 +11834,108 @@ static njs_unit_test_t  njs_test[] =
                  "o.a"),
       njs_str("7") },
 
+    { njs_str("let a = 1;"
+              "function f(x, y) { return x + ':' + y; }"
+              "f(a, a = 2)"),
+      njs_str("1:2") },
+
+    { njs_str("let a = 1, b = 4;"
+              "function f(x, y) { return x + ':' + y; }"
+              "f(a = b, b = 2)"),
+      njs_str("4:2") },
+
+    { njs_str("function f(x, y) { return x + ':' + y; }"
+              "function g(a) { return f(a, a = 2); }"
+              "g(1)"),
+      njs_str("1:2") },
+
+    { njs_str("let a = 1;"
+              "let o = { m: function(x, y) { return x + ':' + y; } };"
+              "o.m(a, a = 2)"),
+      njs_str("1:2") },
+
+    { njs_str("let a = 1;"
+              "let o = { get m() { a = 9;"
+              "    return function(x, y) { return x + ':' + y + ':' + a; };"
+              "} };"
+              "o.m(a, a = 2)"),
+      njs_str("9:2:2") },
+
+    { njs_str("let a = 1;"
+              "let o = { get x() { a = 2; return 3; } };"
+              "function f(x, y) { return x + ':' + y; }"
+              "f(a, o.x)"),
+      njs_str("1:3") },
+
+    { njs_str("let a = 1;"
+              "function f(x, y) { return x + ':' + y; }"
+              "Object.defineProperty(globalThis, 'b', {"
+              "    get: function() { a = 2; return 3; },"
+              "    configurable: true"
+              "});"
+              "let r = f(a, b);"
+              "delete globalThis.b;"
+              "r"),
+      njs_str("1:3") },
+
+    { njs_str("let a = 1;"
+              "function f(x, y, z) { return x + ':' + y + ':' + z; }"
+              "f(a, (a = 2, a), a = 3)"),
+      njs_str("1:2:3") },
+
+    { njs_str("let a = 1;"
+              "function f(x, y, z) { return x + ':' + y + ':' + z; }"
+              "f(a, 0, a = 2)"),
+      njs_str("1:0:2") },
+
+    { njs_str("let a = 1;"
+              "function f(x, y) { return x + ':' + y; }"
+              "f(a, ++a)"),
+      njs_str("1:2") },
+
+    { njs_str("let a = 1;"
+              "function f(x, y) { return x + ':' + y; }"
+              "f(a, true ? (a = 2) : 0)"),
+      njs_str("1:2") },
+
+    { njs_str("let a = 0;"
+              "function f(x, y) { return x + ':' + y; }"
+              "f(a, a ||= 2)"),
+      njs_str("0:2") },
+
+    { njs_str("let a = 1;"
+              "function f(x, y) { return x + ':' + y; }"
+              "function g(v) { return v; }"
+              "f(a, g(a = 2))"),
+      njs_str("1:2") },
+
+    { njs_str("function f(x, y) { return x + ':' + y; }"
+              "function g() { let a = 1;"
+              "    return function() { return f(a, a = 2); };"
+              "}"
+              "g()()"),
+      njs_str("1:2") },
+
+    { njs_str("let a = 1;"
+              "function F(x, y) { this.v = x + ':' + y; }"
+              "new F(a, a = 2).v"),
+      njs_str("1:2") },
+
+    { njs_str("let a = 1;"
+              "function f(x, y, z) { return x + ':' + y + ':' + z; }"
+              "f(a, [a = 2][0], { k: a = 3 }.k)"),
+      njs_str("1:2:3") },
+
+    { njs_str("let o = { x: 1 };"
+              "function f(x, y) { return x + ':' + y; }"
+              "f(o.x, o.x = 2)"),
+      njs_str("1:2") },
+
+    { njs_str("let a = 1;"
+              "let f = function(x, y) { return 'orig:' + x + ':' + y; };"
+              "f(a, (f = function() { return 'new'; }, a = 2))"),
+      njs_str("orig:1:2") },
+
     { njs_str("function F(a, b) { return }"
                  "F.prototype.constructor === F"),
       njs_str("true") },