[FYI 1/2] Disable -Wformat-nonliteral in parts of printcmd.c

[FYI 1/2] Disable -Wformat-nonliteral in parts of printcmd.c

[Tom Tromey]

From: Simon Marchi <simon.marchi@ericsson.com>

commit 3322c5d9a1 ("Remove unneeded explicit .o targets") broke the
build with clang, because -Wformat-nonliteral was in fact needed.
This patch fixes the problem by introducing
DIAGNOSTIC_IGNORE_FORMAT_NONLITERAL and using it in printcmd.c.  This
seems preferable to reverting the patch because now the warning
suppression is more targeted.

gdb/ChangeLog
2018-09-05  Simon Marchi  <simon.marchi@ericsson.com>

	* printcmd.c (printf_c_string): Use
	DIAGNOSTIC_IGNORE_FORMAT_NONLITERAL.
	(printf_wide_c_string, printf_pointer, ui_printf): Likewise.

include/ChangeLog
2018-09-05  Simon Marchi  <simon.marchi@ericsson.com>

	* diagnostics.h (DIAGNOSTIC_IGNORE_FORMAT_NONLITERAL): New macro.
---
 gdb/ChangeLog         |  6 ++++++
 gdb/printcmd.c        | 33 +++++++++++++++++++++++++++++++++
 include/ChangeLog     |  4 ++++
 include/diagnostics.h | 12 ++++++++++++
 4 files changed, 55 insertions(+)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 084765d29c..d8ca6d3dab 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,9 @@
+2018-09-05  Simon Marchi  <simon.marchi@ericsson.com>
+
+	* printcmd.c (printf_c_string): Use
+	DIAGNOSTIC_IGNORE_FORMAT_NONLITERAL.
+	(printf_wide_c_string, printf_pointer, ui_printf): Likewise.
+
 2018-09-05  Tom Tromey  <tom@tromey.com>
 
 	* cli/cli-cmds.c (shell_escape, edit_command): Remove cast.
