Re: [PATCH] Handle overlength strings in the C FE

[Martin Sebor <msebor@gmail.com>]

On 08/01/2018 05:20 AM, Bernd Edlinger wrote:
> On 07/30/18 17:49, Joseph Myers wrote:
>> On Mon, 30 Jul 2018, Bernd Edlinger wrote:
>>
>>> Hi,
>>>
>>> this is how I would like to handle the over length strings issue in the C FE.
>>> If the string constant is exactly the right length and ends in one explicit
>>> NUL character, shorten it by one character.
>>
>> I don't think shortening should be limited to that case.  I think the case
>> where the constant is longer than that (and so gets an unconditional
>> pedwarn) should also have it shortened - any constant that doesn't fit in
>> the object being initialized should be shortened to fit, whether diagnosed
>> or not, we should define GENERIC / GIMPLE to disallow too-large string
>> constants in initializers, and should add an assertion somewhere in the
>> middle-end that no too-large string constants reach it.
>>
>
> Okay, there is an update following your suggestion.
>
> Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
> Is it OK for trunk?

The ChangeLog description says:

	* c-typeck.c (digest_init): Fix overlength strings.

suggesting there is a bug but there is no test case.  If there
is a bug in there that can be triggered by C code (valid or
otherwise), it would be good to have a test case and a bug
in Bugzilla.  If there is no bug and this is just cleanup,
I would suggest to adjust the description.

Other than that, while making improvements here, I think it
would be helpful to also add more detail to the text of
the warning:

1) mention the type of the array being initialized in case
it's not obvious from the declaration (the array bound could
be a symbol, not a literal, or the type could be a typedef)

2) mention the number of elements in the initializer in case
it's a macro (such as __FILE__) whose definition isn't visible
in the diagnostic

3) mention that the excess elements are ignored (since it's
undefined in the standard, it will let users know what
happens in GCC).

Here's a test case and a suggested warning:

   #define S __FILE__ "\000"
   enum { N = sizeof __FILE__ };
   const char a[N] = S;

   warning: discarding 1 excess element from initializer-string for 
'char[4]' [-Wc++-compat]
    #define S __FILE__ "\000"
              ^~~~~~~~
   note: in expansion of macro ‘S’
    const char a[N] = S;
                      ^
(Similarly for more than 1 excess element.)

Martin