From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 12782 invoked by alias); 26 Feb 2015 09:52:44 -0000 Mailing-List: contact gdb-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-owner@sourceware.org Received: (qmail 12756 invoked by uid 89); 26 Feb 2015 09:52:43 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.0 required=5.0 tests=AWL,BAYES_20,SPF_HELO_PASS,SPF_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Thu, 26 Feb 2015 09:52:42 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id t1Q9qSxX013604 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Thu, 26 Feb 2015 04:52:29 -0500 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t1Q9qQ5W001049; Thu, 26 Feb 2015 04:52:26 -0500 Message-ID: <54EEECD9.4050909@redhat.com> Date: Thu, 26 Feb 2015 16:26:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.3.0 MIME-Version: 1.0 To: Jack Howarth , pinskia@gmail.com CC: Simon Marchi , "Paul_Koning@dell.com" , "gdb@sourceware.org" Subject: Re: format string is not a string literal References: <0AB56024-875B-4724-8ED2-A9DDB237CBFF@dell.com> <23CC7871-C616-436C-920C-4A635DC87189@dell.com> <7A311B56-C424-4C4F-A0E4-B12B65131745@gmail.com> In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-SW-Source: 2015-02/txt/msg00065.txt.bz2 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 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 * 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)) = "\n" "\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