From 9f99e9008fd111664381da396483be5677eeac9a Mon Sep 17 00:00:00 2001 From: Sami Vaarala Date: Wed, 16 Aug 2017 00:37:21 +0300 Subject: [PATCH 1/2] Assert for no API calls from debugger transport --- src-input/duk_debugger.c | 34 +++++++++++++++++++++++++++++++++- src-input/duk_heap.h | 3 +++ src-input/duk_hthread.h | 2 ++ 3 files changed, 38 insertions(+), 1 deletion(-) diff --git a/src-input/duk_debugger.c b/src-input/duk_debugger.c index 885d65365d..00ae93f8cd 100644 --- a/src-input/duk_debugger.c +++ b/src-input/duk_debugger.c @@ -6,6 +6,24 @@ #if defined(DUK_USE_DEBUGGER_SUPPORT) +/* + * Assert helpers + */ + +#if defined(DUK_USE_ASSERTIONS) +#define DUK__DBG_TPORT_ENTER() do { \ + DUK_ASSERT(heap->dbg_calling_transport == 0); \ + heap->dbg_calling_transport = 1; \ + } while (0) +#define DUK__DBG_TPORT_EXIT() do { \ + DUK_ASSERT(heap->dbg_calling_transport == 1); \ + heap->dbg_calling_transport = 0; \ + } while (0) +#else +#define DUK__DBG_TPORT_ENTER() do {} while (0) +#define DUK__DBG_TPORT_EXIT() do {} while (0) +#endif + /* * Helper structs */ @@ -147,6 +165,7 @@ DUK_LOCAL void duk__debug_null_most_callbacks(duk_hthread *thr) { DUK_INTERNAL duk_bool_t duk_debug_read_peek(duk_hthread *thr) { duk_heap *heap; + duk_bool_t ret; DUK_ASSERT(thr != NULL); heap = thr->heap; @@ -161,7 +180,10 @@ DUK_INTERNAL duk_bool_t duk_debug_read_peek(duk_hthread *thr) { return 0; } - return (duk_bool_t) (heap->dbg_peek_cb(heap->dbg_udata) > 0); + DUK__DBG_TPORT_ENTER(); + ret = (duk_bool_t) (heap->dbg_peek_cb(heap->dbg_udata) > 0); + DUK__DBG_TPORT_EXIT(); + return ret; } DUK_INTERNAL void duk_debug_read_flush(duk_hthread *thr) { @@ -180,7 +202,9 @@ DUK_INTERNAL void duk_debug_read_flush(duk_hthread *thr) { return; } + DUK__DBG_TPORT_ENTER(); heap->dbg_read_flush_cb(heap->dbg_udata); + DUK__DBG_TPORT_EXIT(); } DUK_INTERNAL void duk_debug_write_flush(duk_hthread *thr) { @@ -199,7 +223,9 @@ DUK_INTERNAL void duk_debug_write_flush(duk_hthread *thr) { return; } + DUK__DBG_TPORT_ENTER(); heap->dbg_write_flush_cb(heap->dbg_udata); + DUK__DBG_TPORT_EXIT(); } /* @@ -277,7 +303,10 @@ DUK_INTERNAL void duk_debug_read_bytes(duk_hthread *thr, duk_uint8_t *data, duk_ #if defined(DUK_USE_DEBUGGER_TRANSPORT_TORTURE) left = 1; #endif + DUK__DBG_TPORT_ENTER(); got = heap->dbg_read_cb(heap->dbg_udata, (char *) p, left); + DUK__DBG_TPORT_EXIT(); + if (got == 0 || got > left) { DUK_D(DUK_DPRINT("connection error during read, return zero data")); duk__debug_null_most_callbacks(thr); /* avoid calling write callback in detach1() */ @@ -634,7 +663,10 @@ DUK_INTERNAL void duk_debug_write_bytes(duk_hthread *thr, const duk_uint8_t *dat #if defined(DUK_USE_DEBUGGER_TRANSPORT_TORTURE) left = 1; #endif + DUK__DBG_TPORT_ENTER(); got = heap->dbg_write_cb(heap->dbg_udata, (const char *) p, left); + DUK__DBG_TPORT_EXIT(); + if (got == 0 || got > left) { duk__debug_null_most_callbacks(thr); /* avoid calling write callback in detach1() */ DUK_D(DUK_DPRINT("connection error during write")); diff --git a/src-input/duk_heap.h b/src-input/duk_heap.h index e5006b2f83..cc0aaf8066 100644 --- a/src-input/duk_heap.h +++ b/src-input/duk_heap.h @@ -527,6 +527,9 @@ struct duk_heap { /* Used to support single-byte stream lookahead. */ duk_bool_t dbg_have_next_byte; duk_uint8_t dbg_next_byte; +#endif /* DUK_USE_DEBUGGER_SUPPORT */ +#if defined(DUK_USE_ASSERTIONS) + duk_bool_t dbg_calling_transport; /* transport call in progress, calling into Duktape forbidden */ #endif /* String intern table (weak refs). */ diff --git a/src-input/duk_hthread.h b/src-input/duk_hthread.h index ccb3025cae..1088c98547 100644 --- a/src-input/duk_hthread.h +++ b/src-input/duk_hthread.h @@ -175,6 +175,8 @@ */ #define DUK_ASSERT_API_ENTRY(thr) do { \ DUK_ASSERT_CTX_VALID((thr)); \ + DUK_ASSERT((thr)->heap != NULL); \ + DUK_ASSERT((thr)->heap->dbg_calling_transport == 0); \ } while (0) /* From e543718bd9ccac983413b03e162fcf9291559f30 Mon Sep 17 00:00:00 2001 From: Sami Vaarala Date: Wed, 16 Aug 2017 00:44:53 +0300 Subject: [PATCH 2/2] Releases: debugger transport API call asserts --- RELEASES.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/RELEASES.rst b/RELEASES.rst index 89c30894c6..8a3e80418e 100644 --- a/RELEASES.rst +++ b/RELEASES.rst @@ -3021,6 +3021,9 @@ Planned without throwing, as internals are not always expecting to deal with an error when reading current time (GH-1666) +* Add assertion coverage for catching calls into Duktape API from debug + transport calls (read, peek, etc) (GH-1681) + * Fix incorrect handling of register bound unary operation target for unary minus, unary plus, and bitwise NOT (GH-1623, GH-1624)