* printcmd.c (printf_command): Make format string checking

	stricter.  Add separate cases for long_arg, ptr_arg, and
	long_double_arg.
	* utils.c (xstrvprintf): Improve the error message issued
	for a bad format string.
	* Makefile.in (GDB_WARN_CFLAGS_NO_FORMAT, INTERNAL_CFLAGS_BASE):
	New variables.
	(gnu-v3-abi.o, monitor.o, procfs.o, linux-thread-db.o): Remove
	$(NO_WERROR_CFLAGS).
	(printcmd.o): Likewise.  Use $(GDB_WARN_CFLAGS_NO_FORMAT) and
	enable -Werror.

4 files changed, 184 insertions, 48 deletions

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index a1ae60a..0fa726d 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,19 @@
 2006-02-01  Daniel Jacobowitz  <dan@codesourcery.com>
 
+	* printcmd.c (printf_command): Make format string checking
+	stricter.  Add separate cases for long_arg, ptr_arg, and
+	long_double_arg.
+	* utils.c (xstrvprintf): Improve the error message issued
+	for a bad format string.
+	* Makefile.in (GDB_WARN_CFLAGS_NO_FORMAT, INTERNAL_CFLAGS_BASE):
+	New variables.
+	(gnu-v3-abi.o, monitor.o, procfs.o, linux-thread-db.o): Remove
+	$(NO_WERROR_CFLAGS).
+	(printcmd.o): Likewise.  Use $(GDB_WARN_CFLAGS_NO_FORMAT) and
+	enable -Werror.
+
+2006-02-01  Daniel Jacobowitz  <dan@codesourcery.com>
+
 	* Makefile.in (remote.o): Update.
 	* remote.c (show_packet_config_cmd): Shorten messages.
 	(remote_set_cmdlist, remote_show_cmdlist): Make file-static.
diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 88c8f11..14188bd 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -132,6 +132,8 @@ WERROR_CFLAGS = @WERROR_CFLAGS@
 GDB_WARN_CFLAGS = $(WARN_CFLAGS)
 GDB_WERROR_CFLAGS = $(WERROR_CFLAGS)
 
+GDB_WARN_CFLAGS_NO_FORMAT = `echo " $(GDB_WARN_CFLAGS) " | sed "s/ -Wformat-nonliteral / /g"`
+
 # Where is the INTL library?  Typically in ../intl.
 INTL_DIR = ../intl
 INTL = @INTLLIBS@
@@ -344,12 +346,12 @@ CFLAGS = @CFLAGS@
 CXXFLAGS = -g -O
 
 # INTERNAL_CFLAGS is the aggregate of all other *CFLAGS macros.
-INTERNAL_WARN_CFLAGS = \
+INTERNAL_CFLAGS_BASE = \
 	$(CFLAGS) $(GLOBAL_CFLAGS) $(PROFILE_CFLAGS) \
 	$(GDB_CFLAGS) $(OPCODES_CFLAGS) $(READLINE_CFLAGS) \
 	$(BFD_CFLAGS) $(INCLUDE_CFLAGS) \
-	$(INTL_CFLAGS) $(ENABLE_CFLAGS) \
-	$(GDB_WARN_CFLAGS)
+	$(INTL_CFLAGS) $(ENABLE_CFLAGS)
+INTERNAL_WARN_CFLAGS = $(INTERNAL_CFLAGS_BASE) $(GDB_WARN_CFLAGS)
 INTERNAL_CFLAGS = $(INTERNAL_WARN_CFLAGS) $(GDB_WERROR_CFLAGS)
 
 # LDFLAGS is specifically reserved for setting from the command line
@@ -1479,8 +1481,7 @@ main.o: main.c
 # return types - "enum gnu_v3_dtor_kinds" vs "enum ctor_kinds" -
 # conflict.
 gnu-v3-abi.o: $(srcdir)/gnu-v3-abi.c
-	$(CC) -c $(INTERNAL_WARN_CFLAGS) $(NO_WERROR_CFLAGS) \
-		$(srcdir)/gnu-v3-abi.c
+	$(CC) -c $(INTERNAL_WARN_CFLAGS) $(srcdir)/gnu-v3-abi.c
 
 # FIXME: cagney/2003-08-10: "monitor.c" gets -Wformat-nonliteral
 # errors.  It turns out that that is the least of monitor.c's
@@ -1489,25 +1490,23 @@ gnu-v3-abi.o: $(srcdir)/gnu-v3-abi.c
 # definitly will not work.  "monitor.c" needs to be rewritten so that
 # it doesn't use format strings and instead uses callbacks.
 monitor.o: $(srcdir)/monitor.c
-	$(CC) -c $(INTERNAL_WARN_CFLAGS) $(NO_WERROR_CFLAGS) $(srcdir)/monitor.c
+	$(CC) -c $(INTERNAL_WARN_CFLAGS) $(srcdir)/monitor.c
 
-# FIXME: cagney/2003-08-10: Do not try to build "printcmd.c" with
-# -Wformat-nonliteral.  It needs to be overhauled so that it doesn't
-# pass user input strings as the format parameter to host printf
-# function calls.
+# Do not try to build "printcmd.c" with -Wformat-nonliteral.  It manually
+# checks format strings.
 printcmd.o: $(srcdir)/printcmd.c
