-
Notifications
You must be signed in to change notification settings - Fork 683
Fix push() and unshift() in case result length is larger than UINT_MAX #462
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
Conversation
if (ecma_object_get_class_name (object) == LIT_MAGIC_STRING_ARRAY_UL | ||
&& unlikely (length > (uint32_t)(-1))) /* Check if length greater than UINT_MAX */ | ||
{ | ||
ret_value = ecma_make_throw_obj_completion_value (ecma_new_standard_error (ECMA_ERROR_RANGE)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dbatyai, the error should be raised in ecma_op_array_object_define_own_property
. Is there any case, where it isn't raised?
…ger than UINT_MAX JerryScript-DCO-1.0-Signed-off-by: Dániel Bátyai dbatyai.u-szeged@partner.samsung.com
8f8bda2
to
e647b4c
Compare
@ruben-ayrapetyan, I've updated the patch. The error does get raised correctly in |
index++, n++) | ||
{ | ||
/* 5.a */ | ||
ecma_value_t e_value = argument_list_p[index]; | ||
|
||
/* 5.b */ | ||
ecma_string_t *n_str_p = ecma_new_ecma_string_from_uint32 (n); | ||
ecma_string_t *n_str_p = ecma_new_ecma_string_from_number (n); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is better to use ecma_new_ecma_string_from_uint32
instead of ecma_new_ecma_string_from_number
, as it reduces memory usage.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can't use ecma_new_ecma_string_from_uint32
here, because the index can go above 2^32.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. Thanks for explanation.
Looks good to me |
Looks good to me. |
Landed: fec5933 |
JerryScript-DCO-1.0-Signed-off-by: Dániel Bátyai dbatyai.u-szeged@partner.samsung.com