Fw: Problems with compiling autogen with GCC8 or newer versions

[Bruce Korb]

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);

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.
> 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;
     }