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