Re: [PATCH v4] tcg: Sanitize shift constants on ppc64le so that shift operations with large constants don't generate invalid instructions.

[Richard Henderson <richard.henderson@linaro.org>]

On 6/3/20 6:50 PM, agrecascino123@gmail.com wrote:
>  static inline void tcg_out_shri64(TCGContext *s, TCGReg dst, TCGReg src, int c)
>  {
> +    tcg_debug_assert((c < 64) && (c >= 0));
>      tcg_out_rld(s, RLDICL, dst, src, 64 - c, c);
>  }
>  
> @@ -2610,21 +2614,33 @@ static void tcg_out_op(TCGContext *s, TCGOpcode opc, const TCGArg *args,
>  
>      case INDEX_op_shl_i32:
>          if (const_args[2]) {
> -            tcg_out_shli32(s, args[0], args[1], args[2]);
> +            /*
> +             * Limit shift immediate to prevent illegal instruction
> +             * from bitmask corruption
> +             */
> +            tcg_out_shli32(s, args[0], args[1], args[2] & 31);

Why are you duplicating these?

I suggested masking, and now you are, but you're also retaining the assert.
What's the point?  Just mask, IMO.

>      case INDEX_op_sar_i64:
>          if (const_args[2]) {
> -            int sh = SH(args[2] & 0x1f) | (((args[2] >> 5) & 1) << 1);
> +            /*
> +             * Same for SRADI, except there's no function
> +             * to call into.
> +             */
> +            int sh = SH(((args[2] & 63) & 0x1f)
> +                        | ((((args[2] & 63) >> 5) & 1) << 1));

As for this, there's zero point.  We are already masking.  You can see it right
there.


r~