This is the mail archive of the
mailing list for the GCC project.
Re: [PATCH] - improve sprintf buffer overflow detection (middle-end/49905)
- From: Richard Biener <rguenther at suse dot de>
- To: Martin Sebor <msebor at gmail dot com>
- Cc: Gcc Patch List <gcc-patches at gcc dot gnu dot org>, Joseph Myers <joseph at codesourcery dot com>, Jakub Jelinek <jakub at redhat dot com>
- Date: Tue, 5 Jul 2016 12:10:45 +0200 (CEST)
- Subject: Re: [PATCH] - improve sprintf buffer overflow detection (middle-end/49905)
- Authentication-results: sourceware.org; auth=none
- References: <5776B33E.email@example.com> <alpine.LSU.firstname.lastname@example.org> <577A8D6A.email@example.com>
On Mon, 4 Jul 2016, Martin Sebor wrote:
> 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=. 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?
Another solution is to implement this somewhen before LTO
bytecode is streamed out thus at the end of early optimizations.
I'm not sure linking with C_COMMON_OBJS does even work (you can try).
Likewise c-format.c may be too entangled with the FEs (maybe just
linking with c-format.o is enough?).
> 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).
Richard Biener <firstname.lastname@example.org>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)