Improve Proxy recrusion check in their internal methods. #4568
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
galpeter
force-pushed
the
proxy_disable_tail_call
branch
from
4c629f4 to
172b643
Compare
galpeter
force-pushed
the
proxy_disable_tail_call
branch
from
172b643 to
fe2b5aa
Compare
rerobika
approved these changes
Feb 5, 2021
|
Maybe it's worth to contribute this testcase to test262 |
lygstate
reviewed
Feb 5, 2021
lygstate
reviewed
Feb 5, 2021
|
Also please update jerryscript/.github/workflows/gh-actions.yml Lines 50 to 52 in fe2b5aa |
true, thanks |
galpeter
force-pushed
the
proxy_disable_tail_call
branch
from
fe2b5aa to
b25bfa1
Compare
|
Oh type in title |
|
For mark, fixes #4530 |
zherczeg
reviewed
Feb 9, 2021
akosthekiss
requested changes
Feb 10, 2021
galpeter
force-pushed
the
proxy_disable_tail_call
branch
from
b25bfa1 to
5031c0b
Compare
galpeter
force-pushed
the
proxy_disable_tail_call
branch
2 times, most recently
from
4f0b049 to
5e5d4dd
Compare
zherczeg
approved these changes
Feb 27, 2021
jerry-core/jrt/jrt.h
Outdated
| #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) |
There was a problem hiding this comment.
I am not sure this syntax is allowed even in macros
There was a problem hiding this comment.
Could you please elaborate? What do you mean?
There was a problem hiding this comment.
newlines
do \
{ \
... \
} \
while (0)
There was a problem hiding this comment.
Ahh... true. I've updated the PR. thx!
galpeter
force-pushed
the
proxy_disable_tail_call
branch
2 times, most recently
from
3f6ecf0 to
3881c07
Compare
galpeter
force-pushed
the
proxy_disable_tail_call
branch
from
3881c07 to
227f0fd
Compare
lygstate
approved these changes
Mar 26, 2021
galpeter
force-pushed
the
proxy_disable_tail_call
branch
from
227f0fd to
7ff8580
Compare
lygstate
approved these changes
Apr 14, 2021
galpeter
force-pushed
the
proxy_disable_tail_call
branch
from
7ff8580 to
7665e2c
Compare
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]
galpeter
force-pushed
the
proxy_disable_tail_call
branch
from
7665e2c to
1e164a6
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 recursion which already very edge case.
The stack limit (--stack-limit=..) should be enabled to correctly report such stack
depth errors.