Skip to content

Commit

Permalink
Improve Proxy recrusion check in their internal methods.
Browse files Browse the repository at this point in the history 
In some parts of Proxy internal methods the compiler can do a bit of
tail call optimalization which would be ok, but the current stack limit
check framework in Jerry uses the stack pointer to calculate the stack depth.
This way of stack depth calculation is affected by the tail call optimalization
(as the stack does not increase).

By disabling the tail call optimalization at given points the stack limit calculation
will work as expected. This causes a bit of stack overhead, but the Proxy in it self
is a fairly big chunk of code and this stack limit would only be relevant if the Proxy
already does recusion which already very edge case.

The stack limit (--stack-limit=..) should be enabled to correctly report such stack
depth errors.

JerryScript-DCO-1.0-Signed-off-by: Peter Gal [email protected]
  • Loading branch information
@galpeter
galpeter committed Jul 15, 2021
1 parent 305741a commit 1e164a6
Show file tree
Hide file tree
Showing 5 changed files with 263 additions and 19 deletions.
7 changes: 3 additions & 4 deletions .github/workflows/gh-actions.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,7 @@ jobs:
runs-on: macos-latest
steps:
- uses: actions/checkout@v2
# FIXME: regression-test-issue-3785.js currently are deadloop on OSX.
- run: $RUNNER -q --jerry-tests --skip-list=regression-test-issue-3785.js
- run: $RUNNER -q --jerry-tests
- run: $RUNNER -q --unittests

OSX_x86-64_Build_Correctness_Unit_Tests_Debug:
Expand Down Expand Up @@ -152,7 +151,7 @@ jobs:
- run: >-
$RUNNER -q --jerry-tests
--buildoptions=--stack-limit=0,--compile-flag=-fsanitize=address,--compile-flag=-m32,--compile-flag=-fno-omit-frame-pointer,--compile-flag=-fno-common,--compile-flag=-O2,--debug,--system-allocator=on,--linker-flag=-fuse-ld=gold
--skip-list=parser-oom.js,parser-oom2.js,stack-limit.js,regression-test-issue-2190.js,regression-test-issue-2258-2963.js,regression-test-issue-2448.js,regression-test-issue-2905.js,regression-test-issue-3785.js
--skip-list=parser-oom.js,parser-oom2.js,stack-limit.js,regression-test-issue-2190.js,regression-test-issue-2258-2963.js,regression-test-issue-2448.js,regression-test-issue-2905.js,regression-test-issue-3785.js,proxy-evil-recursion.js
ASAN_Tests_Debug:
runs-on: ubuntu-latest
Expand All @@ -165,7 +164,7 @@ jobs:
- run: >-
$RUNNER -q --jerry-tests --build-debug
--buildoptions=--stack-limit=0,--compile-flag=-fsanitize=address,--compile-flag=-m32,--compile-flag=-fno-omit-frame-pointer,--compile-flag=-fno-common,--compile-flag=-O2,--debug,--system-allocator=on,--linker-flag=-fuse-ld=gold
--skip-list=parser-oom.js,parser-oom2.js,stack-limit.js,regression-test-issue-2190.js,regression-test-issue-2258-2963.js,regression-test-issue-2448.js,regression-test-issue-2905.js,regression-test-issue-3785.js
--skip-list=parser-oom.js,parser-oom2.js,stack-limit.js,regression-test-issue-2190.js,regression-test-issue-2258-2963.js,regression-test-issue-2448.js,regression-test-issue-2905.js,regression-test-issue-3785.js,proxy-evil-recursion.js
UBSAN_Tests:
runs-on: ubuntu-latest
Expand Down
17 changes: 15 additions & 2 deletions jerry-core/ecma/base/ecma-globals.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2268,16 +2268,29 @@ typedef struct
#if (JERRY_STACK_LIMIT != 0)
/**
* Check the current stack usage. If the limit is reached a RangeError is raised.
* The macro argument specifies the return value which is usally ECMA_VALUE_ERROR or NULL.
*/
#define ECMA_CHECK_STACK_USAGE() \
#define ECMA_CHECK_STACK_USAGE_RETURN(RETURN_VALUE) \
do \
{ \
if (ecma_get_current_stack_usage () > CONFIG_MEM_STACK_LIMIT) \
{ \
return ecma_raise_range_error (ECMA_ERR_MSG ("Maximum call stack size exceeded")); \
ecma_raise_range_error (ECMA_ERR_MSG ("Maximum call stack size exceeded")); \
return RETURN_VALUE; \
} \
} while (0)

