Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

internals: refactor error handling #42

Closed
drsm opened this issue Jul 28, 2018 · 2 comments
Closed

internals: refactor error handling #42

drsm opened this issue Jul 28, 2018 · 2 comments
Assignees
Labels

Comments

@drsm
Copy link
Contributor

drsm commented Jul 28, 2018

>> var a = Array(1111111111)
1111111111
    at native (native)
    at main (native)

While looking into #41 I've found a ...
here njs_memory_error will be called twice:

njs/njs/njs_string.c

Lines 177 to 179 in bd2e5d8

njs_memory_error(vm);
return NXT_ERROR;

here it wont be called at all:

njs/njs/njs_string.c

Lines 149 to 151 in bd2e5d8

if (nxt_slow_path(string == NULL)) {
return NXT_ERROR;
}

but the return value of njs_string_create is used directly in many top level functions.

here:

return NXT_ERROR;

and so on...

@xeioex xeioex added the bug label Jul 28, 2018
@xeioex xeioex self-assigned this Jul 28, 2018
@xeioex
Copy link
Contributor

xeioex commented Aug 1, 2018

Please, try the following patch

# HG changeset patch
# User Dmitry Volyntsev <xeioex@nginx.com>
# Date 1533144721 -10800
#      Wed Aug 01 20:32:01 2018 +0300
# Node ID 3ee49132b47b84b343ff7f02598df29e0f57b38f
# Parent  4586ae051111c1cf40764771f0ceccf76bac524e
Setting exception values where appropriate.

This fixes #42 issue on Github.

