From b5a91069fdf3c2650d7a56defd885e592146f127 Mon Sep 17 00:00:00 2001 From: Zoltan Herczeg Date: Mon, 6 Mar 2017 11:13:25 +0100 Subject: [PATCH] Fix multiple debugger issues. (#1640) - Wait for free byte code pointers during garbage collection. - Detect incorrect free requests in the debugger server. - Ignore byte code blocks loaded from snapshot. - Use memmove instead of memcpy to avoid receive buffer corruption. JerryScript-DCO-1.0-Signed-off-by: Zoltan Herczeg zherczeg.u-szeged@partner.samsung.com --- jerry-core/debugger/jerry-debugger-ws.c | 6 +-- jerry-core/debugger/jerry-debugger.c | 44 ++++++++++----------- jerry-core/debugger/jerry-debugger.h | 5 +-- jerry-core/ecma/base/ecma-gc.c | 12 +++++- jerry-core/ecma/base/ecma-helpers.c | 51 +++++++++++++------------ jerry-core/jcontext/jcontext.h | 1 + jerry-core/jerry-snapshot.c | 4 ++ jerry-core/parser/js/byte-code.h | 21 ++++++---- jerry-core/vm/vm.c | 28 ++++++++++++++ 9 files changed, 110 insertions(+), 62 deletions(-) diff --git a/jerry-core/debugger/jerry-debugger-ws.c b/jerry-core/debugger/jerry-debugger-ws.c index 92b7c3739..8e9a4a37d 100644 --- a/jerry-core/debugger/jerry-debugger-ws.c +++ b/jerry-core/debugger/jerry-debugger-ws.c @@ -551,9 +551,9 @@ jerry_debugger_receive (void) if (message_total_size < offset) { - memcpy (recv_buffer_p, - recv_buffer_p + message_total_size, - offset - message_total_size); + memmove (recv_buffer_p, + recv_buffer_p + message_total_size, + offset - message_total_size); } JERRY_CONTEXT (debugger_receive_buffer_offset) = (uint16_t) (offset - message_total_size); diff --git a/jerry-core/debugger/jerry-debugger.c b/jerry-core/debugger/jerry-debugger.c index 928afab94..dabc82280 100644 --- a/jerry-core/debugger/jerry-debugger.c +++ b/jerry-core/debugger/jerry-debugger.c @@ -45,18 +45,18 @@ jerry_debugger_free_unreferenced_byte_code (void) jerry_debugger_byte_code_free_t *byte_code_free_p; byte_code_free_p = JMEM_CP_GET_POINTER (jerry_debugger_byte_code_free_t, - JERRY_CONTEXT (debugger_byte_code_free_head)); + JERRY_CONTEXT (debugger_byte_code_free_tail)); while (byte_code_free_p != NULL) { - jerry_debugger_byte_code_free_t *next_byte_code_free_p; - next_byte_code_free_p = JMEM_CP_GET_POINTER (jerry_debugger_byte_code_free_t, - byte_code_free_p->next_cp); + jerry_debugger_byte_code_free_t *prev_byte_code_free_p; + prev_byte_code_free_p = JMEM_CP_GET_POINTER (jerry_debugger_byte_code_free_t, + byte_code_free_p->prev_cp); jmem_heap_free_block (byte_code_free_p, ((size_t) byte_code_free_p->size) << JMEM_ALIGNMENT_LOG); - byte_code_free_p = next_byte_code_free_p; + byte_code_free_p = prev_byte_code_free_p; } } /* jerry_debugger_free_unreferenced_byte_code */ @@ -88,6 +88,12 @@ jerry_debugger_send_backtrace (uint8_t *recv_buffer_p) /**< pointer the the rece while (frame_ctx_p != NULL && max_depth > 0) { + if (frame_ctx_p->bytecode_header_p->status_flags & CBC_CODE_FLAGS_DEBUGGER_IGNORE) + { + frame_ctx_p = frame_ctx_p->prev_context_p; + continue; + } + if (current_frame >= JERRY_DEBUGGER_SEND_MAX (jerry_debugger_frame_t)) { if (!jerry_debugger_send (sizeof (jerry_debugger_send_backtrace_t))) @@ -283,31 +289,25 @@ jerry_debugger_process_message (uint8_t *recv_buffer_p, /**< pointer the the rec jmem_cpointer_t byte_code_free_cp; memcpy (&byte_code_free_cp, byte_code_p->byte_code_cp, sizeof (jmem_cpointer_t)); + if (byte_code_free_cp != JERRY_CONTEXT (debugger_byte_code_free_tail)) + { + jerry_port_log (JERRY_LOG_LEVEL_ERROR, "Invalid byte code free order\n"); + jerry_debugger_close_connection (); + return false; + } + jerry_debugger_byte_code_free_t *byte_code_free_p; byte_code_free_p = JMEM_CP_GET_NON_NULL_POINTER (jerry_debugger_byte_code_free_t, byte_code_free_cp); - if (JERRY_CONTEXT (debugger_byte_code_free_head) == byte_code_free_cp) - { - JERRY_CONTEXT (debugger_byte_code_free_head) = byte_code_free_p->next_cp; - } - if (byte_code_free_p->prev_cp != ECMA_NULL_POINTER) { - jerry_debugger_byte_code_free_t *prev_byte_code_free_p; - prev_byte_code_free_p = JMEM_CP_GET_NON_NULL_POINTER (jerry_debugger_byte_code_free_t, - byte_code_free_p->prev_cp); - - prev_byte_code_free_p->next_cp = byte_code_free_p->next_cp; + JERRY_CONTEXT (debugger_byte_code_free_tail) = byte_code_free_p->prev_cp; } - - if (byte_code_free_p->next_cp != ECMA_NULL_POINTER) + else { - jerry_debugger_byte_code_free_t *next_byte_code_free_p; - next_byte_code_free_p = JMEM_CP_GET_NON_NULL_POINTER (jerry_debugger_byte_code_free_t, - byte_code_free_p->next_cp); - - next_byte_code_free_p->prev_cp = byte_code_free_p->prev_cp; + JERRY_CONTEXT (debugger_byte_code_free_head) = ECMA_NULL_POINTER; + JERRY_CONTEXT (debugger_byte_code_free_tail) = ECMA_NULL_POINTER; } jmem_heap_free_block (byte_code_free_p, diff --git a/jerry-core/debugger/jerry-debugger.h b/jerry-core/debugger/jerry-debugger.h index f9017bbc8..f155a0e1b 100644 --- a/jerry-core/debugger/jerry-debugger.h +++ b/jerry-core/debugger/jerry-debugger.h @@ -98,9 +98,8 @@ typedef enum */ typedef struct { - uint16_t size; - jmem_cpointer_t prev_cp; - jmem_cpointer_t next_cp; + uint16_t size; /**< size of the byte code header divided by JMEM_ALIGNMENT */ + jmem_cpointer_t prev_cp; /**< previous byte code data to be freed */ } jerry_debugger_byte_code_free_t; /** diff --git a/jerry-core/ecma/base/ecma-gc.c b/jerry-core/ecma/base/ecma-gc.c index fa7122f6a..2ebf1a872 100644 --- a/jerry-core/ecma/base/ecma-gc.c +++ b/jerry-core/ecma/base/ecma-gc.c @@ -708,10 +708,18 @@ ecma_gc_run (jmem_free_unused_memory_severity_t severity) /**< gc severity */ void ecma_free_unused_memory (jmem_free_unused_memory_severity_t severity) /**< severity of the request */ { +#ifdef JERRY_DEBUGGER + while ((JERRY_CONTEXT (debugger_flags) & JERRY_DEBUGGER_CONNECTED) + && JERRY_CONTEXT (debugger_byte_code_free_tail) != ECMA_NULL_POINTER) + { + /* Wait until all byte code is freed or the connection is aborted. */ + jerry_debugger_receive (); + } +#endif /* JERRY_DEBUGGER */ + if (severity == JMEM_FREE_UNUSED_MEMORY_SEVERITY_LOW) { - - #ifndef CONFIG_ECMA_PROPERTY_HASHMAP_DISABLE +#ifndef CONFIG_ECMA_PROPERTY_HASHMAP_DISABLE if (JERRY_CONTEXT (ecma_prop_hashmap_alloc_state) > ECMA_PROP_HASHMAP_ALLOC_ON) { --JERRY_CONTEXT (ecma_prop_hashmap_alloc_state); diff --git a/jerry-core/ecma/base/ecma-helpers.c b/jerry-core/ecma/base/ecma-helpers.c index 889cbf0aa..4d9e93295 100644 --- a/jerry-core/ecma/base/ecma-helpers.c +++ b/jerry-core/ecma/base/ecma-helpers.c @@ -1377,33 +1377,36 @@ ecma_bytecode_deref (ecma_compiled_code_t *bytecode_p) /**< byte code pointer */ } #ifdef JERRY_DEBUGGER - if (JERRY_CONTEXT (debugger_flags) & JERRY_DEBUGGER_CONNECTED) + if ((JERRY_CONTEXT (debugger_flags) & JERRY_DEBUGGER_CONNECTED) + && !(bytecode_p->status_flags & CBC_CODE_FLAGS_DEBUGGER_IGNORE) + && jerry_debugger_send_function_cp (JERRY_DEBUGGER_RELEASE_BYTE_CODE_CP, bytecode_p)) { - if (jerry_debugger_send_function_cp (JERRY_DEBUGGER_RELEASE_BYTE_CODE_CP, bytecode_p)) + /* Delay the byte code free until the debugger client is notified. + * If the connection is aborted the pointer is still freed by + * jerry_debugger_close_connection(). */ + jerry_debugger_byte_code_free_t *byte_code_free_p = (jerry_debugger_byte_code_free_t *) bytecode_p; + jmem_cpointer_t byte_code_free_head = JERRY_CONTEXT (debugger_byte_code_free_head); + + byte_code_free_p->prev_cp = ECMA_NULL_POINTER; + + jmem_cpointer_t byte_code_free_cp; + JMEM_CP_SET_NON_NULL_POINTER (byte_code_free_cp, byte_code_free_p); + + if (byte_code_free_head == ECMA_NULL_POINTER) { - /* Delay the byte code free until the debugger client is notified. - * If the connection is aborted the pointer is still freed by - * jerry_debugger_close_connection(). */ - jerry_debugger_byte_code_free_t *byte_code_free_p = (jerry_debugger_byte_code_free_t *) bytecode_p; - jmem_cpointer_t byte_code_free_head = JERRY_CONTEXT (debugger_byte_code_free_head); - - byte_code_free_p->prev_cp = ECMA_NULL_POINTER; - byte_code_free_p->next_cp = byte_code_free_head; - - JMEM_CP_SET_NON_NULL_POINTER (JERRY_CONTEXT (debugger_byte_code_free_head), - byte_code_free_p); - - if (byte_code_free_head != ECMA_NULL_POINTER) - { - jerry_debugger_byte_code_free_t *next_byte_code_free_p; - - next_byte_code_free_p = JMEM_CP_GET_NON_NULL_POINTER (jerry_debugger_byte_code_free_t, - byte_code_free_head); - - next_byte_code_free_p->prev_cp = JERRY_CONTEXT (debugger_byte_code_free_head); - } - return; + JERRY_CONTEXT (debugger_byte_code_free_tail) = byte_code_free_cp; } + else + { + jerry_debugger_byte_code_free_t *first_byte_code_free_p; + + first_byte_code_free_p = JMEM_CP_GET_NON_NULL_POINTER (jerry_debugger_byte_code_free_t, + byte_code_free_head); + first_byte_code_free_p->prev_cp = byte_code_free_cp; + } + + JERRY_CONTEXT (debugger_byte_code_free_head) = byte_code_free_cp; + return; } #endif /* JERRY_DEBUGGER */ } diff --git a/jerry-core/jcontext/jcontext.h b/jerry-core/jcontext/jcontext.h index 7b8628725..353627b91 100644 --- a/jerry-core/jcontext/jcontext.h +++ b/jerry-core/jcontext/jcontext.h @@ -88,6 +88,7 @@ typedef struct uint8_t debugger_send_buffer[JERRY_DEBUGGER_MAX_BUFFER_SIZE]; /**< buffer for sending messages */ uint8_t debugger_receive_buffer[JERRY_DEBUGGER_MAX_BUFFER_SIZE]; /**< buffer for receiving messages */ jmem_cpointer_t debugger_byte_code_free_head; /**< head of byte code free linked list */ + jmem_cpointer_t debugger_byte_code_free_tail; /**< tail of byte code free linked list */ int debugger_connection; /**< hold the file descriptor for socket communication */ uint8_t debugger_flags; /**< debugger flags */ vm_frame_ctx_t *debugger_stop_context; /**< stop only if the current context is equal to this context */ diff --git a/jerry-core/jerry-snapshot.c b/jerry-core/jerry-snapshot.c index 5fdb46c2d..9c39ef522 100644 --- a/jerry-core/jerry-snapshot.c +++ b/jerry-core/jerry-snapshot.c @@ -395,6 +395,10 @@ snapshot_load_compiled_code (const uint8_t *snapshot_data_p, /**< snapshot data JERRY_ASSERT (bytecode_p->refs == 1); +#ifdef JERRY_DEBUGGER + bytecode_p->status_flags = (uint16_t) (bytecode_p->status_flags | CBC_CODE_FLAGS_DEBUGGER_IGNORE); +#endif /* JERRY_DEBUGGER */ + jmem_cpointer_t *literal_start_p = (jmem_cpointer_t *) (((uint8_t *) bytecode_p) + header_size); for (uint32_t i = 0; i < const_literal_end; i++) diff --git a/jerry-core/parser/js/byte-code.h b/jerry-core/parser/js/byte-code.h index ad3d13e35..a23ca6d7c 100644 --- a/jerry-core/parser/js/byte-code.h +++ b/jerry-core/parser/js/byte-code.h @@ -650,14 +650,19 @@ typedef struct uint16_t literal_end; /**< end position of the literal group */ } cbc_uint16_arguments_t; -/* When CBC_CODE_FLAGS_FULL_LITERAL_ENCODING - * is not set the small encoding is used. */ -#define CBC_CODE_FLAGS_FUNCTION 0x01 -#define CBC_CODE_FLAGS_FULL_LITERAL_ENCODING 0x02 -#define CBC_CODE_FLAGS_UINT16_ARGUMENTS 0x04 -#define CBC_CODE_FLAGS_STRICT_MODE 0x08 -#define CBC_CODE_FLAGS_ARGUMENTS_NEEDED 0x10 -#define CBC_CODE_FLAGS_LEXICAL_ENV_NOT_NEEDED 0x20 +/** + * Compact byte code status flags. + */ +typedef enum +{ + CBC_CODE_FLAGS_FUNCTION = (1u << 0), /**< compiled code is JavaScript function */ + CBC_CODE_FLAGS_FULL_LITERAL_ENCODING = (1u << 1), /**< full literal encoding mode is enabled */ + CBC_CODE_FLAGS_UINT16_ARGUMENTS = (1u << 2), /**< compiled code data is cbc_uint16_arguments_t */ + CBC_CODE_FLAGS_STRICT_MODE = (1u << 3), /**< strict mode is enabled */ + CBC_CODE_FLAGS_ARGUMENTS_NEEDED = (1u << 4), /**< arguments object must be constructed */ + CBC_CODE_FLAGS_LEXICAL_ENV_NOT_NEEDED = (1u << 5), /**< no need to create a lexical environment */ + CBC_CODE_FLAGS_DEBUGGER_IGNORE = (1u << 6), /**< this function should be ignored by debugger */ +} cbc_code_flags; #define CBC_OPCODE(arg1, arg2, arg3, arg4) arg1, diff --git a/jerry-core/vm/vm.c b/jerry-core/vm/vm.c index 578b6f7e4..5b89f2c43 100644 --- a/jerry-core/vm/vm.c +++ b/jerry-core/vm/vm.c @@ -2321,6 +2321,20 @@ vm_loop (vm_frame_ctx_t *frame_ctx_p) /**< frame context */ JERRY_ASSERT (JERRY_CONTEXT (debugger_flags) & JERRY_DEBUGGER_CONNECTED); + if (frame_ctx_p->bytecode_header_p->status_flags & CBC_CODE_FLAGS_DEBUGGER_IGNORE) + { + /* Messages are still processed regardless of ignore. */ + if (JERRY_CONTEXT (debugger_message_delay) > 0) + { + JERRY_CONTEXT (debugger_message_delay)--; + continue; + } + + JERRY_CONTEXT (debugger_message_delay) = JERRY_DEBUGGER_MESSAGE_FREQUENCY; + jerry_debugger_receive (); + continue; + } + frame_ctx_p->byte_code_p = byte_code_start_p; jerry_debugger_breakpoint_hit (); @@ -2337,6 +2351,20 @@ vm_loop (vm_frame_ctx_t *frame_ctx_p) /**< frame context */ JERRY_ASSERT (JERRY_CONTEXT (debugger_flags) & JERRY_DEBUGGER_CONNECTED); + if (frame_ctx_p->bytecode_header_p->status_flags & CBC_CODE_FLAGS_DEBUGGER_IGNORE) + { + /* Messages are still processed regardless of ignore. */ + if (JERRY_CONTEXT (debugger_message_delay) > 0) + { + JERRY_CONTEXT (debugger_message_delay)--; + continue; + } + + JERRY_CONTEXT (debugger_message_delay) = JERRY_DEBUGGER_MESSAGE_FREQUENCY; + jerry_debugger_receive (); + continue; + } + frame_ctx_p->byte_code_p = byte_code_start_p; if ((JERRY_CONTEXT (debugger_flags) & JERRY_DEBUGGER_VM_STOP)