Skip to content

[debugger enhance] don't block gdbserver thread while executing #989

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 16 commits into from
Feb 16, 2022
Merged

[debugger enhance] don't block gdbserver thread while executing #989

Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
37bce76
don't block gdbserver thread while executing
xujuntwt95329 Jan 26, 2022
21dbc4d
fix some issue about current thread tid
xujuntwt95329 Jan 27, 2022
7b1c105
remove unused functions
xujuntwt95329 Jan 27, 2022
65ccc26
auto format
xujuntwt95329 Jan 27, 2022
02d653d
move mutex lock to debug_engine
xujuntwt95329 Jan 27, 2022
b0570cd
remove more unused codes
xujuntwt95329 Jan 27, 2022
934bf6a
remove unnecessary comment
xujuntwt95329 Jan 27, 2022
5fafce8
add empty line between type declaration
xujuntwt95329 Jan 27, 2022
cbaebb6
fix some warning on windows
xujuntwt95329 Jan 27, 2022
492f5d5
enable windows platform
xujuntwt95329 Jan 27, 2022
80219a6
enhance cmake option check
xujuntwt95329 Jan 28, 2022
ba83116
change vendor string
xujuntwt95329 Feb 4, 2022
f226bed
auto format
xujuntwt95329 Feb 4, 2022
f4c6f10
address PR comments
xujuntwt95329 Feb 15, 2022
7aceef7
ignore empty gdb request
xujuntwt95329 Feb 15, 2022
a5ab4dd
address PR comment
xujuntwt95329 Feb 16, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion build-scripts/runtime_lib.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,12 @@ endif ()
if (WAMR_BUILD_DEBUG_INTERP EQUAL 1)
set (WAMR_BUILD_THREAD_MGR 1)
include (${IWASM_DIR}/libraries/debug-engine/debug_engine.cmake)

if (WAMR_BUILD_FAST_INTERP EQUAL 1)
set (WAMR_BUILD_FAST_INTERP 0)
message(STATUS
"Debugger doesn't work with fast interpreter, switch to classic interpreter")
endif ()
endif ()

if (WAMR_BUILD_THREAD_MGR EQUAL 1)
Expand All @@ -95,7 +101,7 @@ endif ()

if (WAMR_BUILD_LIBC_EMCC EQUAL 1)
include (${IWASM_DIR}/libraries/libc-emcc/libc_emcc.cmake)
endif()
endif ()

####################### Common sources #######################
if (NOT MSVC)
Expand Down
4 changes: 3 additions & 1 deletion core/iwasm/common/wasm_exec_env.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,10 +160,12 @@ wasm_exec_env_destroy(WASMExecEnv *exec_env)
/* Terminate all sub-threads */
WASMCluster *cluster = wasm_exec_env_get_cluster(exec_env);
if (cluster) {
wasm_cluster_terminate_all_except_self(cluster, exec_env);
#if WASM_ENABLE_DEBUG_INTERP != 0
/* Must fire exit event after other threads exits, otherwise
the stopped thread will be overrided by other threads */
wasm_cluster_thread_exited(exec_env);
#endif
wasm_cluster_terminate_all_except_self(cluster, exec_env);
wasm_cluster_del_exec_env(cluster, exec_env);
}
#endif /* end of WASM_ENABLE_THREAD_MGR */
Expand Down
134 changes: 92 additions & 42 deletions core/iwasm/libraries/debug-engine/debug_engine.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,14 @@ typedef struct WASMDebugEngine {
bool active;
} WASMDebugEngine;

void
on_thread_stop_event(WASMDebugInstance *debug_inst, WASMExecEnv *exec_env)
{
os_mutex_lock(&debug_inst->wait_lock);
debug_inst->stopped_thread = exec_env;
os_mutex_unlock(&debug_inst->wait_lock);
}

static WASMDebugEngine *g_debug_engine;

