Re: [Qemu-devel] [PATCH v2 1/3] target-mips: Copy restrictions from ext/ins to dext/dins

[Richard Henderson <rth@twiddle.net>]

On 08/03/2015 02:31 PM, Aurelien Jarno wrote:
> On 2015-08-03 12:35, Richard Henderson wrote:
>>           if (msb != 31) {
>> -            tcg_gen_andi_tl(t0, t0, (1 << (msb + 1)) - 1);
>> +            tcg_gen_andi_tl(t0, t0, (1U << (msb + 1)) - 1);
>
> Is this change really needed?

msb == 30 means 1 << 31.  Which officially must be unsigned to be correct.  If 
we were to run under ubsan, this would trigger an error.

> Note that DEXT can't fail as both lsb and msb are in the range 0..31.
> DEXTU and DEXTM can.
...
> The same way DINSM can't fail.

Yes, I know.  But it seems cleaner to do the checks always, unifying all of the 
code.

> Should we try to get this one into 2.4, if not already too late?

Perhaps.  Otherwise via stable after the fact.


r~