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

[Martin Sebor]

On 07/04/2016 04:59 AM, Richard Biener wrote:

On Fri, 1 Jul 2016, Martin Sebor wrote:

The attached patch enhances compile-time checking for buffer overflow
and output truncation in non-trivial calls to the sprintf family of
functions under a new option -Wformat-length=[12].  This initial
patch handles printf directives with string, integer, and simple
floating arguments but eventually I'd like to extend it all other
functions and directives for which it makes sense.

I made some choices in the implementation that resulted in trade-offs
in the quality of the diagnostics.  I would be grateful for comments
and suggestions how to improve them.  Besides the list I include
Jakub who already gave me some feedback (thanks), Joseph who as
I understand has deep knowledge of the c-format.c code, and Richard
for his input on the LTO concern below.

1) Making use of -Wformat machinery in c-family/c-format.c.  This
    seemed preferable to duplicating some of the same code elsewhere
    (I initially started implementing it in expand_builtin in
    builtins.c).  It makes the implementation readily extensible
    to all the same formats as those already handled for -Wformat.
    One drawback is that unlike in expand_builtin, calls to these
    functions cannot readily be folded.  Another drawback pointed

folded?  You mean this -W option changes code generation?

No, it doesn't.  What I meant is that the same code, when added
in builtins.c instead, could readily be extended to fold into
strings expressions like

  sprintf (buf, "%i", 123);

    out by Jakub is that since the code is only available in the
    C and C++ compilers, it apparently may not be available with
    an LTO compiler (I don't completely understand this problem
    but I mention it in the interest of full disclosure). In light
    of the dependency in (2) below, I don't see a way to avoid it
    (moving c-format.c to the middle end was suggested but seemed
    like too much of a change to me).

Yes, lto1 is not linked with C_COMMON_OBJS (that could be changed
of course at the expense of dragging in some dead code).  Moving
all the format stuff to the middle-end (or separated better so
the overhead in lto1 is lower) would be possible as well.

That said, a langhook as you add it highlights the issue with LTO.

Thanks for the clarification.  IIUC, there are at least three
possibilities for how to proceed: leave it as is (no checking
with LTO), link LTO with C_COMMON_OBJS, or move the c-format.c
code into the middle end.  Do you have a preference for one of
these?  Or is there another solution that I missed?

FWIW, I would expect a good number of other warnings to benefit
from optimization and having a general solution for this problem
to be helpful.  I also suspect this isn't the first time this
issue has come up.  I'm wondering what solutions have already
been considered and with what pros and cons (naively, I would
think that factoring the relevant code out of cc1 into a shared
library that lto1 could load should work).

Martin