Fix an issue while debugging with static snapshots #2606
Conversation
DanielBallaSZTE
force-pushed
the
fix_static_snapshot_exception
branch
from
6cc3839 to
6cb86e3
Compare
|
What exactly is the problem this PR wants to fix, what are the steps to reproduce it? If there is a reported issue, the PR/commit should reference it. If there is no such issue then at least this PR should document it. Is there a way to make the problem tested automatically so that it does not happen again (to prevent regression)? |
LaszloLango
dismissed
their stale review
Dismiss my review to prevent too early merging. Please answer the questions of @akosthekiss
|
@akosthekiss Sorry for not adding it to the PR description, going to amend it to make sure it appears there as well. Sadly we can't really create a regression test for this specific issue. |
The issue was found when debugging IoT.js with static snapshots turned on, if an error was thrown an assertion failure was caused. The reason is the frame context's bytecode_header_p pointer was not a heap pointer. The jerry_debugger_breakpoint_hit function tries to set a compressed pointer which points to bytecode_header_p with the JMEM_CP_SET_NON_NULL_POINTER macro, which has an assertion that requires the above mentioned bytecode_header_p to be a heap pointer, which is obviously not. Co-authored-by: Robert Fancsik frobert@inf.u-szeged.hu JerryScript-DCO-1.0-Signed-off-by: Daniel Balla dballa@inf.u-szeged.hu
DanielBallaSZTE
force-pushed
the
fix_static_snapshot_exception
branch
from
6cb86e3 to
647bd9c
Compare
|
@DanielBallaSZTE Is this issue IoT.js-specific? Or can it be reproduced somehow in a JerryScript-only environment? |
|
@akosthekiss I haven't tried it yet, I could give it a shot, and if it is reproducible try and create a test for it. However, in that case we would need a static snapshot in the test environment to be debugged, while running debugger-tests. |
|
Not a trivial task. You need a unit test which creates such an environment, and you also need the python debugger to do debugging. Hence it needs a new infrastructure basically, where instead of a .js, a binary is run by the debugger tester driver. @DanielBallaSZTE please check how difficult would be to do this. |
|
Any update on this? IMHO the test environment improvement should be done in a separate PR. It is not trivial and might take a while to implement it, but this bug is quite a blocker issue for us. |
LaszloLango
added
bug
There was a problem hiding this comment.
Unfortunately, there has been no update on adding test for the issue. LGTM for now, to lift the block it causes. (I've created an issue, #2637, to track the need for a test environment.)
The issue was present when the executed code would have thrown an error.
Co-authored-by: Robert Fancsik frobert@inf.u-szeged.hu
JerryScript-DCO-1.0-Signed-off-by: Daniel Balla dballa@inf.u-szeged.hu