Re: format string is not a string literal

[Pedro Alves <palves at redhat dot com>]

On 02/26/2015 12:41 AM, Jack Howarth wrote:

>>> I think the warning is relevant. If you instruct the compiler that
>>> inferior_debug takes a format string and format arguments (using a
>>> format attribute, as mentioned by Richard in the bug report), then it
>>> can check if the callers are doing something wrong.
>>>
>>> In the case of inferior_debug, the attribute should be
>>>    __attribute__((format (printf, 2, 3)))
>>>
>>> By adding the attribute, you get nice warnings of this kind:
>>>
>>> test.c: In function âmainâ:
>>> test.c:17:2: warning: too many arguments for format [-Wformat-extra-args]
>>>  inferior_debug (1, "pouet %d", 2, "hello");
>>>

Agreed.  Jack, can you try this patch please?  It applies cleanly
against both mainline and the 7.9 branch.  I think it fixes all the
warnings you reported.

--------
>From bd87d33c7b2236857f2c32ce5da85aae474ce7bd Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Thu, 26 Feb 2015 09:47:21 +0000
Subject: [PATCH] Add ATTRIBUTE_PRINTF attributes, and fix fallout

gdb/ChangeLog:
2015-02-26  Pedro Alves  <palves@redhat.com>

	* auto-load.h (file_is_auto_load_safe): Add ATTRIBUTE_PRINTF.
	* compile/compile-loc2c.c (pushf, unary, binary): Add
	ATTRIBUTE_PRINTF.
	(do_compile_dwarf_expr_to_c): Pass string literal as format string
	to pushf.
	(BINARY): Pass string literal as format string to 'binary'.
	* compile/compile-object-load.c (link_callbacks_einfo): Add
	ATTRIBUTE_PRINTF.
	* complaints.c (vcomplaint): Pass argument FMT directly to
	printf-like functions instead of complaint->fmt.
	* darwin-nat.c (inferior_debug): Add ATTRIBUTE_PRINTF.
---
 gdb/auto-load.h                   |  3 ++-
 gdb/compile/compile-loc2c.c       | 19 ++++++++++++++-----
 gdb/compile/compile-object-load.c |  3 +++
 gdb/complaints.c                  | 13 ++++++++-----
 gdb/darwin-nat.c                  |  3 +++
 gdb/guile/guile-internal.h        |  3 ++-
 gdb/remote.c                      |  3 +++
 7 files changed, 35 insertions(+), 12 deletions(-)

diff --git a/gdb/auto-load.h b/gdb/auto-load.h
index 1d10e91..d241946 100644
--- a/gdb/auto-load.h
+++ b/gdb/auto-load.h
@@ -45,7 +45,8 @@ extern struct cmd_list_element **auto_load_show_cmdlist_get (void);
 extern struct cmd_list_element **auto_load_info_cmdlist_get (void);
 
 extern int file_is_auto_load_safe (const char *filename,
-				   const char *debug_fmt, ...);
+				   const char *debug_fmt, ...)
+  ATTRIBUTE_PRINTF (2, 3);
 
 extern int auto_load_gdb_scripts_enabled
   (const struct extension_language_defn *extlang);
diff --git a/gdb/compile/compile-loc2c.c b/gdb/compile/compile-loc2c.c
index 3f43e58..6a3615d 100644
--- a/gdb/compile/compile-loc2c.c
+++ b/gdb/compile/compile-loc2c.c
@@ -443,6 +443,9 @@ push (int indent, struct ui_file *stream, ULONGEST l)
 /* Emit code to push an arbitrary expression.  This works like
    printf.  */
 
