qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] mips: pass code of conditional trap
@ 2024-06-20 23:46 YunQiang Su
  2024-06-21  0:41 ` Maciej W. Rozycki
  2024-06-21  4:21 ` Richard Henderson
  0 siblings, 2 replies; 8+ messages in thread
From: YunQiang Su @ 2024-06-20 23:46 UTC (permalink / raw)
  To: qemu-devel; +Cc: macro, philmd, YunQiang Su

Linux and We use the code of conditional trap instructions to emit
signals other than simple SIGTRAP.  Currently, code 6 (overflow),
7 (div by zero) are supported. It means that if code 7 is used with
a conditional trap instruction, a SIGFPE instead of SIGTRAP will emit.

But when `gen_trap` we didn't pass the code as we use `generate_exception`,
which has no info about the code.  Let's introduce a new function
`generate_exception_code` for it.
---
 target/mips/tcg/translate.c | 8 +++++++-
 target/mips/tcg/translate.h | 1 +
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/target/mips/tcg/translate.c b/target/mips/tcg/translate.c
index 333469b268..e680a1c2f2 100644
--- a/target/mips/tcg/translate.c
+++ b/target/mips/tcg/translate.c
@@ -1353,6 +1353,12 @@ void generate_exception(DisasContext *ctx, int excp)
     gen_helper_raise_exception(tcg_env, tcg_constant_i32(excp));
 }
 
+void generate_exception_with_code(DisasContext *ctx, int excp, int code)
+{
+    gen_helper_raise_exception_err(tcg_env, tcg_constant_i32(excp),
+                                   tcg_constant_i32(code));
+}
+
 void generate_exception_end(DisasContext *ctx, int excp)
 {
     generate_exception_err(ctx, excp, 0);
@@ -4553,7 +4559,7 @@ static void gen_trap(DisasContext *ctx, uint32_t opc,
         if (ctx->hflags != ctx->saved_hflags) {
             tcg_gen_movi_i32(hflags, ctx->hflags);
         }
-        generate_exception(ctx, EXCP_TRAP);
+        generate_exception_with_code(ctx, EXCP_TRAP, code);
         gen_set_label(l1);
     }
 }
diff --git a/target/mips/tcg/translate.h b/target/mips/tcg/translate.h
index 2b6646b339..e3d544b478 100644
--- a/target/mips/tcg/translate.h
+++ b/target/mips/tcg/translate.h
@@ -134,6 +134,7 @@ enum {
     } while (0)
 
 void generate_exception(DisasContext *ctx, int excp);
+void generate_exception_with_code(DisasContext *ctx, int excp, int code);
 void generate_exception_err(DisasContext *ctx, int excp, int err);
 void generate_exception_end(DisasContext *ctx, int excp);
 void generate_exception_break(DisasContext *ctx, int code);
-- 
2.39.3 (Apple Git-146)



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] mips: pass code of conditional trap
  2024-06-20 23:46 [PATCH] mips: pass code of conditional trap YunQiang Su
@ 2024-06-21  0:41 ` Maciej W. Rozycki
  2024-06-21  0:51   ` YunQiang Su
  2024-06-21  4:21 ` Richard Henderson
  1 sibling, 1 reply; 8+ messages in thread
From: Maciej W. Rozycki @ 2024-06-21  0:41 UTC (permalink / raw)
  To: YunQiang Su; +Cc: qemu-devel, philmd

On Fri, 21 Jun 2024, YunQiang Su wrote:

> Linux and We use the code of conditional trap instructions to emit
> signals other than simple SIGTRAP.  Currently, code 6 (overflow),
> 7 (div by zero) are supported. It means that if code 7 is used with
> a conditional trap instruction, a SIGFPE instead of SIGTRAP will emit.
> 
> But when `gen_trap` we didn't pass the code as we use `generate_exception`,
> which has no info about the code.  Let's introduce a new function
> `generate_exception_code` for it.

 I haven't touched this stuff for ages, but AFAICT the code is already 
passed where applicable via the environment for `do_tr_or_bp' to handle, 
so I can't understand why your change is needed.

 What problem are you trying to solve?

  Maciej


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] mips: pass code of conditional trap
  2024-06-21  0:41 ` Maciej W. Rozycki
@ 2024-06-21  0:51   ` YunQiang Su
  2024-06-21 12:02     ` Maciej W. Rozycki
  0 siblings, 1 reply; 8+ messages in thread
From: YunQiang Su @ 2024-06-21  0:51 UTC (permalink / raw)
  To: Maciej W. Rozycki; +Cc: qemu-devel, philmd

Maciej W. Rozycki <macro@orcam.me.uk> 于2024年6月21日周五 08:41写道:
>
> On Fri, 21 Jun 2024, YunQiang Su wrote:
>
> > Linux and We use the code of conditional trap instructions to emit
> > signals other than simple SIGTRAP.  Currently, code 6 (overflow),
> > 7 (div by zero) are supported. It means that if code 7 is used with
> > a conditional trap instruction, a SIGFPE instead of SIGTRAP will emit.
> >
> > But when `gen_trap` we didn't pass the code as we use `generate_exception`,
> > which has no info about the code.  Let's introduce a new function
> > `generate_exception_code` for it.
>
>  I haven't touched this stuff for ages, but AFAICT the code is already
> passed where applicable via the environment for `do_tr_or_bp' to handle,
> so I can't understand why your change is needed.
>

