Commit a9353fe8 authored by Peter Maydell's avatar Peter Maydell Committed by Stefan Hajnoczi
Browse files

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: default avatarJulian Brown <julian@codesourcery.com>
Signed-off-by: default avatarPeter Maydell <peter.maydell@linaro.org>
Message-id: 1481047629-7763-1-git-send-email-peter.maydell@linaro.org
Signed-off-by: default avatarStefan Hajnoczi <stefanha@redhat.com>
parent d750c3a9

exec.c

+6 −19
Original line number Diff line number Diff line
@@ -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)