Re: -Wformat-security warnings generated in gcc build

[Prathamesh Kulkarni <bilbotheelffriend at gmail dot com>]

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