From 4fe84ebf6c9cc62f36ed272a1317475a524334d1 Mon Sep 17 00:00:00 2001 From: Stefani Seibold Date: Thu, 26 Nov 2015 08:02:40 +0100 Subject: [PATCH] minor bug fixes fix a crash on a race condition during new task creation disable scratch hw bp after use always stop threads when change a non scratch hw bp only change os specific data for the leader --- .gitignore | 1 + breakpoint.c | 31 +++++++++++++++++++-------- event.c | 5 +++++ sysdeps/linux-gnu/proc.c | 8 +++---- task.c | 46 +++++++++++++++++++++++----------------- 5 files changed, 59 insertions(+), 32 deletions(-) diff --git a/.gitignore b/.gitignore index 2e4f1a7..188bfa4 100644 --- a/.gitignore +++ b/.gitignore @@ -35,3 +35,4 @@ mtrace-ng debian/files debian/mtrace-ng.substvars debian/mtrace-ng +tags diff --git a/breakpoint.c b/breakpoint.c index cb0f1a6..3ec4360 100644 --- a/breakpoint.c +++ b/breakpoint.c @@ -171,6 +171,7 @@ void reorder_hw_bp(struct task *task) return; n = 0; + list_for_each(it, &leader->hw_bp_list) { bp = container_of(it, struct breakpoint, link_list); if (bp->enabled) { @@ -180,16 +181,19 @@ void reorder_hw_bp(struct task *task) } } + if (!n) + return; + qsort(bp_list, n, sizeof(*bp_list), hw_bp_sort); - for(i = 0; i < n; ++i) { + for(i = 0; i < n; ++i) bp_list[i]->hwcnt = (i < HW_BREAKPOINTS - 1) ? BP_REORDER_THRESHOLD >> (i + 4) : 0; - } if (n > HW_BREAKPOINTS - 1) n = HW_BREAKPOINTS - 1; p = bp_list; + for(i = 0; i < n; ++i) { if (bp_list[i]->hw) { assert(bp_list[i]->hw_bp_slot != HW_BP_SCRATCH_SLOT); @@ -201,14 +205,23 @@ void reorder_hw_bp(struct task *task) else *p++ = bp_list[i]; } + + if (p == bp_list) + return; + *p = NULL; + stop_threads(leader); + i = HW_BP_SCRATCH_SLOT + 1; - for(p = bp_list; (bp = *p); ++p) { + p = bp_list; + + do { + bp = *p; + while(hw_bp_set & (1 << i)) ++i; - stop_threads(leader); disable_sw_breakpoint(leader, bp); if (leader->hw_bp[i]) @@ -220,7 +233,8 @@ void reorder_hw_bp(struct task *task) each_task(leader, enable_hw_bp_cb, bp); ++i; - } + ++p; + } while(*p); } static int insert_hw_bp_slot(struct task *task, struct breakpoint *bp) @@ -233,7 +247,6 @@ static int insert_hw_bp_slot(struct task *task, struct breakpoint *bp) break; if (bp->type == BP_HW && leader->hw_bp[i]->type == BP_AUTO) { - stop_threads(leader); hw2sw_bp(leader, leader->hw_bp[i]); break; } @@ -396,14 +409,14 @@ void breakpoint_enable(struct task *task, struct breakpoint *bp) bp->enabled = 1; return; } +#endif + stop_threads(task); #if HW_BREAKPOINTS > 1 if (bp->type >= BP_HW) { if (!insert_hw_bp_slot(task, bp)) return; } #endif -#endif - stop_threads(task); bp->hw = 0; enable_sw_breakpoint(task, bp); bp->enabled = 1; @@ -418,6 +431,7 @@ void breakpoint_disable(struct task *task, struct breakpoint *bp) debug(DEBUG_PROCESS, "pid=%d, addr=%#lx", task->pid, bp->addr); if (likely(bp->enabled)) { + stop_threads(task); #if HW_BREAKPOINTS > 0 if (bp->hw) { struct task *leader = task->leader; @@ -439,7 +453,6 @@ void breakpoint_disable(struct task *task, struct breakpoint *bp) return; } #endif - stop_threads(task); disable_sw_breakpoint(task, bp); bp->enabled = 0; } diff --git a/event.c b/event.c index c656bd8..67b19ed 100644 --- a/event.c +++ b/event.c @@ -240,6 +240,11 @@ static int handle_call_after(struct task *task, struct breakpoint *bp) if (unlikely(options.verbose > 1)) start_time(&start); +#if HW_BREAKPOINTS > 0 + if (bp->hw) + disable_scratch_hw_bp(task, bp); +#endif + task->libsym->func->report_out(task, task->libsym); if (unlikely(options.verbose > 1)) diff --git a/sysdeps/linux-gnu/proc.c b/sysdeps/linux-gnu/proc.c index d3821be..ba8c1ff 100644 --- a/sysdeps/linux-gnu/proc.c +++ b/sysdeps/linux-gnu/proc.c @@ -609,10 +609,10 @@ done: int os_task_init(struct task *task) { - struct task *leader = task->leader; - - leader->os.debug_addr = 0; - leader->os.debug_state = RT_ADD; + if (task == task->leader) { + task->os.debug_addr = 0; + task->os.debug_state = RT_ADD; + } return 0; } diff --git a/task.c b/task.c index 5397147..63d6a0c 100644 --- a/task.c +++ b/task.c @@ -131,6 +131,7 @@ static int leader_setup(struct task *leader) static int task_init(struct task *task) { pid_t tgid; + struct task *leader; /* Add process so that we know who the leader is. */ tgid = process_leader(task->pid); @@ -140,7 +141,26 @@ static int task_init(struct task *task) } if (tgid == task->pid) { - task->leader = task; + leader = task; + } + else { + leader = pid2task(tgid); + + if (!leader) { + fprintf(stderr, "%s no leader for tgpid=%d\n", __FUNCTION__, tgid); + return -1; + } + } + + task->leader = leader; + + if (arch_task_init(task) < 0) + return -1; + + if (os_task_init(task) < 0) + return -1; + + if (task == leader) { task->threads = 1; breakpoint_setup(task); @@ -148,27 +168,14 @@ static int task_init(struct task *task) list_add_tail(&task->leader_list, &list_of_leaders); } else { - task->leader = pid2task(tgid); - - if (!task->leader) { - fprintf(stderr, "%s no leader for tgpid=%d\n", __FUNCTION__, tgid); - return -1; - } - - task->leader->threads++; + leader->threads++; task->breakpoints = NULL; - list_add_tail(&task->task_list, &task->leader->task_list); + list_add_tail(&task->task_list, &leader->task_list); } task->attached = 1; - if (arch_task_init(task) < 0) - return -1; - - if (os_task_init(task) < 0) - return -1; - breakpoint_hw_destroy(task); return 0; @@ -243,16 +250,17 @@ struct task *task_new(pid_t pid) library_setup(task); + init_event(task); + if (task_init(task) < 0) goto fail1; - init_event(task); - insert_pid(task); return task; fail1: - task_destroy(task); + free(task); + return NULL; }