Generator: bound value preservation analysis

Value-preservation analysis recursively examines the upcoming expression before
the generator decides whether mutable-slot operands need temporary copies.
Deeply nested binary expressions and arrays could therefore exhaust the native
stack.

Move the generator-specific predicate out of the parser API and limit its
recursive walk to 128 levels.  At the limit, conservatively require
preservation: an extra move is safe, whereas missing a deeper side effect can
change evaluation semantics.

diff --git a/src/njs_generator.c b/src/njs_generator.c
index 78db1b948abbc700b92be464fc72edc78354f902..3d8a3efa50f9855abd062beb26ba8d86b8713ddd 100644 (file)
--- a/src/njs_generator.c
+++ b/src/njs_generator.c
@@ -11,6 +11,7 @@
 
 
 #define NJS_FUNCTION_MAX_DEPTH  128
+#define NJS_PRESERVATION_MAX_DEPTH  128
 
 
 typedef struct njs_generator_patch_s   njs_generator_patch_t;
@@ -224,6 +225,67 @@ njs_generate_function_call_this(njs_parser_node_t *node)
 }
 
 
+static njs_bool_t
+njs_generate_expr_requires_preservation_depth(njs_parser_node_t *node,
+    uint32_t depth)
+{
+    uint32_t                 i;
+    njs_bool_t               preserve;
+    njs_parser_array_item_t  *item;
+
+    if (node == NULL) {
+        return 0;
+    }
+
+    if (depth >= NJS_PRESERVATION_MAX_DEPTH) {
+        return 1;
+    }
+
+    if (node->token_type >= NJS_TOKEN_ASSIGNMENT
+        && node->token_type <= NJS_TOKEN_LAST_ASSIGNMENT)
+    {
+        return 1;
+    }
+
+    if (node->token_type == NJS_TOKEN_FUNCTION_CALL
+        || node->token_type == NJS_TOKEN_METHOD_CALL)
+    {
+        return 1;
+    }
+
+    if (node->token_type == NJS_TOKEN_ARRAY && node->u.array.items != NULL) {
+        for (i = 0; i < node->u.array.items->items; i++) {
+            item = njs_arr_item(node->u.array.items, i);
+
+            if (njs_generate_expr_requires_preservation_depth(item->value,
+                                                              depth + 1))
+            {
+                return 1;
+            }
+        }
+
+        return 0;
+    }
+
+    preserve = njs_generate_expr_requires_preservation_depth(node->left,
+                                                             depth + 1);
+
+    if (njs_fast_path(!preserve)) {
+        return njs_generate_expr_requires_preservation_depth(node->right,
+                                                             depth + 1);
+    }
+
+    return preserve;
+}
+
+
+static njs_bool_t
+njs_generate_expr_requires_preservation(njs_parser_node_t *node)
+{
+    return njs_generate_expr_requires_preservation_depth(node, 0);
+}
+
+
 static u_char *njs_generate_reserve(njs_vm_t *vm, njs_generator_t *generator,
     size_t size);
 static njs_int_t njs_generate_code_map(njs_vm_t *vm, njs_generator_t *generator,
@@ -3238,7 +3300,7 @@ njs_generate_preserve_property_lvalue(njs_vm_t *vm,
     njs_parser_node_t  *object, *property;
     njs_vmcode_move_t  *move;
 
-    if (!njs_slow_path(njs_parser_has_side_effect(expr))) {
+    if (!njs_slow_path(njs_generate_expr_requires_preservation(expr))) {
         return NJS_OK;
     }
 
@@ -3634,7 +3696,7 @@ njs_generate_operation_assignment(njs_vm_t *vm, njs_generator_t *generator,
         index = lvalue->index;
         expr = node->right;
 
-        if (njs_slow_path(njs_parser_has_side_effect(expr))) {
+        if (njs_slow_path(njs_generate_expr_requires_preservation(expr))) {
             /* Preserve variable value if it may be changed by expression. */
 
             njs_generate_code(generator, njs_vmcode_move_t, move,
@@ -4498,7 +4560,8 @@ njs_generate_3addr_operation_name(njs_vm_t *vm, njs_generator_t *generator,
 
     left = node->left;
 
-    if (njs_slow_path(njs_parser_has_side_effect(node->right))) {
+    if (njs_slow_path(njs_generate_expr_requires_preservation(node->right)))
+    {
         njs_generate_code(generator, njs_vmcode_move_t, move,
                           NJS_VMCODE_MOVE, node);
         move->src = left->index;

diff --git a/src/njs_parser.c b/src/njs_parser.c
index 91b324960faf98871d90c12355c01dd2c7419e24..79a07e31612777495cc43463a890b43e97280c35 100644 (file)
--- a/src/njs_parser.c
+++ b/src/njs_parser.c
@@ -8999,51 +8999,6 @@ invalid:
 }
 
 
-njs_bool_t
-njs_parser_has_side_effect(njs_parser_node_t *node)
-{
-    uint32_t                 i;
-    njs_bool_t               side_effect;
-    njs_parser_array_item_t  *item;
-
-    if (node == NULL) {
-        return 0;
-    }
-
-    if (node->token_type >= NJS_TOKEN_ASSIGNMENT
-        && node->token_type <= NJS_TOKEN_LAST_ASSIGNMENT)
-    {
-        return 1;
-    }
-
-    if (node->token_type == NJS_TOKEN_FUNCTION_CALL
-        || node->token_type == NJS_TOKEN_METHOD_CALL)
-    {
-        return 1;
-    }
-
-    if (node->token_type == NJS_TOKEN_ARRAY && node->u.array.items != NULL) {
-        for (i = 0; i < node->u.array.items->items; i++) {
-            item = njs_arr_item(node->u.array.items, i);
-
-            if (njs_parser_has_side_effect(item->value)) {
-                return 1;
-            }
-        }
-
-        return 0;
-    }
-
-    side_effect = njs_parser_has_side_effect(node->left);
-
-    if (njs_fast_path(!side_effect)) {
-        return njs_parser_has_side_effect(node->right);
-    }
-
-    return side_effect;
-}
-
-
 njs_int_t
 njs_parser_variable_reference(njs_parser_t *parser, njs_parser_scope_t *scope,
     njs_parser_node_t *node, uintptr_t atom_id)

diff --git a/src/njs_parser.h b/src/njs_parser.h
index 54d3cd8c474fad57b64792534f4baa4d08ca3ca2..16ceb3a8f612ab314b744e6a79a94a101170a1d6 100644 (file)
--- a/src/njs_parser.h
+++ b/src/njs_parser.h
@@ -155,7 +155,6 @@ njs_bool_t njs_variable_closure_test(njs_parser_scope_t *root,
     njs_parser_scope_t *scope);
 njs_variable_t *njs_variable_resolve(njs_vm_t *vm, njs_parser_node_t *node);
 njs_index_t njs_variable_index(njs_vm_t *vm, njs_parser_node_t *node);
-njs_bool_t njs_parser_has_side_effect(njs_parser_node_t *node);
 njs_int_t njs_parser_variable_reference(njs_parser_t *parser,
     njs_parser_scope_t *scope, njs_parser_node_t *node, uintptr_t atom_id);
 njs_token_type_t njs_parser_unexpected_token(njs_vm_t *vm, njs_parser_t *parser,

diff --git a/src/test/njs_unit_test.c b/src/test/njs_unit_test.c
index b609044c4e62c57aee6745554b37a520e7efcb28..e6368d8ce3cebbb28f937fe5ea07b020cc77c8cf 100644 (file)
--- a/src/test/njs_unit_test.c
+++ b/src/test/njs_unit_test.c
@@ -1158,6 +1158,22 @@ static njs_unit_test_t  njs_test[] =
     { njs_str("var a = 1; function f(x) { a = x; return 2 }; a += f(5)"),
       njs_str("3") },
 
+    { njs_str("var src = 'var a=1;return a+' + '('.repeat(256)"
+              "+ '0+'.repeat(256) + '(a=2)' + ')'.repeat(256);"
+              "(new Function(src))()"),
+      njs_str("3") },
+
+    { njs_str("var src = 'var a=1;a+=' + '('.repeat(256)"
+              "+ '0+'.repeat(256) + '(a=2)' + ')'.repeat(256) + ';return a';"
+              "(new Function(src))()"),
+      njs_str("3") },
+
+    { njs_str("var src = 'var i=0,a=[];a[i]=' + '['.repeat(256)"
+              "+ '(i=1)' + ']'.repeat(256)"
+              "+ ';return a[0] !== undefined && a[1] === undefined';"
+              "(new Function(src))()"),
+      njs_str("true") },
+
     { njs_str("var x; x in (x = 1, [1, 2, 3])"),
       njs_str("false") },