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

login
register
mail settings
Submitter Jason Wessel
Date Jan. 26, 2010, 10:29 p.m.
Message ID <1264544990-7628-2-git-send-email-jason.wessel@windriver.com>
Download mbox | patch
Permalink /patch/43751/
State New
Headers show

Comments

Jason Wessel - Jan. 26, 2010, 10:29 p.m.
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(-)
Aurelien Jarno - Feb. 28, 2010, 6:23 p.m.
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
> 
> 
> 
>

Patch

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)