diff --git a/gdb/printcmd.c b/gdb/printcmd.c
index 1a3d9723d4..8c999188d7 100644
--- a/gdb/printcmd.c
+++ b/gdb/printcmd.c
@@ -2200,7 +2200,10 @@ printf_c_string (struct ui_file *stream, const char *format,
   tem = value_as_address (value);
   if (tem == 0)
     {
+      DIAGNOSTIC_PUSH
+      DIAGNOSTIC_IGNORE_FORMAT_NONLITERAL
       fprintf_filtered (stream, format, "(null)");
+      DIAGNOSTIC_POP
       return;
     }
 
@@ -2221,7 +2224,10 @@ printf_c_string (struct ui_file *stream, const char *format,
     read_memory (tem, str, j);
   str[j] = 0;
 
+  DIAGNOSTIC_PUSH
+  DIAGNOSTIC_IGNORE_FORMAT_NONLITERAL
   fprintf_filtered (stream, format, (char *) str);
+  DIAGNOSTIC_POP
 }
 
 /* Subroutine of ui_printf to simplify it.
@@ -2245,7 +2251,10 @@ printf_wide_c_string (struct ui_file *stream, const char *format,
   tem = value_as_address (value);
   if (tem == 0)
     {
+      DIAGNOSTIC_PUSH
+      DIAGNOSTIC_IGNORE_FORMAT_NONLITERAL
       fprintf_filtered (stream, format, "(null)");
+      DIAGNOSTIC_POP
       return;
     }
 
@@ -2272,7 +2281,10 @@ printf_wide_c_string (struct ui_file *stream, const char *format,
 			     &output, translit_char);
   obstack_grow_str0 (&output, "");
 
+  DIAGNOSTIC_PUSH
+  DIAGNOSTIC_IGNORE_FORMAT_NONLITERAL
   fprintf_filtered (stream, format, obstack_base (&output));
+  DIAGNOSTIC_POP
 }
 
 /* Subroutine of ui_printf to simplify it.
@@ -2400,13 +2412,19 @@ printf_pointer (struct ui_file *stream, const char *format,
       *fmt_p++ = 'l';
       *fmt_p++ = 'x';
       *fmt_p++ = '\0';
+      DIAGNOSTIC_PUSH
+      DIAGNOSTIC_IGNORE_FORMAT_NONLITERAL
       fprintf_filtered (stream, fmt, val);
+      DIAGNOSTIC_POP
     }
   else
     {
       *fmt_p++ = 's';
       *fmt_p++ = '\0';
+      DIAGNOSTIC_PUSH
+      DIAGNOSTIC_IGNORE_FORMAT_NONLITERAL
       fprintf_filtered (stream, fmt, "(nil)");
+      DIAGNOSTIC_POP
     }
 }
 
@@ -2507,8 +2525,11 @@ ui_printf (const char *arg, struct ui_file *stream)
 					 &output, translit_char);
 	      obstack_grow_str0 (&output, "");
 
+	      DIAGNOSTIC_PUSH
+	      DIAGNOSTIC_IGNORE_FORMAT_NONLITERAL
 	      fprintf_filtered (stream, current_substring,
                                 obstack_base (&output));
+	      DIAGNOSTIC_POP
 	    }
 	    break;
 	  case long_long_arg:
@@ -2516,7 +2537,10 @@ ui_printf (const char *arg, struct ui_file *stream)
 	    {
 	      long long val = value_as_long (val_args[i]);
 
+	      DIAGNOSTIC_PUSH
+	      DIAGNOSTIC_IGNORE_FORMAT_NONLITERAL
               fprintf_filtered (stream, current_substring, val);
+	      DIAGNOSTIC_POP
 	      break;
 	    }
 #else
@@ -2526,14 +2550,20 @@ ui_printf (const char *arg, struct ui_file *stream)
 	    {
 	      int val = value_as_long (val_args[i]);
 
+	      DIAGNOSTIC_PUSH
+	      DIAGNOSTIC_IGNORE_FORMAT_NONLITERAL
               fprintf_filtered (stream, current_substring, val);
+	      DIAGNOSTIC_POP
 	      break;
 	    }
 	  case long_arg:
 	    {
 	      long val = value_as_long (val_args[i]);
 
+	      DIAGNOSTIC_PUSH
+	      DIAGNOSTIC_IGNORE_FORMAT_NONLITERAL
               fprintf_filtered (stream, current_substring, val);
+	      DIAGNOSTIC_POP
 	      break;
 	    }
 	  /* Handles floating-point values.  */
@@ -2557,7 +2587,10 @@ ui_printf (const char *arg, struct ui_file *stream)
 	       have modified GCC to include -Wformat-security by
 	       default, which will warn here if there is no
 	       argument.  */
+	    DIAGNOSTIC_PUSH
+	    DIAGNOSTIC_IGNORE_FORMAT_NONLITERAL
 	    fprintf_filtered (stream, current_substring, 0);
+	    DIAGNOSTIC_POP
 	    break;
 	  default:
 	    internal_error (__FILE__, __LINE__,
diff --git a/include/ChangeLog b/include/ChangeLog
index 63fdde6349..c23c743738 100644
--- a/include/ChangeLog
+++ b/include/ChangeLog
@@ -1,3 +1,7 @@
+2018-09-05  Simon Marchi  <simon.marchi@ericsson.com>
+
+	* diagnostics.h (DIAGNOSTIC_IGNORE_FORMAT_NONLITERAL): New macro.
+
 2018-08-31  Alan Modra  <amodra@gmail.com>
 
 	* elf/ppc64.h (R_PPC64_REL16_HIGH, R_PPC64_REL16_HIGHA),
diff --git a/include/diagnostics.h b/include/diagnostics.h
index 9e9d1a832f..79e6779edf 100644
--- a/include/diagnostics.h
+++ b/include/diagnostics.h
@@ -59,6 +59,10 @@
 #  define DIAGNOSTIC_IGNORE_SWITCH_DIFFERENT_ENUM_TYPES \
    DIAGNOSTIC_IGNORE ("-Wenum-compare-switch")
 # endif
+
+# define DIAGNOSTIC_IGNORE_FORMAT_NONLITERAL \
+  DIAGNOSTIC_IGNORE ("-Wformat-nonliteral")
+
 #elif defined (__GNUC__) /* GCC */
 
 # define DIAGNOSTIC_IGNORE_UNUSED_FUNCTION \
@@ -66,6 +70,10 @@
 
 # define DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION \
   DIAGNOSTIC_IGNORE ("-Wstringop-truncation")
+
+# define DIAGNOSTIC_IGNORE_FORMAT_NONLITERAL \
+  DIAGNOSTIC_IGNORE ("-Wformat-nonliteral")
+
 #endif
 
 #ifndef DIAGNOSTIC_IGNORE_SELF_MOVE
@@ -92,4 +100,8 @@
 # define DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION
 #endif
 
+#ifndef DIAGNOSTIC_IGNORE_FORMAT_NONLITERAL
+# define DIAGNOSTIC_IGNORE_FORMAT_NONLITERAL
+#endif
+
 #endif /* DIAGNOSTICS_H */
-- 
2.17.1

[FYI 2/2] Make -Wformat-nonliteral work with gcc

[Tom Tromey]

After looking into why the build failed for Simon but not for me, we
found that the underlying cause was due to how gcc treats
-Wformat-nonliteral.  gcc requires -Wformat to be given first; but
warning.m4 was not doing this, so -Wformat-nonliteral was not being
used.

This patch changes warning.m4 to account gcc's requirement.

This then showed that the target-float.c build change in the earlier
Makefile patch was also incorrect.  Simon didn't see this in his
build, but gcc now points it out.  So, this patch fixes this problem
as well.

gdb/ChangeLog
2018-09-05  Tom Tromey  <tom@tromey.com>

	* warning.m4 (AM_GDB_WARNINGS): Add -Wformat when testing
	-Wformat-nonliteral.
	* target-float.c (host_float_ops<T>::to_string)
	(host_float_ops<T>::from_string): Use
	DIAGNOSTIC_IGNORE_FORMAT_NONLITERAL.
	* configure: Rebuild.

gdb/gdbserver/ChangeLog
2018-09-05  Tom Tromey  <tom@tromey.com>

	* configure: Rebuild.
---
 gdb/ChangeLog           |  9 +++++++++
 gdb/configure           | 11 ++++++++++-
 gdb/gdbserver/ChangeLog |  4 ++++
 gdb/gdbserver/configure | 11 ++++++++++-
 gdb/target-float.c      |  7 +++++++
 gdb/warning.m4          | 11 ++++++++++-
 6 files changed, 50 insertions(+), 3 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index d8ca6d3dab..69e3182ec3 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,12 @@
+2018-09-05  Tom Tromey  <tom@tromey.com>
+
+	* warning.m4 (AM_GDB_WARNINGS): Add -Wformat when testing
+	-Wformat-nonliteral.
+	* target-float.c (host_float_ops<T>::to_string)
+	(host_float_ops<T>::from_string): Use
+	DIAGNOSTIC_IGNORE_FORMAT_NONLITERAL.
+	* configure: Rebuild.
+
 2018-09-05  Simon Marchi  <simon.marchi@ericsson.com>
 
 	* printcmd.c (printf_c_string): Use
diff --git a/gdb/configure b/gdb/configure
index 270657103b..d92a256f1f 100755
--- a/gdb/configure
+++ b/gdb/configure
@@ -15427,7 +15427,10 @@ case "${host}" in
     build_warnings="$build_warnings -Wno-unknown-pragmas"
     # Solaris 11 <unistd.h> marks vfork deprecated.
     build_warnings="$build_warnings -Wno-deprecated-declarations" ;;
-  *) build_warnings="$build_warnings -Wformat-nonliteral" ;;
+  *)
+    # Note that gcc requires -Wformat for -Wformat-nonliteral to work,
+    # but there's a special case for this below.
+    build_warnings="$build_warnings -Wformat-nonliteral" ;;
 esac
 
 # Check whether --enable-build-warnings was given.
