[Qemu-devel] Problems with MIPS full system emulation and breakpoints

[Qemu-devel] Problems with MIPS full system emulation and breakpoints

[Jason Wessel]

[-- Attachment #1: Type: text/plain, Size: 1852 bytes --]


It seems there is an issue with the translation block flushing when 
writing to the code regions in the MIPS full system emulation.  Using a 
2.6 kernel which is basically running in single user mode, I use an 
extremely simple program:

main () {
    int i;
    for (i = 0; i < 10; i++) {
        printf("doing %i\n",i);
    }
}

/ # gdb simple_program
(gdb) break main
Breakpoint 1 at 0x400670: file simple_program.c, line 3.
(gdb) run
Starting program: /simple_program

Breakpoint 1, main () at simple_program.c:3
3               for (i = 0; i < 10; i++) {
(gdb) n
4               printf("doing %i\n",i);
(gdb) n
doing 0
3               for (i = 0; i < 10; i++) {
(gdb) n
doing 1

Program received signal SIGTRAP, Trace/breakpoint trap.
main () at simple_program.c:3
3               for (i = 0; i < 10; i++) {

At this point the program is trashed on the second time through the loop 
because the translated block with the breakpoint op code was executed 
instead of being flushed and translated with the correct original 
instruction.   All the single stepping and jumping over the function 
calls is done by writing a breakpoint op code in and later restoring the 
original instruction.  In the kernel access_process_vm() was used via 
ptrace to correctly read and write the breakpoints, and I have verified 
these writes are occurring.

To illustrate the problem further, I attached a patch that makes this 
problem go away.  Of course this is not the right fix, because it only 
deals with the breakpoint opcode and does not isolate the translated 
block that had the instruction that changed.  In theory you should be 
able to modify any part of the instruction code from another process 
with ptrace. 

Are there any suggestions as to how to fix this the right way?  The real 
hardware of course does not exhibit this issue.

Thanks,
Jason.


[-- Attachment #2: malta_tb_flush_HACK.patch --]
[-- Type: text/x-patch, Size: 372 bytes --]

Index: qemu/target-mips/helper.c
===================================================================
--- qemu.orig/target-mips/helper.c
+++ qemu/target-mips/helper.c
@@ -360,6 +360,7 @@ void do_interrupt (CPUState *env)
         goto set_EPC;
     case EXCP_BREAK:
         cause = 9;
+        tb_flush(env);
         goto set_EPC;
     case EXCP_RI:
         cause = 10;

Re: [Qemu-devel] Problems with MIPS full system emulation and breakpoints

[Daniel Jacobowitz]

On Fri, Apr 20, 2007 at 01:03:07PM -0500, Jason Wessel wrote:
> At this point the program is trashed on the second time through the loop 
> because the translated block with the breakpoint op code was executed instead 
> of being flushed and translated with the correct original
> instruction.

I have an idea.  When I was talking to Paul about breakpoints
recently, I noticed something very strange in the ARM port: it
continues to disassemble the instruction under a breakpoint after
generating the debug op.  This is a waste of CPU and memory, so I
tried taking it out - but he told me that if I did that, things would
go wrong because the size of the tb would be too small.  We'd try to
flush the tb at the breakpoint location, but it wouldn't seem to cover
there.

MIPS doesn't do that extra disassembly because it has a goto instead
of a break from the nested loop.  What happens if you add an extra +1
to the translation block size if there's a breakpoint, in target-mips/translate.c?

-- 
Daniel Jacobowitz
CodeSourcery

Re: [Qemu-devel] Problems with MIPS full system emulation and breakpoints

[Daniel Jacobowitz]

[-- Attachment #1: Type: text/plain, Size: 1393 bytes --]

On Fri, Apr 20, 2007 at 02:22:09PM -0400, Daniel Jacobowitz wrote:
> I have an idea.  When I was talking to Paul about breakpoints
> recently, I noticed something very strange in the ARM port: it
> continues to disassemble the instruction under a breakpoint after
> generating the debug op.  This is a waste of CPU and memory, so I
> tried taking it out - but he told me that if I did that, things would
> go wrong because the size of the tb would be too small.  We'd try to
> flush the tb at the breakpoint location, but it wouldn't seem to cover
> there.
> 
> MIPS doesn't do that extra disassembly because it has a goto instead
> of a break from the nested loop.  What happens if you add an extra
> +1 to the translation block size if there's a breakpoint, in
> target-mips/translate.c?

It won't help because that problem related to "hardware" breakpoints
through QEMU's gdb stub.

The attached patch fixes that, and Jason's issue, and probably the
FPU emulation issue also.  The real problem was "tb->size = 0" in the
search_pc case.  Alpha, ARM, m68k, mips, ppc, sh4, and sparc all
did this.  But it can't be right - the tb passed when searching for a
pc is in the cache, and clearing its size prevents it from being
flushed properly.

I got a couple of strange oopses after this, and one unidentified
lockup.  I don't think they are related, though.

-- 
Daniel Jacobowitz
CodeSourcery

[-- Attachment #2: qemu-mips-breakpoints.patch --]
[-- Type: text/x-diff, Size: 3027 bytes --]

--- target-alpha/translate.c	(revision 181278)
+++ target-alpha/translate.c	(local)
@@ -2047,7 +2047,6 @@ int gen_intermediate_code_internal (CPUS
         lj++;
         while (lj <= j)
             gen_opc_instr_start[lj++] = 0;
-        tb->size = 0;
     } else {
         tb->size = ctx.pc - pc_start;
     }
--- target-arm/translate.c	(revision 181278)
+++ target-arm/translate.c	(local)
@@ -3656,7 +3656,6 @@ static inline int gen_intermediate_code_
         lj++;
         while (lj <= j)
             gen_opc_instr_start[lj++] = 0;
-        tb->size = 0;
     } else {
         tb->size = dc->pc - pc_start;
     }
--- target-m68k/translate.c	(revision 181278)
+++ target-m68k/translate.c	(local)
@@ -3260,7 +3260,6 @@ gen_intermediate_code_internal(CPUState 
         lj++;
         while (lj <= j)
             gen_opc_instr_start[lj++] = 0;
-        tb->size = 0;
     } else {
         tb->size = dc->pc - pc_start;
     }
--- target-mips/translate.c	(revision 181278)
+++ target-mips/translate.c	(local)
@@ -5882,10 +5882,6 @@ static void decode_opc (CPUState *env, D
             generate_exception(ctx, EXCP_SYSCALL);
             break;
         case OPC_BREAK:
-            /* XXX: Hack to work around wrong handling of self-modifying code. */
-            ctx->pc += 4;
-            save_cpu_state(ctx, 1);
-            ctx->pc -= 4;
             generate_exception(ctx, EXCP_BREAK);
             break;
         case OPC_SPIM:
@@ -6433,6 +6429,9 @@ gen_intermediate_code_internal (CPUState
                     save_cpu_state(&ctx, 1);
                     ctx.bstate = BS_BRANCH;
                     gen_op_debug();
+                    /* Include the breakpoint location or the tb won't
+                     * be flushed when it must be.  */
+                    ctx.pc += 4;
                     goto done_generating;
                 }
             }
@@ -6493,7 +6492,6 @@ done_generating:
         lj++;
         while (lj <= j)
             gen_opc_instr_start[lj++] = 0;
-        tb->size = 0;
     } else {
         tb->size = ctx.pc - pc_start;
     }
--- target-ppc/translate.c	(revision 181278)
+++ target-ppc/translate.c	(local)
@@ -5878,7 +5878,6 @@ static inline int gen_intermediate_code_
         lj++;
         while (lj <= j)
             gen_opc_instr_start[lj++] = 0;
-        tb->size = 0;
     } else {
         tb->size = ctx.nip - pc_start;
     }
--- target-sh4/translate.c	(revision 181278)
+++ target-sh4/translate.c	(local)
@@ -1242,7 +1242,6 @@ gen_intermediate_code_internal(CPUState 
         ii++;
         while (ii <= i)
             gen_opc_instr_start[ii++] = 0;
-        tb->size = 0;
     } else {
         tb->size = ctx.pc - pc_start;
     }
--- target-sparc/translate.c	(revision 181278)
+++ target-sparc/translate.c	(local)
@@ -3365,7 +3365,6 @@ static inline int gen_intermediate_code_
         lj++;
         while (lj <= j)
             gen_opc_instr_start[lj++] = 0;
-        tb->size = 0;
 #if 0
         if (loglevel > 0) {
             page_dump(logfile);

Re: [Qemu-devel] Problems with MIPS full system emulation and breakpoints

[Thiemo Seufer]

Daniel Jacobowitz wrote:
> On Fri, Apr 20, 2007 at 02:22:09PM -0400, Daniel Jacobowitz wrote:
> > I have an idea.  When I was talking to Paul about breakpoints
> > recently, I noticed something very strange in the ARM port: it
> > continues to disassemble the instruction under a breakpoint after
> > generating the debug op.  This is a waste of CPU and memory, so I
> > tried taking it out - but he told me that if I did that, things would
> > go wrong because the size of the tb would be too small.  We'd try to
> > flush the tb at the breakpoint location, but it wouldn't seem to cover
> > there.
> > 
> > MIPS doesn't do that extra disassembly because it has a goto instead
> > of a break from the nested loop.  What happens if you add an extra
> > +1 to the translation block size if there's a breakpoint, in
> > target-mips/translate.c?
> 
> It won't help because that problem related to "hardware" breakpoints
> through QEMU's gdb stub.
> 
> The attached patch fixes that, and Jason's issue, and probably the
> FPU emulation issue also.

It fixes the FPU emulation problem.

> The real problem was "tb->size = 0" in the
> search_pc case.  Alpha, ARM, m68k, mips, ppc, sh4, and sparc all
> did this.  But it can't be right - the tb passed when searching for a
> pc is in the cache, and clearing its size prevents it from being
> flushed properly.
> 
> I got a couple of strange oopses after this, and one unidentified
> lockup.  I don't think they are related, though.

Works fine for me.


Thiemo

Re: [Qemu-devel] Problems with MIPS full system emulation and breakpoints: also for FPU emulation

[Stefan Weil]

A similar patch for EXCP_AdEL seems to fix the problems
with Linux floating point emulation.

Linux uses self modifying code on the stack in the FPU emulation
and terminates this code using "lw    zero,1(zero)"
(which raises EXCP_AdEL).

Sometimes, a system call which follows and is also on the stack
does not raise EXCP_SYSCALL but raises EXCP_AdEL, so the calling
process gets a signal SIGILL.

I tested this by running aptitude on Debian 4.0.

It crashs with 4KEc (no FPU, CVS HEAD).
It works with default CPU (FPU, CVS HEAD).
It works with 4KEc (no FPU, patched CVS HEAD).

Stefan

Jason Wessel schrieb:
>
> It seems there is an issue with the translation block flushing when
> writing to the code regions in the MIPS full system emulation.  Using
> a 2.6 kernel which is basically running in single user mode, I use an
> extremely simple program:
>
> ...
> To illustrate the problem further, I attached a patch that makes this
> problem go away.  Of course this is not the right fix, because it only
> deals with the breakpoint opcode and does not isolate the translated
> block that had the instruction that changed.  In theory you should be
> able to modify any part of the instruction code from another process
> with ptrace.
> Are there any suggestions as to how to fix this the right way?  The
> real hardware of course does not exhibit this issue.
>
> Thanks,
> Jason.
>
>
> ------------------------------------------------------------------------
>
> Index: qemu/target-mips/helper.c
> ===================================================================
> --- qemu.orig/target-mips/helper.c
> +++ qemu/target-mips/helper.c
> @@ -360,6 +360,7 @@ void do_interrupt (CPUState *env)
> goto set_EPC;
> case EXCP_BREAK:
> cause = 9;
> + tb_flush(env);
> goto set_EPC;
> case EXCP_RI:
> cause = 10;