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? > 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. Richard. > 2) Optimization. > In keeping with the other -Wformat options, the checking is > enabled without optimization. Especially at level 2, the > warnings can be useful even without it. But to make buffer > sizes and non-constant argument values available in calls to > functions like sprintf (via __builtin_object_size) better > results are obtained with optimization. > > 3) Truncation warnings. > Although calls to bounded functions like snprintf aren't subject > to buffer overflow, they can be subject to accidental truncation > when the destination buffer isn't sized appropriately. With the > patch, such calls are diagnosed under the same option, but I > wonder if have a separate warning option for them might be > preferable (e.g., -Wformat-trunc=[01] or something like that). > Independently, it might be useful to differentiate between > truncating calls that check the return value and those that > don't. > > Besides the usual testing I compiled several packages with the > warning. If found a few bugs in boundary cases in Binutils that > are being fixed. > > Thanks > Martin > > PS There are a few FIXME notes in the patch that I will either > fix or remove, depending on feedback, before committing the > patch. > -- Richard Biener <rguent...@suse.de> SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)
