[PATCH] - improve sprintf buffer overflow detection (middle-end/49905)

Martin Sebor msebor@gmail.com
Mon Sep 12 08:07:00 GMT 2016


On 09/08/2016 04:10 PM, Joseph Myers wrote:
> On Thu, 8 Sep 2016, Martin Sebor wrote:
>
>> diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
>> index da133a4..4607495 100644
>> --- a/gcc/doc/tm.texi.in
>> +++ b/gcc/doc/tm.texi.in
>> @@ -4081,6 +4081,13 @@ In either case, it remains possible to select code-generation for the alternate
>>   scheme, by means of compiler command line switches.
>>   @end defmac
>>
>> +@deftypefn {Target Hook} {const char *} TARGET_LIBC_PRINTF_POINTER_FORMAT (tree, const char **@var{flags})
>> +A hook to determine the target @code{printf} implementation format string
>> +that the most closely corresponds to the @code{%p} format directive.
>> +The object pointed to by the @var{flags} is set to a string consisting
>> +of recognized format flags such as the @code{'#'} character.
>> +@end deftypefn
>
> No, the substance of hook documentation should go in target.def with just
> an @hook line in tm.texi.in leading to the documentation going in tm.texi
> automatically.
>
> You appear to be defining a target macro masquerading as a hook.  Please
> don't (new target macros should be avoided where possible); use a proper
> hook.  (Maybe the settings depending on OS rather than architecture means
> it needs to be one of those whose default is a manual setting in
> target-def.h rather than automatically generated, but that should be the
> limit of deviation from the normal workings of hooks.)
>
>> +  const char *pfmt = TARGET_LIBC_PRINTF_POINTER_FORMAT (arg, &flags);
>
> With a proper hook them you'd call targetm.libc_printf_pointer_format.
>
>> +	inform (callloc,
>> +		(nbytes + exact == 1
>> +		 ? "format output %wu byte into a destination of size %wu"
>> +		 : "format output %wu bytes into a destination of size %wu"),
>> +		nbytes + exact, info.objsize);
>
> You need to use G_() around both format strings in such a case; xgettext
> doesn't know how to extract them both.

Attached is revision 8 of the patch (hopefully) adjusted as
requested above, and with a simple test with of the multiline
diagnostic directives suggested by David.  This revision also
enables the -fprintf-return-value option by default.  The libgomp
test failures I was seeing in my earlier testing must have been
caused by an older version of GMP or MPFR that I had inadvertently
use (normally I use in-tree versions downloaded and expanded there
by the download_prerequisites script but that time I forgot that
step).

David, in the way of feedback, I spent hours debugging the simple
multiline test I added.  It seems that DejaGnu is exquisitely
sensitive to whitespaces in the multiline output.  I appreciate
that spacing is essential to lining up the caret and the tildes
with the text of the program but tests fail even when all the
expected output is lined up right in the multiline directive but
off by a space (or tab) with respect to the actual output.  That
DejaGnu output doesn't make this very clear at all makes debugging
these types of issues a pain.  I wonder if there's a better to do
this.

Thanks
Martin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: gcc-49905.diff
Type: text/x-patch
Size: 217633 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20160912/206f7163/attachment.bin>


More information about the Gcc-patches mailing list