On Wed, Aug 8, 2018 at 9:04 AM, Martin Sebor <mse...@gmail.com> wrote:
> On 08/07/2018 02:57 AM, Jason Merrill wrote:
>>
>> On Wed, Aug 1, 2018 at 12:49 AM, Martin Sebor <mse...@gmail.com> wrote:
>>>
>>> On 07/31/2018 07:38 AM, Jason Merrill wrote:
>>>>
>>>>
>>>> On Tue, Jul 31, 2018 at 9:51 AM, Martin Sebor <mse...@gmail.com> wrote:
>>>>>
>>>>>
>>>>> The middle-end contains code to determine the lengths of constant
>>>>> character arrays initialized by string literals.  The code is used
>>>>> in a number of optimizations and warnings.
>>>>>
>>>>> However, the code is unable to deal with constant arrays initialized
>>>>> using the braced initializer syntax, as in
>>>>>
>>>>>   const char a[] = { '1', '2', '\0' };
>>>>>
>>>>> The attached patch extends the C and C++ front-ends to convert such
>>>>> initializers into a STRING_CST form.
>>>>>
>>>>> The goal of this work is to both enable existing optimizations for
>>>>> such arrays, and to help detect bugs due to using non-nul terminated
>>>>> arrays where nul-terminated strings are expected.  The latter is
>>>>> an extension of the GCC 8 _Wstringop-overflow and
>>>>> -Wstringop-truncation warnings that help detect or prevent reading
>>>>> past the end of dynamically created character arrays.  Future work
>>>>> includes detecting potential past-the-end reads from uninitialized
>>>>> local character arrays.
>>>>
>>>>
>>>>
>>>>>   && TYPE_MAIN_VARIANT (TREE_TYPE (valtype)) == char_type_node)
>>>>
>>>>
>>>>
>>>> Why? Don't we want this for other character types as well?
>>>
>>>
>>> It suppresses narrowing warnings for things like
>>>
>>>   signed char a[] = { 0xff };
>>>
>>> (there are a couple of tests that exercise this).
>>
>>
>> Why is plain char different in this respect?  Presumably one of
>>
>> char a[] = { -1 };
>> char b[] = { 0xff };
>>
>> should give the same narrowing warning, depending on whether char is
>> signed.
>
>
> Right.  I've added more tests to verify that it does.
>
>>> At the same time, STRING_CST is supposed to be able to represent
>>> strings of any integer type so there should be a way to make it
>>> work.  On the flip side, recent discussions of changes in this
>>> area suggest there may be bugs in the wide character handling of
>>> STRING_CST so those would need to be fixed before relying on it
>>> for robust support.
>>>
>>> In any case, if you have a suggestion for how to make it work for
>>> at least the narrow character types I'll adjust the patch.
>>
>>
>> I suppose braced_list_to_string should call check_narrowing for C++.
>
>
> I see.  I've made that change.  That has made it possible to
> convert arrays of all character types.  Thanks!
>
>> Currently it uses tree_fits_shwi_p (signed host_wide_int) and then
>> stores the extracted value in a host unsigned int, which is then
>> converted to host char.  Does the right thing happen for -fsigned-char
>> or targets with a different character set?
>
> I believe so.  I've added tests for these too (ASCII and EBCDIC)
> and also changed the type of the extracted value to HWI to match
> (it doesn't change the results of the tests).
>
> Attached is an updated patch with these changes plus more tests
> as suggested by Joseph.

Great.  Can we also move the call to braced_list_to_string into
check_initializer, so it works for templates as well?  As a case just
before the block that calls reshape_init seems appropriate.

Jason

Reply via email to