[Qemu-devel] [PATCH 0/1] possible fix for hw breakpoint crash on x86 32bit host

[Qemu-devel] [PATCH 0/1] possible fix for hw breakpoint crash on x86 32bit host

[Jason Wessel]

The aim of this patch is to either fix the problem or at least
describe the circumstances around the crash using a 32 bit x86 linux
host with a 32 bit linux guest making use of hw breakpoints.

I had also seen the same problem reported here:

https://bugs.launchpad.net/qemu/+bug/501177


---- running qemu in gdb with a linux kernel as the guest ----
kgdbts:RUN hw breakpoint test
[New Thread 0xb7f1d700 (LWP 31553)]

Program received signal SIGSEGV, Segmentation fault.
raise_interrupt (intno=1, is_int=0, error_code=0, next_eip_addend=0)
    at /ord-lpggp11/jwessel/git/qemu/target-i386/op_helper.c:1335
1335        env->exception_index = intno;
(gdb) info address env
Symbol "env" is a variable in register ebp.
(gdb) bt
#0  raise_interrupt (intno=1, is_int=0, error_code=0, next_eip_addend=0)
    at /ord-lpggp11/jwessel/git/qemu/target-i386/op_helper.c:1335
#1  0x081380b7 in raise_exception (exception_index=1)
    at /ord-lpggp11/jwessel/git/qemu/target-i386/op_helper.c:1351
#2  0x0814db6e in breakpoint_handler (env=0x9ed8b50)
    at /ord-lpggp11/jwessel/git/qemu/target-i386/helper.c:1551
#3  0x080e1643 in cpu_x86_exec (env1=0x9ed8b50)
    at /ord-lpggp11/jwessel/git/qemu/cpu-exec.c:206
#4  0x08053af9 in main (argc=0, argv=0x0, envp=Cannot access memory at address 0x8
)
    at /ord-lpggp11/jwessel/git/qemu/vl.c:3816
(gdb) 

I concluded that env points to the ebp register and it only has a
valid value while executing from the generated code context.  I
created another function to pass in the valid version of env in order
to set ebp in preparation to eventually execute a longjmp() via the
raise_exception().

It is not clear that this is the right way to fix the problem, but it
does allow the guest hw breakpoint tests to work correctly on a 32 bit
and 64 bit x86 linux host.

If there is a better way to solve this, please let me know.

Thanks,
Jason.

---
Jason Wessel (1):
      target-i386: fix crash on x86 32bit linux host with hw breakpoint exceptions

 target-i386/exec.h      |    1 +
 target-i386/helper.c    |    6 +++---
 target-i386/op_helper.c |    5 +++++
 3 files changed, 9 insertions(+), 3 deletions(-)

[Qemu-devel] [PATCH 1/1] target-i386: fix crash on x86 32bit linux host with hw breakpoint exceptions

[Jason Wessel]

If you make use of hw breakpoints on a 32bit x86 linux host, qemu
will segmentation fault when processing the exception.

The problem is that the value of env is stored in $ebp in the op_helper
raise_exception() function, and it can have the wrong value when
calling it from non generated code.

It is possible to work around the problem by restoring the value of
env before calling raise_exception() using a new helper function that
takes (CPUState *) as one of the arguments.

Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
---
 target-i386/exec.h      |    1 +
 target-i386/helper.c    |    6 +++---
 target-i386/op_helper.c |    5 +++++
 3 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/target-i386/exec.h b/target-i386/exec.h
index 1fd74fd..4ff3c57 100644
--- a/target-i386/exec.h
+++ b/target-i386/exec.h
@@ -73,6 +73,7 @@ void do_interrupt_user(int intno, int is_int, int error_code,
                        target_ulong next_eip);
 void QEMU_NORETURN raise_exception_err(int exception_index, int error_code);
 void QEMU_NORETURN raise_exception(int exception_index);
+void QEMU_NORETURN raise_exception_env(int exception_index, CPUState *nenv);
 void do_smm_enter(void);
 
 /* n must be a constant to be efficient */
diff --git a/target-i386/helper.c b/target-i386/helper.c
index 70762bb..736ef16 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -1529,7 +1529,7 @@ int check_hw_breakpoints(CPUState *env, int force_dr6_update)
 
 static CPUDebugExcpHandler *prev_debug_excp_handler;
 