The error_code in env is always zero, as we need to set it here.

>  What problem are you trying to solve?
>

See the talk in GCC mailing list about testsuite/ubsan/overflow-div-3.c
Qemu emits SIGTRAP instead of SIGFPE, due to it didn't initialize the
code of conditional trap to env.

>   Maciej


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] mips: pass code of conditional trap
  2024-06-20 23:46 [PATCH] mips: pass code of conditional trap YunQiang Su
  2024-06-21  0:41 ` Maciej W. Rozycki
@ 2024-06-21  4:21 ` Richard Henderson
  2024-06-21  4:37   ` YunQiang Su
  1 sibling, 1 reply; 8+ messages in thread
From: Richard Henderson @ 2024-06-21  4:21 UTC (permalink / raw)
  To: YunQiang Su, qemu-devel; +Cc: macro, philmd

On 6/20/24 16:46, YunQiang Su wrote:
> @@ -4553,7 +4559,7 @@ static void gen_trap(DisasContext *ctx, uint32_t opc,
>           if (ctx->hflags != ctx->saved_hflags) {
>               tcg_gen_movi_i32(hflags, ctx->hflags);
>           }
> -        generate_exception(ctx, EXCP_TRAP);
> +        generate_exception_with_code(ctx, EXCP_TRAP, code);
>           gen_set_label(l1);
>       }
>   }

There are two instances within gen_trap, one of which *does* store into error_code, but 
that gets overwritten by do_raise_exception_err.

Search for EXCP_TRAP.


r~


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] mips: pass code of conditional trap
  2024-06-21  4:21 ` Richard Henderson
@ 2024-06-21  4:37   ` YunQiang Su
  0 siblings, 0 replies; 8+ messages in thread
From: YunQiang Su @ 2024-06-21  4:37 UTC (permalink / raw)
  To: Richard Henderson; +Cc: qemu-devel, macro, philmd

Richard Henderson <richard.henderson@linaro.org> 于2024年6月21日周五 12:21写道:
>
> On 6/20/24 16:46, YunQiang Su wrote:
> > @@ -4553,7 +4559,7 @@ static void gen_trap(DisasContext *ctx, uint32_t opc,
> >           if (ctx->hflags != ctx->saved_hflags) {
> >               tcg_gen_movi_i32(hflags, ctx->hflags);
> >           }
> > -        generate_exception(ctx, EXCP_TRAP);
> > +        generate_exception_with_code(ctx, EXCP_TRAP, code);
> >           gen_set_label(l1);
> >       }
> >   }
>
> There are two instances within gen_trap, one of which *does* store into error_code, but
> that gets overwritten by do_raise_exception_err.
>

Ohh, yes. There is another `generate_exception_end` if cond == 0.

> Search for EXCP_TRAP.
>
>
> r~


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] mips: pass code of conditional trap
  2024-06-21  0:51   ` YunQiang Su
@ 2024-06-21 12:02     ` Maciej W. Rozycki
  2025-09-27  5:33       ` YunQiang Su
  0 siblings, 1 reply; 8+ messages in thread
From: Maciej W. Rozycki @ 2024-06-21 12:02 UTC (permalink / raw)
  To: YunQiang Su; +Cc: qemu-devel, philmd

On Fri, 21 Jun 2024, YunQiang Su wrote:

