diff mbox series

[RFC] Disable uninitialized warnings around va_arg(, long double)

Message ID 20180310203625.13515-1-zackw@panix.com
State New
Headers show
Series [RFC] Disable uninitialized warnings around va_arg(, long double) | expand

Commit Message

Zack Weinberg March 10, 2018, 8:36 p.m. UTC 
This is the only nontrivial problem I found in build-many-glibcs
testing of my "use more flags parameters instead of global bits in
stdio" patchset.

My changes to vfprintf in that patchset tickle a bug in the
PowerPC-SPE back end, leading to spurious "used uninitialized"
warnings--not on any code we control, but on a scratch variable
emitted as part of the expansion of __builtin_va_arg!  This is
<https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84772>, Jakub already
has a fix in hand, and I'm not sure whether it's worth bothering to
include these DIAG_IGNOREs in our tree.  I suppose it depends on
whether the bug manifests in compilers older than 7.3 as well.

	* stdio-common/vfprintf-internal.c: Include libc-diag.h.
	Disable -Wuninitialized and -Wmaybe-uninitialized around all
	uses of va_arg(..., long double), to work around GCC bug 84772.
---
 stdio-common/vfprintf-internal.c | 37 +++++++++++++++++++++++++++++++++----
 1 file changed, 33 insertions(+), 4 deletions(-)

Comments

Florian Weimer March 11, 2018, 10:34 a.m. UTC | #1 
* Zack Weinberg:

> My changes to vfprintf in that patchset tickle a bug in the
> PowerPC-SPE back end, leading to spurious "used uninitialized"
> warnings--not on any code we control, but on a scratch variable
> emitted as part of the expansion of __builtin_va_arg!  This is
> <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84772>, Jakub already
> has a fix in hand, and I'm not sure whether it's worth bothering to
> include these DIAG_IGNOREs in our tree.  I suppose it depends on
> whether the bug manifests in compilers older than 7.3 as well.

Will the default configuration of build-many-glibcs.py pick up a GCC
fix if it is backported to earlier release branches?  If yes, then I
don't think we need to work around it in the glibc tree.  The GCC will
eventually be able to powerpcspe users through a GCC release, and
until then, they can build with --disable-werror.
diff mbox series

Patch

diff --git a/stdio-common/vfprintf-internal.c b/stdio-common/vfprintf-internal.c
index d8716f4f07..6f7bd0d2da 100644
--- a/stdio-common/vfprintf-internal.c
+++ b/stdio-common/vfprintf-internal.c
@@ -31,6 +31,7 @@ 
 #include <locale/localeinfo.h>
 #include <stdio.h>
 #include <scratch_buffer.h>
+#include <libc-diag.h>
 
 /* This code is shared between the standard stdio implementation found
    in GNU C library and the libio implementation originally found in
@@ -774,9 +775,19 @@  static const uint8_t jump_table[] =
 					.is_binary128 = 0};		      \
 									      \
 	    if (is_long_double)						      \
-	      the_arg.pa_long_double = va_arg (ap, long double);	      \
+	      {								      \
+		/* GCC 7.3 has a bug on some architectures where */	      \
+		/* va_arg (ap, long double) produces spurious warnings.  */   \
+		/* https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84772 */      \
+		DIAG_PUSH_NEEDS_COMMENT;				      \
+		DIAG_IGNORE_NEEDS_COMMENT (7.3, "-Wuninitialized");	      \
+		DIAG_IGNORE_NEEDS_COMMENT (7.3, "-Wmaybe-uninitialized");     \
+		the_arg.pa_long_double = va_arg (ap, long double);	      \
+		DIAG_POP_NEEDS_COMMENT;					      \
+	      }								      \
 	    else							      \
 	      the_arg.pa_double = va_arg (ap, double);			      \
+									      \
 	    ptr = (const void *) &the_arg;				      \
 									      \
 	    function_done = __printf_fp (s, &info, &ptr);		      \
@@ -834,7 +845,16 @@  static const uint8_t jump_table[] =
 					.is_binary128 = 0};		      \
 									      \
 	    if (is_long_double)						      \
-	      the_arg.pa_long_double = va_arg (ap, long double);	      \
+	      {								      \
+		/* GCC 7.3 has a bug on some architectures where */	      \
+		/* va_arg (ap, long double) produces spurious warnings.  */   \
+		/* https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84772 */      \
+		DIAG_PUSH_NEEDS_COMMENT;				      \
+		DIAG_IGNORE_NEEDS_COMMENT (7.3, "-Wuninitialized");	      \
+		DIAG_IGNORE_NEEDS_COMMENT (7.3, "-Wmaybe-uninitialized");     \
+		the_arg.pa_long_double = va_arg (ap, long double);	      \
+		DIAG_POP_NEEDS_COMMENT;					      \
+	      }								      \
 	    else							      \
 	      the_arg.pa_double = va_arg (ap, double);			      \
 	    ptr = (const void *) &the_arg;				      \
@@ -844,7 +864,7 @@  static const uint8_t jump_table[] =
 	else								      \
 	  {								      \
 	    ptr = (const void *) &args_value[fspec->data_arg];		      \
-	    if (LDBL_IS_DBL)                                            \
+	    if (LDBL_IS_DBL)                                                  \
 	      fspec->info.is_long_double = 0;				      \
 	    /* Not supported by *printf functions.  */			      \
 	    fspec->info.is_binary128 = 0;				      \
@@ -1878,7 +1898,16 @@  printf_positional (FILE *s, const CHAR_T *format, int readonly_format,
 	    args_type[cnt] &= ~PA_FLAG_LONG_DOUBLE;
 	  }
 	else
-	  args_value[cnt].pa_long_double = va_arg (*ap_savep, long double);
+	  {
+	    /* GCC 7.3 has a bug on some architectures where
+	       va_arg (ap, long double) produces spurious warnings.
+	       https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84772 */
+	    DIAG_PUSH_NEEDS_COMMENT;
+	    DIAG_IGNORE_NEEDS_COMMENT (7.3, "-Wuninitialized");
+	    DIAG_IGNORE_NEEDS_COMMENT (7.3, "-Wmaybe-uninitialized");
+	    args_value[cnt].pa_long_double = va_arg (*ap_savep, long double);
+	    DIAG_POP_NEEDS_COMMENT;
+	  }
 	break;
       case PA_STRING:				/* All pointers are the same */
       case PA_WSTRING:			/* All pointers are the same */