/**
* Specialized version of ECMA_CHECK_STACK_USAGE_RETURN which returns ECMA_VALUE_ERROR.
* This version should be used in most cases.
*/
#define ECMA_CHECK_STACK_USAGE() ECMA_CHECK_STACK_USAGE_RETURN(ECMA_VALUE_ERROR)
#else /* JERRY_STACK_LIMIT == 0) */
/**
* If the stack limit is unlimited, this check is an empty macro.
*/
#define ECMA_CHECK_STACK_USAGE_RETURN(RETURN_VALUE)

/**
* If the stack limit is unlimited, this check is an empty macro.
*/
Expand Down
63 changes: 50 additions & 13 deletions jerry-core/ecma/operations/ecma-proxy-object.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,7 @@ ecma_value_t
ecma_proxy_object_get_prototype_of (ecma_object_t *obj_p) /**< proxy object */
{
JERRY_ASSERT (ECMA_OBJECT_IS_PROXY (obj_p));
ECMA_CHECK_STACK_USAGE ();

ecma_proxy_object_t *proxy_obj_p = (ecma_proxy_object_t *) obj_p;

Expand All @@ -284,7 +285,9 @@ ecma_proxy_object_get_prototype_of (ecma_object_t *obj_p) /**< proxy object */
/* 7. */
if (ecma_is_value_undefined (trap))
{
return ecma_builtin_object_object_get_prototype_of (target_obj_p);
ecma_value_t result = ecma_builtin_object_object_get_prototype_of (target_obj_p);
JERRY_BLOCK_TAIL_CALL_OPTIMIZATION ();
return result;
}

ecma_object_t *func_obj_p = ecma_get_object_from_value (trap);
Expand Down Expand Up @@ -373,6 +376,7 @@ ecma_proxy_object_set_prototype_of (ecma_object_t *obj_p, /**< proxy object */
ecma_value_t proto) /**< new prototype object */
{
JERRY_ASSERT (ECMA_OBJECT_IS_PROXY (obj_p));
ECMA_CHECK_STACK_USAGE ();

/* 1. */
JERRY_ASSERT (ecma_is_value_object (proto) || ecma_is_value_null (proto));
Expand All @@ -399,10 +403,14 @@ ecma_proxy_object_set_prototype_of (ecma_object_t *obj_p, /**< proxy object */
{
if (ECMA_OBJECT_IS_PROXY (target_obj_p))
{
return ecma_proxy_object_set_prototype_of (target_obj_p, proto);
ecma_value_t result = ecma_proxy_object_set_prototype_of (target_obj_p, proto);
JERRY_BLOCK_TAIL_CALL_OPTIMIZATION ();
return result;
}

return ecma_op_ordinary_object_set_prototype_of (target_obj_p, proto);
ecma_value_t result = ecma_op_ordinary_object_set_prototype_of (target_obj_p, proto);
JERRY_BLOCK_TAIL_CALL_OPTIMIZATION ();
return result;
}

ecma_object_t *func_obj_p = ecma_get_object_from_value (trap);
Expand Down Expand Up @@ -488,6 +496,7 @@ ecma_value_t
ecma_proxy_object_is_extensible (ecma_object_t *obj_p) /**< proxy object */
{
JERRY_ASSERT (ECMA_OBJECT_IS_PROXY (obj_p));
ECMA_CHECK_STACK_USAGE ();

ecma_proxy_object_t *proxy_obj_p = (ecma_proxy_object_t *) obj_p;

Expand All @@ -509,7 +518,9 @@ ecma_proxy_object_is_extensible (ecma_object_t *obj_p) /**< proxy object */
/* 7. */
if (ecma_is_value_undefined (trap))
{
return ecma_builtin_object_object_is_extensible (target_obj_p);
ecma_value_t result = ecma_builtin_object_object_is_extensible (target_obj_p);
JERRY_BLOCK_TAIL_CALL_OPTIMIZATION ();
return result;
}

ecma_object_t *func_obj_p = ecma_get_object_from_value (trap);
Expand Down Expand Up @@ -577,6 +588,7 @@ ecma_value_t
ecma_proxy_object_prevent_extensions (ecma_object_t *obj_p) /**< proxy object */
{
JERRY_ASSERT (ECMA_OBJECT_IS_PROXY (obj_p));
ECMA_CHECK_STACK_USAGE ();

ecma_proxy_object_t *proxy_obj_p = (ecma_proxy_object_t *) obj_p;

Expand Down Expand Up @@ -666,6 +678,8 @@ ecma_proxy_object_get_own_property_descriptor (ecma_object_t *obj_p, /**< proxy
ecma_property_descriptor_t *prop_desc_p) /**< [out] property
* descriptor */
{
ECMA_CHECK_STACK_USAGE ();

ecma_proxy_object_t *proxy_obj_p = (ecma_proxy_object_t *) obj_p;

/* 2. */
Expand All @@ -687,7 +701,9 @@ ecma_proxy_object_get_own_property_descriptor (ecma_object_t *obj_p, /**< proxy
/* 8. */
if (ecma_is_value_undefined (trap))
{
return ecma_op_object_get_own_property_descriptor (target_obj_p, prop_name_p, prop_desc_p);
ecma_value_t result = ecma_op_object_get_own_property_descriptor (target_obj_p, prop_name_p, prop_desc_p);
JERRY_BLOCK_TAIL_CALL_OPTIMIZATION ();
return result;
}

ecma_object_t *func_obj_p = ecma_get_object_from_value (trap);
Expand Down Expand Up @@ -868,6 +884,7 @@ ecma_proxy_object_define_own_property (ecma_object_t *obj_p, /**< proxy object *
const ecma_property_descriptor_t *prop_desc_p) /**< property descriptor */
{
JERRY_ASSERT (ECMA_OBJECT_IS_PROXY (obj_p));
ECMA_CHECK_STACK_USAGE ();

ecma_proxy_object_t *proxy_obj_p = (ecma_proxy_object_t *) obj_p;

Expand All @@ -889,7 +906,9 @@ ecma_proxy_object_define_own_property (ecma_object_t *obj_p, /**< proxy object *
/* 8. */
if (ecma_is_value_undefined (trap))
{
return ecma_op_object_define_own_property (target_obj_p, prop_name_p, prop_desc_p);
ecma_value_t result = ecma_op_object_define_own_property (target_obj_p, prop_name_p, prop_desc_p);
JERRY_BLOCK_TAIL_CALL_OPTIMIZATION ();
return result;
}

/* 9. */
Expand Down Expand Up @@ -1050,7 +1069,9 @@ ecma_proxy_object_has (ecma_object_t *obj_p, /**< proxy object */
/* 8. */
if (ecma_is_value_undefined (trap))
{
return ecma_op_object_has_property (target_obj_p, prop_name_p);
ecma_value_t result = ecma_op_object_has_property (target_obj_p, prop_name_p);
JERRY_BLOCK_TAIL_CALL_OPTIMIZATION ();
return result;
}

ecma_object_t *func_obj_p = ecma_get_object_from_value (trap);
Expand Down Expand Up @@ -1153,7 +1174,9 @@ ecma_proxy_object_get (ecma_object_t *obj_p, /**< proxy object */
if (ecma_is_value_undefined (trap))
{
ecma_object_t *target_obj_p = ecma_get_object_from_value (proxy_obj_p->target);
return ecma_op_object_get_with_receiver (target_obj_p, prop_name_p, receiver);
ecma_value_t result = ecma_op_object_get_with_receiver (target_obj_p, prop_name_p, receiver);
JERRY_BLOCK_TAIL_CALL_OPTIMIZATION ();
return result;
}

ecma_object_t *func_obj_p = ecma_get_object_from_value (trap);
Expand Down Expand Up @@ -1259,7 +1282,9 @@ ecma_proxy_object_set (ecma_object_t *obj_p, /**< proxy object */
/* 8. */
if (ecma_is_value_undefined (trap))
{
return ecma_op_object_put_with_receiver (target_obj_p, prop_name_p, value, receiver, is_strict);
ecma_value_t result = ecma_op_object_put_with_receiver (target_obj_p, prop_name_p, value, receiver, is_strict);
JERRY_BLOCK_TAIL_CALL_OPTIMIZATION ();
return result;
}

ecma_object_t *func_obj_p = ecma_get_object_from_value (trap);
Expand Down Expand Up @@ -1357,6 +1382,7 @@ ecma_proxy_object_delete_property (ecma_object_t *obj_p, /**< proxy object */
bool is_strict) /**< delete in strict mode? */
{
JERRY_ASSERT (ECMA_OBJECT_IS_PROXY (obj_p));
ECMA_CHECK_STACK_USAGE ();

ecma_proxy_object_t *proxy_obj_p = (ecma_proxy_object_t *) obj_p;

Expand All @@ -1378,7 +1404,9 @@ ecma_proxy_object_delete_property (ecma_object_t *obj_p, /**< proxy object */
/* 8. */
if (ecma_is_value_undefined (trap))
{
return ecma_op_object_delete (target_obj_p, prop_name_p, is_strict);
ecma_value_t result = ecma_op_object_delete (target_obj_p, prop_name_p, is_strict);
JERRY_BLOCK_TAIL_CALL_OPTIMIZATION ();
return result;
}

ecma_object_t *func_obj_p = ecma_get_object_from_value (trap);
Expand Down Expand Up @@ -1579,6 +1607,7 @@ ecma_collection_t *
ecma_proxy_object_own_property_keys (ecma_object_t *obj_p) /**< proxy object */
{
JERRY_ASSERT (ECMA_OBJECT_IS_PROXY (obj_p));
ECMA_CHECK_STACK_USAGE_RETURN (NULL);

ecma_proxy_object_t *proxy_obj_p = (ecma_proxy_object_t *) obj_p;

Expand All @@ -1599,7 +1628,9 @@ ecma_proxy_object_own_property_keys (ecma_object_t *obj_p) /**< proxy object */
/* 6. */
if (ecma_is_value_undefined (trap))
{
return ecma_op_object_own_property_keys (target_obj_p);
ecma_collection_t *result = ecma_op_object_own_property_keys (target_obj_p);
JERRY_BLOCK_TAIL_CALL_OPTIMIZATION ();
return result;
}

ecma_object_t *func_obj_p = ecma_get_object_from_value (trap);
Expand Down Expand Up @@ -1740,6 +1771,7 @@ ecma_proxy_object_call (ecma_object_t *obj_p, /**< proxy object */
uint32_t argc) /**< number of arguments */
{
JERRY_ASSERT (ECMA_OBJECT_IS_PROXY (obj_p));
ECMA_CHECK_STACK_USAGE ();

ecma_proxy_object_t *proxy_obj_p = (ecma_proxy_object_t *) obj_p;

Expand All @@ -1761,7 +1793,9 @@ ecma_proxy_object_call (ecma_object_t *obj_p, /**< proxy object */
if (ecma_is_value_undefined (trap))
{
ecma_object_t *target_obj_p = ecma_get_object_from_value (target);
return ecma_op_function_call (target_obj_p, this_argument, args_p, argc);
ecma_value_t result = ecma_op_function_call (target_obj_p, this_argument, args_p, argc);
JERRY_BLOCK_TAIL_CALL_OPTIMIZATION ();
return result;
}

/* 8. */
Expand Down Expand Up @@ -1794,6 +1828,7 @@ ecma_proxy_object_construct (ecma_object_t *obj_p, /**< proxy object */
uint32_t argc) /**< number of arguments */
{
JERRY_ASSERT (ECMA_OBJECT_IS_PROXY (obj_p));
ECMA_CHECK_STACK_USAGE ();

ecma_proxy_object_t * proxy_obj_p = (ecma_proxy_object_t *) obj_p;

Expand All @@ -1817,7 +1852,9 @@ ecma_proxy_object_construct (ecma_object_t *obj_p, /**< proxy object */
{
JERRY_ASSERT (ecma_object_is_constructor (target_obj_p));

return ecma_op_function_construct (target_obj_p, new_target_p, args_p, argc);
ecma_value_t result = ecma_op_function_construct (target_obj_p, new_target_p, args_p, argc);
JERRY_BLOCK_TAIL_CALL_OPTIMIZATION ();
return result;
}

/* 8. */
Expand Down
33 changes: 33 additions & 0 deletions jerry-core/jrt/jrt.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -153,4 +153,37 @@ void JERRY_ATTR_NORETURN jerry_fatal (jerry_fatal_code_t code);
#define JERRY__LOG2_8(n) (((n) >= 1 << 8) ? (8 + JERRY__LOG2_4 ((n) >> 8)) : JERRY__LOG2_4 (n))
#define JERRY_LOG2(n) (((n) >= 1 << 16) ? (16 + JERRY__LOG2_8 ((n) >> 16)) : JERRY__LOG2_8 (n))

/**
* JERRY_BLOCK_TAIL_CALL_OPTIMIZATION
*
* Adapted from abseil ( https://github.com/abseil/ )
*
* Instructs the compiler to avoid optimizing tail-call recursion. This macro is
* useful when you wish to preserve the existing function order within a stack
* trace for logging, debugging, or profiling purposes.
*
* Example:
*
* int f() {
* int result = g();
* JERRY_BLOCK_TAIL_CALL_OPTIMIZATION();
* return result;
* }
*
* This macro is intentionally here as jerryscript-compiler.h is a public header and
* it does not make sense to expose this macro to the public.
*/
#if defined (__clang__) || defined (__GNUC__)
/* Clang/GCC will not tail call given inline volatile assembly. */
#define JERRY_BLOCK_TAIL_CALL_OPTIMIZATION() __asm__ __volatile__ ("")
#else /* !defined(__clang__) && !defined(__GNUC__) */
/* On GCC 10.x this version also works. */
#define JERRY_BLOCK_TAIL_CALL_OPTIMIZATION() \
do \
{ \
JERRY_CONTEXT (status_flags) |= ECMA_STATUS_API_AVAILABLE; \
} \
while (0)
#endif /* defined(__clang__) || defined (__GNUC__) */

#endif /* !JRT_H */
Loading

0 comments on commit 1e164a6

Please sign in to comment.