This is the mail archive of the mailing list for the GCC project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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

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

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.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]