This is the mail archive of the gcc@gcc.gnu.org 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


"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


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