> >  I haven't touched this stuff for ages, but AFAICT the code is already
> > passed where applicable via the environment for `do_tr_or_bp' to handle,
> > so I can't understand why your change is needed.
> >
> 
> The error_code in env is always zero, as we need to set it here.

 There is code to set it there already, so when submitting a fix you need 
to investigate why it doesn't work and describe in the change description.

> >  What problem are you trying to solve?
> >
> 
> See the talk in GCC mailing list about testsuite/ubsan/overflow-div-3.c
> Qemu emits SIGTRAP instead of SIGFPE, due to it didn't initialize the
> code of conditional trap to env.

 Self-contained information about the reproducer needs to be included in 
the change description.  A general statement such as "this and that does 
not work" or referring to another mailing list is not sufficient.

  Maciej


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] mips: pass code of conditional trap
  2024-06-21 12:02     ` Maciej W. Rozycki
@ 2025-09-27  5:33       ` YunQiang Su
  2025-10-01 11:18         ` Maciej W. Rozycki
  0 siblings, 1 reply; 8+ messages in thread
From: YunQiang Su @ 2025-09-27  5:33 UTC (permalink / raw)
  To: Maciej W. Rozycki; +Cc: qemu-devel, philmd

Maciej W. Rozycki <macro@orcam.me.uk> 于2024年6月21日周五 20:02写道:
>
> On Fri, 21 Jun 2024, YunQiang Su wrote:
>
> > >  I haven't touched this stuff for ages, but AFAICT the code is already
> > > passed where applicable via the environment for `do_tr_or_bp' to handle,
> > > so I can't understand why your change is needed.
> > >
> >
> > The error_code in env is always zero, as we need to set it here.
>
>  There is code to set it there already, so when submitting a fix you need
> to investigate why it doesn't work and describe in the change description.
>
> > >  What problem are you trying to solve?
> > >
> >
> > See the talk in GCC mailing list about testsuite/ubsan/overflow-div-3.c
> > Qemu emits SIGTRAP instead of SIGFPE, due to it didn't initialize the
> > code of conditional trap to env.
>
>  Self-contained information about the reproducer needs to be included in
> the change description.  A general statement such as "this and that does
> not work" or referring to another mailing list is not sufficient.
>

I am trying to fix the problem like this
gcc/testsuite/gcc.c-torture/execute/20101011-1.c

void
sigfpe (int signum __attribute__ ((unused)))
{
  exit (0);
}

int
main ()
{
#if DO_TEST
  signal (SIGFPE, sigfpe);
  k = i / j;
  abort ();
#else
  exit (0);
#endif
}



>   Maciej


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] mips: pass code of conditional trap
  2025-09-27  5:33       ` YunQiang Su
@ 2025-10-01 11:18         ` Maciej W. Rozycki
  0 siblings, 0 replies; 8+ messages in thread
From: Maciej W. Rozycki @ 2025-10-01 11:18 UTC (permalink / raw)
  To: YunQiang Su; +Cc: qemu-devel, philmd

On Sat, 27 Sep 2025, YunQiang Su wrote:

> > > >  What problem are you trying to solve?
> > >
> > > See the talk in GCC mailing list about testsuite/ubsan/overflow-div-3.c
> > > Qemu emits SIGTRAP instead of SIGFPE, due to it didn't initialize the
> > > code of conditional trap to env.
> >
> >  Self-contained information about the reproducer needs to be included in
> > the change description.  A general statement such as "this and that does
> > not work" or referring to another mailing list is not sufficient.
> 
> I am trying to fix the problem like this
> gcc/testsuite/gcc.c-torture/execute/20101011-1.c
> 
> void
> sigfpe (int signum __attribute__ ((unused)))
> {
>   exit (0);
> }
> 
> int
> main ()
> {
> #if DO_TEST
>   signal (SIGFPE, sigfpe);
>   k = i / j;
>   abort ();

 I gather QEMU in the user emulation mode fails to interpret the embedded 
break or trap code of a `teq $2,$0,7' or similar instruction produced by 
the compiler as a part of the integer division machine code sequence for 
the source code quoted above, and consequently issues the wrong signal to 
the program emulated.  Is that a correct statement of the problem?

 If so, then it has to be stated in the change description.  Then Richard 
has correctly identified the fix to make.

  Maciej


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2025-10-01 11:19 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-06-20 23:46 [PATCH] mips: pass code of conditional trap YunQiang Su
2024-06-21  0:41 ` Maciej W. Rozycki
2024-06-21  0:51   ` YunQiang Su
2024-06-21 12:02     ` Maciej W. Rozycki
2025-09-27  5:33       ` YunQiang Su
2025-10-01 11:18         ` Maciej W. Rozycki
2024-06-21  4:21 ` Richard Henderson
2024-06-21  4:37   ` YunQiang Su

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).