On 1/10/21 12:28 PM, Bruce Korb wrote:
Hi Martin,
On 1/10/21 11:01 AM, Martin Sebor wrote:
On 1/8/21 12:38 PM, Bruce Korb via Gcc wrote:
This is the code that must be confusing to GCC. "def_str" points to
the second character in the 520 byte buffer. "def_scan" points to a
character that we already know we're going to copy into the
destination, so the "spn" function doesn't look at it:
{
char * end = spn_ag_char_map_chars(def_scan + 1, 31);
size_t len = end - def_scan;
if (len >= 256)
goto fail_return;
memcpy(def_str, def_scan, len);
def_str += len;
*def_str = '\0';
def_scan = end;
}
In the function preamble, "def_str" points to the first character
(character "0") of a 520 byte buffer. Before this fragment, "*def_str"
is set to an apostrophe and the pointer advanced. After execution passes
through this fragment, "def_str" is pointing to a NUL byte that can be
as far as 257 bytes into the buffer (character "257"). That leaves 263
more bytes. The "offending" sprintf is:
sprintf(def_str, " %s'", name_bf);
Sure. I was confirming that based on the GCC dump there is no risk
of an overflow in the translation unit, and so there is no warning.
GCC correctly determines that "name_bf" cannot contain more than 255
bytes. Add 3 bytes of text and a NUL byte and the sprintf will be
dropping *AT MOST* 259 characters into the buffer. The buffer is 4 bytes
longer than necessary.
GCC 8 also doesn't warn but it does determine the size. Here's
the output for the relevant directive (from the output of
-fdump-tree-printf-return-value in GCC versions prior to 10, or
-fdump-tree-strlen in GCC 10 and later). objsize is the size
of the destination, or 520 bytes here (this is in contrast to
the 255 in the originally reported message). The Result numbers
are the minimum and maximum size of the output (between 0 and
255 characters).
Computing maximum subobject size for def_str_146:
getdefs.c:275: sprintf: objsize = 520, fmtstr = " %s'"
Directive 1 at offset 0: " ", length = 2
Result: 2, 2, 2, 2 (2, 2, 2, 2)
Directive 2 at offset 2: "%s"
Result: 0, 255, 255, 255 (2, 257, 257, 257)
Directive 3 at offset 4: "'", length = 1
Result: 1, 1, 1, 1 (3, 258, 258, 258)
Directive 4 at offset 5: "", length = 1
Besides provable overflow, it's worth noting that -Wformat-overflow
It can /not/ overflow. Those compiler stats are not decipherable by me.
They indicate the minimum, likely, maximum, and unlikely maximum
number of bytes of output for each directive and the running totals
for the call (in parentheses). The relevant lines are these:
Directive 2 at offset 2: "%s"
Result: 0, 255, 255, 255 (2, 257, 257, 257)
The result tells us that the length of the %s argument is between
0 and 255 bytes long.
Since objsize (the size of the destination) is 520 there is no
buffer overflow.
The note in the forwarded message indicates that GCC computes
the destination size to be much smaller for some reason:
note: 'sprintf' output between 4 and 259 bytes into a destination of
size 255
I.e., it thinks it's just 255 bytes. As I explained, such a small
size would trigger the warning by design. I can't really think of
a reason why GCC would compute a smaller size here (it looks far
from trivial). We'd need to see the original poster's translation
unit and know the host and the target GCC was configured for.
Martin
also diagnoses a subset of cases where it can't prove that overflow
cannot happen. One common case is:
extern char a[8], b[8];
sprintf (a, "a=%s", b);
My example would have the "a" array sized at 16 bytes and "b" provably
not containing more than 7 characters (plus a NUL). There would be no
overflow.
... The solution is to either use precision
to constrain the amount of output or in GCC 10 and later to assert
that b's length is less than 7.
See the fragment below to see where the characters in name_bf can
*/NOT/* be more than 255. There is no need for either a precision
constraint or an assertion, based on that code fragment.
So if in the autogen file def_str is ever less than 258 bytes/[259 --
NUL byte, too]/
I'd expect the warning to trigger with a message close to
the original.
It can not be less than 263. For the sake of those not reading the code,
here is the fragment that fills in "name_bf[256]":
{
char * end = spn_ag_char_map_chars(def_scan + 1, 26);
size_t len = end - def_scan;
if (len >= 256)
goto fail_return;
memcpy(name_bf, def_scan, len);
name_bf[len] = '\0';
def_scan = end;
}
