| Message ID | 20220921135108.3324737-3-adhemerval.zanella@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | Fix -Os build | expand |
On Wed, Sep 21, 2022 at 10:51:04AM -0300, Adhemerval Zanella via Libc-alpha wrote: > GCC with -Os issues some may uninitialized warnings on fnmatch code. > All of the variables are already set when they are accessed on the > loop prior. > > Checked on x86_64-linux-gnu and i686-linux-gnu. LGTM. This touches gnulib code and may require merging upstream or harmonizing in a different way. For now this looks ok. There are some alternatives, but this is OK. Reviewed-by: Carlos O'Donell <carlos@redhat.com> Tested-by: Carlos O'Donell <carlos@redhat.com> > --- > posix/fnmatch_loop.c | 31 +++++++++++++++++++++++++++++++ > 1 file changed, 31 insertions(+) > > diff --git a/posix/fnmatch_loop.c b/posix/fnmatch_loop.c > index 9445ed9c58..23eeffc79c 100644 > --- a/posix/fnmatch_loop.c > +++ b/posix/fnmatch_loop.c > @@ -530,6 +530,14 @@ FCT (const CHAR *pattern, const CHAR *string, const CHAR *string_end, > { > /* Compare the byte sequence but only if > this is not part of a range. */ > + > + /* The compiler might warn that idx may be > + used uninitialized, however it will be > + reached iff elem < table_size which means > + that it was properly set in the loop > + above. */ > + DIAG_PUSH_NEEDS_COMMENT; > + DIAG_IGNORE_Os_NEEDS_COMMENT (8, "-Wmaybe-uninitialized"); > if (! is_range > > # if WIDE_CHAR_VERSION > @@ -542,11 +550,19 @@ FCT (const CHAR *pattern, const CHAR *string, const CHAR *string_end, > n += c1 - 1; > goto matched; > } > + DIAG_POP_NEEDS_COMMENT; OK. > > /* Get the collation sequence value. */ > is_seqval = true; > # if WIDE_CHAR_VERSION > + /* The compile might warn that wextra may be > + used uninitialized and similar to 'idx' > + above it will be properly set by the loop. > + */ > + DIAG_PUSH_NEEDS_COMMENT; > + DIAG_IGNORE_Os_NEEDS_COMMENT (8, "-Wmaybe-uninitialized"); > cold = wextra[1 + wextra[0]]; > + DIAG_POP_NEEDS_COMMENT; OK. > # else > idx += 1 + extra[idx]; > /* Adjust for the alignment. */ > @@ -723,9 +739,24 @@ FCT (const CHAR *pattern, const CHAR *string, const CHAR *string_end, > /* Get the collation sequence value. */ > is_seqval = true; > # if WIDE_CHAR_VERSION > + /* The compiler might warn that wextra may > + be used uninitialized, however it will > + be reached iff elem < table_size which > + means that it was properly set in the > + loop above. */ > + DIAG_PUSH_NEEDS_COMMENT; > + DIAG_IGNORE_Os_NEEDS_COMMENT (8, "-Wmaybe-uninitialized"); > cend = wextra[1 + wextra[0]]; > + DIAG_POP_NEEDS_COMMENT; OK. > # else > + /* The compile might warn that idx may > + be used uninitialized and similar to > + wextra above it will be properly set by > + the loop. */ > + DIAG_PUSH_NEEDS_COMMENT; > + DIAG_IGNORE_Os_NEEDS_COMMENT (8, "-Wmaybe-uninitialized"); > idx += 1 + extra[idx]; > + DIAG_POP_NEEDS_COMMENT; OK. > /* Adjust for the alignment. */ > idx = (idx + 3) & ~3; > cend = *((int32_t *) &extra[idx]); > -- > 2.34.1 >
On 10/5/22 06:47, Carlos O'Donell via Libc-alpha wrote: > This touches gnulib code and may require merging upstream or harmonizing > in a different way. Shouldn't require any extra work as far as glibc goes, as Gnulib generally disables those diagnostics at the GCC command line level, because c'mon, they're more trouble than their worth. We'll put in no-op macro definitions somewhere else in Gnulib, if needed. The files touched by this patch shouldn't need any further change.
diff --git a/posix/fnmatch_loop.c b/posix/fnmatch_loop.c index 9445ed9c58..23eeffc79c 100644 --- a/posix/fnmatch_loop.c +++ b/posix/fnmatch_loop.c @@ -530,6 +530,14 @@ FCT (const CHAR *pattern, const CHAR *string, const CHAR *string_end, { /* Compare the byte sequence but only if this is not part of a range. */ + + /* The compiler might warn that idx may be + used uninitialized, however it will be + reached iff elem < table_size which means + that it was properly set in the loop + above. */ + DIAG_PUSH_NEEDS_COMMENT; + DIAG_IGNORE_Os_NEEDS_COMMENT (8, "-Wmaybe-uninitialized"); if (! is_range # if WIDE_CHAR_VERSION @@ -542,11 +550,19 @@ FCT (const CHAR *pattern, const CHAR *string, const CHAR *string_end, n += c1 - 1; goto matched; } + DIAG_POP_NEEDS_COMMENT; /* Get the collation sequence value. */ is_seqval = true; # if WIDE_CHAR_VERSION + /* The compile might warn that wextra may be + used uninitialized and similar to 'idx' + above it will be properly set by the loop. + */ + DIAG_PUSH_NEEDS_COMMENT; + DIAG_IGNORE_Os_NEEDS_COMMENT (8, "-Wmaybe-uninitialized"); cold = wextra[1 + wextra[0]]; + DIAG_POP_NEEDS_COMMENT; # else idx += 1 + extra[idx]; /* Adjust for the alignment. */ @@ -723,9 +739,24 @@ FCT (const CHAR *pattern, const CHAR *string, const CHAR *string_end, /* Get the collation sequence value. */ is_seqval = true; # if WIDE_CHAR_VERSION + /* The compiler might warn that wextra may + be used uninitialized, however it will + be reached iff elem < table_size which + means that it was properly set in the + loop above. */ + DIAG_PUSH_NEEDS_COMMENT; + DIAG_IGNORE_Os_NEEDS_COMMENT (8, "-Wmaybe-uninitialized"); cend = wextra[1 + wextra[0]]; + DIAG_POP_NEEDS_COMMENT; # else + /* The compile might warn that idx may + be used uninitialized and similar to + wextra above it will be properly set by + the loop. */ + DIAG_PUSH_NEEDS_COMMENT; + DIAG_IGNORE_Os_NEEDS_COMMENT (8, "-Wmaybe-uninitialized"); idx += 1 + extra[idx]; + DIAG_POP_NEEDS_COMMENT; /* Adjust for the alignment. */ idx = (idx + 3) & ~3; cend = *((int32_t *) &extra[idx]);