This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 3/4] - Change sprintf to use new get_range_strlen overload


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]