From 46e9880c625972c3a9ac7c0b53bc1a30c2989b7c Mon Sep 17 00:00:00 2001 From: Daniel Jacobowitz Date: Thu, 2 Feb 2006 02:26:48 +0000 Subject: [PATCH] * 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. --- gdb/ChangeLog | 14 +++++ gdb/Makefile.in | 27 +++++---- gdb/printcmd.c | 173 ++++++++++++++++++++++++++++++++++++++++++++++++-------- gdb/utils.c | 18 +++--- 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 + * 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 + * 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; } -- 2.7.4