> On Jan 9, 2023, at 2:11 AM, Richard Biener <rguent...@suse.de> wrote:
>
> On Thu, 22 Dec 2022, Qing Zhao wrote:
>
>>
>>
>>> On Dec 22, 2022, at 2:09 AM, Richard Biener <rguent...@suse.de> wrote:
>>>
>>> On Wed, 21 Dec 2022, Qing Zhao wrote:
>>>
>>>> Hi, Richard,
>>>>
>>>> Thanks a lot for your comments.
>>>>
>>>>> On Dec 21, 2022, at 2:12 AM, Richard Biener <rguent...@suse.de> wrote:
>>>>>
>>>>> On Tue, 20 Dec 2022, Qing Zhao wrote:
>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> This is the patch for mentioning -fstrict-flex-arrays and
>>>>>> -Warray-bounds=2 changes in gcc-13/changes.html.
>>>>>>
>>>>>> Let me know if you have any comment or suggestions.
>>>>>
>>>>> Some copy editing below
>>>>>
>>>>>> Thanks.
>>>>>>
>>>>>> Qing.
>>>>>>
>>>>>> =======================================
>>>>>> From c022076169b4f1990b91f7daf4cc52c6c5535228 Mon Sep 17 00:00:00 2001
>>>>>> From: Qing Zhao <qing.z...@oracle.com>
>>>>>> Date: Tue, 20 Dec 2022 16:13:04 +0000
>>>>>> Subject: [PATCH] gcc-13/changes: Mention -fstrict-flex-arrays and its
>>>>>> impact.
>>>>>>
>>>>>> ---
>>>>>> htdocs/gcc-13/changes.html | 15 +++++++++++++++
>>>>>> 1 file changed, 15 insertions(+)
>>>>>>
>>>>>> diff --git a/htdocs/gcc-13/changes.html b/htdocs/gcc-13/changes.html
>>>>>> index 689178f9..47b3d40f 100644
>>>>>> --- a/htdocs/gcc-13/changes.html
>>>>>> +++ b/htdocs/gcc-13/changes.html
>>>>>> @@ -39,6 +39,10 @@ a work-in-progress.</p>
>>>>>> <li>Legacy debug info compression option <code>-gz=zlib-gnu</code> was
>>>>>> removed
>>>>>> and the option is ignored right now.</li>
>>>>>> <li>New debug info compression option value <code>-gz=zstd</code> has
>>>>>> been added.</li>
>>>>>> + <li><code>-Warray-bounds=2</code> will no longer issue warnings for
>>>>>> out of bounds
>>>>>> + accesses to trailing struct members of one-element array type
>>>>>> anymore. Please
>>>>>> + add <code>-fstrict-flex-arrays=level</code> to control how the
>>>>>> compiler treat
>>>>>> + trailing arrays of structures as flexible array members. </li>
>>>>>
>>>>> "Instead it diagnoses accesses to trailing arrays according to
>>>>> <code>-fstrict-flex-arrays</code>."
>>>>
>>>> Okay.
>>>>>
>>>>>> </ul>
>>>>>>
>>>>>>
>>>>>> @@ -409,6 +413,17 @@ a work-in-progress.</p>
>>>>>> <h2>Other significant improvements</h2>
>>>>>>
>>>>>> <!-- <h3 id="uninitialized">Eliminating uninitialized variables</h3> -->
>>>>>> +<h3 id="flexible array">Treating trailing arrays as flexible array
>>>>>> members</h3>
>>>>>> +
>>>>>> +<ul>
>>>>>> + <li>GCC can now control when to treat the trailing array of a
>>>>>> structure as a
>>>>>> + flexible array member for the purpose of accessing the elements of
>>>>>> such
>>>>>> + an array. By default, all trailing arrays of structures are
>>>>>> treated as
>>>>>
>>>>> all trailing arrays in aggregates are treated
>>>> Okay.
>>>>>
>>>>>> + flexible array members. Use the new command-line option
>>>>>> + <code>-fstrict-flex-array=level</code> to control how GCC treats
>>>>>> the trailing
>>>>>> + array of a structure as a flexible array member at different
>>>>>> levels.
>>>>>
>>>>> <code>-fstrict-flex-arrays</code> to control which trailing array
>>>>> members are streated as flexible arrays.
>>>>
>>>> Okay.
>>>>
>>>>>
>>>>> I've also just now noticed that there's now a flag_strict_flex_arrays
>>>>> check in the middle-end (in array bound diagnostics) but this option
>>>>> isn't streamed or handled with LTO. I think you want to replace that
>>>>> with the appropriate DECL_NOT_FLEXARRAY check.
>>>>
>>>> We need to know the level value of the strict_flex_arrays on the struct
>>>> field to issue proper warnings at different levels. DECL_NOT_FLEXARRAY
>>>> does not include such info. So, what should I do? Streaming the
>>>> flag_strict_flex_arrays with LTO?
>>>
>>> But you do
>>>
>>> if (compref)
>>> {
>>> /* Try to determine special array member type for this
>>> COMPONENT_REF. */
>>> sam = component_ref_sam_type (arg);
>>> /* Get the level of strict_flex_array for this array field. */
>>> tree afield_decl = TREE_OPERAND (arg, 1);
>>> strict_flex_array_level = strict_flex_array_level_of (afield_decl);
>>>
>>> I see that function doesn't look at DECL_NOT_FLEXARRAY but just
>>> checks attributes (those are streamed in LTO).
>>
>> Yes, checked both flag_strict_flex_arrays and attributes.
>>
>> There are two places in middle end calling ?strict_flex_array_level_of?
>> function,
>> one inside ?array_bounds_checker::check_array_ref?, another one inside
>> ?component_ref_size?.
>> Shall we check DECL_NOT_FLEXARRAY field instead of calling
>> ?strict_flex_array_level_of? in both places?
>
> I wonder if that function should check DECL_NOT_FLEXARRAY?
The function “strict_flex_array_level_of” is intended to query the LEVEL of
strict_flex_array, only check DECL_NOT_FLEXARRAY is not enough.
So, I think the major question here is:
Do we need the LEVEL of strict_flex_array information in the Middle end?
The current major use of LEVEL of strict_flex_array in the middle end is two
places:
1. In the routine “component_ref_size”: to determine the size of the
trailing array based on the level of the strict_flex_array.
2. In the routine “array_bounds_checker::check_array_ref”: to issue
different information for -Wstrict-flex-array based on different level.
Just double checked the above 1, and 2. Without LEVEL of strict_flex_array
info, 1 should be fine
2, as you mentioned previously, the major impact will be that the LEVEL
information is lost in the issued message, but that might be not a big
issue.
So, I will try to eliminate the reference to “flag_strict_flex_array” in the
middle end, replace it with “DECL_NOT_FLEXARRAY”, and come up with
an updated patch for this change.
How do you think?
>
>>>
>>> OK, so I suppose the diagnostic itself would become just less precise
>>> as in "trailing array %qT should not be used as a flexible array member"
>>> without the "for level N and above" part of the diagnostic?
>>
>> Yes, that might be the major impact.
>>
>> If only check DECL_NOT_FLEXARRAY, we will lose such information. Does that
>> matter?
>
> I think the main information is preserved in diagnosing the flex vs.
> non-flex array assumption.
Yes. Agreed.
>
>>>
>>>>> We might also want
>>>>> to see how inlining accesses from TUs with different -fstrict-flex-arrays
>>>>> setting behaves when accessing the same structure (and whether we might
>>>>> want to issue an ODR style diagnostic there).
>>>
>>> This mixing also means streaming -fstrict-flex-arrays won't be of much
>>> help in general.
>>
>> Then under such situation, i.e, different -fstrict-flex-arrays levels for
>> the same structure from different TUs, how should we handle it?
>
> I think in similar situations we try to DWIM, but in some cases it will
> result in "garbage" behavior. I don't think there's anything we can
> do here besides trying to diagnose such mismatches with -flto at the WPA
> stage.
Shall we issue warning for such mismatches? Where is the place I can add such
warnings?
thanks.
Qing
>
> Richard.
