On Thu, Feb 17, 2022 at 4:48 AM Richard Henderson
wrote:
>
> The dh_alias redirect is intended to handle TCG types as distinguished
> from C types. TCG does not distinguish signed int from unsigned int,
> because they are the same size. However, we need to retain this
> distinction for dh_typecode, lest we fail to extend abi types properly
> for the host call parameters.
Thank you Richard and Keith for the fix for
- https://github.com/keith-packard/snek/issues/58
- https://gitlab.com/qemu-project/qemu/-/issues/876
I did apply that and tested it on s390x with the load that originally found it.
With qemu 6.2 + that patch I ran the full testsuite of snek-arm on
s390x and it works great now.
If you like feel free to add any combination of the following when committing:
Fixes: #876
Fixes: 7319d83a735 ("tcg: Combine dh_is_64bit and dh_is_signed to dh_typecode")
Reported-by: Christian Ehrhardt
Tested-by: Christian Ehrhardt
> This bug was detected when running the 'arm' emulator on an s390
> system. The s390 uses TCG_TARGET_EXTEND_ARGS which triggers code
> in tcg_gen_callN to extend 32 bit values to 64 bits; the incorrect
> sign data in the typemask for each argument caused the values to be
> extended as unsigned values.
>
> This simple program exhibits the problem:
>
> static volatile int num = -9;
> static volatile int den = -5;
>
> int
> main(void)
> {
> int quo = num / den;
> printf("num %d den %d quo %d\n", num, den, quo);
> exit(0);
> }
>
> When run on the broken qemu, this results in:
>
> num -9 den -5 quo 0
>
> The correct result is:
>
> num -9 den -5 quo 1
>
> Reported-by: Keith Packard
> Signed-off-by: Richard Henderson
> ---
> include/exec/helper-head.h | 19 ++-
> target/hppa/helper.h | 2 ++
> target/i386/ops_sse_header.h | 3 +++
> target/m68k/helper.h | 1 +
> target/ppc/helper.h | 3 +++
> 5 files changed, 19 insertions(+), 9 deletions(-)
>
> diff --git a/include/exec/helper-head.h b/include/exec/helper-head.h
> index b974eb394a..734af067fe 100644
> --- a/include/exec/helper-head.h
> +++ b/include/exec/helper-head.h
> @@ -53,13 +53,16 @@
> # ifdef TARGET_LONG_BITS
> # if TARGET_LONG_BITS == 32
> # define dh_alias_tl i32
> +# define dh_typecode_tl dh_typecode_i32
> # else
> # define dh_alias_tl i64
> +# define dh_typecode_tl dh_typecode_i64
> # endif
> # endif
> -# define dh_alias_env ptr
> # define dh_ctype_tl target_ulong
> +# define dh_alias_env ptr
> # define dh_ctype_env CPUArchState *
> +# define dh_typecode_env dh_typecode_ptr
> #endif
>
> /* We can't use glue() here because it falls foul of C preprocessor
> @@ -92,18 +95,16 @@
> #define dh_typecode_i64 4
> #define dh_typecode_s64 5
> #define dh_typecode_ptr 6
> -#define dh_typecode(t) glue(dh_typecode_, dh_alias(t))
> +#define dh_typecode_int dh_typecode_s32
> +#define dh_typecode_f16 dh_typecode_i32
> +#define dh_typecode_f32 dh_typecode_i32
> +#define dh_typecode_f64 dh_typecode_i64
> +#define dh_typecode_cptr dh_typecode_ptr
> +#define dh_typecode(t) dh_typecode_##t
>
> #define dh_callflag_i32 0
> -#define dh_callflag_s32 0
> -#define dh_callflag_int 0
> #define dh_callflag_i64 0
> -#define dh_callflag_s64 0
> -#define dh_callflag_f16 0
> -#define dh_callflag_f32 0
> -#define dh_callflag_f64 0
> #define dh_callflag_ptr 0
> -#define dh_callflag_cptr dh_callflag_ptr
> #define dh_callflag_void 0
> #define dh_callflag_noreturn TCG_CALL_NO_RETURN
> #define dh_callflag(t) glue(dh_callflag_, dh_alias(t))
> diff --git a/target/hppa/helper.h b/target/hppa/helper.h
> index fe8a9ce493..c7e35ce8c7 100644
> --- a/target/hppa/helper.h
> +++ b/target/hppa/helper.h
> @@ -1,7 +1,9 @@
> #if TARGET_REGISTER_BITS == 64
> # define dh_alias_tr i64
> +# define dh_typecode_tr dh_typecode_i64
> #else
> # define dh_alias_tr i32
> +# define dh_typecode_tr dh_typecode_i32
> #endif
> #define dh_ctype_tr target_ureg
>
> diff --git a/target/i386/ops_sse_header.h b/target/i386/ops_sse_header.h
> index e68af5c403..cef28f2aae 100644
> --- a/target/i386/ops_sse_header.h
> +++ b/target/i386/ops_sse_header.h
> @@ -30,6 +30,9 @@
> #define dh_ctype_Reg Reg *
> #define dh_ctype_ZMMReg ZMMReg *
> #define dh_ctype_MMXReg MMXReg *
> +#define dh_typecode_Reg dh_typecode_ptr
> +#define dh_typecode_ZMMReg dh_typecode_ptr
> +#define dh_typecode_MMXReg dh_typecode_ptr
>
> DEF_HELPER_3(glue(psrlw, SUFFIX), void, env, Reg, Reg)
> DEF_HELPER_3(glue(psraw, SUFFIX), void, env, Reg, Reg)
> diff --git a/target/m68k/helper.h b/target/m68k/helper.h
> index 9842eeaa95..0a6b4146f6 100644
> --- a/target/m68k/helper.h
> +++ b/target/m68k/helper.h
> @@ -17,6 +17,7 @@ DEF_HELPER_4(cas2l_parallel, void, env, i32, i32, i32)
>
> #define dh_alias_fp ptr
> #define dh_ctype_fp FPReg *
> +#define dh_typecode_fp dh_typecode_ptr
>
> DEF_HELPER_3(exts32, void, env, fp,
