On Feb 14 17:55, Peter Maydell wrote:
> On Mon, 28 Jan 2019 at 18:11, Peter Maydell <peter.mayd...@linaro.org> wrote:
> >
> > From: Aaron Lindsay OS <aa...@os.amperecomputing.com>
> >
> > 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
