unofficial mirror of libc-alpha@sourceware.org
 help / color / mirror / Atom feed
* Fix another -Os strcoll build issue
@ 2018-02-23 17:41 Joseph Myers
  2018-02-26 18:08 ` Ping " Joseph Myers
  2018-02-26 18:25 ` Adhemerval Zanella
  0 siblings, 2 replies; 3+ messages in thread
From: Joseph Myers @ 2018-02-23 17:41 UTC (permalink / raw)
  To: libc-alpha

While there are now clean -Os build and test results on x86_64 (given
my patch <https://sourceware.org/ml/libc-alpha/2018-02/msg00602.html>,
pending review), testing with -Os with build-many-glibcs.py shows the
build is still failing with -Os everywhere except for x86_64, x86 and
s390x.

There are a variety of different build failures, but the most common
seem to be in strcoll / wcscoll, similar to existing such cases where
DIAG_* are used to disable -Wmaybe-uninitialized.  There are various
different failures even within those functions.  This patch fixes one
particular case that seems quite common, where the warning appears at
the declarations of seq1 and seq2.

Tested with build-many-glibcs.py that this fixes the -Os build for
aarch64-linux-gnu with GCC 7.

2018-02-23  Joseph Myers  <joseph@codesourcery.com>

	* string/strcoll_l.c: Include <libc-diag.h>.
	(STRCOLL): Ignore -Wmaybe-uninitialized for -Os around
	declarations of seq1 and seq2.

diff --git a/string/strcoll_l.c b/string/strcoll_l.c
index 4a63c56..c001ff4 100644
--- a/string/strcoll_l.c
+++ b/string/strcoll_l.c
@@ -24,6 +24,7 @@
 #include <stdint.h>
 #include <string.h>
 #include <sys/param.h>
+#include <libc-diag.h>
 
 #ifndef STRING_TYPE
 # define STRING_TYPE char
@@ -291,7 +292,17 @@ STRCOLL (const STRING_TYPE *s1, const STRING_TYPE *s2, locale_t l)
 
   int result = 0, rule = 0;
 
+  /* With GCC 7 when compiling with -Os the compiler warns that
+     seq1.back_us and seq2.back_us might be used uninitialized.
+     Sometimes this warning appears at locations in locale/weightwc.h
+     where the actual use is, but on architectures other than x86_64,
+     x86 and s390x, a warning appears at the definitions of seq1 and
+     seq2.  This uninitialized use is impossible for the same reason
+     as described in comments in locale/weightwc.h.  */
+  DIAG_PUSH_NEEDS_COMMENT;
+  DIAG_IGNORE_Os_NEEDS_COMMENT (7, "-Wmaybe-uninitialized");
   coll_seq seq1, seq2;
+  DIAG_POP_NEEDS_COMMENT;
   seq1.len = 0;
   seq1.idxmax = 0;
   seq1.rule = 0;

-- 
Joseph S. Myers
joseph@codesourcery.com


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Ping Re: Fix another -Os strcoll build issue
  2018-02-23 17:41 Fix another -Os strcoll build issue Joseph Myers
@ 2018-02-26 18:08 ` Joseph Myers
  2018-02-26 18:25 ` Adhemerval Zanella
  1 sibling, 0 replies; 3+ messages in thread
From: Joseph Myers @ 2018-02-26 18:08 UTC (permalink / raw)
  To: libc-alpha

Ping.  This patch 
<https://sourceware.org/ml/libc-alpha/2018-02/msg00675.html> is pending 
review.

-- 
Joseph S. Myers
joseph@codesourcery.com


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: Fix another -Os strcoll build issue
  2018-02-23 17:41 Fix another -Os strcoll build issue Joseph Myers
  2018-02-26 18:08 ` Ping " Joseph Myers
@ 2018-02-26 18:25 ` Adhemerval Zanella
  1 sibling, 0 replies; 3+ messages in thread
From: Adhemerval Zanella @ 2018-02-26 18:25 UTC (permalink / raw)
  To: libc-alpha



On 23/02/2018 14:41, Joseph Myers wrote:
> While there are now clean -Os build and test results on x86_64 (given
> my patch <https://sourceware.org/ml/libc-alpha/2018-02/msg00602.html>,
> pending review), testing with -Os with build-many-glibcs.py shows the
> build is still failing with -Os everywhere except for x86_64, x86 and
> s390x.
> 
> There are a variety of different build failures, but the most common
> seem to be in strcoll / wcscoll, similar to existing such cases where
> DIAG_* are used to disable -Wmaybe-uninitialized.  There are various
> different failures even within those functions.  This patch fixes one
> particular case that seems quite common, where the warning appears at
> the declarations of seq1 and seq2.
> 
> Tested with build-many-glibcs.py that this fixes the -Os build for
> aarch64-linux-gnu with GCC 7.
> 
> 2018-02-23  Joseph Myers  <joseph@codesourcery.com>
> 
> 	* string/strcoll_l.c: Include <libc-diag.h>.
> 	(STRCOLL): Ignore -Wmaybe-uninitialized for -Os around
> 	declarations of seq1 and seq2.

Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

> 
> diff --git a/string/strcoll_l.c b/string/strcoll_l.c
> index 4a63c56..c001ff4 100644
> --- a/string/strcoll_l.c
> +++ b/string/strcoll_l.c
> @@ -24,6 +24,7 @@
>  #include <stdint.h>
>  #include <string.h>
>  #include <sys/param.h>
> +#include <libc-diag.h>
>  
>  #ifndef STRING_TYPE
>  # define STRING_TYPE char
> @@ -291,7 +292,17 @@ STRCOLL (const STRING_TYPE *s1, const STRING_TYPE *s2, locale_t l)
>  
>    int result = 0, rule = 0;
>  
> +  /* With GCC 7 when compiling with -Os the compiler warns that
> +     seq1.back_us and seq2.back_us might be used uninitialized.
> +     Sometimes this warning appears at locations in locale/weightwc.h
> +     where the actual use is, but on architectures other than x86_64,
> +     x86 and s390x, a warning appears at the definitions of seq1 and
> +     seq2.  This uninitialized use is impossible for the same reason
> +     as described in comments in locale/weightwc.h.  */
> +  DIAG_PUSH_NEEDS_COMMENT;
> +  DIAG_IGNORE_Os_NEEDS_COMMENT (7, "-Wmaybe-uninitialized");
>    coll_seq seq1, seq2;
> +  DIAG_POP_NEEDS_COMMENT;
>    seq1.len = 0;
>    seq1.idxmax = 0;
>    seq1.rule = 0;
> 


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2018-02-26 18:23 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-23 17:41 Fix another -Os strcoll build issue Joseph Myers
2018-02-26 18:08 ` Ping " Joseph Myers
2018-02-26 18:25 ` Adhemerval Zanella

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).