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

[Eric Gallager <egall at gwmail dot gwu dot edu>]

On 8/1/18, Martin Sebor <msebor@gmail.com> wrote:
> 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
>

While modifying -Woverlength-strings, I think a good way to test it
would be to enable it during bootstrap. Even though it is currently
disabled, I have done bootstraps before while manually enabling it,
and they still succeeded. Plus there is documentation that says to
avoid overlength strings in GCC sources, so it would be good if this
were verified. I also brought this up in bug 80942:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80942

Eric