Message ID | 20230111212729.4109942-1-adhemerval.zanella@linaro.org |
---|---|
State | New |
Headers | show |
Series | [v2] string: Suppress -Wmaybe-unitialized for wordcopy [BZ #19444] | expand |
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;
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; /* _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;