-void raise_exception(int exception_index);
+void raise_exception_env(int exception_index, CPUState *env);
 
 static void breakpoint_handler(CPUState *env)
 {
@@ -1539,7 +1539,7 @@ static void breakpoint_handler(CPUState *env)
         if (env->watchpoint_hit->flags & BP_CPU) {
             env->watchpoint_hit = NULL;
             if (check_hw_breakpoints(env, 0))
-                raise_exception(EXCP01_DB);
+                raise_exception_env(EXCP01_DB, env);
             else
                 cpu_resume_from_signal(env, NULL);
         }
@@ -1548,7 +1548,7 @@ static void breakpoint_handler(CPUState *env)
             if (bp->pc == env->eip) {
                 if (bp->flags & BP_CPU) {
                     check_hw_breakpoints(env, 1);
-                    raise_exception(EXCP01_DB);
+                    raise_exception_env(EXCP01_DB, env);
                 }
                 break;
             }
diff --git a/target-i386/op_helper.c b/target-i386/op_helper.c
index 5eea322..4bb4347 100644
--- a/target-i386/op_helper.c
+++ b/target-i386/op_helper.c
@@ -1351,6 +1351,11 @@ void raise_exception(int exception_index)
     raise_interrupt(exception_index, 0, 0, 0);
 }
 
+void raise_exception_env(int exception_index, CPUState *nenv)
+{
+    env = nenv;
+    raise_exception(exception_index);
+}
 /* SMM support */
 
 #if defined(CONFIG_USER_ONLY)
-- 
1.6.3.3

Re: [Qemu-devel] [PATCH 1/1] target-i386: fix crash on x86 32bit linux host with hw breakpoint exceptions

[Aurelien Jarno]

On Tue, Jan 26, 2010 at 04:29:50PM -0600, Jason Wessel wrote:
> If you make use of hw breakpoints on a 32bit x86 linux host, qemu
> will segmentation fault when processing the exception.
> 
> The problem is that the value of env is stored in $ebp in the op_helper
> raise_exception() function, and it can have the wrong value when
> calling it from non generated code.
> 
> It is possible to work around the problem by restoring the value of
> env before calling raise_exception() using a new helper function that
> takes (CPUState *) as one of the arguments.

Thanks, applied.

> Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
> ---
>  target-i386/exec.h      |    1 +
>  target-i386/helper.c    |    6 +++---
>  target-i386/op_helper.c |    5 +++++
>  3 files changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/target-i386/exec.h b/target-i386/exec.h
> index 1fd74fd..4ff3c57 100644
> --- a/target-i386/exec.h
> +++ b/target-i386/exec.h
> @@ -73,6 +73,7 @@ void do_interrupt_user(int intno, int is_int, int error_code,
>                         target_ulong next_eip);
>  void QEMU_NORETURN raise_exception_err(int exception_index, int error_code);
>  void QEMU_NORETURN raise_exception(int exception_index);
> +void QEMU_NORETURN raise_exception_env(int exception_index, CPUState *nenv);
>  void do_smm_enter(void);
>  
>  /* n must be a constant to be efficient */
> diff --git a/target-i386/helper.c b/target-i386/helper.c
> index 70762bb..736ef16 100644
> --- a/target-i386/helper.c
> +++ b/target-i386/helper.c
> @@ -1529,7 +1529,7 @@ int check_hw_breakpoints(CPUState *env, int force_dr6_update)
>  
>  static CPUDebugExcpHandler *prev_debug_excp_handler;
>  
> -void raise_exception(int exception_index);
> +void raise_exception_env(int exception_index, CPUState *env);
>  
>  static void breakpoint_handler(CPUState *env)
>  {
> @@ -1539,7 +1539,7 @@ static void breakpoint_handler(CPUState *env)
>          if (env->watchpoint_hit->flags & BP_CPU) {
>              env->watchpoint_hit = NULL;
>              if (check_hw_breakpoints(env, 0))
> -                raise_exception(EXCP01_DB);
> +                raise_exception_env(EXCP01_DB, env);
>              else
>                  cpu_resume_from_signal(env, NULL);
>          }
> @@ -1548,7 +1548,7 @@ static void breakpoint_handler(CPUState *env)
>              if (bp->pc == env->eip) {
>                  if (bp->flags & BP_CPU) {
>                      check_hw_breakpoints(env, 1);
> -                    raise_exception(EXCP01_DB);
> +                    raise_exception_env(EXCP01_DB, env);
>                  }
>                  break;
>              }
> diff --git a/target-i386/op_helper.c b/target-i386/op_helper.c
> index 5eea322..4bb4347 100644
> --- a/target-i386/op_helper.c
> +++ b/target-i386/op_helper.c
> @@ -1351,6 +1351,11 @@ void raise_exception(int exception_index)
>      raise_interrupt(exception_index, 0, 0, 0);
>  }
>  
> +void raise_exception_env(int exception_index, CPUState *nenv)
> +{
> +    env = nenv;
> +    raise_exception(exception_index);
> +}
>  /* SMM support */
>  
>  #if defined(CONFIG_USER_ONLY)
> -- 
> 1.6.3.3
> 
> 
> 
> 

-- 
Aurelien Jarno                          GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net