* [PATCH] tcg,riscv: Fix illegal shift instructions
@ 2020-12-16 8:12 Zihao Yu
2020-12-16 16:10 ` Richard Henderson
0 siblings, 1 reply; 2+ messages in thread
From: Zihao Yu @ 2020-12-16 8:12 UTC (permalink / raw)
To: qemu-riscv
Cc: Alistair Francis, Richard Henderson, Palmer Dabbelt,
open list:All patches CC here
* This bug can be reproduced by running the following guest instructions
on a RISC-V host.
(1) xor %ecx,%ecx
(2) sar %cl,%eax
(3) cmovne %edi,%eax
After optimization, the tcg instructions of (2) are
movi_i32 tmp3,$0xffffffffffffffff pref=all
sar_i32 tmp3,eax,tmp3 dead: 2 pref=all
mov_i32 cc_dst,eax sync: 0 dead: 1 pref=0xffc0300
mov_i32 cc_src,tmp3 sync: 0 dead: 0 1 pref=all
movi_i32 cc_op,$0x31 sync: 0 dead: 0 pref=all
And the target assembly instructions of (2) are
0x200808d618: fffa5b9b illegal
0x200808d61c: 03442423 sw s4,40(s0)
0x200808d620: 03742623 sw s7,44(s0)
0x200808d624: 03100b93 addi s7,zero,49
0x200808d628: 03742a23 sw s7,52(s0)
* Note that the `illegal` target instruction above should be
`sraiw s7,s4,0x1f` (41fa5b9b). The bug exists because the `imm` of the
shift instruction is too large, and therefore the `funct7` field is
overwritten. To fix this issue, `imm` should be masked for shift
instructions.
Signed-off-by: Zihao Yu <yuzihao@ict.ac.cn>
---
tcg/riscv/tcg-target.c.inc | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/tcg/riscv/tcg-target.c.inc b/tcg/riscv/tcg-target.c.inc
index d536f3ccc1..4089e29cd9 100644
--- a/tcg/riscv/tcg-target.c.inc
+++ b/tcg/riscv/tcg-target.c.inc
@@ -1462,14 +1462,14 @@ static void tcg_out_op(TCGContext *s, TCGOpcode opc,
case INDEX_op_shl_i32:
if (c2) {
- tcg_out_opc_imm(s, OPC_SLLIW, a0, a1, a2);
+ tcg_out_opc_imm(s, OPC_SLLIW, a0, a1, a2 & 0x1f);
} else {
tcg_out_opc_reg(s, OPC_SLLW, a0, a1, a2);
}
break;
case INDEX_op_shl_i64:
if (c2) {
- tcg_out_opc_imm(s, OPC_SLLI, a0, a1, a2);
+ tcg_out_opc_imm(s, OPC_SLLI, a0, a1, a2 & 0x3f);
} else {
tcg_out_opc_reg(s, OPC_SLL, a0, a1, a2);
}
@@ -1477,14 +1477,14 @@ static void tcg_out_op(TCGContext *s, TCGOpcode opc,
case INDEX_op_shr_i32:
if (c2) {
- tcg_out_opc_imm(s, OPC_SRLIW, a0, a1, a2);
+ tcg_out_opc_imm(s, OPC_SRLIW, a0, a1, a2 & 0x1f);
} else {
tcg_out_opc_reg(s, OPC_SRLW, a0, a1, a2);
}
break;
case INDEX_op_shr_i64:
if (c2) {
- tcg_out_opc_imm(s, OPC_SRLI, a0, a1, a2);
+ tcg_out_opc_imm(s, OPC_SRLI, a0, a1, a2 & 0x3f);
} else {
tcg_out_opc_reg(s, OPC_SRL, a0, a1, a2);
}
@@ -1492,14 +1492,14 @@ static void tcg_out_op(TCGContext *s, TCGOpcode opc,
case INDEX_op_sar_i32:
if (c2) {
- tcg_out_opc_imm(s, OPC_SRAIW, a0, a1, a2);
+ tcg_out_opc_imm(s, OPC_SRAIW, a0, a1, a2 & 0x1f);
} else {
tcg_out_opc_reg(s, OPC_SRAW, a0, a1, a2);
}
break;
case INDEX_op_sar_i64:
if (c2) {
- tcg_out_opc_imm(s, OPC_SRAI, a0, a1, a2);
+ tcg_out_opc_imm(s, OPC_SRAI, a0, a1, a2 & 0x3f);
} else {
tcg_out_opc_reg(s, OPC_SRA, a0, a1, a2);
}
--
2.20.1
^ permalink raw reply related [flat|nested] 2+ messages in thread* Re: [PATCH] tcg,riscv: Fix illegal shift instructions
2020-12-16 8:12 [PATCH] tcg,riscv: Fix illegal shift instructions Zihao Yu
@ 2020-12-16 16:10 ` Richard Henderson
0 siblings, 0 replies; 2+ messages in thread
From: Richard Henderson @ 2020-12-16 16:10 UTC (permalink / raw)
To: Zihao Yu, qemu-riscv
Cc: Alistair Francis, Palmer Dabbelt, open list:All patches CC here
On 12/16/20 2:12 AM, Zihao Yu wrote:
> * This bug can be reproduced by running the following guest instructions
> on a RISC-V host.
>
> (1) xor %ecx,%ecx
> (2) sar %cl,%eax
> (3) cmovne %edi,%eax
>
> After optimization, the tcg instructions of (2) are
>
> movi_i32 tmp3,$0xffffffffffffffff pref=all
> sar_i32 tmp3,eax,tmp3 dead: 2 pref=all
> mov_i32 cc_dst,eax sync: 0 dead: 1 pref=0xffc0300
> mov_i32 cc_src,tmp3 sync: 0 dead: 0 1 pref=all
> movi_i32 cc_op,$0x31 sync: 0 dead: 0 pref=all
>
> And the target assembly instructions of (2) are
>
> 0x200808d618: fffa5b9b illegal
> 0x200808d61c: 03442423 sw s4,40(s0)
> 0x200808d620: 03742623 sw s7,44(s0)
> 0x200808d624: 03100b93 addi s7,zero,49
> 0x200808d628: 03742a23 sw s7,52(s0)
>
> * Note that the `illegal` target instruction above should be
> `sraiw s7,s4,0x1f` (41fa5b9b).
More precisely, the 'illegal' instruction should be *anything*, except for
'illegal'. The result of the out-of-range shift is undefined, but TCG requires
that it not trap. The undefined result should be unused.
In this particular case this is so, because cc_op == 0x31 == CC_OP_CLR, which
does not use either of cc_dst or cc_src.
We have make the same patch for other hosts, e.g. 1fd959466574 for tcg/sparc/.
I have queued the patch to tcg-next.
r~
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2020-12-16 16:12 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-12-16 8:12 [PATCH] tcg,riscv: Fix illegal shift instructions Zihao Yu
2020-12-16 16:10 ` Richard Henderson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).