Re: [Qemu-devel] [PATCH for-2.9 4/6] disas/microblaze: Avoid unintended sign extension

[Peter Maydell]

On 3 March 2017 at 15:58, Edgar E. Iglesias <address@hidden> wrote:
> On Fri, Mar 03, 2017 at 03:50:31PM +0000, Peter Maydell wrote:
>> In read_insn_microblaze() we assemble 4 bytes into an 'unsigned
>> long'.  If 'unsigned long' is 64 bits and the high byte has its top
>> bit set, then C's implicit conversion from 'unsigned char' to 'int'
>> for the shift will result in an unintended sign extension which sets
>> the top 32 bits in 'inst'.  Add casts to prevent this.  (Spotted by
>> Coverity, CID 1005401.)
>
> I'm OK with this but perhaps it would have been more readable to
> change inst to uint32_t ? (All microblaze insns are 32bit).

I thought about that, but it was more invasive a change than
I was happy with: you'd want to change not just 'inst' but
also the return type of read_insn_microblaze, the opcode_mask
field of struct op_code_struct, 'inst' and 'prev_inst' in
print_insn_microblaze, the type of the instr arguments to
get_field, get_field_imm, etc...

thanks
-- PMM