+static void pushf (int indent, struct ui_file *stream, const char *format, ...)
+  ATTRIBUTE_PRINTF (3, 4);
+
 static void
 pushf (int indent, struct ui_file *stream, const char *format, ...)
 {
@@ -460,6 +463,9 @@ pushf (int indent, struct ui_file *stream, const char *format, ...)
 /* Emit code for a unary expression -- one which operates in-place on
    the top-of-stack.  This works like printf.  */
 
+static void unary (int indent, struct ui_file *stream, const char *format, ...)
+  ATTRIBUTE_PRINTF (3, 4);
+
 static void
 unary (int indent, struct ui_file *stream, const char *format, ...)
 {
@@ -474,6 +480,8 @@ unary (int indent, struct ui_file *stream, const char *format, ...)
 
 /* Emit code for a unary expression -- one which uses the top two
    stack items, popping the topmost one.  This works like printf.  */
+static void binary (int indent, struct ui_file *stream, const char *format, ...)
+  ATTRIBUTE_PRINTF (3, 4);
 
 static void
 binary (int indent, struct ui_file *stream, const char *format, ...)
@@ -651,7 +659,7 @@ do_compile_dwarf_expr_to_c (int indent, struct ui_file *stream,
   fprintfi_filtered (indent, stream, "int __gdb_tos = -1;\n");
 
   if (initial != NULL)
-    pushf (indent, stream, core_addr_to_string (*initial));
+    pushf (indent, stream, "%s", core_addr_to_string (*initial));
 
   while (op_ptr < op_end)
     {
@@ -911,7 +919,8 @@ do_compile_dwarf_expr_to_c (int indent, struct ui_file *stream,
 
 	case DW_OP_pick:
 	  offset = *op_ptr++;
-	  pushf (indent, stream, "__gdb_stack[__gdb_tos - %d]", offset);
+	  pushf (indent, stream, "__gdb_stack[__gdb_tos - %s]",
+		 plongest (offset));
 	  break;
 
 	case DW_OP_swap:
@@ -1000,8 +1009,8 @@ do_compile_dwarf_expr_to_c (int indent, struct ui_file *stream,
 	  break;
 
 #define BINARY(OP)							\
-	  binary (indent, stream, ("__gdb_stack[__gdb_tos-1] " #OP	\
-				   " __gdb_stack[__gdb_tos]"));	\
+	  binary (indent, stream, "%s", "__gdb_stack[__gdb_tos-1] " #OP \
+				   " __gdb_stack[__gdb_tos]");	\
 	  break
 
 	case DW_OP_and:
@@ -1076,7 +1085,7 @@ do_compile_dwarf_expr_to_c (int indent, struct ui_file *stream,
 					    addr_size,
 					    cfa_start, cfa_end,
 					    &text_offset, per_cu);
-		pushf (indent, stream, cfa_name);
+		pushf (indent, stream, "%s", cfa_name);
 	      }
 	  }
 
diff --git a/gdb/compile/compile-object-load.c b/gdb/compile/compile-object-load.c
index 5903f18..7d2b683 100644
--- a/gdb/compile/compile-object-load.c
+++ b/gdb/compile/compile-object-load.c
@@ -224,6 +224,9 @@ link_callbacks_unattached_reloc (struct bfd_link_info *link_info,
 
 /* Helper for link_callbacks callbacks vector.  */
 
+static void link_callbacks_einfo (const char *fmt, ...)
+  ATTRIBUTE_PRINTF (1, 2);
+
 static void
 link_callbacks_einfo (const char *fmt, ...)
 {
diff --git a/gdb/complaints.c b/gdb/complaints.c
index 7e52656..ab36d9b 100644
--- a/gdb/complaints.c
+++ b/gdb/complaints.c
@@ -183,21 +183,24 @@ vcomplaint (struct complaints **c, const char *file,
   else
     series = complaints->series;
 
+  /* Use FMT from here on instead of complaint->fmt, to avoid "format
+     string is not a string literal" warnings.  */
+  gdb_assert (complaint->fmt == fmt);
+
   if (complaint->file != NULL)
-    internal_vwarning (complaint->file, complaint->line, 
-		       complaint->fmt, args);
+    internal_vwarning (complaint->file, complaint->line, fmt, args);
   else if (deprecated_warning_hook)
-    (*deprecated_warning_hook) (complaint->fmt, args);
+    (*deprecated_warning_hook) (fmt, args);
   else
     {
       if (complaints->explanation == NULL)
 	/* A [v]warning() call always appends a newline.  */
-	vwarning (complaint->fmt, args);
+	vwarning (fmt, args);
       else
 	{
 	  char *msg;
 	  struct cleanup *cleanups;
-	  msg = xstrvprintf (complaint->fmt, args);
+	  msg = xstrvprintf (fmt, args);
 	  cleanups = make_cleanup (xfree, msg);
 	  wrap_here ("");
 	  if (series != SUBSEQUENT_MESSAGE)
diff --git a/gdb/darwin-nat.c b/gdb/darwin-nat.c
index f9481c7..dfce179 100644
--- a/gdb/darwin-nat.c
+++ b/gdb/darwin-nat.c
@@ -171,6 +171,9 @@ __attribute__ ((section ("__TEXT,__info_plist"),used)) =
   "</dict>\n"
   "</plist>\n";
 
+static void inferior_debug (int level, const char *fmt, ...)
+  ATTRIBUTE_PRINTF (2, 3);
+
 static void
 inferior_debug (int level, const char *fmt, ...)
 {
diff --git a/gdb/guile/guile-internal.h b/gdb/guile/guile-internal.h
index 968b4d3..86fc2f6 100644
--- a/gdb/guile/guile-internal.h
+++ b/gdb/guile/guile-internal.h
@@ -484,7 +484,8 @@ extern char *gdbscm_scm_to_c_string (SCM string);
 
 extern SCM gdbscm_scm_from_c_string (const char *string);
 
-extern SCM gdbscm_scm_from_printf (const char *format, ...);
+extern SCM gdbscm_scm_from_printf (const char *format, ...)
+    ATTRIBUTE_PRINTF (1, 2);
 
 extern char *gdbscm_scm_to_string (SCM string, size_t *lenp,
 				   const char *charset,
diff --git a/gdb/remote.c b/gdb/remote.c
index e971a29..0b0770f 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -6973,6 +6973,9 @@ remote_read_bytes (struct target_ops *ops, CORE_ADDR memaddr,
    FORMAT and the remaining arguments, then gets the reply.  Returns
    whether the packet was a success, a failure, or unknown.  */
 
+static enum packet_result remote_send_printf (const char *format, ...)
+  ATTRIBUTE_PRINTF (1, 2);
+
 static enum packet_result
 remote_send_printf (const char *format, ...)
 {
-- 
1.9.3