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

[Martin Sebor <msebor at gmail dot com>]

> Does it warn for the code that Florian actually posted?  I tried it
> with a recent (unpatched) build of trunk and got no warning (with -O2
> -Wall -D_FORTIFY_SOURCE=2), but it strikes me that we ought to warn if
> someone passes about the above code (for the uninitialized format
> string, at least; I don't know if it's legal to pass a string literal
> as the destination).
>
> Should I file a PR for this?

It doesn't warn for it because in C, string literals are of type
char[] and so implicitly convertible to char* (shudder).  GCC
does warn for it with -Wwrite-strings or -Wdiscarded-qualifiers,
and will give a pedantic warning in C++ (it seems an error would
be preferable).

> > Thanks for giving it a try!  Based on the feedback I received
> > I've since updated the patch and will post the latest version
> > for review soon.  In simple cases like this one it warns even
> > without _FORTIFY_SOURCE or optimization (though without the
> > latter it doesn't benefit from VRP information).  Let me see
> > about adding a warning to detect and warn when the arguments
> > are transposed.
> >
> > $ /build/gcc-49905/gcc/xgcc -B /build/gcc-49905/gcc -S -Wall -Wextra
> > -Wpedantic -Wformat-length=2 xyz.c
> >
> > xyz.c: In function ‘format_1’:
> > xyz.c:13:29: warning: may write a terminating nul past the end of the
> > destination [-Wformat-length=]
> >      sprintf (buf, "%u.%u.%u.%u", a, b, c, d);
> >                                ^
> > xyz.c:13:3: note: destination size is ‘15’ bytes, output size between
> > ‘8’ and ‘16’
> >      sprintf (buf, "%u.%u.%u.%u", a, b, c, d);
> >      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> Style question: should the numbers in these diagnostic messages be in
> quotes?  That looks a bit strange to me.  It seems clearer to me to
> have:

You're probably right.  I suspect I have a tendency to overuse
the quotes (e.g, the -Wplacement-new warning also quotes the
sizes).  If there aren't yet (I vague recall coming across
something on the GCC Wiki but can't find it now), it would be
helpful to put in place some diagnostic style conventions like
there are for formatting code to guide us in cases like this.
I'm willing to help put the document together or add this to
it if one already exists.

Martin