diff --git a/njs/njs_array.c b/njs/njs_array.c
--- a/njs/njs_array.c
+++ b/njs/njs_array.c
@@ -122,6 +122,7 @@ njs_array_alloc(njs_vm_t *vm, uint32_t l
     array->data = nxt_mem_cache_align(vm->mem_cache_pool, sizeof(njs_value_t),
                                       size * sizeof(njs_value_t));
     if (nxt_slow_path(array->data == NULL)) {
+        njs_memory_error(vm);
         return NULL;
     }
 
@@ -194,6 +195,7 @@ njs_array_expand(njs_vm_t *vm, njs_array
     start = nxt_mem_cache_align(vm->mem_cache_pool, sizeof(njs_value_t),
                                 (prepend + size) * sizeof(njs_value_t));
     if (nxt_slow_path(start == NULL)) {
+        njs_memory_error(vm);
         return NXT_ERROR;
     }
 
@@ -230,7 +232,7 @@ njs_array_constructor(njs_vm_t *vm, njs_
         size = (uint32_t) num;
 
         if ((double) size != num) {
-            njs_range_error(vm, NULL);
+            njs_range_error(vm, "Invalid array length");
             return NXT_ERROR;
         }
 
@@ -393,7 +395,6 @@ njs_array_prototype_length(njs_vm_t *vm,
         if (size > 0) {
             ret = njs_array_expand(vm, array, 0, size);
             if (nxt_slow_path(ret != NXT_OK)) {
-                njs_memory_error(vm);
                 return NJS_ERROR;
             }
 
@@ -846,6 +847,7 @@ njs_array_prototype_join(njs_vm_t *vm, n
         values = nxt_mem_cache_align(vm->mem_cache_pool, sizeof(njs_value_t),
                                      sizeof(njs_value_t) * max);
         if (nxt_slow_path(values == NULL)) {
+            njs_memory_error(vm);
             return NXT_ERROR;
         }
 
diff --git a/njs/njs_crypto.c b/njs/njs_crypto.c
--- a/njs/njs_crypto.c
+++ b/njs/njs_crypto.c
@@ -142,9 +142,12 @@ njs_crypto_object_value_alloc(njs_vm_t *
         ov->object.extensible = 1;
 
         ov->object.__proto__ = &vm->prototypes[proto].object;
+        return ov;
     }
 
-    return ov;
+    njs_memory_error(vm);
+
+    return NULL;
 }
 
 
@@ -171,12 +174,13 @@ njs_crypto_create_hash(njs_vm_t *vm, njs
 
     hash = njs_crypto_object_value_alloc(vm, NJS_PROTOTYPE_CRYPTO_HASH);
     if (nxt_slow_path(hash == NULL)) {
-        goto memory_error;
+        return NJS_ERROR;
     }
 
     dgst = nxt_mem_cache_alloc(vm->mem_cache_pool, sizeof(njs_digest_t));
     if (nxt_slow_path(dgst == NULL)) {
-        goto memory_error;
+        njs_memory_error(vm);
+        return NJS_ERROR;
     }
 
     dgst->alg = alg;
@@ -190,12 +194,6 @@ njs_crypto_create_hash(njs_vm_t *vm, njs
     vm->retval.data.truth = 1;
 
     return NJS_OK;
-
-memory_error:
-
-    njs_memory_error(vm);
-
-    return NJS_ERROR;
 }
 
 
@@ -412,7 +410,8 @@ njs_crypto_create_hmac(njs_vm_t *vm, njs
 
     ctx = nxt_mem_cache_alloc(vm->mem_cache_pool, sizeof(njs_hmac_t));
     if (nxt_slow_path(ctx == NULL)) {
-        goto memory_error;
+        njs_memory_error(vm);
+        return NJS_ERROR;
     }
 
     ctx->alg = alg;
@@ -443,7 +442,7 @@ njs_crypto_create_hmac(njs_vm_t *vm, njs
 
     hmac = njs_crypto_object_value_alloc(vm, NJS_PROTOTYPE_CRYPTO_HMAC);
     if (nxt_slow_path(hmac == NULL)) {
-        goto memory_error;
+        return NJS_ERROR;
     }
 
     njs_value_data_set(&hmac->value, ctx);
@@ -453,12 +452,6 @@ njs_crypto_create_hmac(njs_vm_t *vm, njs
     vm->retval.data.truth = 1;
 
     return NJS_OK;
-
-memory_error:
-
-    njs_memory_error(vm);
-
-    return NJS_ERROR;
 }
 
 
diff --git a/njs/njs_date.c b/njs/njs_date.c
--- a/njs/njs_date.c
+++ b/njs/njs_date.c
@@ -129,6 +129,7 @@ njs_date_constructor(njs_vm_t *vm, njs_v
 
         date = nxt_mem_cache_alloc(vm->mem_cache_pool, sizeof(njs_date_t));
         if (nxt_slow_path(date == NULL)) {
+            njs_memory_error(vm);
             return NXT_ERROR;
         }
 
@@ -1052,9 +1053,9 @@ njs_date_to_string(njs_vm_t *vm, njs_val
         return njs_string_new(vm, retval, buf, size, size);
     }
 
-    njs_range_error(vm, NULL);
-
-    return NXT_ERROR;
+    vm->retval = njs_string_invalid_date;
+
+    return NXT_OK;
 }
 
 
diff --git a/njs/njs_error.c b/njs/njs_error.c
--- a/njs/njs_error.c
+++ b/njs/njs_error.c
@@ -36,23 +36,15 @@ njs_exception_error_create(njs_vm_t *vm,
 
     ret = njs_string_new(vm, &string, (const u_char *) buf, size, size);
     if (nxt_slow_path(ret != NXT_OK)) {
-        goto memory_error;
+        return;
     }
 
     error = njs_error_alloc(vm, type, NULL, &string);
-    if (nxt_slow_path(error == NULL)) {
-        goto memory_error;
+    if (nxt_fast_path(error != NULL)) {
+        vm->retval.data.u.object = error;
+        vm->retval.type = type;
+        vm->retval.data.truth = 1;
     }
-
-    vm->retval.data.u.object = error;
-    vm->retval.type = type;
-    vm->retval.data.truth = 1;
-
-    return;
-
-memory_error:
-
-    njs_memory_error(vm);
 }
 
 
@@ -67,6 +59,7 @@ njs_error_alloc(njs_vm_t *vm, njs_value_
 
     error = nxt_mem_cache_alloc(vm->mem_cache_pool, sizeof(njs_object_t));
     if (nxt_slow_path(error == NULL)) {
+        njs_memory_error(vm);
         return NULL;
     }
 
@@ -94,6 +87,7 @@ njs_error_alloc(njs_vm_t *vm, njs_value_
 
         ret = nxt_lvlhsh_insert(&error->hash, &lhq);
         if (nxt_slow_path(ret != NXT_OK)) {
+            njs_internal_error(vm, NULL);
             return NULL;
         }
     }
@@ -114,6 +108,7 @@ njs_error_alloc(njs_vm_t *vm, njs_value_
 
         ret = nxt_lvlhsh_insert(&error->hash, &lhq);
         if (nxt_slow_path(ret != NXT_OK)) {
+            njs_internal_error(vm, NULL);
             return NULL;
         }
     }
@@ -138,7 +133,6 @@ njs_error_create(njs_vm_t *vm, njs_value
 
     error = njs_error_alloc(vm, type, NULL, value);
     if (nxt_slow_path(error == NULL)) {
-        njs_memory_error(vm);
         return NXT_ERROR;
     }
 
diff --git a/njs/njs_extern.c b/njs/njs_extern.c
--- a/njs/njs_extern.c
+++ b/njs/njs_extern.c
@@ -81,7 +81,7 @@ njs_vm_external_add(njs_vm_t *vm, nxt_lv
     do {
         ext = nxt_mem_cache_alloc(vm->mem_cache_pool, sizeof(njs_extern_t));
         if (nxt_slow_path(ext == NULL)) {
-            return NULL;
+            goto memory_error;
         }
 
         ext->name = external->name;
@@ -100,7 +100,7 @@ njs_vm_external_add(njs_vm_t *vm, nxt_lv
             function = nxt_mem_cache_zalloc(vm->mem_cache_pool,
                                             sizeof(njs_function_t));
             if (nxt_slow_path(function == NULL)) {
-                return NULL;
+                goto memory_error;
             }
 
             /*
@@ -129,7 +129,7 @@ njs_vm_external_add(njs_vm_t *vm, nxt_lv
             child = njs_vm_external_add(vm, &ext->hash, external->properties,
                                         external->nproperties);
             if (nxt_slow_path(child == NULL)) {
-                return NULL;
+                goto memory_error;
             }
         }
 
@@ -144,6 +144,7 @@ njs_vm_external_add(njs_vm_t *vm, nxt_lv
 
             ret = nxt_lvlhsh_insert(hash, &lhq);
             if (nxt_slow_path(ret != NXT_OK)) {
+                njs_internal_error(vm, NULL);
                 return NULL;
             }
         }
@@ -154,6 +155,12 @@ njs_vm_external_add(njs_vm_t *vm, nxt_lv
     } while (n != 0);
 
     return ext;
+
+memory_error:
+
+    njs_memory_error(vm);
+
+    return NULL;
 }
 
 
@@ -206,6 +213,7 @@ njs_vm_external_bind(njs_vm_t *vm, const
     ev = nxt_mem_cache_align(vm->mem_cache_pool, sizeof(njs_value_t),
                              sizeof(njs_extern_value_t));
     if (nxt_slow_path(ev == NULL)) {
+        njs_memory_error(vm);
         return NXT_ERROR;
     }
 
@@ -221,6 +229,7 @@ njs_vm_external_bind(njs_vm_t *vm, const
 
     ret = nxt_lvlhsh_insert(&vm->externals_hash, &lhq);
     if (nxt_slow_path(ret != NXT_OK)) {
+        njs_internal_error(vm, NULL);
         return ret;
     }
 
diff --git a/njs/njs_fs.c b/njs/njs_fs.c
--- a/njs/njs_fs.c
+++ b/njs/njs_fs.c
@@ -221,7 +221,7 @@ njs_fs_read_file(njs_vm_t *vm, njs_value
 
     start = njs_string_alloc(vm, &arguments[2], sb.st_size, length);
     if (nxt_slow_path(start == NULL)) {
-        goto memory_error;
+        goto fail;
     }
 
     p = start;
@@ -286,14 +286,12 @@ done:
     return njs_function_apply(vm, callback->data.u.function,
                               arguments, 3, (njs_index_t) &vm->retval);
 
-memory_error:
+fail:
 
     if (fd != -1) {
         (void) close(fd);
     }
 
-    njs_memory_error(vm);
-
     return NJS_ERROR;
 }
 
@@ -420,7 +418,7 @@ njs_fs_read_file_sync(njs_vm_t *vm, njs_
 
     start = njs_string_alloc(vm, &vm->retval, sb.st_size, length);
     if (nxt_slow_path(start == NULL)) {
-        goto memory_error;
+        goto fail;
     }
 
     p = start;
@@ -472,14 +470,12 @@ done:
 
     return NJS_OK;
 
-memory_error:
+fail:
 
     if (fd != -1) {
         (void) close(fd);
     }
 
-    njs_memory_error(vm);
-
     return NJS_ERROR;
 }
 
@@ -898,12 +894,12 @@ static njs_ret_t njs_fs_error(njs_vm_t *
 
     ret = njs_string_new(vm, &string, (u_char *) description, size, size);
     if (nxt_slow_path(ret != NXT_OK)) {
-        goto memory_error;
+        return NJS_ERROR;
     }
 
     error = njs_error_alloc(vm, NJS_OBJECT_ERROR, NULL, &string);
     if (nxt_slow_path(error == NULL)) {
-        goto memory_error;
+        return NJS_ERROR;
     }
 
     lhq.replace = 0;
@@ -920,7 +916,7 @@ static njs_ret_t njs_fs_error(njs_vm_t *
 
         prop = njs_object_prop_alloc(vm, &njs_fs_errno_string, &value, 1);
         if (nxt_slow_path(prop == NULL)) {
-            goto memory_error;
+            return NJS_ERROR;
         }
 
         lhq.value = prop;
@@ -939,7 +935,7 @@ static njs_ret_t njs_fs_error(njs_vm_t *
 
         prop = njs_object_prop_alloc(vm, &njs_fs_path_string, path, 1);
         if (nxt_slow_path(prop == NULL)) {
-            goto memory_error;
+            return NJS_ERROR;
         }
 
         lhq.value = prop;
@@ -955,7 +951,7 @@ static njs_ret_t njs_fs_error(njs_vm_t *
         size = strlen(syscall);
         ret = njs_string_new(vm, &string, (u_char *) syscall, size, size);
         if (nxt_slow_path(ret != NXT_OK)) {
-            goto memory_error;
+            return NJS_ERROR;
         }
 
         lhq.key = nxt_string_value("sycall");
@@ -964,7 +960,7 @@ static njs_ret_t njs_fs_error(njs_vm_t *
 
         prop = njs_object_prop_alloc(vm, &njs_fs_syscall_string, &string, 1);
         if (nxt_slow_path(prop == NULL)) {
-            goto memory_error;
+            return NJS_ERROR;
         }
 
         lhq.value = prop;
@@ -981,12 +977,6 @@ static njs_ret_t njs_fs_error(njs_vm_t *
     retval->data.truth = 1;
 
     return NJS_OK;
-
-memory_error:
-
-    njs_memory_error(vm);
-
-    return NJS_ERROR;
 }
 
 
diff --git a/njs/njs_function.c b/njs/njs_function.c
--- a/njs/njs_function.c
+++ b/njs/njs_function.c
@@ -35,11 +35,16 @@ njs_function_alloc(njs_vm_t *vm)
         function->u.lambda = nxt_mem_cache_zalloc(vm->mem_cache_pool,
                                                  sizeof(njs_function_lambda_t));
         if (nxt_slow_path(function->u.lambda == NULL)) {
+            njs_memory_error(vm);
             return NULL;
         }
+
+        return function;
     }
 
-    return function;
+    njs_memory_error(vm);
+
+    return NULL;
 }
 
 
@@ -62,7 +67,8 @@ njs_function_value_copy(njs_vm_t *vm, nj
 
     copy = nxt_mem_cache_alloc(vm->mem_cache_pool, size);
     if (nxt_slow_path(copy == NULL)) {
-        return copy;
+        njs_memory_error(vm);
+        return NULL;
     }
 
     value->data.u.function = copy;
@@ -247,6 +253,7 @@ njs_function_frame_alloc(njs_vm_t *vm, s
         frame = nxt_mem_cache_align(vm->mem_cache_pool, sizeof(njs_value_t),
                                     spare_size);
         if (nxt_slow_path(frame == NULL)) {
+            njs_memory_error(vm);
             return NULL;
         }
 
@@ -365,6 +372,7 @@ njs_function_call(njs_vm_t *vm, njs_inde
             closure = nxt_mem_cache_align(vm->mem_cache_pool,
                                           sizeof(njs_value_t), size);
             if (nxt_slow_path(closure == NULL)) {
+                njs_memory_error(vm);
                 return NXT_ERROR;
             }
 
@@ -618,6 +626,7 @@ njs_function_prototype_bind(njs_vm_t *vm
 
     function = nxt_mem_cache_alloc(vm->mem_cache_pool, sizeof(njs_function_t));
     if (nxt_slow_path(function == NULL)) {
+        njs_memory_error(vm);
         return NXT_ERROR;
     }
 
@@ -640,6 +649,7 @@ njs_function_prototype_bind(njs_vm_t *vm
 
     values = nxt_mem_cache_alloc(vm->mem_cache_pool, size);
     if (nxt_slow_path(values == NULL)) {
+        njs_memory_error(vm);
         nxt_mem_cache_free(vm->mem_cache_pool, function);
         return NXT_ERROR;
     }
diff --git a/njs/njs_object.c b/njs/njs_object.c
--- a/njs/njs_object.c
+++ b/njs/njs_object.c
@@ -34,9 +34,12 @@ njs_object_alloc(njs_vm_t *vm)
         object->type = NJS_OBJECT;
         object->shared = 0;
         object->extensible = 1;
+        return object;
     }
 
-    return object;
+    njs_memory_error(vm);
+
+    return NULL;
 }
 
 
@@ -58,9 +61,12 @@ njs_object_value_copy(njs_vm_t *vm, njs_
         object->__proto__ = &vm->prototypes[NJS_PROTOTYPE_OBJECT].object;
         object->shared = 0;
         value->data.u.object = object;
+        return object;
     }
 
-    return object;
+    njs_memory_error(vm);
+
+    return NULL;
 }
 
 
@@ -83,9 +89,13 @@ njs_object_value_alloc(njs_vm_t *vm, con
         ov->object.__proto__ = &vm->prototypes[index].object;
 
         ov->value = *value;
+
+        return &ov->object;
     }
 
-    return &ov->object;
+    njs_memory_error(vm);
+
+    return NULL;
 }
 
 
@@ -107,6 +117,7 @@ njs_object_hash_create(njs_vm_t *vm, nxt
 
         ret = nxt_lvlhsh_insert(hash, &lhq);
         if (nxt_slow_path(ret != NXT_OK)) {
+            njs_internal_error(vm, NULL);
             return NXT_ERROR;
         }
 
@@ -183,9 +194,12 @@ njs_object_prop_alloc(njs_vm_t *vm, cons
         prop->enumerable = attributes;
         prop->writable = attributes;
         prop->configurable = attributes;
+        return prop;
     }
 
-    return prop;
+    njs_memory_error(vm);
+
+    return NULL;
 }
 
 
@@ -988,6 +1002,7 @@ njs_object_get_own_property_descriptor(n
 
     ret = nxt_lvlhsh_insert(&descriptor->hash, &lhq);
     if (nxt_slow_path(ret != NXT_OK)) {
+        njs_internal_error(vm, NULL);
         return NXT_ERROR;
     }
 
@@ -1005,6 +1020,7 @@ njs_object_get_own_property_descriptor(n
 
     ret = nxt_lvlhsh_insert(&descriptor->hash, &lhq);
     if (nxt_slow_path(ret != NXT_OK)) {
+        njs_internal_error(vm, NULL);
         return NXT_ERROR;
     }
 
@@ -1022,6 +1038,7 @@ njs_object_get_own_property_descriptor(n
 
     ret = nxt_lvlhsh_insert(&descriptor->hash, &lhq);
     if (nxt_slow_path(ret != NXT_OK)) {
+        njs_internal_error(vm, NULL);
         return NXT_ERROR;
     }
 
@@ -1039,6 +1056,7 @@ njs_object_get_own_property_descriptor(n
 
     ret = nxt_lvlhsh_insert(&descriptor->hash, &lhq);
     if (nxt_slow_path(ret != NXT_OK)) {
+        njs_internal_error(vm, NULL);
         return NXT_ERROR;
     }
 
@@ -1395,7 +1413,6 @@ njs_property_prototype_create(njs_vm_t *
         return &prop->value;
     }
 
-    /* Memory allocation or NXT_DECLINED error. */
     njs_internal_error(vm, NULL);
 
     return NULL;
@@ -1638,7 +1655,6 @@ njs_property_constructor_create(njs_vm_t
         return &prop->value;
     }
 
-    /* Memory allocation or NXT_DECLINED error. */
     njs_internal_error(vm, NULL);
 
     return NULL;
diff --git a/njs/njs_regexp.c b/njs/njs_regexp.c
--- a/njs/njs_regexp.c
+++ b/njs/njs_regexp.c
@@ -34,11 +34,13 @@ njs_regexp_init(njs_vm_t *vm)
     vm->regex_context = nxt_regex_context_create(njs_regexp_malloc,
                                           njs_regexp_free, vm->mem_cache_pool);
     if (nxt_slow_path(vm->regex_context == NULL)) {
+        njs_memory_error(vm);
         return NXT_ERROR;
     }
 
     vm->single_match_data = nxt_regex_match_data(NULL, vm->regex_context);
     if (nxt_slow_path(vm->single_match_data == NULL)) {
+        njs_memory_error(vm);
         return NXT_ERROR;
     }
 
@@ -66,6 +68,7 @@ njs_ret_t
 njs_regexp_constructor(njs_vm_t *vm, njs_value_t *args, nxt_uint_t nargs,
     njs_index_t unused)
 {
+    u_char              *start;
     nxt_str_t           string;
     njs_regexp_flags_t  flags;
 
@@ -81,9 +84,12 @@ njs_regexp_constructor(njs_vm_t *vm, njs
     default:
         njs_string_get(&args[2], &string);
 
-        flags = njs_regexp_flags(&string.start, string.start + string.length,
-                                 1);
+        start = string.start;
+
+        flags = njs_regexp_flags(&start, start + string.length, 1);
         if (nxt_slow_path(flags < 0)) {
+            njs_syntax_error(vm, "Invalid RegExp flags \"%.*s\"",
+                             (int) string.length, string.start);
             return NXT_ERROR;
         }
 
@@ -267,6 +273,7 @@ njs_regexp_pattern_create(njs_vm_t *vm, 
                                    sizeof(njs_regexp_pattern_t)
                                    + 1 + length + size + 1);
     if (nxt_slow_path(pattern == NULL)) {
+        njs_memory_error(vm);
         return NULL;
     }
 
@@ -434,9 +441,12 @@ njs_regexp_alloc(njs_vm_t *vm, njs_regex
         regexp->object.extensible = 1;
         regexp->last_index = 0;
         regexp->pattern = pattern;
+        return regexp;
     }
 
-    return regexp;
+    njs_memory_error(vm);
+
+    return NULL;
 }
 
 
@@ -655,6 +665,7 @@ njs_regexp_prototype_exec(njs_vm_t *vm, 
         match_data = nxt_regex_match_data(&pattern->regex[type],
                                           vm->regex_context);
         if (nxt_slow_path(match_data == NULL)) {
+            njs_memory_error(vm);
             return NXT_ERROR;
         }
 
@@ -744,6 +755,7 @@ njs_regexp_exec_result(njs_vm_t *vm, njs
 
     ret = nxt_lvlhsh_insert(&array->object.hash, &lhq);
     if (nxt_slow_path(ret != NXT_OK)) {
+        njs_internal_error(vm, NULL);
         goto fail;
     }
 
diff --git a/njs/njs_string.c b/njs/njs_string.c
--- a/njs/njs_string.c
+++ b/njs/njs_string.c
@@ -147,6 +147,7 @@ njs_string_create(njs_vm_t *vm, njs_valu
 
         string = nxt_mem_cache_alloc(vm->mem_cache_pool, sizeof(njs_string_t));
         if (nxt_slow_path(string == NULL)) {
+            njs_memory_error(vm);
             return NXT_ERROR;
         }
 
@@ -174,8 +175,6 @@ njs_string_new(njs_vm_t *vm, njs_value_t
         return NXT_OK;
     }
 
-    njs_memory_error(vm);
-
     return NXT_ERROR;
 }
 
@@ -491,6 +490,7 @@ njs_string_validate(njs_vm_t *vm, njs_st
 
                     start = nxt_mem_cache_alloc(vm->mem_cache_pool, new_size);
                     if (nxt_slow_path(start == NULL)) {
+                        njs_memory_error(vm);
                         return NXT_ERROR;
                     }
 
@@ -1063,6 +1063,8 @@ njs_string_prototype_to_bytes(njs_vm_t *
         return NXT_OK;
     }
 
+    njs_memory_error(vm);
+
     return NXT_ERROR;
 }
 
diff --git a/njs/njs_time.c b/njs/njs_time.c
--- a/njs/njs_time.c
+++ b/njs/njs_time.c
@@ -71,6 +71,7 @@ njs_set_timeout(njs_vm_t *vm, njs_value_
 memory_error:
 
     njs_memory_error(vm);
+
     return NJS_ERROR;
 }
 
diff --git a/njs/njs_variable.c b/njs/njs_variable.c
--- a/njs/njs_variable.c
+++ b/njs/njs_variable.c
@@ -83,6 +83,7 @@ njs_builtin_add(njs_vm_t *vm, njs_parser
     ret = nxt_lvlhsh_insert(&scope->variables, &lhq);
 
     if (nxt_fast_path(ret == NXT_OK)) {
+        njs_internal_error(vm, NULL);
         return var;
     }
 
@@ -397,6 +398,7 @@ njs_variable_get(njs_vm_t *vm, njs_parse
         value = nxt_mem_cache_align(vm->mem_cache_pool, sizeof(njs_value_t),
                                     sizeof(njs_value_t));
         if (nxt_slow_path(value == NULL)) {
+            njs_memory_error(vm);
             return NULL;
         }
 
@@ -503,6 +505,7 @@ njs_variable_alloc(njs_vm_t *vm, nxt_str
 
     var = nxt_mem_cache_zalloc(vm->mem_cache_pool, sizeof(njs_variable_t));
     if (nxt_slow_path(var == NULL)) {
+        njs_memory_error(vm);
         return NULL;
     }
 
@@ -533,6 +536,8 @@ njs_name_copy(njs_vm_t *vm, nxt_str_t *d
         return NXT_OK;
     }
 
+    njs_memory_error(vm);
+
     return NXT_ERROR;
 }
 
diff --git a/njs/njs_vm.c b/njs/njs_vm.c
--- a/njs/njs_vm.c
+++ b/njs/njs_vm.c
@@ -383,6 +383,7 @@ njs_vmcode_function(njs_vm_t *vm, njs_va
 
     function = nxt_mem_cache_zalloc(vm->mem_cache_pool, size);
     if (nxt_slow_path(function == NULL)) {
+        njs_memory_error(vm);
         return NXT_ERROR;
     }
 
@@ -935,6 +936,7 @@ njs_method_private_copy(njs_vm_t *vm, nj
 
     prop = nxt_mem_cache_alloc(vm->mem_cache_pool, sizeof(njs_object_prop_t));
     if (nxt_slow_path(prop == NULL)) {
+        njs_memory_error(vm);
         return NXT_ERROR;
     }
 
@@ -968,6 +970,7 @@ njs_vmcode_property_foreach(njs_vm_t *vm
         next = nxt_mem_cache_alloc(vm->mem_cache_pool,
                                    sizeof(njs_property_next_t));
         if (nxt_slow_path(next == NULL)) {
+            njs_memory_error(vm);
             return NXT_ERROR;
         }
 
@@ -2721,6 +2724,7 @@ njs_vmcode_try_start(njs_vm_t *vm, njs_v
     if (vm->top_frame->exception.catch != NULL) {
         e = nxt_mem_cache_alloc(vm->mem_cache_pool, sizeof(njs_exception_t));
         if (nxt_slow_path(e == NULL)) {
+            njs_memory_error(vm);
             return NXT_ERROR;
         }
 
@@ -3298,6 +3302,7 @@ again:
             if (size != NJS_STRING_LONG) {
                 start = nxt_mem_cache_alloc(vm->mem_cache_pool, size);
                 if (nxt_slow_path(start == NULL)) {
+                    njs_memory_error(vm);
                     return NXT_ERROR;
                 }
 
@@ -3332,6 +3337,7 @@ again:
 
                 p = nxt_mem_cache_alloc(vm->mem_cache_pool, len);
                 if (p == NULL) {
+                    njs_memory_error(vm);
                     return NXT_ERROR;
                 }
 
diff --git a/njs/test/njs_unit_test.c b/njs/test/njs_unit_test.c
--- a/njs/test/njs_unit_test.c
+++ b/njs/test/njs_unit_test.c
@@ -5566,6 +5566,9 @@ static njs_unit_test_t  njs_test[] =
     { nxt_string("var r = new RegExp('abc', 'i'); r.test('00ABC11')"),
       nxt_string("true") },
 
+    { nxt_string("new RegExp('', 'x')"),
+      nxt_string("SyntaxError: Invalid RegExp flags \"x\"") },
+
     { nxt_string("[0].map(RegExp().toString)"),
       nxt_string("TypeError: 'this' argument is not a regexp") },
 
@@ -6081,16 +6084,19 @@ static njs_unit_test_t  njs_test[] =
       nxt_string("1,two,3") },
 
     { nxt_string("var a = Array(-1)"),
-      nxt_string("RangeError") },
+      nxt_string("RangeError: Invalid array length") },
 
     { nxt_string("var a = Array(2.5)"),
-      nxt_string("RangeError") },
+      nxt_string("RangeError: Invalid array length") },
 
     { nxt_string("var a = Array(NaN)"),
-      nxt_string("RangeError") },
+      nxt_string("RangeError: Invalid array length") },
 
     { nxt_string("var a = Array(Infinity)"),
-      nxt_string("RangeError") },
+      nxt_string("RangeError: Invalid array length") },
+
+    { nxt_string("var a = Array(1111111111)"),
+      nxt_string("MemoryError") },
 
     { nxt_string("var a = new Array(3); a"),
       nxt_string(",,") },
@@ -7533,6 +7539,9 @@ static njs_unit_test_t  njs_test[] =
     { nxt_string("var d = new Date(); d.__proto__ === Date.prototype"),
       nxt_string("true") },
 
+    { nxt_string("new Date(NaN)"),
+      nxt_string("Invalid Date") },
+
     { nxt_string("[0].map(new Date().getDate)"),
       nxt_string("TypeError: cannot convert void to date") },
 

@drsm
Copy link
Contributor Author

drsm commented Aug 1, 2018

The patch works for me.
I think this is also related, please take a look:

diff --git a/njs/njs_json.c b/njs/njs_json.c
--- a/njs/njs_json.c
+++ b/njs/njs_json.c
@@ -535,7 +535,6 @@ njs_json_parse_array(njs_json_parse_ctx_
 
     array = njs_array_alloc(ctx->vm, 0, 0);
     if (nxt_slow_path(array == NULL)) {
-        njs_memory_error(ctx->vm);
         return NULL;
     }
 
@@ -812,7 +811,6 @@ njs_json_parse_string(njs_json_parse_ctx
 
     ret = njs_string_create(ctx->vm, value, (u_char *) start, size, length);
     if (nxt_slow_path(ret != NXT_OK)) {
-        njs_memory_error(ctx->vm);
         return NULL;
     }
 
diff --git a/njs/njs_string.c b/njs/njs_string.c
--- a/njs/njs_string.c
+++ b/njs/njs_string.c
@@ -292,8 +292,6 @@ njs_string_hex(njs_vm_t *vm, njs_value_t
         return NXT_OK;
     }
 
-    njs_memory_error(vm);
-
     return NXT_ERROR;
 }
 
@@ -385,7 +383,6 @@ njs_string_base64(njs_vm_t *vm, njs_valu
 
     dst.start = njs_string_alloc(vm, &vm->retval, dst.length, dst.length);
     if (nxt_slow_path(dst.start == NULL)) {
-        njs_memory_error(vm);
         return NXT_ERROR;
     }
 
@@ -417,7 +414,6 @@ njs_string_base64url(njs_vm_t *vm, njs_v
 
     dst.start = njs_string_alloc(vm, &vm->retval, dst.length, dst.length);
     if (nxt_slow_path(dst.start == NULL)) {
-        njs_memory_error(vm);
         return NXT_ERROR;
     }
 
@@ -1063,8 +1059,6 @@ njs_string_prototype_to_bytes(njs_vm_t *
         return NXT_OK;
     }
 
-    njs_memory_error(vm);
-
     return NXT_ERROR;
 }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants