Skip to content

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
{
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