> On Jul 7, 2022, at 4:02 AM, Richard Biener <[email protected]> wrote:
>
> On Wed, Jul 6, 2022 at 4:20 PM Qing Zhao <[email protected]> wrote:
>>
>> (Sorry for the late reply, just came back from a short vacation.)
>>
>>> On Jul 4, 2022, at 2:49 AM, Richard Biener <[email protected]>
>>> wrote:
>>>
>>> On Fri, Jul 1, 2022 at 5:32 PM Martin Sebor <[email protected]> wrote:
>>>>
>>>> On 7/1/22 08:01, Qing Zhao wrote:
>>>>>
>>>>>
>>>>>> On Jul 1, 2022, at 8:59 AM, Jakub Jelinek <[email protected]> wrote:
>>>>>>
>>>>>> On Fri, Jul 01, 2022 at 12:55:08PM +0000, Qing Zhao wrote:
>>>>>>> If so, comparing to the current implemenation to have all the checking
>>>>>>> in middle-end, what’s the
>>>>>>> major benefit of moving part of the checking into FE, and leaving the
>>>>>>> other part in middle-end?
>>>>>>
>>>>>> The point is recording early what FIELD_DECLs could be vs. can't
>>>>>> possibly be
>>>>>> treated like flexible array members and just use that flag in the
>>>>>> decisions
>>>>>> in the current routines in addition to what it is doing.
>>>>>
>>>>> Okay.
>>>>>
>>>>> Based on the discussion so far, I will do the following:
>>>>>
>>>>> 1. Add a new flag “DECL_NOT_FLEXARRAY” to FIELD_DECL;
>>>>> 2. In C/C++ FE, set the new flag “DECL_NOT_FLEXARRAY” for a FIELD_DECL
>>>>> based on [0], [1],
>>>>> [] and the option -fstrict-flex-array, and whether it’s the last field
>>>>> of the DECL_CONTEXT.
>>>>> 3. In Middle end, Add a new utility routine is_flexible_array_member_p,
>>>>> which bases on
>>>>> DECL_NOT_FLEXARRAY + array_at_struct_end_p to decide whether the array
>>>>> reference is a real flexible array member reference.
>>>
>>> I would just update all existing users, not introduce another wrapper
>>> that takes DECL_NOT_FLEXARRAY
>>> into account additionally.
>>
>> Okay.
>>>
>>>>>
>>>>>
>>>>> Middle end currently is quite mess, array_at_struct_end_p,
>>>>> component_ref_size, and all the phases that
>>>>> use these routines need to be updated, + new testing cases for each of
>>>>> the phases.
>>>>>
>>>>>
>>>>> So, I still plan to separate the patch set into 2 parts:
>>>>>
>>>>> Part A: the above 1 + 2 + 3, and use these new utilities in
>>>>> tree-object-size.cc to resolve PR101836 first.
>>>>> Then kernel can use __FORTIFY_SOURCE correctly;
>>>>>
>>>>> Part B: update all other phases with the new utilities + new testing
>>>>> cases + resolving regressions.
>>>>>
>>>>> Let me know if you have any comment and suggestion.
>>>>
>>>> It might be worth considering whether it should be possible to control
>>>> the "flexible array" property separately for each trailing array member
>>>> via either a #pragma or an attribute in headers that can't change
>>>> the struct layout but that need to be usable in programs compiled with
>>>> stricter -fstrict-flex-array=N settings.
>>>
>>> Or an decl attribute.
>>
>> Yes, it might be necessary to add a corresponding decl attribute
>>
>> strict_flex_array (N)
>>
>> Which is attached to a trailing structure array member to provide the user a
>> finer control when -fstrict-flex-array=N is specified.
>>
>> So, I will do the following:
>>
>>
>> *****User interface:
>>
>> 1. command line option:
>> -fstrict-flex-array=N (N=0, 1, 2, 3)
>> 2. decl attribute:
>> strict_flex_array (N) (N=0, 1, 2, 3)
>>
>>
>> *****Implementation:
>>
>> 1. Add a new flag “DECL_NOT_FLEXARRAY” to FIELD_DECL;
>> 2. In C/C++ FE, set the new flag “DECL_NOT_FLEXARRAY” for a FIELD_DECL based
>> on [0], [1],
>> [], the option -fstrict-flex-array, the attribute strict_flex_array,
>> and whether it’s the last field
>> of the DECL_CONTEXT.
>> 3. In Middle end, update all users of “array_at_struct_end_p" or
>> “component_ref_size”, or any place that treats
>> Trailing array as flexible array member with the new flag
>> DECL_NOT_FLEXARRAY.
>> (Still think we need a new consistent utility routine here).
>>
>>
>> I still plan to separate the patch set into 2 parts:
>>
>> Part A: the above 1 + 2 + 3, and use these new utilities in
>> tree-object-size.cc to resolve PR101836 first.
>> Then kernel can use __FORTIFY_SOURCE correctly.
>> Part B: update all other phases with the new utilities + new testing
>> cases + resolving regressions.
>>
>>
>> Let me know any more comment or suggestion.
>
> Sounds good. Part 3. is "optimization" and reasonable to do
> separately, I'm not sure you need
> 'B' (since we're not supposed to have new utilities), but instead I'd
> do '3.' as part of 'B', just
> changing the pieces th resolve PR101836 for part 'A'.
Okay, I see. Then I will separate the patches to:
Part A: 1 + 2
Part B: In Middle end, use the new flag in tree-object-size.cc to resolve
PR101836, then kernel can use __FORTIFY_SOURCE correctly after this;
Part C: in Middle end, use the new flag in all other places that use
“array_at_struct_end_p” or “component_ref_size” to make GCC consistently
behave for trailing array.
The reason I separate the middle end work into Part B and C is: Part B is
immediately required by PR101836; Part C is needed to fix all other GCC
phases to treat trailing array consistently, it’s not needed for PR101836, but
it’s important to make GCC to consistently behave on trailing arrays.
Qing
