From 0ff6d7ae9e4ec6e33ca799e775ee067383256f89 Mon Sep 17 00:00:00 2001 From: Ruben Ayrapetyan Date: Mon, 21 Jul 2014 16:55:39 +0400 Subject: [PATCH 1/4] Fixing ecma_CopyValue: adding missing breaks in switch. --- src/libecmaobjects/ecma-helpers-value.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/libecmaobjects/ecma-helpers-value.c b/src/libecmaobjects/ecma-helpers-value.c index acf6fe358..f132b132d 100644 --- a/src/libecmaobjects/ecma-helpers-value.c +++ b/src/libecmaobjects/ecma-helpers-value.c @@ -133,6 +133,8 @@ ecma_CopyValue( const ecma_Value_t value) /**< ecma-value */ case ECMA_TYPE_SIMPLE: { value_copy = value; + + break; } case ECMA_TYPE_NUMBER: { @@ -144,6 +146,8 @@ ecma_CopyValue( const ecma_Value_t value) /**< ecma-value */ value_copy = (ecma_Value_t) { .m_ValueType = ECMA_TYPE_NUMBER }; ecma_SetPointer( value_copy.m_Value, number_copy_p); + + break; } case ECMA_TYPE_STRING: { @@ -154,6 +158,8 @@ ecma_CopyValue( const ecma_Value_t value) /**< ecma-value */ value_copy = (ecma_Value_t) { .m_ValueType = ECMA_TYPE_STRING }; ecma_SetPointer( value_copy.m_Value, string_copy_p); + + break; } case ECMA_TYPE_OBJECT: { @@ -163,6 +169,8 @@ ecma_CopyValue( const ecma_Value_t value) /**< ecma-value */ ecma_RefObject( obj_p); value_copy = value; + + break; } case ECMA_TYPE__COUNT: { From 1175526d5275fe061725cc7d4f41ea7ba024267b Mon Sep 17 00:00:00 2001 From: Ruben Ayrapetyan Date: Mon, 21 Jul 2014 17:39:39 +0400 Subject: [PATCH 2/4] Fixing value copying/value leakage issues. --- src/libcoreint/opcodes.c | 10 ++++++- src/libecmaobjects/ecma-helpers-value.c | 34 +++++++++++++++++++--- src/libecmaobjects/ecma-helpers.h | 1 + src/libecmaoperations/ecma-get-put-value.c | 6 ++++ src/libecmaoperations/ecma-lex-env.c | 32 ++++++++++++++++---- src/libecmaoperations/ecma-lex-env.h | 4 +-- src/libecmaoperations/ecma-reference.c | 10 +++---- 7 files changed, 80 insertions(+), 17 deletions(-) diff --git a/src/libcoreint/opcodes.c b/src/libcoreint/opcodes.c index e2dfa1ea5..f87391399 100644 --- a/src/libcoreint/opcodes.c +++ b/src/libcoreint/opcodes.c @@ -191,9 +191,13 @@ opfunc_jmp (OPCODE opdata, struct __int_data *int_data) } /** - * Variable declaration. + * Variable declaration opcode handler. * * See also: ECMA-262 v5, 10.5 - Declaration binding instantiation (block 8). + * + * @return completion value + * Returned value is simple and so need not be freed. + * However, ecma_free_completion_value may be called for it, but it is a no-op. */ ecma_CompletionValue_t opfunc_var_decl(OPCODE opdata, /**< operation data */ @@ -235,6 +239,10 @@ opfunc_var_decl(OPCODE opdata, /**< operation data */ * Note: this is not ECMA specification-defined, but internal * implementation-defined opcode for end of script * and assertions inside of unit tests. + * + * @return completion value + * Returned value is simple and so need not be freed. + * However, ecma_free_completion_value may be called for it, but it is a no-op. */ ecma_CompletionValue_t opfunc_exitval(OPCODE opdata, /**< operation data */ diff --git a/src/libecmaobjects/ecma-helpers-value.c b/src/libecmaobjects/ecma-helpers-value.c index f132b132d..98f0bae7c 100644 --- a/src/libecmaobjects/ecma-helpers-value.c +++ b/src/libecmaobjects/ecma-helpers-value.c @@ -121,6 +121,9 @@ ecma_MakeObjectValue( ecma_Object_t* object_p) /**< object to reference in value * increase reference counter of the object * and return the value as it was passed. * + * TODO: + * reference counter in strings + * * @return See note. */ ecma_Value_t @@ -182,7 +185,7 @@ ecma_CopyValue( const ecma_Value_t value) /**< ecma-value */ } /* ecma_CopyValue */ /** - * Free memory used for the value + * Free the ecma-value */ void ecma_FreeValue( ecma_Value_t value) /**< value description */ @@ -224,6 +227,8 @@ ecma_FreeValue( ecma_Value_t value) /**< value description */ /** * Completion value constructor + * + * @return completion value */ ecma_CompletionValue_t ecma_MakeCompletionValue(ecma_CompletionType_t type, /**< type */ @@ -235,21 +240,42 @@ ecma_MakeCompletionValue(ecma_CompletionType_t type, /**< type */ /** * Throw completion value constructor. + * + * @return 'throw' completion value */ ecma_CompletionValue_t ecma_MakeThrowValue( ecma_Object_t *exception_p) /**< an object */ { JERRY_ASSERT( exception_p != NULL && !exception_p->m_IsLexicalEnvironment ); - ecma_Value_t exception; - exception.m_ValueType = ECMA_TYPE_OBJECT; - ecma_SetPointer( exception.m_Value, exception_p); + ecma_Value_t exception = ecma_MakeObjectValue( exception_p); return ecma_MakeCompletionValue(ECMA_COMPLETION_TYPE_THROW, exception, ECMA_TARGET_ID_RESERVED); } /* ecma_MakeThrowValue */ +/** + * Free the completion value. + */ +void +ecma_free_completion_value( ecma_CompletionValue_t completion_value) /**< completion value */ +{ + switch ( completion_value.type ) + { + case ECMA_COMPLETION_TYPE_NORMAL: + case ECMA_COMPLETION_TYPE_THROW: + case ECMA_COMPLETION_TYPE_RETURN: + ecma_FreeValue( completion_value.value); + break; + case ECMA_COMPLETION_TYPE_CONTINUE: + case ECMA_COMPLETION_TYPE_BREAK: + case ECMA_COMPLETION_TYPE_EXIT: + JERRY_ASSERT( completion_value.value.m_ValueType == ECMA_TYPE_SIMPLE ); + break; + } +} /* ecma_free_completion_value */ + /** * Check if the completion value is specified normal simple value. * diff --git a/src/libecmaobjects/ecma-helpers.h b/src/libecmaobjects/ecma-helpers.h index 8d37c7fb0..9561b478f 100644 --- a/src/libecmaobjects/ecma-helpers.h +++ b/src/libecmaobjects/ecma-helpers.h @@ -54,6 +54,7 @@ extern void ecma_FreeValue( const ecma_Value_t value); extern ecma_CompletionValue_t ecma_MakeCompletionValue( ecma_CompletionType_t type, ecma_Value_t value, uint8_t target); extern ecma_CompletionValue_t ecma_MakeThrowValue( ecma_Object_t *exception_p); +extern void ecma_free_completion_value( ecma_CompletionValue_t completion_value); extern bool ecma_is_completion_value_normal_simple_value( ecma_CompletionValue_t value, ecma_SimpleValue_t simple_value); extern bool ecma_IsCompletionValueNormalFalse( ecma_CompletionValue_t value); diff --git a/src/libecmaoperations/ecma-get-put-value.c b/src/libecmaoperations/ecma-get-put-value.c index 8a632b7a2..2d1e19f0c 100644 --- a/src/libecmaoperations/ecma-get-put-value.c +++ b/src/libecmaoperations/ecma-get-put-value.c @@ -33,6 +33,9 @@ * GetValue operation. * * See also: ECMA-262 v5, 8.7.1 + * + * @return completion value + * Returned value must be freed with ecma_free_completion_value. */ ecma_CompletionValue_t ecma_OpGetValue( ecma_Reference_t *ref_p) /**< ECMA-reference */ @@ -106,6 +109,9 @@ ecma_OpGetValue( ecma_Reference_t *ref_p) /**< ECMA-reference */ * SetValue operation. * * See also: ECMA-262 v5, 8.7.1 + + * @return completion value + * Returned value must be freed with ecma_free_completion_value. */ ecma_CompletionValue_t ecma_OpSetValue(ecma_Reference_t *ref_p, /**< ECMA-reference */ diff --git a/src/libecmaoperations/ecma-lex-env.c b/src/libecmaoperations/ecma-lex-env.c index f1466bc32..e70c23358 100644 --- a/src/libecmaoperations/ecma-lex-env.c +++ b/src/libecmaoperations/ecma-lex-env.c @@ -32,6 +32,10 @@ * HasBinding operation. * * See also: ECMA-262 v5, 10.2.1 + * + * @return completion value + * Return value is simple and so need not be freed. + * However, ecma_free_completion_value may be called for it, but it is a no-op. */ ecma_CompletionValue_t ecma_OpHasBinding(ecma_Object_t *lex_env_p, /**< lexical environment */ @@ -67,6 +71,10 @@ ecma_OpHasBinding(ecma_Object_t *lex_env_p, /**< lexical environment */ * CreateMutableBinding operation. * * see also: ecma-262 v5, 10.2.1 + * + * @return completion value + * Return value is simple and so need not be freed. + * However, ecma_free_completion_value may be called for it, but it is a no-op. */ ecma_CompletionValue_t ecma_OpCreateMutableBinding(ecma_Object_t *lex_env_p, /**< lexical environment */ @@ -107,6 +115,9 @@ ecma_OpCreateMutableBinding(ecma_Object_t *lex_env_p, /**< lexical environment * * SetMutableBinding operation. * * See also: ECMA-262 v5, 10.2.1 + * + * @return completion value + * Returned value must be freed with ecma_free_completion_value. */ ecma_CompletionValue_t ecma_OpSetMutableBinding(ecma_Object_t *lex_env_p, /**< lexical environment */ @@ -127,7 +138,8 @@ ecma_OpSetMutableBinding(ecma_Object_t *lex_env_p, /**< lexical environment */ if ( property_p->u.m_NamedDataProperty.m_Writable == ECMA_PROPERTY_WRITABLE ) { - property_p->u.m_NamedDataProperty.m_Value = value; + ecma_FreeValue( property_p->u.m_NamedDataProperty.m_Value); + property_p->u.m_NamedDataProperty.m_Value = ecma_CopyValue( value); } else if ( is_strict ) { return ecma_MakeThrowValue( ecma_NewStandardError( ECMA_ERROR_TYPE)); @@ -150,6 +162,9 @@ ecma_OpSetMutableBinding(ecma_Object_t *lex_env_p, /**< lexical environment */ * GetBindingValue operation. * * See also: ECMA-262 v5, 10.2.1 + * + * @return completion value + * Returned value must be freed with ecma_free_completion_value. */ ecma_CompletionValue_t ecma_OpGetBindingValue(ecma_Object_t *lex_env_p, /**< lexical environment */ @@ -173,7 +188,7 @@ ecma_OpGetBindingValue(ecma_Object_t *lex_env_p, /**< lexical environment */ if ( property_p->u.m_NamedDataProperty.m_Writable == ECMA_PROPERTY_WRITABLE ) { return ecma_MakeCompletionValue( ECMA_COMPLETION_TYPE_NORMAL, - prop_value, + ecma_CopyValue( prop_value), ECMA_TARGET_ID_RESERVED); } else if ( prop_value.m_ValueType == ECMA_TYPE_SIMPLE && prop_value.m_Value == ECMA_SIMPLE_VALUE_EMPTY ) @@ -205,6 +220,10 @@ ecma_OpGetBindingValue(ecma_Object_t *lex_env_p, /**< lexical environment */ * DeleteBinding operation. * * See also: ECMA-262 v5, 10.2.1 + * + * @return completion value + * Return value is simple and so need not be freed. + * However, ecma_free_completion_value may be called for it, but it is a no-op. */ ecma_CompletionValue_t ecma_OpDeleteBinding(ecma_Object_t *lex_env_p, /**< lexical environment */ @@ -255,6 +274,9 @@ ecma_OpDeleteBinding(ecma_Object_t *lex_env_p, /**< lexical environment */ * ImplicitThisValue operation. * * See also: ECMA-262 v5, 10.2.1 + * + * @return completion value + * Returned value must be freed with ecma_free_completion_value. */ ecma_CompletionValue_t ecma_OpImplicitThisValue( ecma_Object_t *lex_env_p) /**< lexical environment */ @@ -283,7 +305,7 @@ ecma_OpImplicitThisValue( ecma_Object_t *lex_env_p) /**< lexical environment */ * * See also: ECMA-262 v5, 10.2.1 */ -ecma_CompletionValue_t +void ecma_OpCreateImmutableBinding(ecma_Object_t *lex_env_p, /**< lexical environment */ ecma_Char_t *name_p) /**< argument N */ { @@ -323,7 +345,7 @@ ecma_OpCreateImmutableBinding(ecma_Object_t *lex_env_p, /**< lexical environment * * See also: ECMA-262 v5, 10.2.1 */ -ecma_CompletionValue_t +void ecma_OpInitializeImmutableBinding(ecma_Object_t *lex_env_p, /**< lexical environment */ ecma_Char_t *name_p, /**< argument N */ ecma_Value_t value) /**< argument V */ @@ -343,7 +365,7 @@ ecma_OpInitializeImmutableBinding(ecma_Object_t *lex_env_p, /**< lexical environ && prop_p->u.m_NamedDataProperty.m_Value.m_ValueType == ECMA_TYPE_SIMPLE && prop_p->u.m_NamedDataProperty.m_Value.m_Value == ECMA_SIMPLE_VALUE_EMPTY ); - prop_p->u.m_NamedDataProperty.m_Value = value; + prop_p->u.m_NamedDataProperty.m_Value = ecma_CopyValue( value); } case ECMA_LEXICAL_ENVIRONMENT_OBJECTBOUND: { diff --git a/src/libecmaoperations/ecma-lex-env.h b/src/libecmaoperations/ecma-lex-env.h index c1768bfe6..6aafa2990 100644 --- a/src/libecmaoperations/ecma-lex-env.h +++ b/src/libecmaoperations/ecma-lex-env.h @@ -37,8 +37,8 @@ extern ecma_CompletionValue_t ecma_OpDeleteBinding( ecma_Object_t *lex_env_p, ec extern ecma_CompletionValue_t ecma_OpImplicitThisValue( ecma_Object_t *lex_env_p); /* ECMA-262 v5, Table 18. Additional methods of Declarative Environment Records */ -extern ecma_CompletionValue_t ecma_OpCreateImmutableBinding( ecma_Object_t *lex_env_p, ecma_Char_t *name_p); -extern ecma_CompletionValue_t ecma_OpInitializeImmutableBinding( ecma_Object_t *lex_env_p, ecma_Char_t *name_p, ecma_Value_t value); +extern void ecma_OpCreateImmutableBinding( ecma_Object_t *lex_env_p, ecma_Char_t *name_p); +extern void ecma_OpInitializeImmutableBinding( ecma_Object_t *lex_env_p, ecma_Char_t *name_p, ecma_Value_t value); /** * @} diff --git a/src/libecmaoperations/ecma-reference.c b/src/libecmaoperations/ecma-reference.c index 5e957b5e7..c4db51579 100644 --- a/src/libecmaoperations/ecma-reference.c +++ b/src/libecmaoperations/ecma-reference.c @@ -36,8 +36,8 @@ * must not be freed or reused * until the reference is freed. * - * @return ECMA-reference (if base value is an object, upon return - * it's reference counter is increased by one). + * @return ECMA-reference + * Returned value must be freed through ecma_FreeReference. */ ecma_Reference_t ecma_OpGetIdentifierReference(ecma_Object_t *lex_env_p, /**< lexical environment */ @@ -78,8 +78,8 @@ ecma_OpGetIdentifierReference(ecma_Object_t *lex_env_p, /**< lexical environment * must not be freed or reused * until the reference is freed. * - * @return ECMA-reference (if base_p it not NULL, then upon return - * corresponding object's reference counter is increased by one). + * @return ECMA-reference + * Returned value must be freed through ecma_FreeReference. */ ecma_Reference_t ecma_MakeReference(ecma_Value_t base, /**< base value */ @@ -95,7 +95,7 @@ ecma_MakeReference(ecma_Value_t base, /**< base value */ * Free specified ECMA-reference. * * Warning: - * after freeing all copy of the reference become invalid. + * freeing invalidates all copies of the reference. */ void ecma_FreeReference( const ecma_Reference_t ref) /**< reference */ From 57629912e4ad8a3a4206ef37d8effba4877076f9 Mon Sep 17 00:00:00 2001 From: Ruben Ayrapetyan Date: Mon, 21 Jul 2014 17:47:39 +0400 Subject: [PATCH 3/4] Rename: ecma_OpGetValue -> ecma_op_get_value; ecma_OpSetValue -> ecma_op_put_value. --- src/libecmaoperations/ecma-get-put-value.c | 10 +++++----- src/libecmaoperations/ecma-operations.h | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/libecmaoperations/ecma-get-put-value.c b/src/libecmaoperations/ecma-get-put-value.c index 2d1e19f0c..8e20bf11d 100644 --- a/src/libecmaoperations/ecma-get-put-value.c +++ b/src/libecmaoperations/ecma-get-put-value.c @@ -38,7 +38,7 @@ * Returned value must be freed with ecma_free_completion_value. */ ecma_CompletionValue_t -ecma_OpGetValue( ecma_Reference_t *ref_p) /**< ECMA-reference */ +ecma_op_get_value( ecma_Reference_t *ref_p) /**< ECMA-reference */ { const ecma_Value_t base = ref_p->base; const bool is_unresolvable_reference = ecma_IsValueUndefined( base); @@ -103,7 +103,7 @@ ecma_OpGetValue( ecma_Reference_t *ref_p) /**< ECMA-reference */ return ecma_OpGetBindingValue( lex_env_p, ref_p->referenced_name_p, ref_p->is_strict); } -} /* ecma_OpGetValue */ +} /* ecma_op_get_value */ /** * SetValue operation. @@ -114,8 +114,8 @@ ecma_OpGetValue( ecma_Reference_t *ref_p) /**< ECMA-reference */ * Returned value must be freed with ecma_free_completion_value. */ ecma_CompletionValue_t -ecma_OpSetValue(ecma_Reference_t *ref_p, /**< ECMA-reference */ - ecma_Value_t value) /**< ECMA-value */ +ecma_op_put_value(ecma_Reference_t *ref_p, /**< ECMA-reference */ + ecma_Value_t value) /**< ECMA-value */ { const ecma_Value_t base = ref_p->base; const bool is_unresolvable_reference = ecma_IsValueUndefined( base); @@ -227,7 +227,7 @@ ecma_OpSetValue(ecma_Reference_t *ref_p, /**< ECMA-reference */ return ecma_OpSetMutableBinding( lex_env_p, ref_p->referenced_name_p, value, ref_p->is_strict); } -} /* ecma_OpSetValue */ +} /* ecma_op_put_value */ /** * @} diff --git a/src/libecmaoperations/ecma-operations.h b/src/libecmaoperations/ecma-operations.h index 9f4fe9aee..a2629664e 100644 --- a/src/libecmaoperations/ecma-operations.h +++ b/src/libecmaoperations/ecma-operations.h @@ -29,8 +29,8 @@ extern ecma_Reference_t ecma_OpGetIdentifierReference( ecma_Object_t *lex_env_p, ecma_Char_t *name_p, bool is_strict); -extern ecma_CompletionValue_t ecma_OpGetValue( ecma_Reference_t *ref_p); -extern ecma_CompletionValue_t ecma_OpSetValue( ecma_Reference_t *ref_p, ecma_Value_t value); +extern ecma_CompletionValue_t ecma_op_get_value( ecma_Reference_t *ref_p); +extern ecma_CompletionValue_t ecma_op_put_value( ecma_Reference_t *ref_p, ecma_Value_t value); /** * @} From 5e503ced32109559e77ef48b6686439c8b553f72 Mon Sep 17 00:00:00 2001 From: Ruben Ayrapetyan Date: Mon, 21 Jul 2014 17:51:11 +0400 Subject: [PATCH 4/4] Fixing ecma_op_get_value, ecma_op_put_value: correctly distinguishing whether the base is object or lexical environment. --- src/libecmaoperations/ecma-get-put-value.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/libecmaoperations/ecma-get-put-value.c b/src/libecmaoperations/ecma-get-put-value.c index 8e20bf11d..94c7efdc6 100644 --- a/src/libecmaoperations/ecma-get-put-value.c +++ b/src/libecmaoperations/ecma-get-put-value.c @@ -45,7 +45,9 @@ ecma_op_get_value( ecma_Reference_t *ref_p) /**< ECMA-reference */ const bool has_primitive_base = ( ecma_IsValueBoolean( base) || base.m_ValueType == ECMA_TYPE_NUMBER || base.m_ValueType == ECMA_TYPE_STRING ); - const bool is_property_reference = has_primitive_base || ( base.m_ValueType == ECMA_TYPE_OBJECT ); + const bool has_object_base = ( base.m_ValueType == ECMA_TYPE_OBJECT + && !((ecma_Object_t*)ecma_GetPointer(base.m_Value))->m_IsLexicalEnvironment ); + const bool is_property_reference = has_primitive_base || has_object_base; // GetValue_3 if ( is_unresolvable_reference ) @@ -73,7 +75,7 @@ ecma_op_get_value( ecma_Reference_t *ref_p) /**< ECMA-reference */ if ( property->m_Type == ECMA_PROPERTY_NAMEDDATA ) { return ecma_MakeCompletionValue( ECMA_COMPLETION_TYPE_NORMAL, - property->u.m_NamedDataProperty.m_Value, + ecma_CopyValue( property->u.m_NamedDataProperty.m_Value), ECMA_TARGET_ID_RESERVED); } else { @@ -122,7 +124,9 @@ ecma_op_put_value(ecma_Reference_t *ref_p, /**< ECMA-reference */ const bool has_primitive_base = ( ecma_IsValueBoolean( base) || base.m_ValueType == ECMA_TYPE_NUMBER || base.m_ValueType == ECMA_TYPE_STRING ); - const bool is_property_reference = has_primitive_base || ( base.m_ValueType == ECMA_TYPE_OBJECT ); + const bool has_object_base = ( base.m_ValueType == ECMA_TYPE_OBJECT + && !((ecma_Object_t*)ecma_GetPointer(base.m_Value))->m_IsLexicalEnvironment ); + const bool is_property_reference = has_primitive_base || has_object_base; if ( is_unresolvable_reference ) // PutValue_3 {