gcc-13/changes.html: Mention -fstrict-flex-arrays and its impact

[Qing Zhao]

> On Dec 22, 2022, at 2:09 AM, Richard Biener <rguenther@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 <rguenther@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.zhao@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?

> 
> 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?

> 
>>> 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? 
> 
>> Yes, good point, I will check on this part.
>> 
>> BTW, a stupid question: what does ODR mean?
> 
> It's the One-Definition-Rule (of C++).  Basically we'd diagnose
> same struct declarations with different -fstrict-flex-arrays setting.
> I see we miss comparing DECL_NOT_FLEXARRAY for tree merging, I'm
> testing a patch to fix that now.

Okay, thanks a lot for the help.

Qing
> 
> Richard.
> 
>> thanks.
>> 
>> Qing
>>> 
>>> Thanks,
>>> Richard.
>> 
>> 
> 
> -- 
> Richard Biener <rguenther@suse.de>
> SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg,
> Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman;
> HRB 36809 (AG Nuernberg)