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] avoid assuming known string length is constant (PR 83896)


On Tue, Jan 16, 2018 at 03:20:24PM -0700, Martin Sebor wrote:
> gcc/ChangeLog:
> 
> 	PR tree-optimization/83896
> 	* tree-ssa-strlen.c (get_string_len): Rename...
> 	(get_string_cst_length): ...to this.  Return HOST_WIDE_INT.
> 	Avoid assuming length is constant.
> 	(handle_char_store): Use HOST_WIDE_INT for string length.
> 
> gcc/testsuite/ChangeLog:
> 
> 	PR tree-optimization/83896
> 	* gcc.dg/strlenopt-43.c: New test.
...
> 
>    if (TREE_CODE (rhs) == MEM_REF
>        && integer_zerop (TREE_OPERAND (rhs, 1)))

For the future, there is no reason it couldn't handle
also non-zero offsets if the to be returned length is bigger or
equal than that offset, where the offset can be then subtracted.
But let's defer that for GCC9.

> @@ -2789,7 +2793,8 @@ handle_pointer_plus (gimple_stmt_iterator *gsi)
>  	      if (idx > 0)
>  		{
>  		  strinfo *si = get_strinfo (idx);
> -		  if (si && si->full_string_p)
> +		  if (si && si->full_string_p
> +		      && tree_fits_shwi_p (si->nonzero_chars))

If a && or || using condition doesn't fit onto a single line, it should be
split into one condition per line, so please change the above into:
		  if (si
		      && si->full_string_p
		      && tree_fits_shwi_p (si->nonzero_chars))

> @@ -2822,7 +2824,7 @@ handle_char_store (gimple_stmt_iterator *gsi)
>    unsigned HOST_WIDE_INT offset = 0;
>  
>    /* Set to the length of the string being assigned if known.  */
> -  int rhslen;
> +  HOST_WIDE_INT rhslen;

Please remove the empty line above the above comment, the function
starts with definitions of multiple variables, rhslen isn't even
initialized and there is nothing special on it compared to others,
so they should be in one block.

Ok for trunk with those changes.

> --- gcc/testsuite/gcc.dg/strlenopt-43.c	(nonexistent)
> +++ gcc/testsuite/gcc.dg/strlenopt-43.c	(working copy)
> @@ -0,0 +1,13 @@
> +/* PR tree-optimization/83896 - ice in get_string_len on a call to strlen
> +   with non-constant length
> +   { dg-do compile }
> +   { dg-options "-O2 -Wall -fdump-tree-optimized" } */
> +
> +extern char a[5];
> +extern char b[];
> +
> +void f (void)
> +{
> +  if (__builtin_strlen (b) != 4)
> +    __builtin_memcpy (a, b, sizeof a);
> +}

Most of the strlenopt*.c tests use the strlenopt.h header and then
use the non-__builtin_ function names, if you want to change that too,
please do so, but not a requirement.

	Jakub


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