This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH 3/4] - Change sprintf to use new get_range_strlen overload
- From: Jeff Law <law at redhat dot com>
- To: Martin Sebor <msebor at gmail dot com>, Gcc Patch List <gcc-patches at gcc dot gnu dot org>
- Date: Wed, 10 Oct 2018 16:36:20 -0600
- Subject: Re: [PATCH 3/4] - Change sprintf to use new get_range_strlen overload
- References: <58fbb060-152b-be69-7078-8051f7de8a7f@gmail.com>
On 10/2/18 10:37 AM, Martin Sebor wrote:
> [3/4] - Change sprintf to use new get_range_strlen overload
>
> This change makes use of the new get_range_strlen() overload
> in gimple-ssa-sprintf.c. This necessitated a few changes to
> the API but also enabled the removal of the flexarray member
> from strlen_data_t.
>
> This also patch restores the bool return value for the public
> get_strlen_range function but with a different meaning (to
> indicate whether the computed range is suitable as is to rely
> on for optimization, rather than whether the argument may
> refer to a flexible array member).
>
> The changes to gimple-ssa-sprintf.c involve more indentation
> adjustments than new functionality so to make the review easier
> I attach gcc-99999-3-gimple-ssa-sprintf.c.diff-b with the white
> space changes stripped.
>
> gcc-99999-3.diff
>
> [3/4] - Change sprintf to use new get_range_strlen overload
>
> gcc/ChangeLog:
>
> * gimple-fold.c (get_range_strlen): Avoid clearing minlen after
> recursive call fails for COND_EXPR and GIMPLE_PHI.
> Set minlen to ssize_type rather than size_type. Remove flexarray.
> (get_range_strlen): Return bool.
> (get_range_strlen): Return bool. Clear minmaxlen[0].
> * gimple-fold.h (strlen_data_t): Remove member. Update comments.
> (get_range_strlen): Return bool.
> * tree-ssa-strlen.c (maybe_diag_stxncpy_trunc): Reset lenrange[0]
> when maxlen is unbounded.
> * gimple-ssa-sprintf.c (get_string_length): Call new overload of
> get_range_strlen. Adjust max, likely, and unlikely counters for
> strings of unbounded lengths.
This fixes most, but not all of the regressions introduced in patch #2
(pr79376 continues to regress). However it introduces a bunch of
regressions on warn-sprintf-no-nul.c.
I wouldn't be surprised if we're losing range information. Essentially
the sprintf bits want to say "give me whatever data you've got. I don't
care if there's an unterminated string." I wouldn't be surprised if
the changes to get_range_strlen are running afoul of those needs as the
changes to the sprintf bits themselves look pretty reasonable.
> @@ -2055,12 +2068,12 @@ get_string_length (tree str, unsigned eltsize)
> of an array at the end of a struct assume that it's longer than
> the array bound says it is in case it's used as a poor man's
> flexible array member, such as in struct S { char a[4]; }; */
> - res.range.unlikely = flexarray ? HOST_WIDE_INT_MAX : res.range.max;
> + if (/* lendata.flexarray || */ unbounded)
> + res.range.unlikely = HOST_WIDE_INT_MAX;
> + else
> + res.range.unlikely = res.range.max;
Please remove the commented out code if indeed it's no longer desired.
Jeff