[Qemu-devel] defining VIXL_DEBUG?

[Qemu-devel] defining VIXL_DEBUG?

[Paolo Bonzini]

I've gotten four new Coverity reports in libvixl.  All of them are
caused by Coverity assuming that LaneSizeInBitsFromFormat can return 0.
 The actual code in the function is

    default: VIXL_UNREACHABLE(); return 0;

so this is obviously a false positive.  Defining VIXL_DEBUG would cause
VIXL_UNREACHABLE() to call abort().  Any opinion about whether/where to
do so?

Thanks,

Paolo

Re: [Qemu-devel] defining VIXL_DEBUG?

[Peter Maydell]

On 18 January 2016 at 10:21, Paolo Bonzini <pbonzini@redhat.com> wrote:
> I've gotten four new Coverity reports in libvixl.  All of them are
> caused by Coverity assuming that LaneSizeInBitsFromFormat can return 0.

Yeah, I fed these back to upstream.

(There are also some others do do with integer overflow etc which
I think are unrelated to LaneSizeInBitsFromFormat.)

>  The actual code in the function is
>
>     default: VIXL_UNREACHABLE(); return 0;
>
> so this is obviously a false positive.  Defining VIXL_DEBUG would cause
> VIXL_UNREACHABLE() to call abort().  Any opinion about whether/where to
> do so?

Does defining it to call abort() result in unreachable-code warnings
for the "return 0;" ?

thanks
-- PMM

Re: [Qemu-devel] defining VIXL_DEBUG?

[Paolo Bonzini]

On 18/01/2016 11:51, Peter Maydell wrote:
>>  The actual code in the function is
>> >
>> >     default: VIXL_UNREACHABLE(); return 0;
>> >
>> > so this is obviously a false positive.  Defining VIXL_DEBUG would cause
>> > VIXL_UNREACHABLE() to call abort().  Any opinion about whether/where to
>> > do so?
> Does defining it to call abort() result in unreachable-code warnings
> for the "return 0;" ?

I'm not sure, it would be minor though.

One issue I have found after posting is that I'm not sure whether bad
instructions (aka reserved encodings) are handled properly by libvixl.
See for example this:

    case 'A': {  // IAddSub.
      VIXL_ASSERT(instr->ShiftAddSub() <= 1);
      int64_t imm = instr->ImmAddSub() << (12 * instr->ShiftAddSub());
      AppendToOutput("#0x%" PRIx64 " (%" PRId64 ")", imm, imm);
      return 7;
    }

where the '1x' encodings of bits 22:23 (marked as reserved in the ARMv8
ARM) would cause an abort as far as I can see.

Paolo

Re: [Qemu-devel] defining VIXL_DEBUG?

[Peter Maydell]

On 18 January 2016 at 10:57, Paolo Bonzini <pbonzini@redhat.com> wrote:
> One issue I have found after posting is that I'm not sure whether bad
> instructions (aka reserved encodings) are handled properly by libvixl.
> See for example this:
>
>     case 'A': {  // IAddSub.
>       VIXL_ASSERT(instr->ShiftAddSub() <= 1);
>       int64_t imm = instr->ImmAddSub() << (12 * instr->ShiftAddSub());
>       AppendToOutput("#0x%" PRIx64 " (%" PRId64 ")", imm, imm);
>       return 7;
>     }
>
> where the '1x' encodings of bits 22:23 (marked as reserved in the ARMv8
> ARM) would cause an abort as far as I can see.

Isn't this handled by  Decoder::DecodeAddSubImmediate(), which checks
bit 23?

In any case if we're worried it would be easy to set up a trivial
test loop that just feeds all 2^32 integers to the disassembler.

thanks
-- PMM

Re: [Qemu-devel] defining VIXL_DEBUG?

[Paolo Bonzini]

On 18/01/2016 12:04, Peter Maydell wrote:
>> >
>> > where the '1x' encodings of bits 22:23 (marked as reserved in the ARMv8
>> > ARM) would cause an abort as far as I can see.
> Isn't this handled by  Decoder::DecodeAddSubImmediate(), which checks
> bit 23?

Yes, that's right.

Paolo

> In any case if we're worried it would be easy to set up a trivial
> test loop that just feeds all 2^32 integers to the disassembler.