target/i386/tcg/sysemu/bpt_helper.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-)
The loop is performing a simple boolean test for the existence
of a BP_CPU breakpoint at EIP. Plus it gets the iteration wrong,
if we happen to have a BP_GDB breakpoint at the same address.
We have a function for this: cpu_breakpoint_test.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/i386/tcg/sysemu/bpt_helper.c | 12 +++---------
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/target/i386/tcg/sysemu/bpt_helper.c b/target/i386/tcg/sysemu/bpt_helper.c
index 9bdf7e170b..f1fb479ad9 100644
--- a/target/i386/tcg/sysemu/bpt_helper.c
+++ b/target/i386/tcg/sysemu/bpt_helper.c
@@ -210,7 +210,6 @@ void breakpoint_handler(CPUState *cs)
{
X86CPU *cpu = X86_CPU(cs);
CPUX86State *env = &cpu->env;
- CPUBreakpoint *bp;
if (cs->watchpoint_hit) {
if (cs->watchpoint_hit->flags & BP_CPU) {
@@ -222,14 +221,9 @@ void breakpoint_handler(CPUState *cs)
}
}
} else {
- QTAILQ_FOREACH(bp, &cs->breakpoints, entry) {
- if (bp->pc == env->eip) {
- if (bp->flags & BP_CPU) {
- check_hw_breakpoints(env, true);
- raise_exception(env, EXCP01_DB);
- }
- break;
- }
+ if (cpu_breakpoint_test(cs, env->eip, BP_CPU)) {
+ check_hw_breakpoints(env, true);
+ raise_exception(env, EXCP01_DB);
}
}
}
--
2.25.1
Ping. On 6/19/21 11:23 PM, Richard Henderson wrote: > The loop is performing a simple boolean test for the existence > of a BP_CPU breakpoint at EIP. Plus it gets the iteration wrong, > if we happen to have a BP_GDB breakpoint at the same address. > > We have a function for this: cpu_breakpoint_test. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > target/i386/tcg/sysemu/bpt_helper.c | 12 +++--------- > 1 file changed, 3 insertions(+), 9 deletions(-) > > diff --git a/target/i386/tcg/sysemu/bpt_helper.c b/target/i386/tcg/sysemu/bpt_helper.c > index 9bdf7e170b..f1fb479ad9 100644 > --- a/target/i386/tcg/sysemu/bpt_helper.c > +++ b/target/i386/tcg/sysemu/bpt_helper.c > @@ -210,7 +210,6 @@ void breakpoint_handler(CPUState *cs) > { > X86CPU *cpu = X86_CPU(cs); > CPUX86State *env = &cpu->env; > - CPUBreakpoint *bp; > > if (cs->watchpoint_hit) { > if (cs->watchpoint_hit->flags & BP_CPU) { > @@ -222,14 +221,9 @@ void breakpoint_handler(CPUState *cs) > } > } > } else { > - QTAILQ_FOREACH(bp, &cs->breakpoints, entry) { > - if (bp->pc == env->eip) { > - if (bp->flags & BP_CPU) { > - check_hw_breakpoints(env, true); > - raise_exception(env, EXCP01_DB); > - } > - break; > - } > + if (cpu_breakpoint_test(cs, env->eip, BP_CPU)) { > + check_hw_breakpoints(env, true); > + raise_exception(env, EXCP01_DB); > } > } > } >
On Fri, Jul 09, 2021 at 09:00:30AM -0700, Richard Henderson wrote: > Ping. > > On 6/19/21 11:23 PM, Richard Henderson wrote: > > The loop is performing a simple boolean test for the existence > > of a BP_CPU breakpoint at EIP. Plus it gets the iteration wrong, > > if we happen to have a BP_GDB breakpoint at the same address. > > > > We have a function for this: cpu_breakpoint_test. > > > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> cpu_breakpoint_test() logic matches the existing code being replaced, so: Reviewed-by: Eduardo Habkost <ehabkost@redhat.com> I wouldn't have objected if you simply merged this without waiting for review from others, though. I believe you know this code better than anybody else. -- Eduardo
© 2016 - 2024 Red Hat, Inc.