Skip to content

Fix possible failure in backtrace info #2608

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

DanielBallaSZTE
Copy link

Make sure to ignore static snapshots when sending backtrace information to the debugger.

JerryScript-DCO-1.0-Signed-off-by: Daniel Balla dballa@inf.u-szeged.hu

@akosthekiss
Copy link
Member

Related / follow-up to #2606 .

@akosthekiss akosthekiss added the debugger Related to the debugger label Nov 22, 2018
Make sure to ignore static snapshots when sending backtrace information to the debugger.
The `JMEM_SET_NON_NULL_POINTER` macro requires the frame context's `bytecode_header_p` pointer to be a heap pointer, but due to the static snapshot it might not be, causing an assertion failure.

JerryScript-DCO-1.0-Signed-off-by: Daniel Balla dballa@inf.u-szeged.hu
@@ -160,7 +160,8 @@ jerry_debugger_send_backtrace (const uint8_t *recv_buffer_p) /**< pointer to the

while (frame_ctx_p != NULL && min_depth_offset++ < max_depth)
{
if (frame_ctx_p->bytecode_header_p->status_flags & CBC_CODE_FLAGS_DEBUGGER_IGNORE)
if (frame_ctx_p->bytecode_header_p->status_flags
& (CBC_CODE_FLAGS_DEBUGGER_IGNORE | CBC_CODE_FLAGS_STATIC_FUNCTION))
Copy link
Member

Choose a reason for hiding this comment

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

There is the total frame count computing above, that is probably invalid as well. Maybe @robertsipka scope chain code is affected as well, since parts of the scope chain should be hidden as well.

@DanielBallaSZTE DanielBallaSZTE force-pushed the fix_static_snapshot_error2 branch from f83a578 to 6966695 Compare November 22, 2018 09:15
@akosthekiss
Copy link
Member

@DanielBallaSZTE ping. According to @zherczeg 's comment, it seems that this fix may need further changes. Will those be worked on?

@DanielBallaSZTE
Copy link
Author

@akosthekiss I think this one can be closed now, as this is not a blocker issue.
This PR needs some more work, however, it would be nice to also add a test environment for these bugs. I will update later with a new Pull Request, or reopen this one with some changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
debugger Related to the debugger
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants