[PATCH] mips: pass code of conditional trap

YunQiang Su posted 1 patch 1 year, 7 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20240620234633.74447-1-syq@debian.org
Maintainers: "Philippe Mathieu-Daudé" <philmd@linaro.org>, Aurelien Jarno <aurelien@aurel32.net>, Jiaxun Yang <jiaxun.yang@flygoat.com>, Aleksandar Rikalo <arikalo@gmail.com>
target/mips/tcg/translate.c | 8 +++++++-
target/mips/tcg/translate.h | 1 +
2 files changed, 8 insertions(+), 1 deletion(-)
[PATCH] mips: pass code of conditional trap
Posted by YunQiang Su 1 year, 7 months ago
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)
Re: [PATCH] mips: pass code of conditional trap
Posted by Richard Henderson 1 year, 7 months ago
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~
Re: [PATCH] mips: pass code of conditional trap
Posted by YunQiang Su 1 year, 7 months ago
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~
Re: [PATCH] mips: pass code of conditional trap
Posted by Maciej W. Rozycki 1 year, 7 months ago
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
Re: [PATCH] mips: pass code of conditional trap
Posted by YunQiang Su 1 year, 7 months ago
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
Re: [PATCH] mips: pass code of conditional trap
Posted by Maciej W. Rozycki 1 year, 7 months ago
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
Re: [PATCH] mips: pass code of conditional trap
Posted by YunQiang Su 4 months, 2 weeks ago
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
Re: [PATCH] mips: pass code of conditional trap
Posted by Maciej W. Rozycki 4 months, 1 week ago
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