[PATCH v2] string: Suppress -Wmaybe-unitialized for wordcopy [BZ #19444]

[Carlos O'Donell]

On 1/11/23 16:27, Adhemerval Zanella wrote:
> When compiling with GCC 6+ the sparc build 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.

LGTM.

Reviewed-by: Carlos O'Donell <carlos@redhat.com>

> 
> Changes from v1:
> * Update patch description and the explanation for the suppresion.
> 
> ---
>  string/wordcopy.c | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 
> diff --git a/string/wordcopy.c b/string/wordcopy.c
> index ae5ccd793c..a8a40c9f01 100644
> --- a/string/wordcopy.c
> +++ b/string/wordcopy.c
> @@ -19,7 +19,19 @@
>  /* BE VERY CAREFUL IF YOU CHANGE THIS CODE...!  */
>  
>  #include <stddef.h>
> +#include <libc-diag.h>
> +/* 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.  */
> +DIAG_PUSH_NEEDS_COMMENT;
> +DIAG_IGNORE_NEEDS_COMMENT (6, "-Wmaybe-uninitialized");
>  #include <memcopy.h>
> +DIAG_POP_NEEDS_COMMENT;

OK.

>  
>  /* _wordcopy_fwd_aligned -- Copy block beginning at SRCP to
>     block beginning at DSTP with LEN `op_t' words (not LEN bytes!).
> @@ -94,7 +106,15 @@ 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' 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;
> @@ -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;

-- 
Cheers,
Carlos.