Fix -Os strcoll, wcscoll, build (bug 21313)

Message ID alpine.DEB.2.20.1802012037470.13065@digraph.polyomino.org.uk
State New, archived
Headers

Commit Message

Joseph Myers Feb. 1, 2018, 8:38 p.m. UTC
  The findidx functions used in implementing strcoll / wcscoll already
use DIAG_IGNORE_Os_NEEDS_COMMENT for spurious -Wmaybe-uninitialized
warnings that appear with -Os.  In building with GCC 7 for x86_64 with
-Os, I find there are additional such warnings, for the same structure
elements, which are spurious for the same reasons given in the
existing comments (and this was also reported for MIPS with GCC 5 in
bug 21313).  This patch adds corresponding uses of DIAG_* in the
places that get the additional warnings.

Tested for x86_64 with -Os that this eliminates those warnings and so
allows the build to progress further.

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

	[BZ #21313]
	* locale/weight.h (findidx): Disable -Wmaybe-uninitialized for -Os
	in another place.
	* locale/weightwc.h (findidx): Likewise.
  

Comments

Joseph Myers Feb. 6, 2018, 6:03 p.m. UTC | #1
Ping.  This patch 
<https://sourceware.org/ml/libc-alpha/2018-02/msg00038.html> is pending 
review.
  
Adhemerval Zanella Feb. 7, 2018, 12:14 p.m. UTC | #2
On 01/02/2018 18:38, Joseph Myers wrote:
> The findidx functions used in implementing strcoll / wcscoll already
> use DIAG_IGNORE_Os_NEEDS_COMMENT for spurious -Wmaybe-uninitialized
> warnings that appear with -Os.  In building with GCC 7 for x86_64 with
> -Os, I find there are additional such warnings, for the same structure
> elements, which are spurious for the same reasons given in the
> existing comments (and this was also reported for MIPS with GCC 5 in
> bug 21313).  This patch adds corresponding uses of DIAG_* in the
> places that get the additional warnings.
> 
> Tested for x86_64 with -Os that this eliminates those warnings and so
> allows the build to progress further.
> 
> 2018-02-01  Joseph Myers  <joseph@codesourcery.com>
> 
> 	[BZ #21313]
> 	* locale/weight.h (findidx): Disable -Wmaybe-uninitialized for -Os
> 	in another place.
> 	* locale/weightwc.h (findidx): Likewise.

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

> 
> diff --git a/locale/weight.h b/locale/weight.h
> index e2fdae1..6028d35 100644
> --- a/locale/weight.h
> +++ b/locale/weight.h
> @@ -132,7 +132,15 @@ findidx (const int32_t *table,
>  	      do
>  		{
>  		  offset <<= 8;
> +		  /* With GCC 7 when compiling with -Os the compiler
> +		     warns that seq1.back_us and seq2.back_us, which
> +		     become usrc, might be used uninitialized.  This
> +		     is impossible for the same reason as described
> +		     above.  */
> +		  DIAG_PUSH_NEEDS_COMMENT;
> +		  DIAG_IGNORE_Os_NEEDS_COMMENT (7, "-Wmaybe-uninitialized");
>  		  offset += usrc[cnt] - cp[cnt];
> +		  DIAG_POP_NEEDS_COMMENT;
>  		}
>  	      while (++cnt < nhere);
>  	    }
> diff --git a/locale/weightwc.h b/locale/weightwc.h
> index 682d2b4..ac25ba9 100644
> --- a/locale/weightwc.h
> +++ b/locale/weightwc.h
> @@ -90,9 +90,16 @@ findidx (const int32_t *table,
>  	  size_t cnt;
>  	  size_t offset;
>  
> +	  /* With GCC 7 when compiling with -Os the compiler warns
> +	     that seq1.back_us and seq2.back_us, which become usrc,
> +	     might be used uninitialized.  This is impossible for the
> +	     same reason as described above.  */
> +	  DIAG_PUSH_NEEDS_COMMENT;
> +	  DIAG_IGNORE_Os_NEEDS_COMMENT (7, "-Wmaybe-uninitialized");
>  	  for (cnt = 0; cnt < nhere - 1 && cnt < len; ++cnt)
>  	    if (cp[cnt] != usrc[cnt])
>  	      break;
> +	  DIAG_POP_NEEDS_COMMENT;
>  
>  	  if (cnt < nhere - 1)
>  	    {
>
  

Patch

diff --git a/locale/weight.h b/locale/weight.h
index e2fdae1..6028d35 100644
--- a/locale/weight.h
+++ b/locale/weight.h
@@ -132,7 +132,15 @@  findidx (const int32_t *table,
 	      do
 		{
 		  offset <<= 8;
+		  /* With GCC 7 when compiling with -Os the compiler
+		     warns that seq1.back_us and seq2.back_us, which
+		     become usrc, might be used uninitialized.  This
+		     is impossible for the same reason as described
+		     above.  */
+		  DIAG_PUSH_NEEDS_COMMENT;
+		  DIAG_IGNORE_Os_NEEDS_COMMENT (7, "-Wmaybe-uninitialized");
 		  offset += usrc[cnt] - cp[cnt];
+		  DIAG_POP_NEEDS_COMMENT;
 		}
 	      while (++cnt < nhere);
 	    }
diff --git a/locale/weightwc.h b/locale/weightwc.h
index 682d2b4..ac25ba9 100644
--- a/locale/weightwc.h
+++ b/locale/weightwc.h
@@ -90,9 +90,16 @@  findidx (const int32_t *table,
 	  size_t cnt;
 	  size_t offset;
 
+	  /* With GCC 7 when compiling with -Os the compiler warns
+	     that seq1.back_us and seq2.back_us, which become usrc,
+	     might be used uninitialized.  This is impossible for the
+	     same reason as described above.  */
+	  DIAG_PUSH_NEEDS_COMMENT;
+	  DIAG_IGNORE_Os_NEEDS_COMMENT (7, "-Wmaybe-uninitialized");
 	  for (cnt = 0; cnt < nhere - 1 && cnt < len; ++cnt)
 	    if (cp[cnt] != usrc[cnt])
 	      break;
+	  DIAG_POP_NEEDS_COMMENT;
 
 	  if (cnt < nhere - 1)
 	    {