diff mbox series

[v3,15/22] linux-user/hppa: fix -Werror=maybe-uninitialized false-positive

Message ID 20240930081458.1926382-16-marcandre.lureau@redhat.com (mailing list archive)
State New, archived
Headers show
Series -Werror=maybe-uninitialized fixes | expand

Commit Message

Marc-André Lureau Sept. 30, 2024, 8:14 a.m. UTC 
From: Marc-André Lureau <marcandre.lureau@redhat.com>

../linux-user/hppa/cpu_loop.c: In function ‘hppa_lws’:
../linux-user/hppa/cpu_loop.c:106:17: error: ‘ret’ may be used uninitialized [-Werror=maybe-uninitialized]
  106 |     env->gr[28] = ret;

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 linux-user/hppa/cpu_loop.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

Comments

Laurent Vivier Sept. 30, 2024, 12:18 p.m. UTC | #1 
CC Helge Deller

Le 30/09/2024 à 10:14, marcandre.lureau@redhat.com a écrit :
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> ../linux-user/hppa/cpu_loop.c: In function ‘hppa_lws’:
> ../linux-user/hppa/cpu_loop.c:106:17: error: ‘ret’ may be used uninitialized [-Werror=maybe-uninitialized]
>    106 |     env->gr[28] = ret;
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>   linux-user/hppa/cpu_loop.c | 10 +++++-----
>   1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/linux-user/hppa/cpu_loop.c b/linux-user/hppa/cpu_loop.c
> index bc093b8fe8..f4da95490e 100644
> --- a/linux-user/hppa/cpu_loop.c
> +++ b/linux-user/hppa/cpu_loop.c
> @@ -43,7 +43,7 @@ static abi_ulong hppa_lws(CPUHPPAState *env)
>           old = tswap32(old);
>           new = tswap32(new);
>           ret = qatomic_cmpxchg((uint32_t *)g2h(cs, addr), old, new);
> -        ret = tswap32(ret);
> +        env->gr[28] = tswap32(ret);
>           break;
>   
>       case 2: /* elf32 atomic "new" cmpxchg */
> @@ -64,19 +64,19 @@ static abi_ulong hppa_lws(CPUHPPAState *env)
>               old = *(uint8_t *)g2h(cs, old);
>               new = *(uint8_t *)g2h(cs, new);
>               ret = qatomic_cmpxchg((uint8_t *)g2h(cs, addr), old, new);
> -            ret = ret != old;
> +            env->gr[28] = ret != old;
>               break;
>           case 1:
>               old = *(uint16_t *)g2h(cs, old);
>               new = *(uint16_t *)g2h(cs, new);
>               ret = qatomic_cmpxchg((uint16_t *)g2h(cs, addr), old, new);
> -            ret = ret != old;
> +            env->gr[28] = ret != old;
>               break;
>           case 2:
>               old = *(uint32_t *)g2h(cs, old);
>               new = *(uint32_t *)g2h(cs, new);
>               ret = qatomic_cmpxchg((uint32_t *)g2h(cs, addr), old, new);
> -            ret = ret != old;
> +            env->gr[28] = ret != old;
>               break;
>           case 3:
>               {
> @@ -97,13 +97,13 @@ static abi_ulong hppa_lws(CPUHPPAState *env)
>                   }
>                   end_exclusive();
>   #endif
> +                env->gr[28] = ret;
>               }
>               break;
>           }
>           break;
>       }
>   
> -    env->gr[28] = ret;
>       return 0;
>   }
>   

Did you try to put a g_assert_not_reached() in a "default:" for "switch(size)"?
This should help the compiler to know that "env->gr[28] = ret;" will not be reached if ret is not set.

Thanks,
Laurent
Marc-André Lureau Sept. 30, 2024, 1:26 p.m. UTC | #2 
Hi Laurent

