From: Dmitry Volyntsev Date: Fri, 15 May 2026 23:28:47 +0000 (-0700) Subject: Fixed call argument value snapshotting. X-Git-Tag: 0.9.9~2 X-Git-Url: http://git.kaiwu.me/postgresql/log/contrib/postgres_fdw/static/gitweb.js?a=commitdiff_plain;h=7d1706c20a71d3e864d61b74f9aa2448d265cce6;p=njs.git 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 89d08a11..1cd57df6 100644 --- 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 30b7ba1b..edbddd79 100644 --- 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") },