Skip to content

Commit

Permalink
Fixing ecma_op_from_property_descriptor semantics (type check for inp…
Browse files Browse the repository at this point in the history
…ut property descriptor was implemented incorrectly).

Related issue: #70

JerryScript-DCO-1.0-Signed-off-by: Ruben Ayrapetyan [email protected]
  • Loading branch information
ruben-ayrapetyan committed May 15, 2015
1 parent 5841491 commit 5c76b3f
Showing 1 changed file with 4 additions and 3 deletions.
7 changes: 4 additions & 3 deletions jerry-core/ecma/operations/ecma-conversion.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -449,8 +449,8 @@ ecma_op_from_property_descriptor (const ecma_property_descriptor_t* src_prop_des
}

// 3.
if (prop_desc.is_value_defined
|| prop_desc.is_writable_defined)
if (src_prop_desc_p->is_value_defined
|| src_prop_desc_p->is_writable_defined)
{
JERRY_ASSERT (prop_desc.is_value_defined && prop_desc.is_writable_defined);

This comment has been minimized.

Copy link
@kkristof

kkristof May 19, 2015

Contributor

Hi, this assert should have been changed also. However what happens if someone define property as below.

obj = { bar: 42 };

In this case I think the property descriptor is_writable_defined will be false which causes assert.


Expand Down Expand Up @@ -481,7 +481,8 @@ ecma_op_from_property_descriptor (const ecma_property_descriptor_t* src_prop_des
else
{
// 4.
JERRY_ASSERT (prop_desc.is_get_defined && prop_desc.is_set_defined);
JERRY_ASSERT (src_prop_desc_p->is_get_defined
&& src_prop_desc_p->is_set_defined);

This comment has been minimized.

Copy link
@kkristof

kkristof May 19, 2015

Contributor

Same as above if the property doesn't have defined setter and getter at the same time then it will cause assert.


// a.
if (src_prop_desc_p->get_p == NULL)
Expand Down

1 comment on commit 5c76b3f

@kkristof
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please take a look.
cc: @ruben-ayrapetyan @egavrin

Please sign in to comment.