diff options
author | Peter Maydell <peter.maydell@linaro.org> | 2016-12-06 18:07:09 +0000 |
---|---|---|
committer | Stefan Hajnoczi <stefanha@redhat.com> | 2016-12-06 20:21:46 +0000 |
commit | a9353fe897ca2687e5b3385ed39e3db3927a90e0 (patch) | |
tree | cdcbdc899172840177ee1330e69d5a101c07eb6d | |
parent | d750c3a966d6409356739e97fa4d4a6446754e1e (diff) |
exec.c: Fix breakpoint invalidation race
A bug (1647683) was reported showing a crash when removing
breakpoints. The reproducer was bisected to 3359baad when tb_flush
was finally made thread safe. While in MTTCG the locking in
breakpoint_invalidate would have prevented any problems, but
currently tb_lock() is a NOP for system emulation.
The race is between a tb_flush from the gdbstub and the
tb_invalidate_phys_addr() in breakpoint_invalidate().
Ideally we'd have actual locking here; for the moment the
simple fix is to do a full tb_flush() for a bp invalidate,
since that is thread-safe even if no lock is taken.
Reported-by: Julian Brown <julian@codesourcery.com>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Message-id: 1481047629-7763-1-git-send-email-peter.maydell@linaro.org
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
-rw-r--r-- | exec.c | 25 |
1 files changed, 6 insertions, 19 deletions
@@ -684,28 +684,15 @@ void cpu_exec_realizefn(CPUState *cpu, Error **errp) #endif } -#if defined(CONFIG_USER_ONLY) -static void breakpoint_invalidate(CPUState *cpu, target_ulong pc) -{ - mmap_lock(); - tb_lock(); - tb_invalidate_phys_page_range(pc, pc + 1, 0); - tb_unlock(); - mmap_unlock(); -} -#else static void breakpoint_invalidate(CPUState *cpu, target_ulong pc) { - MemTxAttrs attrs; - hwaddr phys = cpu_get_phys_page_attrs_debug(cpu, pc, &attrs); - int asidx = cpu_asidx_from_attrs(cpu, attrs); - if (phys != -1) { - /* Locks grabbed by tb_invalidate_phys_addr */ - tb_invalidate_phys_addr(cpu->cpu_ases[asidx].as, - phys | (pc & ~TARGET_PAGE_MASK)); - } + /* Flush the whole TB as this will not have race conditions + * even if we don't have proper locking yet. + * Ideally we would just invalidate the TBs for the + * specified PC. + */ + tb_flush(cpu); } -#endif #if defined(CONFIG_USER_ONLY) void cpu_watchpoint_remove_all(CPUState *cpu, int mask) |