From 00bbf631f5e9a1f37c74518c1c51ebc83f0bed2f Mon Sep 17 00:00:00 2001 From: Sami Vaarala Date: Thu, 27 Aug 2015 23:56:03 +0300 Subject: [PATCH] Force recheck of breakpoints on return --- src/duk_api_debug.c | 1 + src/duk_debugger.c | 1 + src/duk_heap.h | 1 + src/duk_js_call.c | 22 ++++++++++++++++++++++ src/duk_js_executor.c | 14 ++++++++++++++ 5 files changed, 39 insertions(+) diff --git a/src/duk_api_debug.c b/src/duk_api_debug.c index 540ede07..0765a66b 100644 --- a/src/duk_api_debug.c +++ b/src/duk_api_debug.c @@ -71,6 +71,7 @@ DUK_EXTERNAL void duk_debugger_attach(duk_context *ctx, heap->dbg_processing = 0; heap->dbg_paused = 1; heap->dbg_state_dirty = 1; + heap->dbg_force_restart = 0; heap->dbg_step_type = 0; heap->dbg_step_thread = NULL; heap->dbg_step_csindex = 0; diff --git a/src/duk_debugger.c b/src/duk_debugger.c index a65b6994..3bd60159 100644 --- a/src/duk_debugger.c +++ b/src/duk_debugger.c @@ -44,6 +44,7 @@ DUK_INTERNAL void duk_debug_do_detach(duk_heap *heap) { heap->dbg_processing = 0; heap->dbg_paused = 0; heap->dbg_state_dirty = 0; + heap->dbg_force_restart = 0; heap->dbg_step_type = 0; heap->dbg_step_thread = NULL; heap->dbg_step_csindex = 0; diff --git a/src/duk_heap.h b/src/duk_heap.h index a51531c3..63a6ba4a 100644 --- a/src/duk_heap.h +++ b/src/duk_heap.h @@ -433,6 +433,7 @@ struct duk_heap { duk_bool_t dbg_processing; /* currently processing messages or breakpoints: don't enter message processing recursively (e.g. no breakpoints when processing debugger eval) */ duk_bool_t dbg_paused; /* currently paused: talk with debug client until step/resume */ duk_bool_t dbg_state_dirty; /* resend state next time executor is about to run */ + duk_bool_t dbg_force_restart; /* force executor restart to recheck breakpoints; used to handle function returns (see GH-303) */ duk_small_uint_t dbg_step_type; /* step type: none, step into, step over, step out */ duk_hthread *dbg_step_thread; /* borrowed; NULL if no step state (NULLed in unwind) */ duk_size_t dbg_step_csindex; /* callstack index */ diff --git a/src/duk_js_call.c b/src/duk_js_call.c index 0c6a2da0..63c74d5a 100644 --- a/src/duk_js_call.c +++ b/src/duk_js_call.c @@ -1605,6 +1605,24 @@ duk_int_t duk_handle_call(duk_hthread *thr, thr->heap->call_recursion_depth = entry_call_recursion_depth; + /* If the debugger is active we need to force an interrupt so that + * debugger breakpoints are rechecked. This is important for function + * calls caused by side effects (e.g. when doing a DUK_OP_GETPROP), see + * GH-303. Only needed for success path, error path always causes a + * breakpoint recheck in the executor. It would be enough to set this + * only when returning to an Ecmascript activation, but setting the flag + * on every return should have no ill effect. + */ +#if defined(DUK_USE_DEBUGGER_SUPPORT) + if (DUK_HEAP_IS_DEBUGGER_ATTACHED(thr->heap)) { + DUK_DD(DUK_DDPRINT("returning to ecmascript activation with debugger enabled, force interrupt")); + DUK_ASSERT(thr->interrupt_counter <= thr->interrupt_init); + thr->interrupt_init -= thr->interrupt_counter; + thr->interrupt_counter = 0; + thr->heap->dbg_force_restart = 1; + } +#endif + #if defined(DUK_USE_INTERRUPT_COUNTER) && defined(DUK_USE_DEBUG) duk__interrupt_fixup(thr, entry_curr_thread); #endif @@ -1987,6 +2005,10 @@ duk_int_t duk_handle_safe_call(duk_hthread *thr, /* stack discipline consistency check */ DUK_ASSERT(duk_get_top(ctx) == idx_retbase + num_stack_rets); + /* A debugger forced interrupt check is not needed here, as + * problematic safe calls are not caused by side effects. + */ + #if defined(DUK_USE_INTERRUPT_COUNTER) && defined(DUK_USE_DEBUG) duk__interrupt_fixup(thr, entry_curr_thread); #endif diff --git a/src/duk_js_executor.c b/src/duk_js_executor.c index 3051e718..4866e660 100644 --- a/src/duk_js_executor.c +++ b/src/duk_js_executor.c @@ -1564,6 +1564,7 @@ DUK_LOCAL void duk__interrupt_handle_debugger(duk_hthread *thr, duk_bool_t *out_ if (bp == NULL) { break; } + DUK_ASSERT(bp->filename != NULL); if (act->prev_line != bp->line && line == bp->line) { DUK_D(DUK_DPRINT("BREAKPOINT TRIGGERED at %!O:%ld", @@ -2260,6 +2261,19 @@ DUK_INTERNAL void duk_js_execute_bytecode(duk_hthread *exec_thr) { act->curr_pc = (duk_instr_t *) curr_pc; } + /* Force restart caused by a function return; must recheck + * debugger breakpoints before checking line transitions, + * see GH-303. Restart and then handle interrupt_counter + * zero again. + */ +#if defined(DUK_USE_DEBUGGER_SUPPORT) + if (thr->heap->dbg_force_restart) { + DUK_DD(DUK_DDPRINT("dbg_force_restart flag forced restart execution")); /* GH-303 */ + thr->heap->dbg_force_restart = 0; + goto restart_execution; + } +#endif + exec_int_ret = duk__executor_interrupt(thr); if (exec_int_ret == DUK__INT_RESTART) { /* curr_pc synced back above */