[3/4] string: Suppress -Wmaybe-unitialized for wordcopy [BZ #19444]

Message ID 20221229125802.2715435-4-adhemerval.zanella@linaro.org
State Superseded
Headers
Series Fix remaining -Os/-O1 compile issues [BZ #19444] |

Checks

Context Check Description
dj/TryBot-apply_patch success Patch applied to master at the time it was sent

Commit Message

Adhemerval Zanella Netto Dec. 29, 2022, 12:58 p.m. UTC
  With GCC 6+ when compiling with -O1 warns that some MERGE macro usage
might be used uninitialized.  The issue is calling the function with
len equal to 0 is undefined since the first 'switch' will not trigger
any case and then subsequent loop will potentially use uninitialized
variables.

However all usages on mem routines always called the function for
sizes larger than OP_T_THRES.
---
 string/wordcopy.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)
  

Comments

Carlos O'Donell Jan. 10, 2023, 10:47 p.m. UTC | #1
On 12/29/22 07:58, Adhemerval Zanella via Libc-alpha wrote:

I think this particular change needs a v2 and some more detailed analysis.

> With GCC 6+ when compiling with -O1 warns that some MERGE macro usage
> might be used uninitialized.  The issue is calling the function with
> len equal to 0 is undefined since the first 'switch' will not trigger
> any case and then subsequent loop will potentially use uninitialized
> variables.

The comment does not seem to match what the code does.

Calling the function with len equal to 0 results in 'case 0' executing in all of these
cases.

> However all usages on mem routines always called the function for
> sizes larger than OP_T_THRES.

This isn't always the case, you could have a call to copy just 8 bytes, which is
below the 16-byt OP_T_THRES.

> ---
>  string/wordcopy.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/string/wordcopy.c b/string/wordcopy.c
> index d05718322c..3b6344115d 100644
> --- a/string/wordcopy.c
> +++ b/string/wordcopy.c
> @@ -18,8 +18,19 @@
>  
>  /* BE VERY CAREFUL IF YOU CHANGE THIS CODE...!  */
>  
> +#include <assert.h>
>  #include <stddef.h>
> +#include <libc-diag.h>
> +/* With GCC 6 when compiling with -O1 warns that some MERGE macro usage might
> +   be used uninitialized.  The issue is calling the function with len equal to
> +   0 is undefined since the first 'switch' will not trigger any case and then
> +   subsequent loop will potentially use uninitialized variables.  However all
> +   usages on mem routines always called the function for sizes larger than

This comment does not match what the code does. The switch 'case 0:' case should execute.

> +   OP_T_THRES.  */
> +DIAG_PUSH_NEEDS_COMMENT;
> +DIAG_IGNORE_NEEDS_COMMENT (6, "-Wmaybe-uninitialized");
>  #include <memcopy.h>
> +DIAG_POP_NEEDS_COMMENT;
>  
>  /* _wordcopy_fwd_aligned -- Copy block beginning at SRCP to
>     block beginning at DSTP with LEN `op_t' words (not LEN bytes!).
> @@ -94,7 +105,11 @@ WORDCOPY_FWD_ALIGNED (long int dstp, long int srcp, size_t len)
>      {
>      do8:
>        a0 = ((op_t *) srcp)[0];
> +      /* Check the comment on memcopy.h inclusion.  */
> +      DIAG_PUSH_NEEDS_COMMENT;
> +      DIAG_IGNORE_NEEDS_COMMENT (6, "-Wmaybe-uninitialized");
>        ((op_t *) dstp)[0] = a1;
> +      DIAG_POP_NEEDS_COMMENT;

Why is a1 considered uninitialized?

The switch has case statements for every possible value.

In case 1 we unconditionally set a1.

We can't enter case 1 unless len was exactly 1 or any value of 1+8*n for valid n.

It seems like the compiler can't see that 'OP_T_THRES <= 3 * OPSIZ' is always a true.

>      do7:
>        a1 = ((op_t *) srcp)[1];
>        ((op_t *) dstp)[1] = a0;
> @@ -291,7 +306,11 @@ WORDCOPY_BWD_ALIGNED (long int dstp, long int srcp, size_t len)
>      {
>      do8:
>        a0 = ((op_t *) srcp)[7];
> +      /* Check the comment on memcopy.h inclusion.  */
> +      DIAG_PUSH_NEEDS_COMMENT;
> +      DIAG_IGNORE_NEEDS_COMMENT (6, "-Wmaybe-uninitialized");
>        ((op_t *) dstp)[7] = a1;
> +      DIAG_POP_NEEDS_COMMENT;

Likewise.

>      do7:
>        a1 = ((op_t *) srcp)[6];
>        ((op_t *) dstp)[6] = a0;
  
Adhemerval Zanella Netto Jan. 11, 2023, 1:14 p.m. UTC | #2
On 10/01/23 19:47, Carlos O'Donell wrote:
> On 12/29/22 07:58, Adhemerval Zanella via Libc-alpha wrote:
> 
> I think this particular change needs a v2 and some more detailed analysis.
> 
>> With GCC 6+ when compiling with -O1 warns that some MERGE macro usage
>> might be used uninitialized.  The issue is calling the function with
>> len equal to 0 is undefined since the first 'switch' will not trigger
>> any case and then subsequent loop will potentially use uninitialized
>> variables.
> 
> The comment does not seem to match what the code does.
> 
> Calling the function with len equal to 0 results in 'case 0' executing in all of these
> cases.
> 
>> However all usages on mem routines always called the function for
>> sizes larger than OP_T_THRES.
> 
> This isn't always the case, you could have a call to copy just 8 bytes, which is
> below the 16-byt OP_T_THRES.

The usage are in fact all done on generic mem functions.  The _wordcopy_fwd_aligned
and/or _wordcopy_fwd_dest_aligned are called by the WORD_COPY_FWD which, used by both
generic memcpy and memmove.  And both implementations only call the macro for length 
larger than OP_T_THRES.

This is similar for WORD_COPY_BWD, which is only used for memmove.c.

> 
>> ---
>>  string/wordcopy.c | 19 +++++++++++++++++++
>>  1 file changed, 19 insertions(+)
>>
>> diff --git a/string/wordcopy.c b/string/wordcopy.c
>> index d05718322c..3b6344115d 100644
>> --- a/string/wordcopy.c
>> +++ b/string/wordcopy.c
>> @@ -18,8 +18,19 @@
>>  
>>  /* BE VERY CAREFUL IF YOU CHANGE THIS CODE...!  */
>>  
>> +#include <assert.h>
>>  #include <stddef.h>
>> +#include <libc-diag.h>
>> +/* With GCC 6 when compiling with -O1 warns that some MERGE macro usage might
>> +   be used uninitialized.  The issue is calling the function with len equal to
>> +   0 is undefined since the first 'switch' will not trigger any case and then
>> +   subsequent loop will potentially use uninitialized variables.  However all
>> +   usages on mem routines always called the function for sizes larger than
> 
> This comment does not match what the code does. The switch 'case 0:' case should execute.

Right, reading again the code I think it might be indeed an issue with the
sparc backend since it is the only arch that triggers it, even though powerpc
also build the same code for both 64 and 32 bits.  What about:


  /* Compiling with -O1 might warn that 'a2' and 'a3' may be used
     uninitialized.  However, 'do3' (which uses 'a3') is only reachable either
     by 'case 0' or 'case 1', which initializes 'a3' (and it is also set at
     'do1' for subsequent loop iterations).  This is similar for 'do4'
     (which uses 'a2') that is only reachable by 'case 1' which initializes
     'a2').
     Since the usage is within the MERGE macro, we need to disable the warning
     on its definition.  */
> 
>> +   OP_T_THRES.  */
>> +DIAG_PUSH_NEEDS_COMMENT;
>> +DIAG_IGNORE_NEEDS_COMMENT (6, "-Wmaybe-uninitialized");
>>  #include <memcopy.h>
>> +DIAG_POP_NEEDS_COMMENT;
>>  
>>  /* _wordcopy_fwd_aligned -- Copy block beginning at SRCP to
>>     block beginning at DSTP with LEN `op_t' words (not LEN bytes!).
>> @@ -94,7 +105,11 @@ WORDCOPY_FWD_ALIGNED (long int dstp, long int srcp, size_t len)
>>      {
>>      do8:
>>        a0 = ((op_t *) srcp)[0];
>> +      /* Check the comment on memcopy.h inclusion.  */
>> +      DIAG_PUSH_NEEDS_COMMENT;
>> +      DIAG_IGNORE_NEEDS_COMMENT (6, "-Wmaybe-uninitialized");
>>        ((op_t *) dstp)[0] = a1;
>> +      DIAG_POP_NEEDS_COMMENT;
> 
> Why is a1 considered uninitialized?
> 
> The switch has case statements for every possible value.
> 
> In case 1 we unconditionally set a1.
> 
> We can't enter case 1 unless len was exactly 1 or any value of 1+8*n for valid n.
> 
> It seems like the compiler can't see that 'OP_T_THRES <= 3 * OPSIZ' is always a true.

I think it is unlikely because both OP_T_THRES and OPSIZ are constant macros,
although it might be case that the sparc backend is doing something fuzzy that
is maybe confusing some other passes.  I changed to:

      /* Compiling with -O1 might warn that 'a1' in 'do8' switch may be used
         uninitialized.  However, 'do8' is only reachable through 'case 1', 
         since all possible modulo value are handling in the initial switch).
         */

> 
>>      do7:
>>        a1 = ((op_t *) srcp)[1];
>>        ((op_t *) dstp)[1] = a0;
>> @@ -291,7 +306,11 @@ WORDCOPY_BWD_ALIGNED (long int dstp, long int srcp, size_t len)
>>      {
>>      do8:
>>        a0 = ((op_t *) srcp)[7];
>> +      /* Check the comment on memcopy.h inclusion.  */
>> +      DIAG_PUSH_NEEDS_COMMENT;
>> +      DIAG_IGNORE_NEEDS_COMMENT (6, "-Wmaybe-uninitialized");
>>        ((op_t *) dstp)[7] = a1;
>> +      DIAG_POP_NEEDS_COMMENT;
> 
> Likewise.
> 
>>      do7:
>>        a1 = ((op_t *) srcp)[6];
>>        ((op_t *) dstp)[6] = a0;
> 

I am attaching the fixed patch.
From 86b83da8c7836484654fbacaf54f764df378a31e Mon Sep 17 00:00:00 2001
From: Adhemerval Zanella <adhemerval.zanella@linaro.org>
Date: Thu, 29 Dec 2022 09:15:49 -0300
Subject: [PATCH 3/4] string: Suppress -Wmaybe-unitialized for wordcopy [BZ
 #19444]

The GCC 6+ when compiling for sparc warns that some variables might be
used uninitialized.  However it does not seem the fact, since the
variables are really initialized (and also other targets that use the
same code, like powerpc, do not warn about it).

So suppress the warning for now.
---
 string/wordcopy.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/string/wordcopy.c b/string/wordcopy.c
index ae5ccd793c..d0cfc22082 100644
--- a/string/wordcopy.c
+++ b/string/wordcopy.c
@@ -19,7 +19,12 @@
 /* BE VERY CAREFUL IF YOU CHANGE THIS CODE...!  */
 
 #include <stddef.h>
+#include <libc-diag.h>
+/* Check comment of WORDCOPY_FWD_DEST_ALIGNED.  */
+DIAG_PUSH_NEEDS_COMMENT;
+DIAG_IGNORE_NEEDS_COMMENT (6, "-Wmaybe-uninitialized");
 #include <memcopy.h>
+DIAG_POP_NEEDS_COMMENT;
 
 /* _wordcopy_fwd_aligned -- Copy block beginning at SRCP to
    block beginning at DSTP with LEN `op_t' words (not LEN bytes!).
@@ -94,7 +99,14 @@ WORDCOPY_FWD_ALIGNED (long int dstp, long int srcp, size_t len)
     {
     do8:
       a0 = ((op_t *) srcp)[0];
+      /* Compiling with -O1 may warn that 'a1' in 'do8' switch may be used
+	 uninitialized.  However, 'do8' is only reachable through 'case 1',
+	 since all possible modulo values are handling in the initial
+	 switch).  */
+      DIAG_PUSH_NEEDS_COMMENT;
+      DIAG_IGNORE_NEEDS_COMMENT (6, "-Wmaybe-uninitialized");
       ((op_t *) dstp)[0] = a1;
+      DIAG_POP_NEEDS_COMMENT;
     do7:
       a1 = ((op_t *) srcp)[1];
       ((op_t *) dstp)[1] = a0;
@@ -190,6 +202,14 @@ WORDCOPY_FWD_DEST_ALIGNED (long int dstp, long int srcp, size_t len)
       goto do4;			/* No-op.  */
     }
 
+  /* Compiling with -O1 might warn that 'a2' and 'a3' may be used
+     uninitialized.  However, 'do3' (which uses 'a3') is only reachable either
+     by 'case 0' or 'case 1', which initializes 'a3' (and it is also set at
+     'do1' for subsequent loop iterations).  This is similar for 'do4'
+     (which uses 'a2') that is only reachable by 'case 1' which initializes
+     'a2').
+     Since the usage is within the MERGE macro, we need to disable the warning
+     on its definition.  */
   do
     {
     do4:
@@ -291,7 +311,11 @@ WORDCOPY_BWD_ALIGNED (long int dstp, long int srcp, size_t len)
     {
     do8:
       a0 = ((op_t *) srcp)[7];
+      /* Check the comment on WORDCOPY_FWD_ALIGNED.  */
+      DIAG_PUSH_NEEDS_COMMENT;
+      DIAG_IGNORE_NEEDS_COMMENT (6, "-Wmaybe-uninitialized");
       ((op_t *) dstp)[7] = a1;
+      DIAG_POP_NEEDS_COMMENT;
     do7:
       a1 = ((op_t *) srcp)[6];
       ((op_t *) dstp)[6] = a0;
  
Carlos O'Donell Jan. 11, 2023, 7:33 p.m. UTC | #3
On 1/11/23 08:14, Adhemerval Zanella Netto wrote:
> I am attaching the fixed patch.

Could you please submit this in a new thread as a distinct patch from the series?

I think all the other patches can go in immediately.

This also helps me track in patchwork and for CI to re-test your patch.
  
Carlos O'Donell Jan. 11, 2023, 8:12 p.m. UTC | #4
On 1/11/23 08:14, Adhemerval Zanella Netto wrote:
> On 10/01/23 19:47, Carlos O'Donell wrote:
>> On 12/29/22 07:58, Adhemerval Zanella via Libc-alpha wrote:
>>
>> I think this particular change needs a v2 and some more detailed analysis.

Downthread I've asked you to repost in a new thread, but I'll carry out a review
here to reduce the turnaround time.

>>> With GCC 6+ when compiling with -O1 warns that some MERGE macro usage
>>> might be used uninitialized.  The issue is calling the function with
>>> len equal to 0 is undefined since the first 'switch' will not trigger
>>> any case and then subsequent loop will potentially use uninitialized
>>> variables.
>>
>> The comment does not seem to match what the code does.
>>
>> Calling the function with len equal to 0 results in 'case 0' executing in all of these
>> cases.
>>
>>> However all usages on mem routines always called the function for
>>> sizes larger than OP_T_THRES.
>>
>> This isn't always the case, you could have a call to copy just 8 bytes, which is
>> below the 16-byt OP_T_THRES.
> 
> The usage are in fact all done on generic mem functions.  The _wordcopy_fwd_aligned
> and/or _wordcopy_fwd_dest_aligned are called by the WORD_COPY_FWD which, used by both
> generic memcpy and memmove.  And both implementations only call the macro for length 
> larger than OP_T_THRES.

We are talking about examples like those in string/memmove.c:

 58       if (len >= OP_T_THRES)
 59         {
 60           /* Copy just a few bytes to make DSTP aligned.  */
 61           len -= (-dstp) % OPSIZ;
 62           BYTE_COPY_FWD (dstp, srcp, (-dstp) % OPSIZ);
 63 
 64           /* Copy whole pages from SRCP to DSTP by virtual address
 65              manipulation, as much as possible.  */
 66 
 67           PAGE_COPY_FWD_MAYBE (dstp, srcp, len, len);
 68 
 69           /* Copy from SRCP to DSTP taking advantage of the known
 70              alignment of DSTP.  Number of bytes remaining is put
 71              in the third argument, i.e. in LEN.  This number may
 72              vary from machine to machine.  */
 73 
 74           WORD_COPY_FWD (dstp, srcp, len, len);
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Only called with len >= OP_T_THRES
 75 
 76           /* Fall out and copy the tail.  */
 77         }

I agree completely with you here.

My point earlier was that the macros themselves have no 'assert(len > OP_T_THRES);'
or any indication that they should not be called with len < OP_T_THRES.

> This is similar for WORD_COPY_BWD, which is only used for memmove.c.

Agreed.

>>
>>> ---
>>>  string/wordcopy.c | 19 +++++++++++++++++++
>>>  1 file changed, 19 insertions(+)
>>>
>>> diff --git a/string/wordcopy.c b/string/wordcopy.c
>>> index d05718322c..3b6344115d 100644
>>> --- a/string/wordcopy.c
>>> +++ b/string/wordcopy.c
>>> @@ -18,8 +18,19 @@
>>>  
>>>  /* BE VERY CAREFUL IF YOU CHANGE THIS CODE...!  */
>>>  
>>> +#include <assert.h>
>>>  #include <stddef.h>
>>> +#include <libc-diag.h>
>>> +/* With GCC 6 when compiling with -O1 warns that some MERGE macro usage might
>>> +   be used uninitialized.  The issue is calling the function with len equal to
>>> +   0 is undefined since the first 'switch' will not trigger any case and then
>>> +   subsequent loop will potentially use uninitialized variables.  However all
>>> +   usages on mem routines always called the function for sizes larger than
>>
>> This comment does not match what the code does. The switch 'case 0:' case should execute.
> 
> Right, reading again the code I think it might be indeed an issue with the
> sparc backend since it is the only arch that triggers it, even though powerpc
> also build the same code for both 64 and 32 bits.  What about:

Right.
 
>   /* Compiling with -O1 might warn that 'a2' and 'a3' may be used
>      uninitialized.  However, 'do3' (which uses 'a3') is only reachable either
>      by 'case 0' or 'case 1', which initializes 'a3' (and it is also set at
>      'do1' for subsequent loop iterations).  This is similar for 'do4'
>      (which uses 'a2') that is only reachable by 'case 1' which initializes
>      'a2').
>      Since the usage is within the MERGE macro, we need to disable the warning
>      on its definition.  */

OK.

>>
>>> +   OP_T_THRES.  */
>>> +DIAG_PUSH_NEEDS_COMMENT;
>>> +DIAG_IGNORE_NEEDS_COMMENT (6, "-Wmaybe-uninitialized");
>>>  #include <memcopy.h>
>>> +DIAG_POP_NEEDS_COMMENT;
>>>  
>>>  /* _wordcopy_fwd_aligned -- Copy block beginning at SRCP to
>>>     block beginning at DSTP with LEN `op_t' words (not LEN bytes!).
>>> @@ -94,7 +105,11 @@ WORDCOPY_FWD_ALIGNED (long int dstp, long int srcp, size_t len)
>>>      {
>>>      do8:
>>>        a0 = ((op_t *) srcp)[0];
>>> +      /* Check the comment on memcopy.h inclusion.  */
>>> +      DIAG_PUSH_NEEDS_COMMENT;
>>> +      DIAG_IGNORE_NEEDS_COMMENT (6, "-Wmaybe-uninitialized");
>>>        ((op_t *) dstp)[0] = a1;
>>> +      DIAG_POP_NEEDS_COMMENT;
>>
>> Why is a1 considered uninitialized?
>>
>> The switch has case statements for every possible value.
>>
>> In case 1 we unconditionally set a1.
>>
>> We can't enter case 1 unless len was exactly 1 or any value of 1+8*n for valid n.
>>
>> It seems like the compiler can't see that 'OP_T_THRES <= 3 * OPSIZ' is always a true.
> 
> I think it is unlikely because both OP_T_THRES and OPSIZ are constant macros,
> although it might be case that the sparc backend is doing something fuzzy that
> is maybe confusing some other passes.  I changed to:
> 
>       /* Compiling with -O1 might warn that 'a1' in 'do8' switch may be used
>          uninitialized.  However, 'do8' is only reachable through 'case 1', 
>          since all possible modulo value are handling in the initial switch).
>          */

OK.

> 
>>
>>>      do7:
>>>        a1 = ((op_t *) srcp)[1];
>>>        ((op_t *) dstp)[1] = a0;
>>> @@ -291,7 +306,11 @@ WORDCOPY_BWD_ALIGNED (long int dstp, long int srcp, size_t len)
>>>      {
>>>      do8:
>>>        a0 = ((op_t *) srcp)[7];
>>> +      /* Check the comment on memcopy.h inclusion.  */
>>> +      DIAG_PUSH_NEEDS_COMMENT;
>>> +      DIAG_IGNORE_NEEDS_COMMENT (6, "-Wmaybe-uninitialized");
>>>        ((op_t *) dstp)[7] = a1;
>>> +      DIAG_POP_NEEDS_COMMENT;
>>
>> Likewise.
>>
>>>      do7:
>>>        a1 = ((op_t *) srcp)[6];
>>>        ((op_t *) dstp)[6] = a0;
>>
> 
> I am attaching the fixed patch.

> From 86b83da8c7836484654fbacaf54f764df378a31e Mon Sep 17 00:00:00 2001
> From: Adhemerval Zanella <adhemerval.zanella@linaro.org>
> Date: Thu, 29 Dec 2022 09:15:49 -0300
> Subject: [PATCH 3/4] string: Suppress -Wmaybe-unitialized for wordcopy [BZ
>  #19444]
> 
> The GCC 6+ when compiling for sparc warns that some variables might be
> used uninitialized.  However it does not seem the fact, since the
> variables are really initialized (and also other targets that use the
> same code, like powerpc, do not warn about it).

Suggest:

When compiling with GCC 6+ the sparc build warns that some variables might
be used uninitialized. 


> 
> So suppress the warning for now.
> ---
>  string/wordcopy.c | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 
> diff --git a/string/wordcopy.c b/string/wordcopy.c
> index ae5ccd793c..d0cfc22082 100644
> --- a/string/wordcopy.c
> +++ b/string/wordcopy.c
> @@ -19,7 +19,12 @@
>  /* BE VERY CAREFUL IF YOU CHANGE THIS CODE...!  */
>  
>  #include <stddef.h>
> +#include <libc-diag.h>
> +/* Check comment of WORDCOPY_FWD_DEST_ALIGNED.  */

Please move the comment up to here.

> +DIAG_PUSH_NEEDS_COMMENT;
> +DIAG_IGNORE_NEEDS_COMMENT (6, "-Wmaybe-uninitialized");
>  #include <memcopy.h>
> +DIAG_POP_NEEDS_COMMENT;
>  
>  /* _wordcopy_fwd_aligned -- Copy block beginning at SRCP to
>     block beginning at DSTP with LEN `op_t' words (not LEN bytes!).
> @@ -94,7 +99,14 @@ WORDCOPY_FWD_ALIGNED (long int dstp, long int srcp, size_t len)
>      {
>      do8:
>        a0 = ((op_t *) srcp)[0];
> +      /* Compiling with -O1 may warn that 'a1' in 'do8' switch may be used
> +	 uninitialized.  However, 'do8' is only reachable through 'case 1',
> +	 since all possible modulo values are handling in the initial
> +	 switch).  */

Please move the comment to just above the store of a1 into dstp.

Suggest:

/* Compiling with -O1 may warn that 'a1' may be used uninitialized. There are only
   two ways to arrive at label 'do8' and they are via a do-while loop iteration
   or directly via the earlier switch 'case 1:' case. The switch case always sets
   'a1' and all previous loop iterations will also have set 'a1' before the use.  */



> +      DIAG_PUSH_NEEDS_COMMENT;
> +      DIAG_IGNORE_NEEDS_COMMENT (6, "-Wmaybe-uninitialized");
>        ((op_t *) dstp)[0] = a1;
> +      DIAG_POP_NEEDS_COMMENT;
>      do7:
>        a1 = ((op_t *) srcp)[1];
>        ((op_t *) dstp)[1] = a0;
> @@ -190,6 +202,14 @@ WORDCOPY_FWD_DEST_ALIGNED (long int dstp, long int srcp, size_t len)
>        goto do4;			/* No-op.  */
>      }
>  

Please move this comment up to the point at which you have the DIAG* since
it should apply to both FWD and BWD uses that use the MERGE macros.

> +  /* Compiling with -O1 might warn that 'a2' and 'a3' may be used
> +     uninitialized.  However, 'do3' (which uses 'a3') is only reachable either
> +     by 'case 0' or 'case 1', which initializes 'a3' (and it is also set at
> +     'do1' for subsequent loop iterations).  This is similar for 'do4'
> +     (which uses 'a2') that is only reachable by 'case 1' which initializes
> +     'a2').
> +     Since the usage is within the MERGE macro, we need to disable the warning
> +     on its definition.  */

Suggest:

/* Compiling with -O1 might warn that 'a2' and 'a3' may be used
   uninitialized.  There are only two ways to arrive at labels
   'do4', 'do3' or 'do1', all of which use 'a2' or 'a3' in the
   MERGE macro: either from the earlier switch case statement
   or via a loop iteration.  In all cases the switch statement
   or previous loop sets both 'a2' and 'a3'.

   Since the usage is within the MERGE macro we disable the
   warning in the definition, but only in this file.  */

>    do
>      {
>      do4:
> @@ -291,7 +311,11 @@ WORDCOPY_BWD_ALIGNED (long int dstp, long int srcp, size_t len)
>      {
>      do8:
>        a0 = ((op_t *) srcp)[7];
> +      /* Check the comment on WORDCOPY_FWD_ALIGNED.  */
> +      DIAG_PUSH_NEEDS_COMMENT;
> +      DIAG_IGNORE_NEEDS_COMMENT (6, "-Wmaybe-uninitialized");
>        ((op_t *) dstp)[7] = a1;
> +      DIAG_POP_NEEDS_COMMENT;
>      do7:
>        a1 = ((op_t *) srcp)[6];
>        ((op_t *) dstp)[6] = a0;
> -- 
> 2.34.1
  

Patch

diff --git a/string/wordcopy.c b/string/wordcopy.c
index d05718322c..3b6344115d 100644
--- a/string/wordcopy.c
+++ b/string/wordcopy.c
@@ -18,8 +18,19 @@ 
 
 /* BE VERY CAREFUL IF YOU CHANGE THIS CODE...!  */
 
+#include <assert.h>
 #include <stddef.h>
+#include <libc-diag.h>
+/* With GCC 6 when compiling with -O1 warns that some MERGE macro usage might
+   be used uninitialized.  The issue is calling the function with len equal to
+   0 is undefined since the first 'switch' will not trigger any case and then
+   subsequent loop will potentially use uninitialized variables.  However all
+   usages on mem routines always called the function for sizes larger than
+   OP_T_THRES.  */
+DIAG_PUSH_NEEDS_COMMENT;
+DIAG_IGNORE_NEEDS_COMMENT (6, "-Wmaybe-uninitialized");
 #include <memcopy.h>
+DIAG_POP_NEEDS_COMMENT;
 
 /* _wordcopy_fwd_aligned -- Copy block beginning at SRCP to
    block beginning at DSTP with LEN `op_t' words (not LEN bytes!).
@@ -94,7 +105,11 @@  WORDCOPY_FWD_ALIGNED (long int dstp, long int srcp, size_t len)
     {
     do8:
       a0 = ((op_t *) srcp)[0];
+      /* Check the comment on memcopy.h inclusion.  */
+      DIAG_PUSH_NEEDS_COMMENT;
+      DIAG_IGNORE_NEEDS_COMMENT (6, "-Wmaybe-uninitialized");
       ((op_t *) dstp)[0] = a1;
+      DIAG_POP_NEEDS_COMMENT;
     do7:
       a1 = ((op_t *) srcp)[1];
       ((op_t *) dstp)[1] = a0;
@@ -291,7 +306,11 @@  WORDCOPY_BWD_ALIGNED (long int dstp, long int srcp, size_t len)
     {
     do8:
       a0 = ((op_t *) srcp)[7];
+      /* Check the comment on memcopy.h inclusion.  */
+      DIAG_PUSH_NEEDS_COMMENT;
+      DIAG_IGNORE_NEEDS_COMMENT (6, "-Wmaybe-uninitialized");
       ((op_t *) dstp)[7] = a1;
+      DIAG_POP_NEEDS_COMMENT;
     do7:
       a1 = ((op_t *) srcp)[6];
       ((op_t *) dstp)[6] = a0;