On Mon, Sep 30, 2024 at 4:19 PM Laurent Vivier <laurent@vivier.eu> wrote:
>
> CC Helge Deller
>
> Le 30/09/2024 à 10:14, marcandre.lureau@redhat.com a écrit :
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > ../linux-user/hppa/cpu_loop.c: In function ‘hppa_lws’:
> > ../linux-user/hppa/cpu_loop.c:106:17: error: ‘ret’ may be used uninitialized [-Werror=maybe-uninitialized]
> >    106 |     env->gr[28] = ret;
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >   linux-user/hppa/cpu_loop.c | 10 +++++-----
> >   1 file changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/linux-user/hppa/cpu_loop.c b/linux-user/hppa/cpu_loop.c
> > index bc093b8fe8..f4da95490e 100644
> > --- a/linux-user/hppa/cpu_loop.c
> > +++ b/linux-user/hppa/cpu_loop.c
> > @@ -43,7 +43,7 @@ static abi_ulong hppa_lws(CPUHPPAState *env)
> >           old = tswap32(old);
> >           new = tswap32(new);
> >           ret = qatomic_cmpxchg((uint32_t *)g2h(cs, addr), old, new);
> > -        ret = tswap32(ret);
> > +        env->gr[28] = tswap32(ret);
> >           break;
> >
> >       case 2: /* elf32 atomic "new" cmpxchg */
> > @@ -64,19 +64,19 @@ static abi_ulong hppa_lws(CPUHPPAState *env)
> >               old = *(uint8_t *)g2h(cs, old);
> >               new = *(uint8_t *)g2h(cs, new);
> >               ret = qatomic_cmpxchg((uint8_t *)g2h(cs, addr), old, new);
> > -            ret = ret != old;
> > +            env->gr[28] = ret != old;
> >               break;
> >           case 1:
> >               old = *(uint16_t *)g2h(cs, old);
> >               new = *(uint16_t *)g2h(cs, new);
> >               ret = qatomic_cmpxchg((uint16_t *)g2h(cs, addr), old, new);
> > -            ret = ret != old;
> > +            env->gr[28] = ret != old;
> >               break;
> >           case 2:
> >               old = *(uint32_t *)g2h(cs, old);
> >               new = *(uint32_t *)g2h(cs, new);
> >               ret = qatomic_cmpxchg((uint32_t *)g2h(cs, addr), old, new);
> > -            ret = ret != old;
> > +            env->gr[28] = ret != old;
> >               break;
> >           case 3:
> >               {
> > @@ -97,13 +97,13 @@ static abi_ulong hppa_lws(CPUHPPAState *env)
> >                   }
> >                   end_exclusive();
> >   #endif
> > +                env->gr[28] = ret;
> >               }
> >               break;
> >           }
> >           break;
> >       }
> >
> > -    env->gr[28] = ret;
> >       return 0;
> >   }
> >
>
> Did you try to put a g_assert_not_reached() in a "default:" for "switch(size)"?
> This should help the compiler to know that "env->gr[28] = ret;" will not be reached if ret is not set.