-	$(CC) -c $(INTERNAL_WARN_CFLAGS) $(NO_WERROR_CFLAGS) $(srcdir)/printcmd.c
+	$(CC) -c $(INTERNAL_CFLAGS_BASE) $(GDB_WARN_CFLAGS_NO_FORMAT) \
+	   $(GDB_WERROR_CFLAGS) $(srcdir)/printcmd.c
 
 # FIXME: Procfs.o gets -Wformat errors because things like pid_t don't
 # match output format strings.
 procfs.o: $(srcdir)/procfs.c
-	$(CC) -c $(INTERNAL_WARN_CFLAGS) $(NO_WERROR_CFLAGS) $(srcdir)/procfs.c
+	$(CC) -c $(INTERNAL_WARN_CFLAGS) $(srcdir)/procfs.c
 
 # FIXME: Thread-db.o gets warnings because the definitions of the register
 # sets are different from kernel to kernel.
 linux-thread-db.o: $(srcdir)/linux-thread-db.c
-	$(CC) -c $(INTERNAL_WARN_CFLAGS) $(NO_WERROR_CFLAGS) \
-		$(srcdir)/linux-thread-db.c
+	$(CC) -c $(INTERNAL_WARN_CFLAGS) $(srcdir)/linux-thread-db.c
 
 v850ice.o: $(srcdir)/v850ice.c
 	$(CC) -c $(INTERNAL_CFLAGS) $(IDE_CFLAGS) $(ITCL_CFLAGS) \
diff --git a/gdb/printcmd.c b/gdb/printcmd.c
index b6f3a7d..962c269 100644
--- a/gdb/printcmd.c
+++ b/gdb/printcmd.c
@@ -1836,13 +1836,13 @@ printf_command (char *arg, int from_tty)
 
     enum argclass
       {
-	no_arg, int_arg, string_arg, double_arg, long_long_arg
+	int_arg, long_arg, long_long_arg, ptr_arg, string_arg,
+	double_arg, long_double_arg
       };
     enum argclass *argclass;
     enum argclass this_argclass;
     char *last_arg;
     int nargs_wanted;
-    int lcount;
     int i;
 
     argclass = (enum argclass *) alloca (strlen (s) * sizeof *argclass);