static uint32 current_instance_id = 1;
Expand Down Expand Up @@ -104,6 +112,43 @@ control_thread_routine(void *arg)
while (true) {
os_mutex_lock(&control_thread->wait_lock);
if (!should_stop(control_thread)) {
/* send thread stop reply */
if (debug_inst->stopped_thread
&& debug_inst->current_state == APP_RUNNING) {
uint32 status;
korp_tid tid;

status =
(uint32)
debug_inst->stopped_thread->current_status->signal_flag;
tid = debug_inst->stopped_thread->handle;

if (debug_inst->stopped_thread->current_status->running_status
== STATUS_EXIT) {
/* If the thread exits, report "W00" if it's the last thread
* in the cluster, otherwise ignore this event */
status = 0;

/* By design, all the other threads should stopped at this
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should stop or should have been stopped?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

* moment, so it is safe to access the exec_env_list.len
* without lock */
if (debug_inst->cluster->exec_env_list.len != 1) {
debug_inst->stopped_thread = NULL;
os_mutex_unlock(&control_thread->wait_lock);
continue;
}
}

wasm_debug_instance_set_cur_thread(
debug_inst, debug_inst->stopped_thread->handle);

send_thread_stop_status(control_thread->server, status, tid);

debug_inst->current_state = APP_STOPPED;
debug_inst->stopped_thread = NULL;
}

/* Processing incoming requests */
if (!wasm_gdbserver_handle_packet(control_thread->server)) {
control_thread->status = STOPPED;
}
Expand Down Expand Up @@ -148,8 +193,10 @@ wasm_debug_control_thread_create(WASMDebugInstance *debug_instance)
/* wait until the debug control thread ready */
os_cond_wait(&debug_instance->wait_cond, &debug_instance->wait_lock);
os_mutex_unlock(&debug_instance->wait_lock);
if (!control_thread->server)
if (!control_thread->server) {
os_thread_join(control_thread->tid, NULL);
goto fail1;
}

os_mutex_lock(&g_debug_engine->instance_list_lock);
/* create control thread success, append debug instance to debug engine */
Expand Down Expand Up @@ -467,46 +514,6 @@ wasm_debug_instance_get_tids(WASMDebugInstance *instance, korp_tid tids[],
return threads_num;
}

static WASMExecEnv *
get_stopped_thread(WASMCluster *cluster)
{
WASMExecEnv *exec_env;

exec_env = bh_list_first_elem(&cluster->exec_env_list);
while (exec_env) {
if (exec_env->current_status->running_status != STATUS_RUNNING) {
return exec_env;
}
exec_env = bh_list_elem_next(exec_env);
}

return NULL;
}

korp_tid
wasm_debug_instance_wait_thread(WASMDebugInstance *instance, korp_tid tid,
uint32 *status)
{
WASMExecEnv *exec_env = NULL;

os_mutex_lock(&instance->wait_lock);
while ((instance->cluster->exec_env_list.len != 0)
&& ((exec_env = get_stopped_thread(instance->cluster)) == NULL)) {
os_cond_wait(&instance->wait_cond, &instance->wait_lock);
}
os_mutex_unlock(&instance->wait_lock);

/* If cluster has no exec_env, then this whole cluster is exiting */
if (instance->cluster->exec_env_list.len == 0) {
*status = 0;
return 0;
}

instance->current_tid = exec_env->handle;
*status = (uint32)exec_env->current_status->signal_flag;
return exec_env->handle;
}

uint32
wasm_debug_instance_get_thread_status(WASMDebugInstance *instance, korp_tid tid)
{
Expand Down Expand Up @@ -538,7 +545,8 @@ wasm_debug_instance_get_pc(WASMDebugInstance *instance)
return 0;

exec_env = wasm_debug_instance_get_current_env(instance);
if ((exec_env->cur_frame != NULL) && (exec_env->cur_frame->ip != NULL)) {
if ((exec_env != NULL) && (exec_env->cur_frame != NULL)
&& (exec_env->cur_frame->ip != NULL)) {
WASMModuleInstance *module_inst =
(WASMModuleInstance *)exec_env->module_inst;
return WASM_ADDR(
Expand Down Expand Up @@ -935,6 +943,11 @@ wasm_debug_instance_continue(WASMDebugInstance *instance)
if (!instance)
return false;

if (instance->current_state == APP_RUNNING) {
LOG_VERBOSE("Already in running state, ignore continue request");
return false;
}

exec_env = bh_list_first_elem(&instance->cluster->exec_env_list);
if (!exec_env)
return false;
Expand All @@ -943,6 +956,28 @@ wasm_debug_instance_continue(WASMDebugInstance *instance)
wasm_cluster_thread_continue(exec_env);
exec_env = bh_list_elem_next(exec_env);
}

instance->current_state = APP_RUNNING;

return true;
}

bool
wasm_debug_instance_interrupt_all_threads(WASMDebugInstance *instance)
{
WASMExecEnv *exec_env;

if (!instance)
return false;

exec_env = bh_list_first_elem(&instance->cluster->exec_env_list);
if (!exec_env)
return false;

while (exec_env) {
wasm_cluster_thread_send_signal(exec_env, WAMR_SIG_TRAP);
exec_env = bh_list_elem_next(exec_env);
}
return true;
}

Expand All @@ -960,8 +995,15 @@ wasm_debug_instance_kill(WASMDebugInstance *instance)

while (exec_env) {
wasm_cluster_thread_send_signal(exec_env, WAMR_SIG_TERM);
if (instance->current_state == APP_STOPPED) {
/* Resume all threads so they can receive the TERM signal */
exec_env->current_status->running_status = STATUS_RUNNING;
os_cond_signal(&exec_env->wait_cond);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we wrap os_cond_signal with os_mutex_lock(&exec_env->wait_lock) and os_mutex_unlock(&exec_env->wait_lock)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

}
exec_env = bh_list_elem_next(exec_env);
}

instance->current_state = APP_RUNNING;
return true;
}

Expand All @@ -973,6 +1015,11 @@ wasm_debug_instance_singlestep(WASMDebugInstance *instance, korp_tid tid)
if (!instance)
return false;

if (instance->current_state == APP_RUNNING) {
LOG_VERBOSE("Already in running state, ignore step request");
return false;
}

exec_env = bh_list_first_elem(&instance->cluster->exec_env_list);
if (!exec_env)
return false;
Expand All @@ -984,6 +1031,9 @@ wasm_debug_instance_singlestep(WASMDebugInstance *instance, korp_tid tid)
}
exec_env = bh_list_elem_next(exec_env);
}

instance->current_state = APP_RUNNING;

return true;
}

