[PATCH] - improve sprintf buffer overflow detection (middle-end/49905)
Jeff Law
law@redhat.com
Thu Jul 21 21:48:00 GMT 2016
I saw a few places in GCC itself where you increased buffer sizes. Were
any of those level 1 failures?
In c.opt:
> +C ObjC C++ ObjC++ Warning Alias (Wformat-length=, 1, 0)
Can't this take on values 0, 1, 2? And if so, is the "1" above correct?
In invoke.texi we have:
+-Wno-format-contains-nul -Wno-format-extra-args -Wformat-length=2 @gol
Note Wformat-length=2.
Then in the actual documentation it says the default level is 1.
> +in output truncation. GCC counts the number of bytes that each format
> +string and directive within it writes into the provided buffer and, when
> +it detects that more bytes that fit in the destination buffer may be output,
> +it emits a warning.
s/that fit/than fit
In that same section is the text which says the default level is 1
I'm curious about the heuristics for level 1. I guess if you get a
warning at level 1, then you've got a situation where we know we'll
write out of bounds. level 2 is a may write out of bounds. Which makes
me wonder if rather than levels we should use symbolic names?
I guess my worry here is that if we don't message this right folks will
make their code level 1 clean and think they're done. When in reality
all they've done is mitigate the most egregious problems.
Elsewhere in invoke.texi:
> +buffer in an inforational note following the warning.
informational I assume
> +the minimum buffer size. For exampe, if @var{a} and @var{b} above can
example
In genmatch.c:
> + char id[13]; /* Big enough for a 32-bit UINT_MAX. */
In general we don't add comments to the end of a line like that. If a
comment is needed, put it before the declaration. I'm not sure if a
comment is needed here or not -- your call.
Similarly for various other places were you increased buffer lengths.
In passes.c:
> +
> + // inform (0, "executing pass: %s", pass->name);
> +
> if (execute_one_pass (pass) && pass->sub)
> - execute_pass_list_1 (pass->sub);
> + {
> + // inform (0, "executing subpass: %s", pass->sub->name);
> + execute_pass_list_1 (pass->sub);
> + }
> +
The comments left over from debugging?
Looks like ~1/3 of the patch are tests. Good stuff. I'm going to
assume those are correct.
I'll get into gimple-ssa-sprintf.c either later tonight or tomorrow.
But so far nothing major, just nits.
Jeff
More information about the Gcc-patches
mailing list