That works! I'll change the patch and include your r-b then?
Laurent Vivier Oct. 1, 2024, 9:20 a.m. UTC | #3 
Le 30/09/2024 à 15:26, Marc-André Lureau a écrit :
> Hi Laurent
> 
> On Mon, Sep 30, 2024 at 4:19 PM Laurent Vivier <laurent@vivier.eu> wrote:
>>
>> CC Helge Deller
>>
>> Le 30/09/2024 à 10:14, marcandre.lureau@redhat.com a écrit :
>>> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>>>
>>> ../linux-user/hppa/cpu_loop.c: In function ‘hppa_lws’:
>>> ../linux-user/hppa/cpu_loop.c:106:17: error: ‘ret’ may be used uninitialized [-Werror=maybe-uninitialized]
>>>     106 |     env->gr[28] = ret;
>>>
>>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>>> ---
>>>    linux-user/hppa/cpu_loop.c | 10 +++++-----
>>>    1 file changed, 5 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/linux-user/hppa/cpu_loop.c b/linux-user/hppa/cpu_loop.c
>>> index bc093b8fe8..f4da95490e 100644
>>> --- a/linux-user/hppa/cpu_loop.c
>>> +++ b/linux-user/hppa/cpu_loop.c
>>> @@ -43,7 +43,7 @@ static abi_ulong hppa_lws(CPUHPPAState *env)
>>>            old = tswap32(old);
>>>            new = tswap32(new);
>>>            ret = qatomic_cmpxchg((uint32_t *)g2h(cs, addr), old, new);
>>> -        ret = tswap32(ret);
>>> +        env->gr[28] = tswap32(ret);
>>>            break;
>>>
>>>        case 2: /* elf32 atomic "new" cmpxchg */
>>> @@ -64,19 +64,19 @@ static abi_ulong hppa_lws(CPUHPPAState *env)
>>>                old = *(uint8_t *)g2h(cs, old);
>>>                new = *(uint8_t *)g2h(cs, new);
>>>                ret = qatomic_cmpxchg((uint8_t *)g2h(cs, addr), old, new);
>>> -            ret = ret != old;
>>> +            env->gr[28] = ret != old;
>>>                break;
>>>            case 1:
>>>                old = *(uint16_t *)g2h(cs, old);
>>>                new = *(uint16_t *)g2h(cs, new);
>>>                ret = qatomic_cmpxchg((uint16_t *)g2h(cs, addr), old, new);
>>> -            ret = ret != old;
>>> +            env->gr[28] = ret != old;
>>>                break;
>>>            case 2:
>>>                old = *(uint32_t *)g2h(cs, old);
>>>                new = *(uint32_t *)g2h(cs, new);
>>>                ret = qatomic_cmpxchg((uint32_t *)g2h(cs, addr), old, new);
>>> -            ret = ret != old;
>>> +            env->gr[28] = ret != old;
>>>                break;
>>>            case 3:
>>>                {
>>> @@ -97,13 +97,13 @@ static abi_ulong hppa_lws(CPUHPPAState *env)
>>>                    }
>>>                    end_exclusive();
>>>    #endif
>>> +                env->gr[28] = ret;
>>>                }
>>>                break;
>>>            }
>>>            break;
>>>        }
>>>
>>> -    env->gr[28] = ret;
>>>        return 0;
>>>    }
>>>
>>
>> Did you try to put a g_assert_not_reached() in a "default:" for "switch(size)"?
>> This should help the compiler to know that "env->gr[28] = ret;" will not be reached if ret is not set.
> 
> That works! I'll change the patch and include your r-b then?
> 
> 

Yes, you can

Thanks,
Laurent
diff mbox series

Patch

diff --git a/linux-user/hppa/cpu_loop.c b/linux-user/hppa/cpu_loop.c
index bc093b8fe8..f4da95490e 100644
--- a/linux-user/hppa/cpu_loop.c
+++ b/linux-user/hppa/cpu_loop.c
@@ -43,7 +43,7 @@  static abi_ulong hppa_lws(CPUHPPAState *env)
         old = tswap32(old);
         new = tswap32(new);
         ret = qatomic_cmpxchg((uint32_t *)g2h(cs, addr), old, new);
-        ret = tswap32(ret);
+        env->gr[28] = tswap32(ret);
         break;
 
     case 2: /* elf32 atomic "new" cmpxchg */
@@ -64,19 +64,19 @@  static abi_ulong hppa_lws(CPUHPPAState *env)
             old = *(uint8_t *)g2h(cs, old);
             new = *(uint8_t *)g2h(cs, new);
             ret = qatomic_cmpxchg((uint8_t *)g2h(cs, addr), old, new);
-            ret = ret != old;
+            env->gr[28] = ret != old;
             break;
         case 1:
             old = *(uint16_t *)g2h(cs, old);
             new = *(uint16_t *)g2h(cs, new);
             ret = qatomic_cmpxchg((uint16_t *)g2h(cs, addr), old, new);
-            ret = ret != old;
+            env->gr[28] = ret != old;
             break;
         case 2:
             old = *(uint32_t *)g2h(cs, old);
             new = *(uint32_t *)g2h(cs, new);
             ret = qatomic_cmpxchg((uint32_t *)g2h(cs, addr), old, new);
-            ret = ret != old;
+            env->gr[28] = ret != old;
             break;
         case 3:
             {
@@ -97,13 +97,13 @@  static abi_ulong hppa_lws(CPUHPPAState *env)
                 }
                 end_exclusive();
 #endif
+                env->gr[28] = ret;
             }
             break;
         }
         break;
     }
 
-    env->gr[28] = ret;
     return 0;
 }