On Wed, Mar 29, 2017 at 7:07 PM, Martin Sebor <mse...@gmail.com> wrote:
> On 03/21/2017 01:33 PM, Jason Merrill wrote:
>> On Tue, Mar 21, 2017 at 11:08 AM, Martin Sebor <mse...@gmail.com> wrote:
>>> On 03/20/2017 10:27 PM, Jason Merrill wrote:
>>>> On Mon, Mar 20, 2017 at 7:58 PM, Martin Sebor <mse...@gmail.com> wrote:
>>>>> On 03/20/2017 05:51 PM, Jason Merrill wrote:
>>>>>> On Mon, Mar 20, 2017 at 7:04 PM, Martin Sebor <mse...@gmail.com>
>>>>>> wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Attached is a minimal patch to avoid an ICE in CHKP upon
>>>>>>> encountering one form of an initializer for a flexible array
>>>>>>> member, specifically the empty string:
>>>>>>>
>>>>>>> int f ()
>>>>>>> {
>>>>>>> struct B { int n; char a[]; };
>>>>>>>
>>>>>>> return ((struct B){ 1, "" }).a[0];
>>>>>>> }
>>>>>>>
>>>>>>> Although GCC accepts (and doesn't ICE on) non-empty initializers
>>>>>>> for flexible array members, such as
>>>>>>>
>>>>>>> (struct B){ 1, "123" }
>>>>>>
>>>>>>
>>>>>> How do you mean? When I compile this with the C front end, I get
>>>>>>
>>>>>> error: non-static initialization of a flexible array member
>>>>>
>>>>> I meant that G++ accepts it, doesn't ICE, but emits wrong code.
>>>>> (it's consistently rejected by the C front end). Sorry for not
>>>>> being clear about it.
>>>>
>>>> Ah, OK. It seems to me that the wrong code bug is worth addressing;
>>>> why does rejecting the code seem risky to you?
>>>
>>> I have a few reasons: First, it's not a regression (I raised
>>> bug 69696 for it in early 2016) so strictly it's out of scope
>>> for this stage. Second, there are a number of bugs related
>>> to the initialization of flexible array members so the fixes
>>> are probably not going to be contained to a single function
>>> or file. Third, the flexible member array changes I made in
>>> the past were not trivial, took time to develop, and the two
>>> of us iterated over some of them for weeks. Despite your
>>> careful review and my extensive testing some of them
>>> introduced regressions that are still being patched up.
>>> Fourth, making a change to reject code this close to a release
>>> runs the risk of breaking code that has successfully compiled
>>> in mass rebuilds and others' tests with the new compiler.
>>> While that could be viewed as a good change for invalid code
>>> that's exercised at run time, it could also break programs
>>> where the bad code is never exercised.
>>
>> Fair enough. But I think the ICE is preferable to wrong code.
>
> I agree in general, although in this case it seems that ICE is
> more likely to penalize -fcheck-pointer-bounds users without
> benefiting anyone else.
>
> I decided to go ahead and implemented your suggestion just in
> case it was easier and safer than I thought. The attached patch
> is the result. I think the changes are actually not as risky as
> I had initially feared but they are still fairly extensive and
> I would hesitate to recommend they be made at this stage.
Agreed. A safer patch for GCC 7 might be to add your find_flexarray
function and call that from finish_compound_literal.
> Regardless of what happens to this patch, for GCC 8, I'd like
> to look into making the initialization work correctly in all
> contexts (i.e., including auto variables) . As I said in my
> response to Marek, I expect rejecting it will only lead users
> determined enough to make use of flexible array members for
> local variables to invent error-prone hacks.
I would expect such users of C++ to be vanishingly few; who would be
determined to use a C feature in a way not allowed by C?
Jason