@@ -15483,6 +15486,12 @@ $as_echo_n "checking compiler warning flags... " >&6; }
 	case $w in
 	-Wno-*)
 		wtest=`echo $w | sed 's/-Wno-/-W/g'` ;;
+        -Wformat-nonliteral)
+		# gcc requires -Wformat before -Wformat-nonliteral
+		# will work, so stick them together.
+		w="-Wformat $w"
+		wtest="$w"
+		;;
 	*)
 		wtest=$w ;;
 	esac
diff --git a/gdb/gdbserver/ChangeLog b/gdb/gdbserver/ChangeLog
index cd0318c79a..0c5c32b22e 100644
--- a/gdb/gdbserver/ChangeLog
+++ b/gdb/gdbserver/ChangeLog
@@ -1,3 +1,7 @@
+2018-09-05  Tom Tromey  <tom@tromey.com>
+
+	* configure: Rebuild.
+
 2018-08-28  Simon Marchi  <simon.marchi@polymtl.ca>
 
 	PR build/23399
diff --git a/gdb/gdbserver/configure b/gdb/gdbserver/configure
index 7454cd8ad3..f5cbbaea78 100755
--- a/gdb/gdbserver/configure
+++ b/gdb/gdbserver/configure
@@ -7258,7 +7258,10 @@ case "${host}" in
     build_warnings="$build_warnings -Wno-unknown-pragmas"
     # Solaris 11 <unistd.h> marks vfork deprecated.
     build_warnings="$build_warnings -Wno-deprecated-declarations" ;;
