This is the mail archive of the mailing list for the GCC project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: -Wformat-security warnings generated in gcc build

On Thu, Jan 23, 2014 at 12:32:34PM +0100, Dodji Seketeli wrote:
> "Joseph S. Myers" <> 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


> I hope this helps.
> -- 
> 		Dodji

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]