-Wformat-security warnings generated in gcc build

-Wformat-security warnings generated in gcc build

[Prathamesh Kulkarni]

[-- Attachment #1: Type: text/plain, Size: 805 bytes --]

There are about 35 warnings of type "format not a string literal and
no formal arguments [-Wformat-security]" generated during gcc-4.9.0
build (revision 206867)
I have attached them in orig-warnings.txt.

Souce of these warnings are typically calls to error() and friends.
In  C and C++ front ends there are many calls of error (errmsg).
errmsg is in many cases, assigned the return value of targetm hooks
(tagetm.invalid_return_type(), etc.)  Is it correct to replace error
(errmsg) by
error ("%s", errmsg) in these cases ?

I have attached a patch that removes 25 of these warnings
(attached in removed-warnings.txt).
I didn't replace the calls to error() and friends where gmsgid was passed.
(eg: c-typeck.c: error (gmsgid) called by error_init() function at line 6390)

Thanks and Regards,
Prathamesh

[-- Attachment #2: removed-warnings.txt --]
[-- Type: text/plain, Size: 2872 bytes --]

../../src/libcpp/expr.c:672:18: warning: format not a string literal and no format arguments [-Wformat-security]
../../src/libcpp/expr.c:675:39: warning: format not a string literal and no format arguments [-Wformat-security]
../../src/libcpp/macro.c:2972:58: warning: format not a string literal and no format arguments [-Wformat-security]
../../src/libcpp/macro.c:2985:58: warning: format not a string literal and no format arguments [-Wformat-security]
../../src/gcc/c/c-decl.c:5701:16: warning: format not a string literal and no format arguments [-Wformat-security]
../../src/gcc/c/c-decl.c:6482:21: warning: format not a string literal and no format arguments [-Wformat-security]
../../src/gcc/c/c-typeck.c:3303:28: warning: format not a string literal and no format arguments [-Wformat-security]
../../src/gcc/c/c-typeck.c:3798:42: warning: format not a string literal and no format arguments [-Wformat-security]
../../src/gcc/c/c-typeck.c:10066:42: warning: format not a string literal and no format arguments [-Wformat-security]
../../src/gcc/c/c-convert.c:82:31: warning: format not a string literal and no format arguments [-Wformat-security]
../../src/gcc/tree-sra.c:3864:26: warning: format not a string literal and no format arguments [-Wformat-security]
../../src/gcc/tree-ssa-uninit.c:767:26: warning: format not a string literal and no format arguments [-Wformat-security]
../../src/gcc/opts.c:1042:33: warning: format not a string literal and no format arguments [-Wformat-security]
../../src/gcc/opts.c:1042:33: warning: format not a string literal and no format arguments [-Wformat-security]
../../src/gcc/cp/decl.c:9553:16: warning: format not a string literal and no format arguments [-Wformat-security]
../../src/gcc/cp/decl.c:11122:17: warning: format not a string literal and no format arguments [-Wformat-security]
../../src/gcc/cp/pt.c:13968:20: warning: format not a string literal and no format arguments [-Wformat-security]
../../src/gcc/cp/typeck.c:3986:24: warning: format not a string literal and no format arguments [-Wformat-security]
../../src/gcc/cp/typeck.c:5562:24: warning: format not a string literal and no format arguments [-Wformat-security]
../../src/gcc/cp/cvt.c:672:26: warning: format not a string literal and no format arguments [-Wformat-security]
../../src/gcc/collect2.c:1985:21: warning: format not a string literal and no format arguments [-Wformat-security]
../../src/gcc/collect2.c:2536:21: warning: format not a string literal and no format arguments [-Wformat-security]
../../src/gcc/lto-wrapper.c:195:24: warning: format not a string literal and no format arguments [-Wformat-security]
../../src/gcc/lto-wrapper.c:198:15: warning: format not a string literal and no format arguments [-Wformat-security]
../../src/gcc/gcc.c:2785:25: warning: format not a string literal and no format arguments [-Wformat-security]

[-- Attachment #3: orig-warnings.txt --]
[-- Type: text/plain, Size: 4038 bytes --]

../../src/libcpp/expr.c:672:18: warning: format not a string literal and no format arguments [-Wformat-security]
../../src/libcpp/expr.c:675:39: warning: format not a string literal and no format arguments [-Wformat-security]
../../src/libcpp/macro.c:2972:58: warning: format not a string literal and no format arguments [-Wformat-security]
../../src/libcpp/macro.c:2985:58: warning: format not a string literal and no format arguments [-Wformat-security]
../../src/gcc/c/c-decl.c:5701:16: warning: format not a string literal and no format arguments [-Wformat-security]
../../src/gcc/c/c-decl.c:6482:21: warning: format not a string literal and no format arguments [-Wformat-security]
../../src/gcc/c/c-typeck.c:3303:28: warning: format not a string literal and no format arguments [-Wformat-security]
../../src/gcc/c/c-typeck.c:3798:42: warning: format not a string literal and no format arguments [-Wformat-security]
../../src/gcc/c/c-typeck.c:6390:16: warning: format not a string literal and no format arguments [-Wformat-security]
../../src/gcc/c/c-typeck.c:6407:33: warning: format not a string literal and no format arguments [-Wformat-security]
../../src/gcc/c/c-typeck.c:6425:23: warning: format not a string literal and no format arguments [-Wformat-security]
../../src/gcc/c/c-typeck.c:10066:42: warning: format not a string literal and no format arguments [-Wformat-security]
../../src/gcc/c/c-convert.c:82:31: warning: format not a string literal and no format arguments [-Wformat-security]
../../src/gcc/c-family/c-common.c:9524:18: warning: format not a string literal and no format arguments [-Wformat-security]
../../src/gcc/c-family/c-common.c:9528:21: warning: format not a string literal and no format arguments [-Wformat-security]
../../src/gcc/dwarf2asm.c:167:50: warning: format not a string literal and no format arguments [-Wformat-security]
../../src/gcc/fold-const.c:315:42: warning: format not a string literal and no format arguments [-Wformat-security]
../../src/gcc/tree-sra.c:3864:26: warning: format not a string literal and no format arguments [-Wformat-security]
../../src/gcc/tree-ssa-uninit.c:767:26: warning: format not a string literal and no format arguments [-Wformat-security]
../../src/gcc/opts.c:1042:33: warning: format not a string literal and no format arguments [-Wformat-security]
../../src/gcc/opts.c:1042:33: warning: format not a string literal and no format arguments [-Wformat-security]
../../src/gcc/opts.c:1056:8: warning: format not a string literal and no format arguments [-Wformat-security]
../../src/gcc/opts.c:1056:8: warning: format not a string literal and no format arguments [-Wformat-security]
../../src/gcc/cp/decl.c:9553:16: warning: format not a string literal and no format arguments [-Wformat-security]
../../src/gcc/cp/decl.c:11122:17: warning: format not a string literal and no format arguments [-Wformat-security]
../../src/gcc/cp/pt.c:13968:20: warning: format not a string literal and no format arguments [-Wformat-security]
../../src/gcc/cp/parser.c:2658:55: warning: format not a string literal and no format arguments [-Wformat-security]
../../src/gcc/cp/typeck.c:3986:24: warning: format not a string literal and no format arguments [-Wformat-security]
../../src/gcc/cp/typeck.c:5562:24: warning: format not a string literal and no format arguments [-Wformat-security]
../../src/gcc/cp/cvt.c:672:26: warning: format not a string literal and no format arguments [-Wformat-security]
../../src/gcc/collect2.c:1985:21: warning: format not a string literal and no format arguments [-Wformat-security]
../../src/gcc/collect2.c:2536:21: warning: format not a string literal and no format arguments [-Wformat-security]
../../src/gcc/lto-wrapper.c:195:24: warning: format not a string literal and no format arguments [-Wformat-security]
../../src/gcc/lto-wrapper.c:198:15: warning: format not a string literal and no format arguments [-Wformat-security]
../../src/gcc/gcc.c:2785:25: warning: format not a string literal and no format arguments [-Wformat-security]

[-- Attachment #4: format-warnings.patch --]
[-- Type: text/x-patch, Size: 8446 bytes --]

Index: gcc/c/c-convert.c
===================================================================
--- gcc/c/c-convert.c	(revision 206867)
+++ gcc/c/c-convert.c	(working copy)
@@ -79,7 +79,7 @@ convert (tree type, tree expr)
   if ((invalid_conv_diag
        = targetm.invalid_conversion (TREE_TYPE (expr), type)))
     {
-      error (invalid_conv_diag);
+      error ("%s", invalid_conv_diag);
       return error_mark_node;
     }
 
Index: gcc/c/c-decl.c
===================================================================
--- gcc/c/c-decl.c	(revision 206867)
+++ gcc/c/c-decl.c	(working copy)
@@ -5698,7 +5698,7 @@ grokdeclarator (const struct c_declarato
 	    errmsg = targetm.invalid_return_type (type);
 	    if (errmsg)
 	      {
-		error (errmsg);
+		error ("%s", errmsg);
 		type = integer_type_node;
 	      }
 
@@ -6479,7 +6479,7 @@ grokparms (struct c_arg_info *arg_info,
 	  errmsg = targetm.invalid_parameter_type (type);
 	  if (errmsg)
 	    {
-	      error (errmsg);
+	      error ("%s", errmsg);
 	      TREE_VALUE (typelt) = error_mark_node;
 	      TREE_TYPE (parm) = error_mark_node;
 	      arg_types = NULL_TREE;
Index: gcc/c/c-typeck.c
===================================================================
--- gcc/c/c-typeck.c	(revision 206867)
+++ gcc/c/c-typeck.c	(working copy)
@@ -3300,7 +3300,7 @@ convert_arguments (tree typelist, vec<tr
       else if ((invalid_func_diag =
 		targetm.calls.invalid_arg_for_unprototyped_fn (typelist, fundecl, val)))
 	{
-	  error (invalid_func_diag);
+	  error ("%s", invalid_func_diag);
 	  return -1;
 	}
       else
@@ -3795,7 +3795,7 @@ build_unary_op (location_t location,
   if ((invalid_op_diag
        = targetm.invalid_unary_op (code, TREE_TYPE (xarg))))
     {
-      error_at (location, invalid_op_diag);
+      error_at (location, "%s", invalid_op_diag);
       return error_mark_node;
     }
 
@@ -10063,7 +10063,7 @@ build_binary_op (location_t location, en
   if ((invalid_op_diag
        = targetm.invalid_binary_op (code, type0, type1)))
     {
-      error_at (location, invalid_op_diag);
+      error_at (location, "%s", invalid_op_diag);
       return error_mark_node;
     }

Index: gcc/collect2.c
===================================================================
--- gcc/collect2.c	(revision 206867)
+++ gcc/collect2.c	(working copy)
@@ -1982,7 +1982,7 @@ collect_execute (const char *prog, char
 	  fatal_error ("%s: %m", _(errmsg));
 	}
       else
-	fatal_error (errmsg);
+	fatal_error ("%s", errmsg);
     }
 
   free (response_arg);
@@ -2533,7 +2533,7 @@ scan_prog_file (const char *prog_name, s
 	  fatal_error ("%s: %m", _(errmsg));
 	}
       else
-	fatal_error (errmsg);
+	fatal_error ("%s", errmsg);
     }
 
   int_handler  = (void (*) (int)) signal (SIGINT,  SIG_IGN);
Index: gcc/cp/cvt.c
===================================================================
--- gcc/cp/cvt.c	(revision 206867)
+++ gcc/cp/cvt.c	(working copy)
@@ -669,7 +669,7 @@ ocp_convert (tree type, tree expr, int c
        = targetm.invalid_conversion (TREE_TYPE (expr), type)))
     {
       if (complain & tf_error)
-	error (invalid_conv_diag);
+	error ("%s", invalid_conv_diag);
       return error_mark_node;
     }
 
Index: gcc/cp/decl.c
===================================================================
--- gcc/cp/decl.c	(revision 206867)
+++ gcc/cp/decl.c	(working copy)
@@ -9550,7 +9550,7 @@ grokdeclarator (const cp_declarator *dec
 	    errmsg = targetm.invalid_return_type (type);
 	    if (errmsg)
 	      {
-		error (errmsg);
+		error ("%s", errmsg);
 		type = integer_type_node;
 	      }
 
@@ -11119,7 +11119,7 @@ grokparms (tree parmlist, tree *parms)
       if (type != error_mark_node
 	  && (errmsg = targetm.invalid_parameter_type (type)))
 	{
-	  error (errmsg);
+	  error ("%s", errmsg);
 	  type = error_mark_node;
 	  TREE_TYPE (decl) = error_mark_node;
 	}
Index: gcc/cp/pt.c
===================================================================
--- gcc/cp/pt.c	(revision 206867)
+++ gcc/cp/pt.c	(working copy)
@@ -13965,7 +13965,7 @@ tsubst_copy_and_build (tree t,
 				     &error_msg,
 				     input_location);
 	if (error_msg)
-	  error (error_msg);
+	  error ("%s", error_msg);
 	if (!function_p && identifier_p (decl))
 	  {
 	    if (complain & tf_error)
Index: gcc/cp/typeck.c
===================================================================
--- gcc/cp/typeck.c	(revision 206867)
+++ gcc/cp/typeck.c	(working copy)
@@ -3983,7 +3983,7 @@ cp_build_binary_op (location_t location,
        = targetm.invalid_binary_op (code, type0, type1)))
     {
       if (complain & tf_error)
-	error (invalid_op_diag);
+	error ("%s", invalid_op_diag);
       return error_mark_node;
     }
 
@@ -5559,7 +5559,7 @@ cp_build_unary_op (enum tree_code code,
 				   TREE_TYPE (xarg))))
     {
       if (complain & tf_error)
-	error (invalid_op_diag);
+	error ("%s", invalid_op_diag);
       return error_mark_node;
     }
 
Index: gcc/gcc.c
===================================================================
--- gcc/gcc.c	(revision 206867)
+++ gcc/gcc.c	(working copy)
@@ -2782,7 +2782,7 @@ execute (void)
       if (errmsg != NULL)
 	{
 	  if (err == 0)
-	    fatal_error (errmsg);
+	    fatal_error ("%s", errmsg);
 	  else
 	    {
 	      errno = err;
Index: gcc/lto-wrapper.c
===================================================================
--- gcc/lto-wrapper.c	(revision 206867)
+++ gcc/lto-wrapper.c	(working copy)
@@ -192,10 +192,10 @@ collect_execute (char **argv)
       if (err != 0)
 	{
 	  errno = err;
-	  fatal_perror (errmsg);
+	  fatal_perror ("%s", errmsg);
 	}
       else
-	fatal (errmsg);
+	fatal ("%s", errmsg);
     }
 
   return pex;
Index: gcc/opts.c
===================================================================
--- gcc/opts.c	(revision 206867)
+++ gcc/opts.c	(working copy)
@@ -1039,7 +1039,7 @@ print_filtered_help (unsigned int includ
 		      if (* (const char **) flag_var != NULL)
 			snprintf (new_help + strlen (new_help),
 				  sizeof (new_help) - strlen (new_help),
-				  * (const char **) flag_var);
+				  "%s", * (const char **) flag_var);
 		    }
 		  else if (option->var_type == CLVC_ENUM)
 		    {
Index: gcc/tree-sra.c
===================================================================
--- gcc/tree-sra.c	(revision 206867)
+++ gcc/tree-sra.c	(working copy)
@@ -3861,7 +3861,7 @@ dump_dereferences_table (FILE *f, const
 {
   basic_block bb;
 
-  fprintf (dump_file, str);
+  fprintf (dump_file, "%s", str);
   FOR_BB_BETWEEN (bb, ENTRY_BLOCK_PTR_FOR_FN (cfun),
 		  EXIT_BLOCK_PTR_FOR_FN (cfun), next_bb)
     {
Index: gcc/tree-ssa-uninit.c
===================================================================
--- gcc/tree-ssa-uninit.c	(revision 206867)
+++ gcc/tree-ssa-uninit.c	(working copy)
@@ -764,7 +764,7 @@ dump_predicates (gimple usestmt, pred_ch
 {
   size_t i, j;
   pred_chain one_pred_chain = vNULL;
-  fprintf (dump_file, msg);
+  fprintf (dump_file, "%s", msg);
   print_gimple_stmt (dump_file, usestmt, 0, 0);
   fprintf (dump_file, "is guarded by :\n\n");
   size_t num_preds = preds.length ();
Index: libcpp/expr.c
===================================================================
--- libcpp/expr.c	(revision 206867)
+++ libcpp/expr.c	(working copy)
@@ -669,10 +669,10 @@ cpp_classify_number (cpp_reader *pfile,
 
 	  if (CPP_OPTION (pfile, c99))
             cpp_warning_with_line (pfile, CPP_W_LONG_LONG, virtual_location,
-				   0, message);
+				   0, "%s", message);
           else
             cpp_pedwarning_with_line (pfile, CPP_W_LONG_LONG,
-				      virtual_location, 0, message);
+				      virtual_location, 0, "%s", message);
         }
 
       result |= CPP_N_INTEGER;
Index: libcpp/macro.c
===================================================================
--- libcpp/macro.c	(revision 206867)
+++ libcpp/macro.c	(working copy)
@@ -2969,7 +2969,7 @@ create_iso_definition (cpp_reader *pfile
 	     function-like macros, but not at the end.  */
 	  if (following_paste_op)
 	    {
-	      cpp_error (pfile, CPP_DL_ERROR, paste_op_error_msg);
+	      cpp_error (pfile, CPP_DL_ERROR, "%s", paste_op_error_msg);
 	      return false;
 	    }
 	  break;
@@ -2982,7 +2982,7 @@ create_iso_definition (cpp_reader *pfile
 	     function-like macros, but not at the beginning.  */
 	  if (macro->count == 1)
 	    {
-	      cpp_error (pfile, CPP_DL_ERROR, paste_op_error_msg);
+	      cpp_error (pfile, CPP_DL_ERROR, "%s", paste_op_error_msg);
 	      return false;
 	    }
 

Re: -Wformat-security warnings generated in gcc build

[Jakub Jelinek]

On Tue, Jan 21, 2014 at 09:09:25PM +0530, Prathamesh Kulkarni wrote:
> --- gcc/c/c-convert.c	(revision 206867)
> +++ gcc/c/c-convert.c	(working copy)
> @@ -79,7 +79,7 @@ convert (tree type, tree expr)
>    if ((invalid_conv_diag
>         = targetm.invalid_conversion (TREE_TYPE (expr), type)))
>      {
> -      error (invalid_conv_diag);
> +      error ("%s", invalid_conv_diag);

This looks wrong.  error/error_at/fatal_error and I think cpp_error
too mark the format string argument for translation (as in all these
cases the format string is actually a variable, not string literal,
that doesn't perform anything, supposedly the actual string literal
is marked with N_(...) earlier) and also the functions translate it using
gettext, which won't happen for the string passed to %s.
So I believe you actually need to use
      error ("%s", _(invalid_conv_diag));
etc. instead.  Of course not for the fprintf case.

	Jakub

Re: -Wformat-security warnings generated in gcc build

[Joseph S. Myers]

On Tue, 21 Jan 2014, Prathamesh Kulkarni wrote:

> Souce of these warnings are typically calls to error() and friends.
> In  C and C++ front ends there are many calls of error (errmsg).
> errmsg is in many cases, assigned the return value of targetm hooks
> (tagetm.invalid_return_type(), etc.)  Is it correct to replace error
> (errmsg) by
> error ("%s", errmsg) in these cases ?

No.  Typically the message returned by the hook may contain no-arguments 
format specifiers such as %< and %>.  Instead, to avoid such warnings you 
need to add a new function error_at_no_args (location, message) that 
accepts and processes only formats taking no arguments (and probably 
aborts if given a format that needs arguments).

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: -Wformat-security warnings generated in gcc build

[Florian Weimer]

On 01/21/2014 06:50 PM, Joseph S. Myers wrote:
> On Tue, 21 Jan 2014, Prathamesh Kulkarni wrote:
>
>> Souce of these warnings are typically calls to error() and friends.
>> In  C and C++ front ends there are many calls of error (errmsg).
>> errmsg is in many cases, assigned the return value of targetm hooks
>> (tagetm.invalid_return_type(), etc.)  Is it correct to replace error
>> (errmsg) by
>> error ("%s", errmsg) in these cases ?
>
> No.  Typically the message returned by the hook may contain no-arguments
> format specifiers such as %< and %>.  Instead, to avoid such warnings you
> need to add a new function error_at_no_args (location, message) that
> accepts and processes only formats taking no arguments (and probably
> aborts if given a format that needs arguments).

And printf format strings also can contain %% an %m (the latter is a GNU 
extension).  That's why we cannot perform the arg -> "%s", arg 
transformation unconditionally in the compiler, rendering 
-Wformat-security pointless.  Which is a bit disappointing.

-- 
Florian Weimer / Red Hat Product Security Team

Re: -Wformat-security warnings generated in gcc build

[Prathamesh Kulkarni]

On Tue, Jan 21, 2014 at 11:20 PM, Joseph S. Myers
<joseph@codesourcery.com> wrote:
> On Tue, 21 Jan 2014, Prathamesh Kulkarni wrote:
>
>> Souce of these warnings are typically calls to error() and friends.
>> In  C and C++ front ends there are many calls of error (errmsg).
>> errmsg is in many cases, assigned the return value of targetm hooks
>> (tagetm.invalid_return_type(), etc.)  Is it correct to replace error
>> (errmsg) by
>> error ("%s", errmsg) in these cases ?
>
> No.  Typically the message returned by the hook may contain no-arguments
> format specifiers such as %< and %>.  Instead, to avoid such warnings you
> need to add a new function error_at_no_args (location, message) that
> accepts and processes only formats taking no arguments (and probably
> aborts if given a format that needs arguments).
>
Here's an initial try:

void
error_at_no_args (location_t loc, const char *gmsgid)
{
  diagnostic_info diagnostic;
  diagnostic_set_info (&diagnostic, gmsgid, NULL, loc, DK_ERROR);
  report_diagnostic (&diagnostic);
}

I guess this shall work fine for no-argument specifiers ? (I tested it
with %%, %m, %', %<, %>)
However this causes segmentation fault, when format string contains
argument specifier (which is anyways incorrect), I believe that's
because I am passing NULL in place of argument pointer.

Unfortunately, I am not clear on how to check for format specifiers in string.
Should I do it manually by checking the format string for specifiers
and call abort if found a no-argument specifier,
or is there a better way to do it ?

Thanks!

> --
> Joseph S. Myers
> joseph@codesourcery.com

Re: -Wformat-security warnings generated in gcc build

[Joseph S. Myers]

On Wed, 22 Jan 2014, Prathamesh Kulkarni wrote:

> Unfortunately, I am not clear on how to check for format specifiers in string.
> Should I do it manually by checking the format string for specifiers
> and call abort if found a no-argument specifier,
> or is there a better way to do it ?

I'll leave it to Dodji as diagnostics maintainer to advise on the best way 
to implement error_at_no_args.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: -Wformat-security warnings generated in gcc build

[Dodji Seketeli]

"Joseph S. Myers" <joseph@codesourcery.com> a écrit:

> On Wed, 22 Jan 2014, Prathamesh Kulkarni wrote:
>
>> Unfortunately, I am not clear on how to check for format specifiers in string.
>> Should I do it manually by checking the format string for specifiers
>> and call abort if found a no-argument specifier,
>> or is there a better way to do it ?
>
> I'll leave it to Dodji as diagnostics maintainer to advise on the best way 
> to implement error_at_no_args.

Thanks for the heads-up, Joseph.

Assuming we want to do something more than just segfaulting if
error_at_no_args is given a format specifier that needs an argument, I
think the function pretty-print.c:pp_format is the place where the
core of the change has to be done.  Basically, that function walks the
format string and expands format specifiers.

Thus, in that function, if a format specifier needs an argument (that
is, each time we try to access text->args_ptr) but we are in a context
where we want to accept only no-argument specifiers, we can call abort
or internal_error with a meaningful error message.

I guess we can assume that we know that we are in a
oo-argument-specifier context if text->args_ptr is NULL in pp_format()
That text->args_ptr is actually the va_ap that you (Prathamesh) set to
NULL when you did:

    void
    error_at_no_args (location_t loc, const char *gmsgid)
    {
      diagnostic_info diagnostic;
      diagnostic_set_info (&diagnostic, gmsgid, NULL, loc, DK_ERROR);
                                                ^^^^
						 ^
                                                 |
                                         here: __|

      report_diagnostic (&diagnostic);
    }


And then, just keep the error_at_no_args() implementation like what
you did already.

Also, you'd need to modify cp/error.c:cp_printer in a similar way, to
issue an internal_error each time we try to access a null test->args_ptr.

But then, I guess some people might argue that we could as well let
the code as is and just segfault on accessing a NULL test->args_ptr.
I would personally lean towards aborting with a meaningful error
message, but I'd like to hear what others think about this.

I hope this helps.

-- 
		Dodji

Re: -Wformat-security warnings generated in gcc build

[Trevor Saunders]

On Thu, Jan 23, 2014 at 12:32:34PM +0100, Dodji Seketeli wrote:
> "Joseph S. Myers" <joseph@codesourcery.com> a écrit:
> 
> > On Wed, 22 Jan 2014, Prathamesh Kulkarni wrote:
> >
> >> Unfortunately, I am not clear on how to check for format specifiers in string.
> >> Should I do it manually by checking the format string for specifiers
> >> and call abort if found a no-argument specifier,
> >> or is there a better way to do it ?
> >
> > I'll leave it to Dodji as diagnostics maintainer to advise on the best way 
> > to implement error_at_no_args.
> 
> Thanks for the heads-up, Joseph.
> 
> Assuming we want to do something more than just segfaulting if
> error_at_no_args is given a format specifier that needs an argument, I
> think the function pretty-print.c:pp_format is the place where the
> core of the change has to be done.  Basically, that function walks the
> format string and expands format specifiers.
> 
> Thus, in that function, if a format specifier needs an argument (that
> is, each time we try to access text->args_ptr) but we are in a context
> where we want to accept only no-argument specifiers, we can call abort
> or internal_error with a meaningful error message.
> 
> I guess we can assume that we know that we are in a
> oo-argument-specifier context if text->args_ptr is NULL in pp_format()
> That text->args_ptr is actually the va_ap that you (Prathamesh) set to
> NULL when you did:
> 
>     void
>     error_at_no_args (location_t loc, const char *gmsgid)
>     {
>       diagnostic_info diagnostic;
>       diagnostic_set_info (&diagnostic, gmsgid, NULL, loc, DK_ERROR);
>                                                 ^^^^
> 						 ^
>                                                  |
>                                          here: __|
> 
>       report_diagnostic (&diagnostic);
>     }
> 
> 
> And then, just keep the error_at_no_args() implementation like what
> you did already.
> 
> Also, you'd need to modify cp/error.c:cp_printer in a similar way, to
> issue an internal_error each time we try to access a null test->args_ptr.
> 
> But then, I guess some people might argue that we could as well let
> the code as is and just segfault on accessing a NULL test->args_ptr.
> I would personally lean towards aborting with a meaningful error
> message, but I'd like to hear what others think about this.

It seems like this is prime teritory for an assert, and then documenting
the API to be if you use format specifiers then you must pass args as
well.  Considering that both makes it easy to see what went wrong, and
doesn't trade performance to get most of what you get with
internal_error()

Trev

> 
> I hope this helps.
> 
> -- 
> 		Dodji

Re: -Wformat-security warnings generated in gcc build

[Prathamesh Kulkarni]

On Thu, Jan 23, 2014 at 5:02 PM, Dodji Seketeli <dodji@redhat.com> wrote:
> "Joseph S. Myers" <joseph@codesourcery.com> a écrit:
>
>> On Wed, 22 Jan 2014, Prathamesh Kulkarni wrote:
>>
>>> Unfortunately, I am not clear on how to check for format specifiers in string.
>>> Should I do it manually by checking the format string for specifiers
>>> and call abort if found a no-argument specifier,
>>> or is there a better way to do it ?
>>
>> I'll leave it to Dodji as diagnostics maintainer to advise on the best way
>> to implement error_at_no_args.
>
> Thanks for the heads-up, Joseph.
>
> Assuming we want to do something more than just segfaulting if
> error_at_no_args is given a format specifier that needs an argument, I
> think the function pretty-print.c:pp_format is the place where the
> core of the change has to be done.  Basically, that function walks the
> format string and expands format specifiers.
>
> Thus, in that function, if a format specifier needs an argument (that
> is, each time we try to access text->args_ptr) but we are in a context
> where we want to accept only no-argument specifiers, we can call abort
> or internal_error with a meaningful error message.
>
> I guess we can assume that we know that we are in a
> oo-argument-specifier context if text->args_ptr is NULL in pp_format()
> That text->args_ptr is actually the va_ap that you (Prathamesh) set to
> NULL when you did:
>
>     void
>     error_at_no_args (location_t loc, const char *gmsgid)
>     {
>       diagnostic_info diagnostic;
>       diagnostic_set_info (&diagnostic, gmsgid, NULL, loc, DK_ERROR);
>                                                 ^^^^
>                                                  ^
>                                                  |
>                                          here: __|
>
>       report_diagnostic (&diagnostic);
>     }
>
>
> And then, just keep the error_at_no_args() implementation like what
> you did already.

Shall it be correct then to replace calls to error() and friends,
taking only format string with no-argument specifiers
to error_at_no_args() ? (similarly we shall need warning_at_no_args,
pedwarn_no_args, etc.)

>
> Also, you'd need to modify cp/error.c:cp_printer in a similar way, to
> issue an internal_error each time we try to access a null test->args_ptr.

Shall check for text->args_ptr be required in each case label of
argument specifier in pp_format()
and client-specific functions like cp_printer() ?

eg:
case 'c':
    gcc_assert (text->args_ptr);
    pp_character (pp, va_arg (*text->args_ptr, int));
    break;

>
> But then, I guess some people might argue that we could as well let
> the code as is and just segfault on accessing a NULL test->args_ptr.
> I would personally lean towards aborting with a meaningful error
> message, but I'd like to hear what others think about this.
>
> I hope this helps.
>
> --
>                 Dodji

Re: -Wformat-security warnings generated in gcc build

[Dodji Seketeli]

Prathamesh Kulkarni <bilbotheelffriend@gmail.com> writes:

>
> Shall it be correct then to replace calls to error() and friends,
> taking only format string with no-argument specifiers
> to error_at_no_args() ? (similarly we shall need warning_at_no_args,
> pedwarn_no_args, etc.)

I would guess so, yes.

>>
>> Also, you'd need to modify cp/error.c:cp_printer in a similar way, to
>> issue an internal_error each time we try to access a null test->args_ptr.
>
> Shall check for text->args_ptr be required in each case label of
> argument specifier in pp_format()
> and client-specific functions like cp_printer() ?

Yes, I think so.  Maybe you can make that a bit more maintainable by
creating a macro like those used to access text->args_ptr in cp_printer,
e.g:

    #define next_int     va_arg (*text->args_ptr, int)

In that macro, make the check for text->args_ptr before accessing it,
and then use that macro to access text->args_ptr through the function.

-- 
		Dodji

Re: -Wformat-security warnings generated in gcc build

[Prathamesh Kulkarni]

[-- Attachment #1: Type: text/plain, Size: 1764 bytes --]

On Thu, Jan 23, 2014 at 8:24 PM, Dodji Seketeli <dodji@redhat.com> wrote:
> Prathamesh Kulkarni <bilbotheelffriend@gmail.com> writes:
>
>>
>> Shall it be correct then to replace calls to error() and friends,
>> taking only format string with no-argument specifiers
>> to error_at_no_args() ? (similarly we shall need warning_at_no_args,
>> pedwarn_no_args, etc.)
>
> I would guess so, yes.
>
>>>
>>> Also, you'd need to modify cp/error.c:cp_printer in a similar way, to
>>> issue an internal_error each time we try to access a null test->args_ptr.
>>
>> Shall check for text->args_ptr be required in each case label of
>> argument specifier in pp_format()
>> and client-specific functions like cp_printer() ?
>
> Yes, I think so.  Maybe you can make that a bit more maintainable by
> creating a macro like those used to access text->args_ptr in cp_printer,
> e.g:
>
>     #define next_int     va_arg (*text->args_ptr, int)
>
> In that macro, make the check for text->args_ptr before accessing it,
> and then use that macro to access text->args_ptr through the function.
>

I was going through diagnostic.c, it appears that many functions in
(error(), error_at(), warning(), etc.) share common code (mostly
contains call to diagnostic_set_info() followed by call to
report_diagnostic()), which I guess can be re-written in terms of
emit_diagnostic(), however since it's variadic that's not possible. I
wrote a v_emit_diagnostic() function, that takes same arguments as
emit_diagnostic(), with additional va_list * at end (va_list * instead
of va_list, so I could pass NULL for error_no_args() and friends). Is
it correct to write these other functions (emit_diagnostic(), error(),
inform(), etc.) in terms of v_emit_diagnostic() ?





> --
>                 Dodji

[-- Attachment #2: diagnostic.patch --]
[-- Type: text/x-patch, Size: 5704 bytes --]

Index: diagnostic.c
===================================================================
--- diagnostic.c	(revision 206867)
+++ diagnostic.c	(working copy)
@@ -884,29 +884,38 @@ diagnostic_append_note (diagnostic_conte
   va_end (ap);
 }
 
-bool
-emit_diagnostic (diagnostic_t kind, location_t location, int opt,
-		 const char *gmsgid, ...)
+static bool
+v_emit_diagnostic (diagnostic_t kind, location_t location, int opt, const char *gmsgid, va_list *ap_p)
 {
   diagnostic_info diagnostic;
-  va_list ap;
   bool ret;
-
-  va_start (ap, gmsgid);
+  
   if (kind == DK_PERMERROR)
     {
-      diagnostic_set_info (&diagnostic, gmsgid, &ap, location,
+      diagnostic_set_info (&diagnostic, gmsgid, ap_p, location,
 			   permissive_error_kind (global_dc));
       diagnostic.option_index = permissive_error_option (global_dc);
     }
   else {
-      diagnostic_set_info (&diagnostic, gmsgid, &ap, location, kind);
+      diagnostic_set_info (&diagnostic, gmsgid, ap_p, location, kind);
       if (kind == DK_WARNING || kind == DK_PEDWARN)
 	diagnostic.option_index = opt;
   }
 
   ret = report_diagnostic (&diagnostic);
-  va_end (ap);
+  return ret;
+}
+
+bool
+emit_diagnostic (diagnostic_t kind, location_t location, int opt,
+		 const char *gmsgid, ...)
+{
+  va_list ap;
+  bool ret;
+
+  va_start (ap, gmsgid);
+  ret = v_emit_diagnostic (kind, location, opt, gmsgid, &ap);
+  va_end (ap); 
   return ret;
 }
 
@@ -915,12 +924,10 @@ emit_diagnostic (diagnostic_t kind, loca
 void
 inform (location_t location, const char *gmsgid, ...)
 {
-  diagnostic_info diagnostic;
   va_list ap;
 
   va_start (ap, gmsgid);
-  diagnostic_set_info (&diagnostic, gmsgid, &ap, location, DK_NOTE);
-  report_diagnostic (&diagnostic);
+  v_emit_diagnostic (DK_NOTE, location, 0, gmsgid, &ap);
   va_end (ap);
 }
 
@@ -947,15 +954,11 @@ inform_n (location_t location, int n, co
 bool
 warning (int opt, const char *gmsgid, ...)
 {
-  diagnostic_info diagnostic;
   va_list ap;
   bool ret;
 
   va_start (ap, gmsgid);
-  diagnostic_set_info (&diagnostic, gmsgid, &ap, input_location, DK_WARNING);
-  diagnostic.option_index = opt;
-
-  ret = report_diagnostic (&diagnostic);
+  ret = v_emit_diagnostic (DK_WARNING, input_location, opt, gmsgid, &ap);
   va_end (ap);
   return ret;
 }
@@ -967,14 +970,11 @@ warning (int opt, const char *gmsgid, ..
 bool
 warning_at (location_t location, int opt, const char *gmsgid, ...)
 {
-  diagnostic_info diagnostic;
   va_list ap;
   bool ret;
 
   va_start (ap, gmsgid);
-  diagnostic_set_info (&diagnostic, gmsgid, &ap, location, DK_WARNING);
-  diagnostic.option_index = opt;
-  ret = report_diagnostic (&diagnostic);
+  ret = emit_diagnostic (DK_WARNING, location, opt, gmsgid, &ap);
   va_end (ap);
   return ret;
 }
@@ -995,14 +995,11 @@ warning_at (location_t location, int opt
 bool
 pedwarn (location_t location, int opt, const char *gmsgid, ...)
 {
-  diagnostic_info diagnostic;
   va_list ap;
   bool ret;
 
   va_start (ap, gmsgid);
-  diagnostic_set_info (&diagnostic, gmsgid, &ap, location,  DK_PEDWARN);
-  diagnostic.option_index = opt;
-  ret = report_diagnostic (&diagnostic);
+  ret = v_emit_diagnostic (DK_PEDWARN, location, opt, gmsgid, &ap);
   va_end (ap);
   return ret;
 }
@@ -1017,15 +1014,11 @@ pedwarn (location_t location, int opt, c
 bool
 permerror (location_t location, const char *gmsgid, ...)
 {
-  diagnostic_info diagnostic;
   va_list ap;
   bool ret;
 
   va_start (ap, gmsgid);
-  diagnostic_set_info (&diagnostic, gmsgid, &ap, location,
-                       permissive_error_kind (global_dc));
-  diagnostic.option_index = permissive_error_option (global_dc);
-  ret = report_diagnostic (&diagnostic);
+  ret = v_emit_diagnostic (DK_PERMERROR, location, 0, gmsgid, &ap);
   va_end (ap);
   return ret;
 }
@@ -1035,12 +1028,10 @@ permerror (location_t location, const ch
 void
 error (const char *gmsgid, ...)
 {
-  diagnostic_info diagnostic;
   va_list ap;
 
   va_start (ap, gmsgid);
-  diagnostic_set_info (&diagnostic, gmsgid, &ap, input_location, DK_ERROR);
-  report_diagnostic (&diagnostic);
+  v_emit_diagnostic (DK_ERROR, input_location, 0, gmsgid, &ap);
   va_end (ap);
 }
 
@@ -1065,12 +1056,10 @@ error_n (location_t location, int n, con
 void
 error_at (location_t loc, const char *gmsgid, ...)
 {
-  diagnostic_info diagnostic;
   va_list ap;
 
   va_start (ap, gmsgid);
-  diagnostic_set_info (&diagnostic, gmsgid, &ap, loc, DK_ERROR);
-  report_diagnostic (&diagnostic);
+  v_emit_diagnostic (DK_ERROR, loc, 0, gmsgid, &ap); 
   va_end (ap);
 }
 
@@ -1080,12 +1069,10 @@ error_at (location_t loc, const char *gm
 void
 sorry (const char *gmsgid, ...)
 {
-  diagnostic_info diagnostic;
   va_list ap;
 
   va_start (ap, gmsgid);
-  diagnostic_set_info (&diagnostic, gmsgid, &ap, input_location, DK_SORRY);
-  report_diagnostic (&diagnostic);
+  v_emit_diagnostic (DK_SORRY, input_location, 0, gmsgid, &ap);
   va_end (ap);
 }
 
@@ -1103,12 +1090,10 @@ seen_error (void)
 void
 fatal_error (const char *gmsgid, ...)
 {
-  diagnostic_info diagnostic;
   va_list ap;
 
   va_start (ap, gmsgid);
-  diagnostic_set_info (&diagnostic, gmsgid, &ap, input_location, DK_FATAL);
-  report_diagnostic (&diagnostic);
+  v_emit_diagnostic (DK_FATAL, input_location, 0, gmsgid, &ap);
   va_end (ap);
 
   gcc_unreachable ();
@@ -1121,12 +1106,10 @@ fatal_error (const char *gmsgid, ...)
 void
 internal_error (const char *gmsgid, ...)
 {
-  diagnostic_info diagnostic;
   va_list ap;
 
   va_start (ap, gmsgid);
-  diagnostic_set_info (&diagnostic, gmsgid, &ap, input_location, DK_ICE);
-  report_diagnostic (&diagnostic);
+  v_emit_diagnostic (DK_ICE, input_location, 0, gmsgid, &ap);
   va_end (ap);
 
   gcc_unreachable ();

Re: -Wformat-security warnings generated in gcc build

[Prathamesh Kulkarni]

On Thu, Jan 23, 2014 at 9:09 PM, Prathamesh Kulkarni
<bilbotheelffriend@gmail.com> wrote:
> On Thu, Jan 23, 2014 at 8:24 PM, Dodji Seketeli <dodji@redhat.com> wrote:
>> Prathamesh Kulkarni <bilbotheelffriend@gmail.com> writes:
>>
>>>
>>> Shall it be correct then to replace calls to error() and friends,
>>> taking only format string with no-argument specifiers
>>> to error_at_no_args() ? (similarly we shall need warning_at_no_args,
>>> pedwarn_no_args, etc.)
>>
>> I would guess so, yes.
>>
>>>>
>>>> Also, you'd need to modify cp/error.c:cp_printer in a similar way, to
>>>> issue an internal_error each time we try to access a null test->args_ptr.
>>>
>>> Shall check for text->args_ptr be required in each case label of
>>> argument specifier in pp_format()
>>> and client-specific functions like cp_printer() ?
>>
>> Yes, I think so.  Maybe you can make that a bit more maintainable by
>> creating a macro like those used to access text->args_ptr in cp_printer,
>> e.g:
>>
>>     #define next_int     va_arg (*text->args_ptr, int)
>>
>> In that macro, make the check for text->args_ptr before accessing it,
>> and then use that macro to access text->args_ptr through the function.
>>
>
> I was going through diagnostic.c, it appears that many functions in
> (error(), error_at(), warning(), etc.) share common code (mostly
> contains call to diagnostic_set_info() followed by call to
> report_diagnostic()), which I guess can be re-written in terms of
> emit_diagnostic(), however since it's variadic that's not possible. I
> wrote a v_emit_diagnostic() function, that takes same arguments as
> emit_diagnostic(), with additional va_list * at end (va_list * instead
> of va_list, so I could pass NULL for error_no_args() and friends). Is
> it correct to write these other functions (emit_diagnostic(), error(),
> inform(), etc.) in terms of v_emit_diagnostic() ?

silly mistake in warning_at(), it should be:
ret = v_emit_diagnostic (DK_WARNING, location, opt, gmsgid, &ap);

>
>
>
>
>
>> --
>>                 Dodji

Re: -Wformat-security warnings generated in gcc build

[Prathamesh Kulkarni]

[-- Attachment #1: Type: text/plain, Size: 3078 bytes --]

On Fri, Jan 24, 2014 at 9:19 PM, Prathamesh Kulkarni
<bilbotheelffriend@gmail.com> wrote:
> On Thu, Jan 23, 2014 at 9:09 PM, Prathamesh Kulkarni
> <bilbotheelffriend@gmail.com> wrote:
>> On Thu, Jan 23, 2014 at 8:24 PM, Dodji Seketeli <dodji@redhat.com> wrote:
>>> Prathamesh Kulkarni <bilbotheelffriend@gmail.com> writes:
>>>
>>>>
>>>> Shall it be correct then to replace calls to error() and friends,
>>>> taking only format string with no-argument specifiers
>>>> to error_at_no_args() ? (similarly we shall need warning_at_no_args,
>>>> pedwarn_no_args, etc.)
>>>
>>> I would guess so, yes.
>>>
>>>>>
>>>>> Also, you'd need to modify cp/error.c:cp_printer in a similar way, to
>>>>> issue an internal_error each time we try to access a null test->args_ptr.
>>>>
>>>> Shall check for text->args_ptr be required in each case label of
>>>> argument specifier in pp_format()
>>>> and client-specific functions like cp_printer() ?
>>>
>>> Yes, I think so.  Maybe you can make that a bit more maintainable by
>>> creating a macro like those used to access text->args_ptr in cp_printer,
>>> e.g:
>>>
>>>     #define next_int     va_arg (*text->args_ptr, int)
>>>
>>> In that macro, make the check for text->args_ptr before accessing it,
>>> and then use that macro to access text->args_ptr through the function.
>>>
>>
>> I was going through diagnostic.c, it appears that many functions in
>> (error(), error_at(), warning(), etc.) share common code (mostly
>> contains call to diagnostic_set_info() followed by call to
>> report_diagnostic()), which I guess can be re-written in terms of
>> emit_diagnostic(), however since it's variadic that's not possible. I
>> wrote a v_emit_diagnostic() function, that takes same arguments as
>> emit_diagnostic(), with additional va_list * at end (va_list * instead
>> of va_list, so I could pass NULL for error_no_args() and friends). Is
>> it correct to write these other functions (emit_diagnostic(), error(),
>> inform(), etc.) in terms of v_emit_diagnostic() ?
>
> silly mistake in warning_at(), it should be:
> ret = v_emit_diagnostic (DK_WARNING, location, opt, gmsgid, &ap);
The attached patch removes all -Wformat-security warnings
I put assertions for tree->args_ptr in pretty-print.c:pp_format(),
cp/error.c:cp_printer() and
c/c-objc-common.c:c_tree_printer().

However, adding *_no_args() functions in include/cpplib.h (eg:
cpp_error_no_args(), etc.), generates a new set of warnings
of type -Wsuggest-attribute=format gnu_printf for these functions.

I removed them by adding the following in include/ansidecl.h
+#ifndef ATTRIBUTE_GNU_PRINTF
+#define ATTRIBUTE_GNU_PRINTF(m, n) __attribute__ ((__format__
(__gnu_printf__, m, n))) ATTRIBUTE_NONNULL(m)
+#define ATTRIBUTE_GNU_PRINTF_3_0 ATTRIBUTE_GNU_PRINTF(3, 0)
+#define ATTRIBUTE_GNU_PRINTF_5_0 ATTRIBUTE_GNU_PRINTF(5, 0)
+#endif /* ATTRIBUTE_GNU_PRINTF */
+

and declarations changed to:
+extern bool cpp_error_no_args (cpp_reader *, int, const char *)
+  ATTRIBUTE_GNU_PRINTF_3_0;
+

Is this correct way to fix it ?
Thanks!

>
>>
>>
>>
>>
>>
>>> --
>>>                 Dodji

[-- Attachment #2: format-warnings.patch --]
[-- Type: text/x-patch, Size: 29417 bytes --]

Index: gcc/c/c-convert.c
===================================================================
--- gcc/c/c-convert.c	(revision 207074)
+++ gcc/c/c-convert.c	(working copy)
@@ -79,7 +79,7 @@ convert (tree type, tree expr)
   if ((invalid_conv_diag
        = targetm.invalid_conversion (TREE_TYPE (expr), type)))
     {
-      error (invalid_conv_diag);
+      error_no_args (invalid_conv_diag);
       return error_mark_node;
     }
 
Index: gcc/c/c-decl.c
===================================================================
--- gcc/c/c-decl.c	(revision 207074)
+++ gcc/c/c-decl.c	(working copy)
@@ -5698,7 +5698,7 @@ grokdeclarator (const struct c_declarato
 	    errmsg = targetm.invalid_return_type (type);
 	    if (errmsg)
 	      {
-		error (errmsg);
+		error_no_args (errmsg);
 		type = integer_type_node;
 	      }
 
@@ -6479,7 +6479,7 @@ grokparms (struct c_arg_info *arg_info,
 	  errmsg = targetm.invalid_parameter_type (type);
 	  if (errmsg)
 	    {
-	      error (errmsg);
+	      error_no_args (errmsg);
 	      TREE_VALUE (typelt) = error_mark_node;
 	      TREE_TYPE (parm) = error_mark_node;
 	      arg_types = NULL_TREE;
Index: gcc/c/c-typeck.c
===================================================================
--- gcc/c/c-typeck.c	(revision 207074)
+++ gcc/c/c-typeck.c	(working copy)
@@ -3300,7 +3300,7 @@ convert_arguments (tree typelist, vec<tr
       else if ((invalid_func_diag =
 		targetm.calls.invalid_arg_for_unprototyped_fn (typelist, fundecl, val)))
 	{
-	  error (invalid_func_diag);
+	  error_no_args (invalid_func_diag);
 	  return -1;
 	}
       else
@@ -3798,7 +3798,7 @@ build_unary_op (location_t location,
   if ((invalid_op_diag
        = targetm.invalid_unary_op (code, TREE_TYPE (xarg))))
     {
-      error_at (location, invalid_op_diag);
+      error_at_no_args (location, invalid_op_diag);
       return error_mark_node;
     }
 
@@ -6409,7 +6409,7 @@ error_init (const char *gmsgid)
   char *ofwhat;
 
   /* The gmsgid may be a format string with %< and %>. */
-  error (gmsgid);
+  error_no_args (gmsgid);
   ofwhat = print_spelling ((char *) alloca (spelling_length () + 1));
   if (*ofwhat)
     error ("(near initialization for %qs)", ofwhat);
@@ -6426,7 +6426,7 @@ pedwarn_init (location_t location, int o
   char *ofwhat;
 
   /* The gmsgid may be a format string with %< and %>. */
-  pedwarn (location, opt, gmsgid);
+  pedwarn_no_args (location, opt, gmsgid);
   ofwhat = print_spelling ((char *) alloca (spelling_length () + 1));
   if (*ofwhat)
     pedwarn (location, opt, "(near initialization for %qs)", ofwhat);
@@ -6444,7 +6444,7 @@ warning_init (int opt, const char *gmsgi
   char *ofwhat;
 
   /* The gmsgid may be a format string with %< and %>. */
-  warning (opt, gmsgid);
+  warning_no_args (opt, gmsgid);
   ofwhat = print_spelling ((char *) alloca (spelling_length () + 1));
   if (*ofwhat)
     warning (opt, "(near initialization for %qs)", ofwhat);
@@ -10102,7 +10102,7 @@ build_binary_op (location_t location, en
   if ((invalid_op_diag
        = targetm.invalid_binary_op (code, type0, type1)))
     {
-      error_at (location, invalid_op_diag);
+      error_at_no_args (location, invalid_op_diag);
       return error_mark_node;
     }
 
Index: gcc/c-family/c-common.c
===================================================================
--- gcc/c-family/c-common.c	(revision 207074)
+++ gcc/c-family/c-common.c	(working copy)
@@ -9522,11 +9522,11 @@ c_parse_error (const char *gmsgid, enum
       message = NULL;
     }
   else
-    error (gmsgid);
+    error_no_args (gmsgid);
 
   if (message)
     {
-      error (message);
+      error_no_args (message);
       free (message);
     }
 #undef catenate_messages
Index: gcc/collect2.c
===================================================================
--- gcc/collect2.c	(revision 207074)
+++ gcc/collect2.c	(working copy)
@@ -1982,7 +1982,7 @@ collect_execute (const char *prog, char
 	  fatal_error ("%s: %m", _(errmsg));
 	}
       else
-	fatal_error (errmsg);
+	fatal_error_no_args (errmsg);
     }
 
   free (response_arg);
@@ -2533,7 +2533,7 @@ scan_prog_file (const char *prog_name, s
 	  fatal_error ("%s: %m", _(errmsg));
 	}
       else
-	fatal_error (errmsg);
+	fatal_error_no_args (errmsg);
     }
 
   int_handler  = (void (*) (int)) signal (SIGINT,  SIG_IGN);
Index: gcc/cp/cvt.c
===================================================================
--- gcc/cp/cvt.c	(revision 207074)
+++ gcc/cp/cvt.c	(working copy)
@@ -669,7 +669,7 @@ ocp_convert (tree type, tree expr, int c
        = targetm.invalid_conversion (TREE_TYPE (expr), type)))
     {
       if (complain & tf_error)
-	error (invalid_conv_diag);
+	error_no_args (invalid_conv_diag);
       return error_mark_node;
     }
 
Index: gcc/cp/decl.c
===================================================================
--- gcc/cp/decl.c	(revision 207074)
+++ gcc/cp/decl.c	(working copy)
@@ -9550,7 +9550,7 @@ grokdeclarator (const cp_declarator *dec
 	    errmsg = targetm.invalid_return_type (type);
 	    if (errmsg)
 	      {
-		error (errmsg);
+		error_no_args (errmsg);
 		type = integer_type_node;
 	      }
 
@@ -11124,7 +11124,7 @@ grokparms (tree parmlist, tree *parms)
       if (type != error_mark_node
 	  && (errmsg = targetm.invalid_parameter_type (type)))
 	{
-	  error (errmsg);
+	  error_no_args (errmsg);
 	  type = error_mark_node;
 	  TREE_TYPE (decl) = error_mark_node;
 	}
Index: gcc/cp/parser.c
===================================================================
--- gcc/cp/parser.c	(revision 207074)
+++ gcc/cp/parser.c	(working copy)
@@ -2660,7 +2660,7 @@ cp_parser_check_type_definition (cp_pars
     {
       /* Don't use `%s' to print the string, because quotations (`%<', `%>')
 	 in the message need to be interpreted.  */
-      error (parser->type_definition_forbidden_message);
+      error_no_args (parser->type_definition_forbidden_message);
       return false;
     }
   return true;
Index: gcc/cp/pt.c
===================================================================
--- gcc/cp/pt.c	(revision 207074)
+++ gcc/cp/pt.c	(working copy)
@@ -13965,7 +13965,7 @@ tsubst_copy_and_build (tree t,
 				     &error_msg,
 				     input_location);
 	if (error_msg)
-	  error (error_msg);
+	  error_no_args (error_msg);
 	if (!function_p && identifier_p (decl))
 	  {
 	    if (complain & tf_error)
Index: gcc/cp/typeck.c
===================================================================
--- gcc/cp/typeck.c	(revision 207074)
+++ gcc/cp/typeck.c	(working copy)
@@ -3983,7 +3983,7 @@ cp_build_binary_op (location_t location,
        = targetm.invalid_binary_op (code, type0, type1)))
     {
       if (complain & tf_error)
-	error (invalid_op_diag);
+	error_no_args (invalid_op_diag);
       return error_mark_node;
     }
 
@@ -5568,7 +5568,7 @@ cp_build_unary_op (enum tree_code code,
 				   TREE_TYPE (xarg))))
     {
       if (complain & tf_error)
-	error (invalid_op_diag);
+	error_no_args (invalid_op_diag);
       return error_mark_node;
     }
 
Index: gcc/diagnostic-core.h
===================================================================
--- gcc/diagnostic-core.h	(revision 207074)
+++ gcc/diagnostic-core.h	(working copy)
@@ -56,24 +56,42 @@ extern const char *trim_filename (const
 #endif
 extern void internal_error (const char *, ...) ATTRIBUTE_GCC_DIAG(1,2)
      ATTRIBUTE_NORETURN;
+extern void internal_error_no_args (const char *) ATTRIBUTE_NORETURN;
+
 /* Pass one of the OPT_W* from options.h as the first parameter.  */
 extern bool warning (int, const char *, ...) ATTRIBUTE_GCC_DIAG(2,3);
 extern bool warning_at (location_t, int, const char *, ...)
     ATTRIBUTE_GCC_DIAG(3,4);
+extern bool warning_no_args (int, const char *);
+extern bool warning_at_no_args (location_t, int, const char *);
+
 extern void error (const char *, ...) ATTRIBUTE_GCC_DIAG(1,2);
 extern void error_n (location_t, int, const char *, const char *, ...)
     ATTRIBUTE_GCC_DIAG(3,5) ATTRIBUTE_GCC_DIAG(4,5);
 extern void error_at (location_t, const char *, ...) ATTRIBUTE_GCC_DIAG(2,3);
+extern void error_no_args (const char *);
+extern void error_at_no_args (location_t, const char *);
+
 extern void fatal_error (const char *, ...) ATTRIBUTE_GCC_DIAG(1,2)
      ATTRIBUTE_NORETURN;
+extern void fatal_error_no_args (const char *) ATTRIBUTE_NORETURN;
+
 /* Pass one of the OPT_W* from options.h as the second parameter.  */
 extern bool pedwarn (location_t, int, const char *, ...)
      ATTRIBUTE_GCC_DIAG(3,4);
+extern bool pedwarn_no_args (location_t, int, const char *);
+
 extern bool permerror (location_t, const char *, ...) ATTRIBUTE_GCC_DIAG(2,3);
+extern bool permerror_no_args (location_t, const char *);
+
 extern void sorry (const char *, ...) ATTRIBUTE_GCC_DIAG(1,2);
+extern void sorry_no_args (const char *);
+
 extern void inform (location_t, const char *, ...) ATTRIBUTE_GCC_DIAG(2,3);
 extern void inform_n (location_t, int, const char *, const char *, ...)
     ATTRIBUTE_GCC_DIAG(3,5) ATTRIBUTE_GCC_DIAG(4,5);
+extern void inform_no_args (location_t, const char *);
+
 extern void verbatim (const char *, ...) ATTRIBUTE_GCC_DIAG(1,2);
 extern bool emit_diagnostic (diagnostic_t, location_t, int,
 			     const char *, ...) ATTRIBUTE_GCC_DIAG(4,5);
Index: gcc/diagnostic.c
===================================================================
--- gcc/diagnostic.c	(revision 207074)
+++ gcc/diagnostic.c	(working copy)
@@ -891,29 +891,38 @@ diagnostic_append_note (diagnostic_conte
   va_end (ap);
 }
 
-bool
-emit_diagnostic (diagnostic_t kind, location_t location, int opt,
-		 const char *gmsgid, ...)
+static bool
+v_emit_diagnostic (diagnostic_t kind, location_t location, int opt, const char *gmsgid, va_list *ap_p)
 {
   diagnostic_info diagnostic;
-  va_list ap;
   bool ret;
-
-  va_start (ap, gmsgid);
+  
   if (kind == DK_PERMERROR)
     {
-      diagnostic_set_info (&diagnostic, gmsgid, &ap, location,
+      diagnostic_set_info (&diagnostic, gmsgid, ap_p, location,
 			   permissive_error_kind (global_dc));
       diagnostic.option_index = permissive_error_option (global_dc);
     }
   else {
-      diagnostic_set_info (&diagnostic, gmsgid, &ap, location, kind);
+      diagnostic_set_info (&diagnostic, gmsgid, ap_p, location, kind);
       if (kind == DK_WARNING || kind == DK_PEDWARN)
 	diagnostic.option_index = opt;
   }
 
   ret = report_diagnostic (&diagnostic);
-  va_end (ap);
+  return ret;
+}
+
+bool
+emit_diagnostic (diagnostic_t kind, location_t location, int opt,
+		 const char *gmsgid, ...)
+{
+  va_list ap;
+  bool ret;
+
+  va_start (ap, gmsgid);
+  ret = v_emit_diagnostic (kind, location, opt, gmsgid, &ap);
+  va_end (ap); 
   return ret;
 }
 
@@ -922,12 +931,10 @@ emit_diagnostic (diagnostic_t kind, loca
 void
 inform (location_t location, const char *gmsgid, ...)
 {
-  diagnostic_info diagnostic;
   va_list ap;
 
   va_start (ap, gmsgid);
-  diagnostic_set_info (&diagnostic, gmsgid, &ap, location, DK_NOTE);
-  report_diagnostic (&diagnostic);
+  v_emit_diagnostic (DK_NOTE, location, 0, gmsgid, &ap);
   va_end (ap);
 }
 
@@ -948,21 +955,23 @@ inform_n (location_t location, int n, co
   va_end (ap);
 }
 
+void
+inform_no_args (location_t location, const char *gmsgid)
+{
+  v_emit_diagnostic (DK_NOTE, location, 0, gmsgid, NULL);
+}
+
 /* A warning at INPUT_LOCATION.  Use this for code which is correct according
    to the relevant language specification but is likely to be buggy anyway.
    Returns true if the warning was printed, false if it was inhibited.  */
 bool
 warning (int opt, const char *gmsgid, ...)
 {
-  diagnostic_info diagnostic;
   va_list ap;
   bool ret;
 
   va_start (ap, gmsgid);
-  diagnostic_set_info (&diagnostic, gmsgid, &ap, input_location, DK_WARNING);
-  diagnostic.option_index = opt;
-
-  ret = report_diagnostic (&diagnostic);
+  ret = v_emit_diagnostic (DK_WARNING, input_location, opt, gmsgid, &ap);
   va_end (ap);
   return ret;
 }
@@ -974,18 +983,27 @@ warning (int opt, const char *gmsgid, ..
 bool
 warning_at (location_t location, int opt, const char *gmsgid, ...)
 {
-  diagnostic_info diagnostic;
   va_list ap;
   bool ret;
 
   va_start (ap, gmsgid);
-  diagnostic_set_info (&diagnostic, gmsgid, &ap, location, DK_WARNING);
-  diagnostic.option_index = opt;
-  ret = report_diagnostic (&diagnostic);
+  ret = v_emit_diagnostic (DK_WARNING, location, opt, gmsgid, &ap);
   va_end (ap);
   return ret;
 }
 
+bool
+warning_at_no_args (location_t location, int opt, const char *gmsgid)
+{
+  return v_emit_diagnostic (DK_WARNING, location, opt, gmsgid, NULL);
+}
+
+bool
+warning_no_args (int opt, const char *gmsgid)
+{
+  return warning_at_no_args (input_location, opt, gmsgid);
+}
+
 /* A "pedantic" warning at LOCATION: issues a warning unless
    -pedantic-errors was given on the command line, in which case it
    issues an error.  Use this for diagnostics required by the relevant
@@ -1002,18 +1020,21 @@ warning_at (location_t location, int opt
 bool
 pedwarn (location_t location, int opt, const char *gmsgid, ...)
 {
-  diagnostic_info diagnostic;
   va_list ap;
   bool ret;
 
   va_start (ap, gmsgid);
-  diagnostic_set_info (&diagnostic, gmsgid, &ap, location,  DK_PEDWARN);
-  diagnostic.option_index = opt;
-  ret = report_diagnostic (&diagnostic);
+  ret = v_emit_diagnostic (DK_PEDWARN, location, opt, gmsgid, &ap);
   va_end (ap);
   return ret;
 }
 
+bool
+pedwarn_no_args (location_t location, int opt, const char *gmsgid)
+{
+  return v_emit_diagnostic (DK_PEDWARN, location, opt, gmsgid, NULL);
+}
+
 /* A "permissive" error at LOCATION: issues an error unless
    -fpermissive was given on the command line, in which case it issues
    a warning.  Use this for things that really should be errors but we
@@ -1024,30 +1045,30 @@ pedwarn (location_t location, int opt, c
 bool
 permerror (location_t location, const char *gmsgid, ...)
 {
-  diagnostic_info diagnostic;
   va_list ap;
   bool ret;
 
   va_start (ap, gmsgid);
-  diagnostic_set_info (&diagnostic, gmsgid, &ap, location,
-                       permissive_error_kind (global_dc));
-  diagnostic.option_index = permissive_error_option (global_dc);
-  ret = report_diagnostic (&diagnostic);
+  ret = v_emit_diagnostic (DK_PERMERROR, location, 0, gmsgid, &ap);
   va_end (ap);
   return ret;
 }
 
+bool
+permerror_no_args (location_t location, const char *gmsgid)
+{
+  return v_emit_diagnostic (DK_PERMERROR, location, 0, gmsgid, NULL);
+}
+
 /* A hard error: the code is definitely ill-formed, and an object file
    will not be produced.  */
 void
 error (const char *gmsgid, ...)
 {
-  diagnostic_info diagnostic;
   va_list ap;
 
   va_start (ap, gmsgid);
-  diagnostic_set_info (&diagnostic, gmsgid, &ap, input_location, DK_ERROR);
-  report_diagnostic (&diagnostic);
+  v_emit_diagnostic (DK_ERROR, input_location, 0, gmsgid, &ap);
   va_end (ap);
 }
 
@@ -1072,30 +1093,44 @@ error_n (location_t location, int n, con
 void
 error_at (location_t loc, const char *gmsgid, ...)
 {
-  diagnostic_info diagnostic;
   va_list ap;
 
   va_start (ap, gmsgid);
-  diagnostic_set_info (&diagnostic, gmsgid, &ap, loc, DK_ERROR);
-  report_diagnostic (&diagnostic);
+  v_emit_diagnostic (DK_ERROR, loc, 0, gmsgid, &ap); 
   va_end (ap);
 }
 
+void
+error_at_no_args (location_t loc, const char *gmsgid)
+{
+  v_emit_diagnostic (DK_ERROR, loc, 0, gmsgid, NULL);
+}
+
+void
+error_no_args (const char *gmsgid)
+{
+  error_at_no_args (input_location, gmsgid);
+}
+
 /* "Sorry, not implemented."  Use for a language feature which is
    required by the relevant specification but not implemented by GCC.
    An object file will not be produced.  */
 void
 sorry (const char *gmsgid, ...)
 {
-  diagnostic_info diagnostic;
   va_list ap;
 
   va_start (ap, gmsgid);
-  diagnostic_set_info (&diagnostic, gmsgid, &ap, input_location, DK_SORRY);
-  report_diagnostic (&diagnostic);
+  v_emit_diagnostic (DK_SORRY, input_location, 0, gmsgid, &ap);
   va_end (ap);
 }
 
+void
+sorry_no_args (const char *gmsgid)
+{
+  v_emit_diagnostic (DK_SORRY, input_location, 0, gmsgid, NULL);
+}
+
 /* Return true if an error or a "sorry" has been seen.  Various
    processing is disabled after errors.  */
 bool
@@ -1110,17 +1145,22 @@ seen_error (void)
 void
 fatal_error (const char *gmsgid, ...)
 {
-  diagnostic_info diagnostic;
   va_list ap;
 
   va_start (ap, gmsgid);
-  diagnostic_set_info (&diagnostic, gmsgid, &ap, input_location, DK_FATAL);
-  report_diagnostic (&diagnostic);
+  v_emit_diagnostic (DK_FATAL, input_location, 0, gmsgid, &ap);
   va_end (ap);
 
   gcc_unreachable ();
 }
 
+void
+fatal_error_no_args (const char *gmsgid)
+{
+  v_emit_diagnostic (DK_FATAL, input_location, 0, gmsgid, NULL);
+  gcc_unreachable ();
+}
+
 /* An internal consistency check has failed.  We make no attempt to
    continue.  Note that unless there is debugging value to be had from
    a more specific message, or some other good reason, you should use
@@ -1128,16 +1168,23 @@ fatal_error (const char *gmsgid, ...)
 void
 internal_error (const char *gmsgid, ...)
 {
-  diagnostic_info diagnostic;
   va_list ap;
 
   va_start (ap, gmsgid);
-  diagnostic_set_info (&diagnostic, gmsgid, &ap, input_location, DK_ICE);
-  report_diagnostic (&diagnostic);
+  v_emit_diagnostic (DK_ICE, input_location, 0, gmsgid, &ap);
   va_end (ap);
 
   gcc_unreachable ();
 }
+
+void
+internal_error_no_args (const char *gmsgid)
+{
+  v_emit_diagnostic (DK_ICE, input_location, 0, gmsgid, NULL);
+  gcc_unreachable ();
+}
+
+
 \f
 /* Special case error functions.  Most are implemented in terms of the
    above, or should be.  */
Index: gcc/dwarf2asm.c
===================================================================
--- gcc/dwarf2asm.c	(revision 207074)
+++ gcc/dwarf2asm.c	(working copy)
@@ -164,7 +164,7 @@ dw2_asm_output_vms_delta (int size ATTRI
 #ifndef ASM_OUTPUT_DWARF_VMS_DELTA
   /* VMS Delta is only special on ia64-vms, but this function also gets
      called on alpha-vms so it has to do something sane.  */
-  dw2_asm_output_delta (size, lab1, lab2, comment);
+  dw2_asm_output_delta (size, lab1, lab2, "%s", comment);
 #else
   ASM_OUTPUT_DWARF_VMS_DELTA (asm_out_file, size, lab1, lab2);
   if (flag_debug_asm && comment)
Index: gcc/fold-const.c
===================================================================
--- gcc/fold-const.c	(revision 207074)
+++ gcc/fold-const.c	(working copy)
@@ -312,7 +312,7 @@ fold_overflow_warning (const char* gmsgi
 	}
     }
   else if (issue_strict_overflow_warning (wc))
-    warning (OPT_Wstrict_overflow, gmsgid);
+    warning_no_args (OPT_Wstrict_overflow, gmsgid);
 }
 \f
 /* Return true if the built-in mathematical function specified by CODE
Index: gcc/gcc.c
===================================================================
--- gcc/gcc.c	(revision 207074)
+++ gcc/gcc.c	(working copy)
@@ -2782,7 +2782,7 @@ execute (void)
       if (errmsg != NULL)
 	{
 	  if (err == 0)
-	    fatal_error (errmsg);
+	    fatal_error_no_args (errmsg);
 	  else
 	    {
 	      errno = err;
Index: gcc/lto-wrapper.c
===================================================================
--- gcc/lto-wrapper.c	(revision 207074)
+++ gcc/lto-wrapper.c	(working copy)
@@ -192,10 +192,10 @@ collect_execute (char **argv)
       if (err != 0)
 	{
 	  errno = err;
-	  fatal_perror (errmsg);
+	  fatal_perror ("%s", errmsg);
 	}
       else
-	fatal (errmsg);
+	fatal ("%s", errmsg);
     }
 
   return pex;
Index: gcc/opts.c
===================================================================
--- gcc/opts.c	(revision 207074)
+++ gcc/opts.c	(working copy)
@@ -1039,7 +1039,7 @@ print_filtered_help (unsigned int includ
 		      if (* (const char **) flag_var != NULL)
 			snprintf (new_help + strlen (new_help),
 				  sizeof (new_help) - strlen (new_help),
-				  * (const char **) flag_var);
+				  "%s", * (const char **) flag_var);
 		    }
 		  else if (option->var_type == CLVC_ENUM)
 		    {
@@ -1053,7 +1053,7 @@ print_filtered_help (unsigned int includ
 			arg = _("[default]");
 		      snprintf (new_help + strlen (new_help),
 				sizeof (new_help) - strlen (new_help),
-				arg);
+				"%s", arg);
 		    }
 		  else
 		    sprintf (new_help + strlen (new_help),
Index: gcc/tree-sra.c
===================================================================
--- gcc/tree-sra.c	(revision 207074)
+++ gcc/tree-sra.c	(working copy)
@@ -3861,7 +3861,7 @@ dump_dereferences_table (FILE *f, const
 {
   basic_block bb;
 
-  fprintf (dump_file, str);
+  fprintf (dump_file, "%s", str);
   FOR_BB_BETWEEN (bb, ENTRY_BLOCK_PTR_FOR_FN (cfun),
 		  EXIT_BLOCK_PTR_FOR_FN (cfun), next_bb)
     {
Index: gcc/tree-ssa-uninit.c
===================================================================
--- gcc/tree-ssa-uninit.c	(revision 207074)
+++ gcc/tree-ssa-uninit.c	(working copy)
@@ -764,7 +764,7 @@ dump_predicates (gimple usestmt, pred_ch
 {
   size_t i, j;
   pred_chain one_pred_chain = vNULL;
-  fprintf (dump_file, msg);
+  fprintf (dump_file, "%s", msg);
   print_gimple_stmt (dump_file, usestmt, 0, 0);
   fprintf (dump_file, "is guarded by :\n\n");
   size_t num_preds = preds.length ();
Index: include/ansidecl.h
===================================================================
--- include/ansidecl.h	(revision 207074)
+++ include/ansidecl.h	(working copy)
@@ -205,6 +205,12 @@ So instead we use the macro below and te
 #define ATTRIBUTE_PRINTF_5 ATTRIBUTE_PRINTF(5, 6)
 #endif /* ATTRIBUTE_PRINTF */
 
+#ifndef ATTRIBUTE_GNU_PRINTF
+#define ATTRIBUTE_GNU_PRINTF(m, n) __attribute__ ((__format__ (__gnu_printf__, m, n))) ATTRIBUTE_NONNULL(m)
+#define ATTRIBUTE_GNU_PRINTF_3_0 ATTRIBUTE_GNU_PRINTF(3, 0)
+#define ATTRIBUTE_GNU_PRINTF_5_0 ATTRIBUTE_GNU_PRINTF(5, 0)
+#endif /* ATTRIBUTE_GNU_PRINTF */
+
 /* Use ATTRIBUTE_FPTR_PRINTF when the format attribute is to be set on
    a function pointer.  Format attributes were allowed on function
    pointers as of gcc 3.1.  */
Index: libcpp/errors.c
===================================================================
--- libcpp/errors.c	(revision 207074)
+++ libcpp/errors.c	(working copy)
@@ -81,6 +81,12 @@ cpp_error (cpp_reader * pfile, int level
   return ret;
 }
 
+bool
+cpp_error_no_args (cpp_reader *pfile, int level, const char *msgid)
+{
+  return cpp_diagnostic (pfile, level, CPP_W_NONE, msgid, NULL);
+}
+
 /* Print a warning.  The warning reason may be given in REASON.  */
 
 bool
@@ -97,6 +103,12 @@ cpp_warning (cpp_reader * pfile, int rea
   return ret;
 }
 
+bool
+cpp_warning_no_args (cpp_reader *pfile, int reason, const char *msgid)
+{
+  return cpp_diagnostic (pfile, CPP_DL_WARNING, reason, msgid, NULL);
+}
+
 /* Print a pedantic warning.  The warning reason may be given in REASON.  */
 
 bool
@@ -113,6 +125,12 @@ cpp_pedwarning (cpp_reader * pfile, int
   return ret;
 }
 
+bool
+cpp_pedwarning_no_args (cpp_reader *pfile, int reason, const char *msgid)
+{
+  return cpp_diagnostic (pfile, CPP_DL_PEDWARN, reason, msgid, NULL);
+}
+
 /* Print a warning, including system headers.  The warning reason may be
    given in REASON.  */
 
@@ -130,6 +148,12 @@ cpp_warning_syshdr (cpp_reader * pfile,
   return ret;
 }
 
+bool
+cpp_warning_syshdr_no_args (cpp_reader *pfile, int reason, const char *msgid)
+{
+  return cpp_diagnostic (pfile, CPP_DL_WARNING_SYSHDR, reason, msgid, NULL);
+}
+
 /* Print a diagnostic at a specific location.  */
 
 ATTRIBUTE_FPTR_PRINTF(6,0)
@@ -166,6 +190,15 @@ cpp_error_with_line (cpp_reader *pfile,
   return ret;
 }
 
+bool
+cpp_error_with_line_no_args (cpp_reader *pfile, int level,
+         source_location src_loc, unsigned int column,
+         const char *msgid)
+{
+  return cpp_diagnostic_with_line (pfile, level, CPP_W_NONE, src_loc,
+                                   column, msgid, NULL);
+}
+
 /* Print a warning.  The warning reason may be given in REASON.  */
 
 bool
@@ -185,6 +218,15 @@ cpp_warning_with_line (cpp_reader *pfile
   return ret;
 }
 
+bool
+cpp_warning_with_line_no_args (cpp_reader *pfile, int reason,
+           source_location src_loc, unsigned int column,
+           const char *msgid)
+{
+  return cpp_diagnostic_with_line (pfile, CPP_DL_WARNING, reason, src_loc,
+                                   column, msgid, NULL);
+}
+
 /* Print a pedantic warning.  The warning reason may be given in REASON.  */
 
 bool
@@ -204,6 +246,15 @@ cpp_pedwarning_with_line (cpp_reader *pf
   return ret;
 }
 
+bool
+cpp_pedwarning_with_line_no_args (cpp_reader *pfile, int reason,
+              source_location src_loc, unsigned int column,
+              const char *msgid)
+{
+  return cpp_diagnostic_with_line (pfile, CPP_DL_PEDWARN, reason, src_loc,
+                                   column, msgid, NULL);
+}
+
 /* Print a warning, including system headers.  The warning reason may be
    given in REASON.  */
 
@@ -224,6 +275,15 @@ cpp_warning_with_line_syshdr (cpp_reader
   return ret;
 }
 
+bool
+cpp_warning_with_line_syshdr_no_args (cpp_reader *pfile, int reason,
+                     source_location src_loc, unsigned int column,
+                     const char *msgid)
+{
+  return cpp_diagnostic_with_line (pfile, CPP_DL_WARNING_SYSHDR, reason, src_loc,
+                                   column, msgid, NULL);
+}
+
 /* Print a warning or error, depending on the value of LEVEL.  Include
    information from errno.  */
 
Index: libcpp/expr.c
===================================================================
--- libcpp/expr.c	(revision 207074)
+++ libcpp/expr.c	(working copy)
@@ -668,10 +668,10 @@ cpp_classify_number (cpp_reader *pfile,
 		                : N_("use of C99 long long integer constant");
 
 	  if (CPP_OPTION (pfile, c99))
-            cpp_warning_with_line (pfile, CPP_W_LONG_LONG, virtual_location,
+            cpp_warning_with_line_no_args (pfile, CPP_W_LONG_LONG, virtual_location,
 				   0, message);
           else
-            cpp_pedwarning_with_line (pfile, CPP_W_LONG_LONG,
+            cpp_pedwarning_with_line_no_args (pfile, CPP_W_LONG_LONG,
 				      virtual_location, 0, message);
         }
 
Index: libcpp/include/cpplib.h
===================================================================
--- libcpp/include/cpplib.h	(revision 207074)
+++ libcpp/include/cpplib.h	(working copy)
@@ -939,12 +939,23 @@ enum {
 /* Output a diagnostic of some kind.  */
 extern bool cpp_error (cpp_reader *, int, const char *msgid, ...)
   ATTRIBUTE_PRINTF_3;
+extern bool cpp_error_no_args (cpp_reader *, int, const char *)
+  ATTRIBUTE_GNU_PRINTF_3_0;
+
 extern bool cpp_warning (cpp_reader *, int, const char *msgid, ...)
   ATTRIBUTE_PRINTF_3;
+extern bool cpp_warning_no_args (cpp_reader *, int, const char *)
+  ATTRIBUTE_GNU_PRINTF_3_0;
+
 extern bool cpp_pedwarning (cpp_reader *, int, const char *msgid, ...)
   ATTRIBUTE_PRINTF_3;
+extern bool cpp_pedwarning_no_args (cpp_reader *, int, const char *)
+  ATTRIBUTE_GNU_PRINTF_3_0;
+
 extern bool cpp_warning_syshdr (cpp_reader *, int, const char *msgid, ...)
   ATTRIBUTE_PRINTF_3;
+extern bool cpp_warning_syshdr_no_args (cpp_reader *, int, const char *)
+  ATTRIBUTE_GNU_PRINTF_3_0;
 
 /* Output a diagnostic with "MSGID: " preceding the
    error string of errno.  No location is printed.  */
@@ -956,15 +967,31 @@ extern bool cpp_errno (cpp_reader *, int
 extern bool cpp_error_with_line (cpp_reader *, int, source_location,
                                  unsigned, const char *msgid, ...)
   ATTRIBUTE_PRINTF_5;
+extern bool cpp_error_with_line_no_args (cpp_reader *, int, source_location,
+                                         unsigned, const char *)
+  ATTRIBUTE_GNU_PRINTF_5_0;
+
 extern bool cpp_warning_with_line (cpp_reader *, int, source_location,
                                    unsigned, const char *msgid, ...)
   ATTRIBUTE_PRINTF_5;
+
+extern bool cpp_warning_with_line_no_args (cpp_reader *, int, source_location,
+                                           unsigned, const char *)
+  ATTRIBUTE_GNU_PRINTF_5_0;
+
 extern bool cpp_pedwarning_with_line (cpp_reader *, int, source_location,
                                       unsigned, const char *msgid, ...)
   ATTRIBUTE_PRINTF_5;
+extern bool cpp_pedwarning_with_line_no_args (cpp_reader *, int, source_location,
+                                              unsigned, const char *)
+  ATTRIBUTE_GNU_PRINTF_5_0;
+
 extern bool cpp_warning_with_line_syshdr (cpp_reader *, int, source_location,
                                           unsigned, const char *msgid, ...)
   ATTRIBUTE_PRINTF_5;
+extern bool cpp_warning_with_line_syshdr_no_args (cpp_reader *, int, source_location,
+                                                  unsigned, const char *)
+  ATTRIBUTE_GNU_PRINTF_5_0;
 
 /* In lex.c */
 extern int cpp_ideq (const cpp_token *, const char *);
Index: libcpp/macro.c
===================================================================
--- libcpp/macro.c	(revision 207074)
+++ libcpp/macro.c	(working copy)
@@ -2969,7 +2969,7 @@ create_iso_definition (cpp_reader *pfile
 	     function-like macros, but not at the end.  */
 	  if (following_paste_op)
 	    {
-	      cpp_error (pfile, CPP_DL_ERROR, paste_op_error_msg);
+	      cpp_error_no_args (pfile, CPP_DL_ERROR, paste_op_error_msg);
 	      return false;
 	    }
 	  break;
@@ -2982,7 +2982,7 @@ create_iso_definition (cpp_reader *pfile
 	     function-like macros, but not at the beginning.  */
 	  if (macro->count == 1)
 	    {
-	      cpp_error (pfile, CPP_DL_ERROR, paste_op_error_msg);
+	      cpp_error_no_args (pfile, CPP_DL_ERROR, paste_op_error_msg);
 	      return false;
 	    }
 

Re: -Wformat-security warnings generated in gcc build

[Prathamesh Kulkarni]

On Sun, Jan 26, 2014 at 3:56 PM, Prathamesh Kulkarni
<bilbotheelffriend@gmail.com> wrote:
> On Fri, Jan 24, 2014 at 9:19 PM, Prathamesh Kulkarni
> <bilbotheelffriend@gmail.com> wrote:
>> On Thu, Jan 23, 2014 at 9:09 PM, Prathamesh Kulkarni
>> <bilbotheelffriend@gmail.com> wrote:
>>> On Thu, Jan 23, 2014 at 8:24 PM, Dodji Seketeli <dodji@redhat.com> wrote:
>>>> Prathamesh Kulkarni <bilbotheelffriend@gmail.com> writes:
>>>>
>>>>>
>>>>> Shall it be correct then to replace calls to error() and friends,
>>>>> taking only format string with no-argument specifiers
>>>>> to error_at_no_args() ? (similarly we shall need warning_at_no_args,
>>>>> pedwarn_no_args, etc.)
>>>>
>>>> I would guess so, yes.
>>>>
>>>>>>
>>>>>> Also, you'd need to modify cp/error.c:cp_printer in a similar way, to
>>>>>> issue an internal_error each time we try to access a null test->args_ptr.
>>>>>
>>>>> Shall check for text->args_ptr be required in each case label of
>>>>> argument specifier in pp_format()
>>>>> and client-specific functions like cp_printer() ?
>>>>
>>>> Yes, I think so.  Maybe you can make that a bit more maintainable by
>>>> creating a macro like those used to access text->args_ptr in cp_printer,
>>>> e.g:
>>>>
>>>>     #define next_int     va_arg (*text->args_ptr, int)
>>>>
>>>> In that macro, make the check for text->args_ptr before accessing it,
>>>> and then use that macro to access text->args_ptr through the function.
>>>>
>>>
>>> I was going through diagnostic.c, it appears that many functions in
>>> (error(), error_at(), warning(), etc.) share common code (mostly
>>> contains call to diagnostic_set_info() followed by call to
>>> report_diagnostic()), which I guess can be re-written in terms of
>>> emit_diagnostic(), however since it's variadic that's not possible. I
>>> wrote a v_emit_diagnostic() function, that takes same arguments as
>>> emit_diagnostic(), with additional va_list * at end (va_list * instead
>>> of va_list, so I could pass NULL for error_no_args() and friends). Is
>>> it correct to write these other functions (emit_diagnostic(), error(),
>>> inform(), etc.) in terms of v_emit_diagnostic() ?
>>
>> silly mistake in warning_at(), it should be:
>> ret = v_emit_diagnostic (DK_WARNING, location, opt, gmsgid, &ap);
> The attached patch removes all -Wformat-security warnings
> I put assertions for tree->args_ptr in pretty-print.c:pp_format(),
> cp/error.c:cp_printer() and
> c/c-objc-common.c:c_tree_printer().
>
> However, adding *_no_args() functions in include/cpplib.h (eg:
> cpp_error_no_args(), etc.), generates a new set of warnings
> of type -Wsuggest-attribute=format gnu_printf for these functions.
>
> I removed them by adding the following in include/ansidecl.h
> +#ifndef ATTRIBUTE_GNU_PRINTF
> +#define ATTRIBUTE_GNU_PRINTF(m, n) __attribute__ ((__format__
> (__gnu_printf__, m, n))) ATTRIBUTE_NONNULL(m)
> +#define ATTRIBUTE_GNU_PRINTF_3_0 ATTRIBUTE_GNU_PRINTF(3, 0)
> +#define ATTRIBUTE_GNU_PRINTF_5_0 ATTRIBUTE_GNU_PRINTF(5, 0)
> +#endif /* ATTRIBUTE_GNU_PRINTF */
> +
>
> and declarations changed to:
> +extern bool cpp_error_no_args (cpp_reader *, int, const char *)
> +  ATTRIBUTE_GNU_PRINTF_3_0;
> +
>
> Is this correct way to fix it ?
> Thanks!
Ping ?
>
>>
>>>
>>>
>>>
>>>
>>>
>>>> --
>>>>                 Dodji