Expand Down
26 changes: 22 additions & 4 deletions core/iwasm/libraries/debug-engine/debug_engine.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,15 @@ typedef struct WASMDebugBreakPoint {
uint64 orignal_data;
} WASMDebugBreakPoint;

typedef enum debug_state_t {
/* Debugger state conversion sequence:
* DBG_LAUNCHING ---> APP_STOPPED <---> APP_RUNNING
*/
DBG_LAUNCHING,
APP_RUNNING,
APP_STOPPED
} debug_state_t;

typedef struct WASMDebugInstance {
struct WASMDebugInstance *next;
WASMDebugControlThread *control_thread;
Expand All @@ -44,6 +53,13 @@ typedef struct WASMDebugInstance {
korp_tid current_tid;
korp_mutex wait_lock;
korp_cond wait_cond;
/* Last stopped thread, it should be set to NULL when sending
* out the thread stop reply */
WASMExecEnv *volatile stopped_thread;
/* Currently status of the debug instance, it will be set to
* RUNNING when receiving STEP/CONTINUE commands, and set to
* STOPPED when any thread stopped */
volatile debug_state_t current_state;
} WASMDebugInstance;

typedef enum WASMDebugEventKind {
Expand Down Expand Up @@ -77,6 +93,9 @@ typedef enum WasmAddressType {

#define INVALIED_ADDR (0xFFFFFFFFFFFFFFFF)

void
on_thread_stop_event(WASMDebugInstance *debug_inst, WASMExecEnv *exec_env);

WASMDebugInstance *
wasm_debug_instance_create(WASMCluster *cluster);

Expand Down Expand Up @@ -152,16 +171,15 @@ bool
wasm_debug_instance_remove_breakpoint(WASMDebugInstance *instance, uint64 addr,
uint64 length);

bool
wasm_debug_instance_interrupt_all_threads(WASMDebugInstance *instance);

bool
wasm_debug_instance_continue(WASMDebugInstance *instance);

bool
wasm_debug_instance_kill(WASMDebugInstance *instance);

korp_tid
wasm_debug_instance_wait_thread(WASMDebugInstance *instance, korp_tid tid,
uint32 *status);

uint32
wasm_debug_instance_get_thread_status(WASMDebugInstance *instance,
korp_tid tid);
Expand Down
Loading