Re: format string is not a string literal

Pedro Alves
2015-02-26T09:52:44Z

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