Re: [Qemu-devel] [PULL 09/26] target/arm: Don't clear supported PMU events when initializing PMCEID1

[Aaron Lindsay OS]

On Feb 14 17:55, Peter Maydell wrote:
> On Mon, 28 Jan 2019 at 18:11, Peter Maydell <address@hidden> wrote:
> >
> > From: Aaron Lindsay OS <address@hidden>
> >
> > A bug was introduced during a respin of:
> >
> >         commit 57a4a11b2b281bb548b419ca81bfafb214e4c77a
> >         target/arm: Add array for supported PMU events, generate 
> > PMCEID[01]_EL0
> 
> 
> 
> > @@ -1113,13 +1115,16 @@ uint64_t get_pmceid(CPUARMState *env, unsigned 
> > which)
> >          /* We do not currently support events in the 0x40xx range */
> >          assert(cnt->number <= 0x3f);
> >
> > -        if ((cnt->number & 0x20) == (which << 6) &&
> > -                cnt->supported(env)) {
> > -            pmceid |= (1 << (cnt->number & 0x1f));
> > +        if (cnt->supported(&cpu->env)) {
> >              supported_event_map[cnt->number] = i;
> > +            uint64_t event_mask = 1 << (cnt->number & 0x1f);
> 
> Coverity complains about this line (CID 1398645). The
> RHS is evaluated using 32-bit signed arithmetic (because
> cnt->number is uint16_t and so integer promotion means it
> ends up working with the 'int' type. If cnt->number is
> 31 then this means that the assignment will do an
> unintended sign-extension that sets the top 32 bits
> of event_mask.
> 
> Fix is probably just to use "1ULL" instead of "1" on the LHS of <<.

I registered for a Coverity account and am awaiting approval for adding
me to the QEMU project so I can test this myself (let me know if this
isn't the right way to go about this).

Would you prefer I wait until I'm able to test that this is resolved
myself, or just submit a patch and have you test it?

-Aaron