Re: [Qemu-devel] [PATCH v2 1/1] target/arm: Fix vector operation segfault

2019-05-15 Thread Philippe Mathieu-Daudé
On 5/15/19 2:21 AM, Alistair Francis wrote:
> Commit 89e68b575 "target/arm: Use vector operations for saturation"
> causes this abort() when booting QEMU ARM with a Cortex-A15:
> 
> 0  0x74c2382f in raise () at /usr/lib/libc.so.6
> 1  0x74c0e672 in abort () at /usr/lib/libc.so.6
> 2  0x559c1839 in disas_neon_data_insn (insn=, 
> s=) at ./target/arm/translate.c:6673
> 3  0x559c1839 in disas_neon_data_insn (s=, 
> insn=) at ./target/arm/translate.c:6386
> 4  0x559cd8a4 in disas_arm_insn (insn=4081107068, s=0x7fffe59a9510) 
> at ./target/arm/translate.c:9289
> 5  0x559cd8a4 in arm_tr_translate_insn (dcbase=0x7fffe59a9510, 
> cpu=) at ./target/arm/translate.c:13612
> 6  0x558d1d39 in translator_loop (ops=0x561cc580 
> , db=0x7fffe59a9510, cpu=0x5686a2f0, tb= out>, max_insns=) at ./accel/tcg/translator.c:96
> 7  0x559d10d4 in gen_intermediate_code (cpu=cpu@entry=0x5686a2f0, 
> tb=tb@entry=0x7fffd7840080 , 
> max_insns=max_insns@entry=512) at ./target/arm/translate.c:13901
> 8  0x558d06b9 in tb_gen_code (cpu=cpu@entry=0x5686a2f0, 
> pc=3067096216, cs_base=0, flags=192, cflags=-16252928, cflags@entry=524288) 
> at ./accel/tcg/translate-all.c:1736
> 9  0x558ce467 in tb_find (cf_mask=524288, tb_exit=1, 
> last_tb=0x7fffd783e640 , cpu=0x1) at 
> ./accel/tcg/cpu-exec.c:407
> 10 0x558ce467 in cpu_exec (cpu=cpu@entry=0x5686a2f0) at 
> ./accel/tcg/cpu-exec.c:728
> 11 0x5588b0cf in tcg_cpu_exec (cpu=0x5686a2f0) at ./cpus.c:1431
> 12 0x5588d223 in qemu_tcg_cpu_thread_fn (arg=0x5686a2f0) at 
> ./cpus.c:1735
> 13 0x5588d223 in qemu_tcg_cpu_thread_fn 
> (arg=arg@entry=0x5686a2f0) at ./cpus.c:1709
> 14 0x55d2629a in qemu_thread_start (args=) at 
> ./util/qemu-thread-posix.c:502
> 15 0x74db8a92 in start_thread () at /usr/lib/libpthread.
> 
> This patch ensures that we don't hit the abort() in the second switch
> case in disas_neon_data_insn() as we will return from the first case.
> 
> Signed-off-by: Alistair Francis 
> ---
>  target/arm/translate.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/target/arm/translate.c b/target/arm/translate.c
> index dd053c80d6..298c262825 100644
> --- a/target/arm/translate.c
> +++ b/target/arm/translate.c
> @@ -6598,13 +6598,13 @@ static int disas_neon_data_insn(DisasContext *s, 
> uint32_t insn)
>  tcg_gen_gvec_4(rd_ofs, offsetof(CPUARMState, vfp.qc),
> rn_ofs, rm_ofs, vec_size, vec_size,
> (u ? uqadd_op : sqadd_op) + size);
> -break;
> +return 0;
>  
>  case NEON_3R_VQSUB:
>  tcg_gen_gvec_4(rd_ofs, offsetof(CPUARMState, vfp.qc),
> rn_ofs, rm_ofs, vec_size, vec_size,
> (u ? uqsub_op : sqsub_op) + size);
> -break;
> +return 0;
>  
>  case NEON_3R_VMUL: /* VMUL */
>  if (u) {
> 

Reviewed-by: Philippe Mathieu-Daudé 



Re: [Qemu-devel] [PATCH v2 1/1] target/arm: Fix vector operation segfault

2019-05-14 Thread Richard Henderson
On 5/14/19 5:21 PM, Alistair Francis wrote:
> diff --git a/target/arm/translate.c b/target/arm/translate.c
> index dd053c80d6..298c262825 100644
> --- a/target/arm/translate.c
> +++ b/target/arm/translate.c
> @@ -6598,13 +6598,13 @@ static int disas_neon_data_insn(DisasContext *s, 
> uint32_t insn)
>  tcg_gen_gvec_4(rd_ofs, offsetof(CPUARMState, vfp.qc),
> rn_ofs, rm_ofs, vec_size, vec_size,
> (u ? uqadd_op : sqadd_op) + size);
> -break;
> +return 0;
>  
>  case NEON_3R_VQSUB:
>  tcg_gen_gvec_4(rd_ofs, offsetof(CPUARMState, vfp.qc),
> rn_ofs, rm_ofs, vec_size, vec_size,
> (u ? uqsub_op : sqsub_op) + size);
> -break;
> +return 0;
>  
>  case NEON_3R_VMUL: /* VMUL */
>  if (u) {

Reviewed-by: Richard Henderson 


r~



[Qemu-devel] [PATCH v2 1/1] target/arm: Fix vector operation segfault

2019-05-14 Thread Alistair Francis
Commit 89e68b575 "target/arm: Use vector operations for saturation"
causes this abort() when booting QEMU ARM with a Cortex-A15:

0  0x74c2382f in raise () at /usr/lib/libc.so.6
1  0x74c0e672 in abort () at /usr/lib/libc.so.6
2  0x559c1839 in disas_neon_data_insn (insn=, 
s=) at ./target/arm/translate.c:6673
3  0x559c1839 in disas_neon_data_insn (s=, 
insn=) at ./target/arm/translate.c:6386
4  0x559cd8a4 in disas_arm_insn (insn=4081107068, s=0x7fffe59a9510) at 
./target/arm/translate.c:9289
5  0x559cd8a4 in arm_tr_translate_insn (dcbase=0x7fffe59a9510, 
cpu=) at ./target/arm/translate.c:13612
6  0x558d1d39 in translator_loop (ops=0x561cc580 
, db=0x7fffe59a9510, cpu=0x5686a2f0, tb=, max_insns=) at ./accel/tcg/translator.c:96
7  0x559d10d4 in gen_intermediate_code (cpu=cpu@entry=0x5686a2f0, 
tb=tb@entry=0x7fffd7840080 , 
max_insns=max_insns@entry=512) at ./target/arm/translate.c:13901
8  0x558d06b9 in tb_gen_code (cpu=cpu@entry=0x5686a2f0, 
pc=3067096216, cs_base=0, flags=192, cflags=-16252928, cflags@entry=524288) at 
./accel/tcg/translate-all.c:1736
9  0x558ce467 in tb_find (cf_mask=524288, tb_exit=1, 
last_tb=0x7fffd783e640 , cpu=0x1) at 
./accel/tcg/cpu-exec.c:407
10 0x558ce467 in cpu_exec (cpu=cpu@entry=0x5686a2f0) at 
./accel/tcg/cpu-exec.c:728
11 0x5588b0cf in tcg_cpu_exec (cpu=0x5686a2f0) at ./cpus.c:1431
12 0x5588d223 in qemu_tcg_cpu_thread_fn (arg=0x5686a2f0) at 
./cpus.c:1735
13 0x5588d223 in qemu_tcg_cpu_thread_fn (arg=arg@entry=0x5686a2f0) 
at ./cpus.c:1709
14 0x55d2629a in qemu_thread_start (args=) at 
./util/qemu-thread-posix.c:502
15 0x74db8a92 in start_thread () at /usr/lib/libpthread.

This patch ensures that we don't hit the abort() in the second switch
case in disas_neon_data_insn() as we will return from the first case.

Signed-off-by: Alistair Francis 
---
 target/arm/translate.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/arm/translate.c b/target/arm/translate.c
index dd053c80d6..298c262825 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -6598,13 +6598,13 @@ static int disas_neon_data_insn(DisasContext *s, 
uint32_t insn)
 tcg_gen_gvec_4(rd_ofs, offsetof(CPUARMState, vfp.qc),
rn_ofs, rm_ofs, vec_size, vec_size,
(u ? uqadd_op : sqadd_op) + size);
-break;
+return 0;
 
 case NEON_3R_VQSUB:
 tcg_gen_gvec_4(rd_ofs, offsetof(CPUARMState, vfp.qc),
rn_ofs, rm_ofs, vec_size, vec_size,
(u ? uqsub_op : sqsub_op) + size);
-break;
+return 0;
 
 case NEON_3R_VMUL: /* VMUL */
 if (u) {
-- 
2.21.0