* 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

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~