On Mon, 28 Jan 2019 at 18:11, Peter Maydell <[email protected]> wrote:
>
> From: Aaron Lindsay OS <[email protected]>
>
> 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 <<.
> + if (cnt->number & 0x20) {
> + cpu->pmceid1 |= event_mask;
> + } else {
> + cpu->pmceid0 |= event_mask;
> + }
> }
> }
> - return pmceid;
> }
thanks
-- PMM