[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH for-2.9 4/6] disas/microblaze: Avoid unintended
From: |
Edgar E. Iglesias |
Subject: |
Re: [Qemu-devel] [PATCH for-2.9 4/6] disas/microblaze: Avoid unintended sign extension |
Date: |
Fri, 3 Mar 2017 17:06:01 +0100 |
User-agent: |
Mutt/1.5.24 (2015-08-30) |
On Fri, Mar 03, 2017 at 04:02:07PM +0000, Peter Maydell wrote:
> 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...
Ah, I see, didn't think about that...
Cheers,
Edgar
- [Qemu-devel] [PATCH for-2.9 0/6] disas: Fix various coverity nits, Peter Maydell, 2017/03/03
- [Qemu-devel] [PATCH for-2.9 2/6] disas/i386: Avoid NULL pointer dereference in error case, Peter Maydell, 2017/03/03
- [Qemu-devel] [PATCH for-2.9 1/6] disas/hppa: Remove dead code, Peter Maydell, 2017/03/03
- [Qemu-devel] [PATCH for-2.9 5/6] disas/cris: Avoid unintended sign extension, Peter Maydell, 2017/03/03
- Re: [Qemu-devel] [PATCH for-2.9 0/6] disas: Fix various coverity nits, no-reply, 2017/03/03
- Re: [Qemu-devel] [PATCH for-2.9 0/6] disas: Fix various coverity nits, no-reply, 2017/03/03