Coverity warns that we shift a 32-bit value by N, and then
accumulate it into a 64-bit type (target_ulong on ppc64).

The ccr is always 8 * 4-bit fields, and thus is always a
32-bit quantity; narrow the type to avoid the warning.

Fixes: Coverity CID 1487223
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/ppc/signal.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/linux-user/ppc/signal.c b/linux-user/ppc/signal.c
index ec0b9c0df3..ce5a4682cd 100644
--- a/linux-user/ppc/signal.c
+++ b/linux-user/ppc/signal.c
@@ -229,7 +229,7 @@ static void save_user_regs(CPUPPCState *env, struct target_mcontext *frame)
 {
     target_ulong msr = env->msr;
     int i;
-    target_ulong ccr = 0;
+    uint32_t ccr = 0;
 
     /* In general, the kernel attempts to be intelligent about what it
        needs to save for Altivec/FP/SPE registers.  We don't care that
-- 
2.25.1

On 4/1/22 21:16, Richard Henderson wrote:
> Coverity warns that we shift a 32-bit value by N, and then
> accumulate it into a 64-bit type (target_ulong on ppc64).
> 
> The ccr is always 8 * 4-bit fields, and thus is always a
> 32-bit quantity; narrow the type to avoid the warning.
> 
> Fixes: Coverity CID 1487223
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   linux-user/ppc/signal.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

Queued for ppc-7.0

Thanks,

C.


> 
> diff --git a/linux-user/ppc/signal.c b/linux-user/ppc/signal.c
> index ec0b9c0df3..ce5a4682cd 100644
> --- a/linux-user/ppc/signal.c
> +++ b/linux-user/ppc/signal.c
> @@ -229,7 +229,7 @@ static void save_user_regs(CPUPPCState *env, struct target_mcontext *frame)
>   {
>       target_ulong msr = env->msr;
>       int i;
> -    target_ulong ccr = 0;
> +    uint32_t ccr = 0;
>   
>       /* In general, the kernel attempts to be intelligent about what it
>          needs to save for Altivec/FP/SPE registers.  We don't care that

On Mon, 4 Apr 2022 at 07:55, Cédric Le Goater <clg@kaod.org> wrote:
>
> On 4/1/22 21:16, Richard Henderson wrote:
> > Coverity warns that we shift a 32-bit value by N, and then
> > accumulate it into a 64-bit type (target_ulong on ppc64).
> >
> > The ccr is always 8 * 4-bit fields, and thus is always a
> > 32-bit quantity; narrow the type to avoid the warning.
> >
> > Fixes: Coverity CID 1487223
> > Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> > ---
> >   linux-user/ppc/signal.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
>
> Queued for ppc-7.0

NB that this is only suppressing a coverity warning, not
correcting any incorrect behaviour, so if you don't have
anything else you were planning to send for 7.0 it could
also wait til 7.1.

thanks
-- PMM

On 4/4/22 10:41, Peter Maydell wrote:
> On Mon, 4 Apr 2022 at 07:55, Cédric Le Goater <clg@kaod.org> wrote:
>>
>> On 4/1/22 21:16, Richard Henderson wrote:
>>> Coverity warns that we shift a 32-bit value by N, and then
>>> accumulate it into a 64-bit type (target_ulong on ppc64).
>>>
>>> The ccr is always 8 * 4-bit fields, and thus is always a
>>> 32-bit quantity; narrow the type to avoid the warning.
>>>
>>> Fixes: Coverity CID 1487223
>>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>>> ---
>>>    linux-user/ppc/signal.c | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> Queued for ppc-7.0
> 
> NB that this is only suppressing a coverity warning, not
> correcting any incorrect behaviour, so if you don't have
> anything else you were planning to send for 7.0 it could
> also wait til 7.1.

I have a couple of small fixes in :

   https://github.com/legoater/qemu/commits/ppc-for-upstream

   linux-user/ppc: Narrow type of ccr in save_user_regs
   ppc/pnv: Fix number of registers in the PCIe controller on POWER9
   hw/ppc: free env->tb_env in spapr_unrealize_vcpu()

Nothing critical indeed. So these can wait 7.1 ?

Thanks,

C.

On Mon, 4 Apr 2022 at 10:09, Cédric Le Goater <clg@kaod.org> wrote:
>
> On 4/4/22 10:41, Peter Maydell wrote:
> > On Mon, 4 Apr 2022 at 07:55, Cédric Le Goater <clg@kaod.org> wrote:
> >>
> >> On 4/1/22 21:16, Richard Henderson wrote:
> >>> Coverity warns that we shift a 32-bit value by N, and then
> >>> accumulate it into a 64-bit type (target_ulong on ppc64).
> >>>
> >>> The ccr is always 8 * 4-bit fields, and thus is always a
> >>> 32-bit quantity; narrow the type to avoid the warning.
> >>>
> >>> Fixes: Coverity CID 1487223
> >>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> >>> ---
> >>>    linux-user/ppc/signal.c | 2 +-
> >>>    1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> Queued for ppc-7.0
> >
> > NB that this is only suppressing a coverity warning, not
> > correcting any incorrect behaviour, so if you don't have
> > anything else you were planning to send for 7.0 it could
> > also wait til 7.1.
>
> I have a couple of small fixes in :
>
>    https://github.com/legoater/qemu/commits/ppc-for-upstream
>
>    linux-user/ppc: Narrow type of ccr in save_user_regs
>    ppc/pnv: Fix number of registers in the PCIe controller on POWER9
>    hw/ppc: free env->tb_env in spapr_unrealize_vcpu()
>
> Nothing critical indeed. So these can wait 7.1 ?

Up to you -- they're all small enough to be OK going into
7.0, and the other two are fixing real bugs.

thanks
-- PMM

On 4/1/22 21:16, Richard Henderson wrote:
> Coverity warns that we shift a 32-bit value by N, and then
> accumulate it into a 64-bit type (target_ulong on ppc64).
> 
> The ccr is always 8 * 4-bit fields, and thus is always a
> 32-bit quantity; narrow the type to avoid the warning.
> 
> Fixes: Coverity CID 1487223
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.


> ---
>   linux-user/ppc/signal.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/linux-user/ppc/signal.c b/linux-user/ppc/signal.c
> index ec0b9c0df3..ce5a4682cd 100644
> --- a/linux-user/ppc/signal.c
> +++ b/linux-user/ppc/signal.c
> @@ -229,7 +229,7 @@ static void save_user_regs(CPUPPCState *env, struct target_mcontext *frame)
>   {
>       target_ulong msr = env->msr;
>       int i;
> -    target_ulong ccr = 0;
> +    uint32_t ccr = 0;
>   
>       /* In general, the kernel attempts to be intelligent about what it
>          needs to save for Altivec/FP/SPE registers.  We don't care that