diff mbox series

mips: pass code of conditional trap

Message ID 20240620234633.74447-1-syq@debian.org (mailing list archive)
State New, archived
Headers show
Series mips: pass code of conditional trap | expand

Commit Message

YunQiang Su June 20, 2024, 11:46 p.m. UTC
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(-)

Comments

Maciej W. Rozycki June 21, 2024, 12:41 a.m. UTC | #1
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
YunQiang Su June 21, 2024, 12:51 a.m. UTC | #2
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
Richard Henderson June 21, 2024, 4:21 a.m. UTC | #3
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~
YunQiang Su June 21, 2024, 4:37 a.m. UTC | #4
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~
Maciej W. Rozycki June 21, 2024, 12:02 p.m. UTC | #5
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
diff mbox series

Patch

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);