[PATCH, C++] Fix PR c++/88261

[Bernd Edlinger]

On 12/20/18 6:50 PM, Martin Sebor wrote:
> On 12/20/18 10:46 AM, Martin Sebor wrote:
>> On 12/17/18 7:58 AM, Jason Merrill wrote:
>>> On 12/15/18 3:36 AM, Bernd Edlinger wrote:
>>>> this patch implements an error message, for non-static initialization of a flexible array member.
>>>> This duplicates the existing error message from the C-FE, to avoid ICE and wrong code generation
>>>> issues, as pointed out in the PR.
>>>>
>>>> It is a bit funny that a non-functional feature like that has already rather much test coverage.
>>>> The most easy adjustment seems to change the existing test cases to use static declarations.
>>>
>>> Martin, thoughts?
>>
>> Our high-level goal when tightening up how flexible array members
>> are handled in C++ was to accept what's accepted in standard C mode
>> and reject (or, at a minimum, warn for) C++ extensions that could
>> be relied on in existing code.
> 
> I meant "reject what couldn't be relied on" and "warn for that could
> be."
> 

I believe the problem here is effectively that initializing non-static
flexible array is not supported by the middle-end.  All examples
where flexible array members are initialized on automatic variable
work only as long as they are simple enough that they are optimized
away so that they do not survive until expansion.

Take as example gcc/testsuite/g++.dg/ext/flexary13.C,
it compiles and runs successfully, but the assertions start to
fail if Ax is declared volatile, and at the same time, we know
that the automatic variables are allocated in a way that they
can overlap and crash at any time.

My impression is that the existing C error made the middle-end kind of rely
on this behavior.

So I think the right thing to do is duplicate the existing C error in
the C++ FE.  I do not see any automatic variable with initialized flexible
data members where it would be safe to only warn about them.


> (Sorry for the delay, by the way.  I've been migrating to a new machine
> this week and things aren't yet working quite like I'm used to.)
> 
>>
>> The flexarray tests I added back then were for features that looked
>> like intentional extensions and that seemed to work for at least
>> some use cases as far as I could tell.  What I noticed didn't work
>> I created bugs for: 69338, 69696, and 69338 look related, but there
>> are others.
>>
>> I think all these bugs should all be reviewed and a decision made
>> about what's intended to work and what continues to be accepted as
>> an accident and should be rejected.  After that, we can adjust
>> the existing tests.
>>

I would not rule out the possibility that there can be more bugs.
But I think the existing tests need to avoid the case which evokes
the new error.  The question is, if changing from automatic to static
objects prevents those tests to test what they were originally written for.
I believe this is not the case, but I do probably not know all the
background here.


Thanks
Bernd.