| Message ID | 20240620234633.74447-1-syq@debian.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mips: pass code of conditional trap | expand |
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
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
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~
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~
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 --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);