Re: [PULL 12/29] target/ppc: move FP and VMX registers into aligned vsr register array

[Peter Maydell]

On Tue, 8 Jan 2019 at 22:46, David Gibson <address@hidden> wrote:
>
> From: Mark Cave-Ayland <address@hidden>
>
> The VSX register array is a block of 64 128-bit registers where the first 32
> registers consist of the existing 64-bit FP registers extended to 128-bit
> using new VSR registers, and the last 32 registers are the VMX 128-bit
> registers as show below:
>
>             64-bit               64-bit
>     +--------------------+--------------------+
>     |        FP0         |                    |  VSR0
>     +--------------------+--------------------+
>     |        FP1         |                    |  VSR1
>     +--------------------+--------------------+
>     |        ...         |        ...         |  ...
>     +--------------------+--------------------+
>     |        FP30        |                    |  VSR30
>     +--------------------+--------------------+
>     |        FP31        |                    |  VSR31
>     +--------------------+--------------------+
>     |                  VMX0                   |  VSR32
>     +-----------------------------------------+
>     |                  VMX1                   |  VSR33
>     +-----------------------------------------+
>     |                  ...                    |  ...
>     +-----------------------------------------+
>     |                  VMX30                  |  VSR62
>     +-----------------------------------------+
>     |                  VMX31                  |  VSR63
>     +-----------------------------------------+
>
> In order to allow for future conversion of VSX instructions to use TCG vector
> operations, recreate the same layout using an aligned version of the existing
> vsr register array.
>
> Since the old fpr and avr register arrays are removed, the existing callers
> must also be updated to use the correct offset in the vsr register array. This
> also includes switching the relevant VMState fields over to using subarrays
> to make sure that migration is preserved.



> @@ -281,15 +281,17 @@ static void save_user_regs(CPUPPCState *env, struct 
> target_mcontext *frame)
>      /* Save VSX second halves */
>      if (env->insns_flags2 & PPC2_VSX) {
>          uint64_t *vsregs = (uint64_t *)&frame->mc_vregs.altivec[34];

This line didn't change in this patch, but the code change seems
to have prompted Coverity to warn (CID 1396862):

 illegal_address: &frame->mc_vregs.altivec[34] evaluates to an address that is
 at byte offset 544 of an array of 528 bytes.

> @@ -393,16 +395,18 @@ static void restore_user_regs(CPUPPCState *env,
>      /* Restore VSX second halves */
>      if (env->insns_flags2 & PPC2_VSX) {
>          uint64_t *vsregs = (uint64_t *)&frame->mc_vregs.altivec[34];

...and then CID 1396863 is the same issue for this line.


Is this legit, or a false-positive ?

thanks
-- PMM