-  *) build_warnings="$build_warnings -Wformat-nonliteral" ;;
+  *)
+    # Note that gcc requires -Wformat for -Wformat-nonliteral to work,
+    # but there's a special case for this below.
+    build_warnings="$build_warnings -Wformat-nonliteral" ;;
 esac
 
 # Check whether --enable-build-warnings was given.
@@ -7314,6 +7317,12 @@ $as_echo_n "checking compiler warning flags... " >&6; }
 	case $w in
 	-Wno-*)
 		wtest=`echo $w | sed 's/-Wno-/-W/g'` ;;
+        -Wformat-nonliteral)
+		# gcc requires -Wformat before -Wformat-nonliteral
+		# will work, so stick them together.
+		w="-Wformat $w"
+		wtest="$w"
+		;;
 	*)
 		wtest=$w ;;
 	esac
diff --git a/gdb/target-float.c b/gdb/target-float.c
index c2878cb0a0..a12f216b53 100644
--- a/gdb/target-float.c
+++ b/gdb/target-float.c
@@ -948,7 +948,11 @@ host_float_ops<T>::to_string (const gdb_byte *addr, const struct type *type,
 
   T host_float;
   from_target (type, addr, &host_float);
+
+  DIAGNOSTIC_PUSH
+  DIAGNOSTIC_IGNORE_FORMAT_NONLITERAL
   return string_printf (host_format.c_str (), host_float);
+  DIAGNOSTIC_POP
 }
 
 /* Parse string IN into a target floating-number of type TYPE and
@@ -977,7 +981,10 @@ host_float_ops<T>::from_string (gdb_byte *addr, const struct type *type,
     scan_format += scanf_length_modifier<T>::value;
   scan_format += "g%n";
 
+  DIAGNOSTIC_PUSH
+  DIAGNOSTIC_IGNORE_FORMAT_NONLITERAL
   num = sscanf (in.c_str (), scan_format.c_str(), &host_float, &n);
+  DIAGNOSTIC_POP
 
   /* The sscanf man page suggests not making any assumptions on the effect
      of %n on the result, so we don't.
diff --git a/gdb/warning.m4 b/gdb/warning.m4
index dd338493f9..82170acc80 100644
--- a/gdb/warning.m4
+++ b/gdb/warning.m4
@@ -58,7 +58,10 @@ case "${host}" in
     build_warnings="$build_warnings -Wno-unknown-pragmas"
     # Solaris 11 <unistd.h> marks vfork deprecated.
     build_warnings="$build_warnings -Wno-deprecated-declarations" ;;
-  *) build_warnings="$build_warnings -Wformat-nonliteral" ;;
+  *)
+    # Note that gcc requires -Wformat for -Wformat-nonliteral to work,
+    # but there's a special case for this below.
+    build_warnings="$build_warnings -Wformat-nonliteral" ;;
 esac
 
 AC_ARG_ENABLE(build-warnings,
@@ -106,6 +109,12 @@ then
 	case $w in
 	-Wno-*)
 		wtest=`echo $w | sed 's/-Wno-/-W/g'` ;;
+        -Wformat-nonliteral)
+		# gcc requires -Wformat before -Wformat-nonliteral
+		# will work, so stick them together.
+		w="-Wformat $w"
+		wtest="$w"
+		;;
 	*)
 		wtest=$w ;;
 	esac
-- 
2.17.1

[FYI 0/2] Fix -Wformat-nonliteral

[Tom Tromey]

Simon pointed out today that my earlier patch to remove some custom
rules from the Makefile had broken his build.

We tracked this difference down to a bug in warning.m4:
-Wformat-nonliteral was not being enabled for builds with gcc.  This
is why my build and the builders did not warn -- in fact the original
patch was incorrect.

This series fixes the problem using the diagnostic suppression code.
This seems a bit better because now the warning avoidance is more
targeted.

The first patch here is from Simon (I wrote the commit text and the
ChangeLog entry, in case they aren't up to par); but I'm committing it
on his behalf as he is traveling.

I'm checking this in.

Tom

Re: [FYI 1/2] Disable -Wformat-nonliteral in parts of printcmd.c

[Tom Tromey]

>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:

Tom> From: Simon Marchi <simon.marchi@ericsson.com>
Tom> commit 3322c5d9a1 ("Remove unneeded explicit .o targets") broke the
Tom> build with clang, because -Wformat-nonliteral was in fact needed.

Oops, this should read -Wno-format-nonliteral, I'll fix before pushing.

Tom