@@ -1852,23 +1852,136 @@ printf_command (char *arg, int from_tty)
     while (*f)
       if (*f++ == '%')
 	{
-	  lcount = 0;
-	  while (strchr ("0123456789.hlL-+ #", *f))
+	  int seen_hash = 0, seen_zero = 0, lcount = 0, seen_prec = 0;
+	  int seen_space = 0, seen_plus = 0;
+	  int seen_big_l = 0, seen_h = 0;
+	  int bad = 0;
+
+	  /* Check the validity of the format specifier, and work
+	     out what argument it expects.  We only accept C89
+	     format strings, with the exception of long long (which
+	     we autoconf for).  */
+
+	  /* Skip over "%%".  */
+	  if (*f == '%')
 	    {
-	      if (*f == 'l' || *f == 'L')
-		lcount++;
 	      f++;
+	      continue;
 	    }
+
+	  /* The first part of a format specifier is a set of flag
+	     characters.  */
+	  while (strchr ("0-+ #", *f))
+	    {
+	      if (*f == '#')
+		seen_hash = 1;
+	      else if (*f == '0')
+		seen_zero = 1;
+	      else if (*f == ' ')
+		seen_space = 1;
+	      else if (*f == '+')
+		seen_plus = 1;
+	      f++;
+	    }
+
+	  /* The next part of a format specifier is a width.  */
+	  while (strchr ("0123456789", *f))
+	    f++;
+
+	  /* The next part of a format specifier is a precision.  */
+	  if (*f == '.')
+	    {
+	      seen_prec = 1;
+	      f++;
+	      while (strchr ("0123456789", *f))
+		f++;
+	    }
+
+	  /* The next part of a format specifier is a length modifier.  */
+	  if (*f == 'h')
+	    {
+	      seen_h = 1;
+	      f++;
+	    }
+	  else if (*f == 'l')
+	    {
+	      f++;
+	      lcount++;
+	      if (*f == 'l')
+		{
+		  f++;
+		  lcount++;
+		}
+	    }
+	  else if (*f == 'L')
+	    {
+	      seen_big_l = 1;
+	      f++;
+	    }
+
 	  switch (*f)
 	    {
+	    case 'u':
+	      if (seen_hash)
+		bad = 1;
+	      /* FALLTHROUGH */
+
+	    case 'o':
+	    case 'x':
+	    case 'X':
+	      if (seen_space || seen_plus)
+		bad = 1;
+	      /* FALLTHROUGH */
+
+	    case 'd':
+	    case 'i':
+	      if (lcount == 0)
+		this_argclass = int_arg;
+	      else if (lcount == 1)
+		this_argclass = long_arg;
+	      else
+		this_argclass = long_long_arg;
+
+	      if (seen_big_l)
+		bad = 1;
+	      break;
+
+	    case 'c':
+	      this_argclass = int_arg;
+	      if (lcount || seen_h || seen_big_l)
+		bad = 1;
+	      if (seen_prec || seen_zero || seen_space || seen_plus)
+		bad = 1;
+	      break;
+
+	    case 'p':
+	      this_argclass = ptr_arg;
+	      if (lcount || seen_h || seen_big_l)
+		bad = 1;
+	      if (seen_prec || seen_zero || seen_space || seen_plus)
+		bad = 1;
+	      break;
+
 	    case 's':
 	      this_argclass = string_arg;
+	      if (lcount || seen_h || seen_big_l)
+		bad = 1;
+	      if (seen_zero || seen_space || seen_plus)
+		bad = 1;
 	      break;
 
 	    case 'e':
 	    case 'f':
 	    case 'g':
-	      this_argclass = double_arg;
+	    case 'E':
+	    case 'G':
+	      if (seen_big_l)
+		this_argclass = long_double_arg;
+	      else
+		this_argclass = double_arg;
+
+	      if (lcount || seen_h)
+		bad = 1;
 	      break;
 
 	    case '*':
@@ -1877,26 +1990,23 @@ printf_command (char *arg, int from_tty)
 	    case 'n':
 	      error (_("Format specifier `n' not supported in printf"));
 
-	    case '%':
-	      this_argclass = no_arg;
-	      break;
+	    case '\0':
+	      error (_("Incomplete format specifier at end of format string"));
 
 	    default:
-	      if (lcount > 1)
-		this_argclass = long_long_arg;
-	      else
-		this_argclass = int_arg;
-	      break;
+	      error (_("Unrecognized format specifier '%c' in printf"), *f);
 	    }
+
+	  if (bad)
+	    error (_("Inappropriate modifiers to format specifier '%c' in printf"),
+		   *f);
+
 	  f++;
-	  if (this_argclass != no_arg)
-	    {
-	      strncpy (current_substring, last_arg, f - last_arg);
-	      current_substring += f - last_arg;
-	      *current_substring++ = '\0';
-	      last_arg = f;
-	      argclass[nargs_wanted++] = this_argclass;
-	    }
+	  strncpy (current_substring, last_arg, f - last_arg);
+	  current_substring += f - last_arg;
+	  *current_substring++ = '\0';
+	  last_arg = f;
+	  argclass[nargs_wanted++] = this_argclass;
 	}
 
     /* Now, parse all arguments and evaluate them.
@@ -1970,6 +2080,16 @@ printf_command (char *arg, int from_tty)
 	      printf_filtered (current_substring, val);
 	      break;
 	    }
+	  case long_double_arg:
+#ifdef HAVE_LONG_DOUBLE
+	    {
+	      long double val = value_as_double (val_args[i]);
+	      printf_filtered (current_substring, val);
+	      break;
+	    }
+#else
+	    error (_("long double not supported in printf"));
+#endif
 	  case long_long_arg:
 #if defined (CC_HAS_LONG_LONG) && defined (PRINTF_HAS_LONG_LONG)
 	    {
@@ -1982,7 +2102,12 @@ printf_command (char *arg, int from_tty)
 #endif
 	  case int_arg:
 	    {
-	      /* FIXME: there should be separate int_arg and long_arg.  */
+	      int val = value_as_long (val_args[i]);
+	      printf_filtered (current_substring, val);
+	      break;
+	    }
+	  case long_arg:
+	    {
 	      long val = value_as_long (val_args[i]);
 	      printf_filtered (current_substring, val);
 	      break;
diff --git a/gdb/utils.c b/gdb/utils.c
index fdebbdc..dff5cb8 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -1,8 +1,8 @@
 /* General utility routines for GDB, the GNU debugger.
 
    Copyright (C) 1986, 1988, 1989, 1990, 1991, 1992, 1993, 1994, 1995,
-   1996, 1997, 1998, 1999, 2000, 2001, 2002, 2003, 2004, 2005 Free
-   Software Foundation, Inc.
+   1996, 1997, 1998, 1999, 2000, 2001, 2002, 2003, 2004, 2005, 2006
+   Free Software Foundation, Inc.
 
    This file is part of GDB.
 
@@ -1069,14 +1069,12 @@ xstrvprintf (const char *format, va_list ap)
 {
   char *ret = NULL;
   int status = vasprintf (&ret, format, ap);
-  /* NULL is returned when there was a memory allocation problem.  */
-  if (ret == NULL)
-    nomem (0);
-  /* A negative status (the printed length) with a non-NULL buffer
-     should never happen, but just to be sure.  */
-  if (status < 0)
-    internal_error (__FILE__, __LINE__,
-		    _("vasprintf call failed (errno %d)"), errno);
+  /* NULL is returned when there was a memory allocation problem, or
+     any other error (for instance, a bad format string).  A negative
+     status (the printed length) with a non-NULL buffer should never
+     happen, but just to be sure.  */
+  if (ret == NULL || status < 0)
+    internal_error (__FILE__, __LINE__, _("vasprintf call failed"));
   return ret;
 }