[committed] avoid assuming array indices are zero-based (PR 92952)

Message ID 6e60a49e-34c5-90e2-502a-57bc2d9e832b@gmail.com
State New
Headers show
Series
  • [committed] avoid assuming array indices are zero-based (PR 92952)
Related show

Commit Message

Martin Sebor Dec. 16, 2019, 10:28 p.m.
The gfortran.dg/lto/pr87689 test fails as a result of
a -Wstringop-overflow warning.  The warning is only enabled for
the C family of languages and LTO but it looks like LTO obviates
this distinction.  Last week's enhancement to compute_objsize
seems like the most likely culprit, although the root cause of
the problem -- assuming array indices begin with zero, was
probably latent before then.  In r279445 I have committed
the attached patch to remove the assumption that array indices
are zero-based to let the test pass again.

Martin

Comments

Jakub Jelinek Dec. 16, 2019, 10:44 p.m. | #1
On Mon, Dec 16, 2019 at 03:28:29PM -0700, Martin Sebor wrote:
> PR middle-end/92952 - gfortran.dg/lto/pr87689 FAILs

> 

> gcc/ChangeLog:

> 

> 	PR middle-end/92952

> 	* builtins.c (compute_objsize): Adjust offset by the array low bound.

> 

> Index: gcc/builtins.c

> ===================================================================

> --- gcc/builtins.c	(revision 279443)

> +++ gcc/builtins.c	(working copy)

> @@ -3999,6 +3999,16 @@ compute_objsize (tree dest, int ostype, tree *pdec

>  	     above.  */

>  	  if (TREE_CODE (dest) == ARRAY_REF)

>  	    {

> +	      tree lowbnd = array_ref_low_bound (dest);

> +	      if (!integer_zerop (lowbnd) && tree_fits_uhwi_p (lowbnd))

> +		{

> +		  /* Adjust the offset by the low bound of the array

> +		     domain (normally zero but 1 in Fortran).  */

> +		  unsigned HOST_WIDE_INT lb = tree_to_uhwi (lowbnd);

> +		  offrng[0] -= lb;

> +		  offrng[1] -= lb;

> +		}


I don't understand why uhwi is used.  offrng is an array of wide_int with
precision of sizetype, so why not instead:
	      if (!integer_zerop (lowbnd) && TREE_CODE (lowbnd) == INTEGER_CST)
		{
		  /* Adjust the offset by the low bound of the array
		     domain (normally zero but 1 in Fortran).  */
		  wide_int lb = wi::to_wide (lowbnd, sizprec);
		  offrng[0] -= lb;
		  offrng[1] -= lb;
		}
?

	Jakub
Martin Sebor Dec. 16, 2019, 11:35 p.m. | #2
On 12/16/19 3:44 PM, Jakub Jelinek wrote:
> On Mon, Dec 16, 2019 at 03:28:29PM -0700, Martin Sebor wrote:

>> PR middle-end/92952 - gfortran.dg/lto/pr87689 FAILs

>>

>> gcc/ChangeLog:

>>

>> 	PR middle-end/92952

>> 	* builtins.c (compute_objsize): Adjust offset by the array low bound.

>>

>> Index: gcc/builtins.c

>> ===================================================================

>> --- gcc/builtins.c	(revision 279443)

>> +++ gcc/builtins.c	(working copy)

>> @@ -3999,6 +3999,16 @@ compute_objsize (tree dest, int ostype, tree *pdec

>>   	     above.  */

>>   	  if (TREE_CODE (dest) == ARRAY_REF)

>>   	    {

>> +	      tree lowbnd = array_ref_low_bound (dest);

>> +	      if (!integer_zerop (lowbnd) && tree_fits_uhwi_p (lowbnd))

>> +		{

>> +		  /* Adjust the offset by the low bound of the array

>> +		     domain (normally zero but 1 in Fortran).  */

>> +		  unsigned HOST_WIDE_INT lb = tree_to_uhwi (lowbnd);

>> +		  offrng[0] -= lb;

>> +		  offrng[1] -= lb;

>> +		}

> 

> I don't understand why uhwi is used.  offrng is an array of wide_int with

> precision of sizetype, so why not instead:

> 	      if (!integer_zerop (lowbnd) && TREE_CODE (lowbnd) == INTEGER_CST)

> 		{

> 		  /* Adjust the offset by the low bound of the array

> 		     domain (normally zero but 1 in Fortran).  */

> 		  wide_int lb = wi::to_wide (lowbnd, sizprec);

> 		  offrng[0] -= lb;

> 		  offrng[1] -= lb;

> 		}

> ?


I don't see what difference it makes.  They should both do the same
thing.  If you just prefer your alternative, feel free to change it.

Martin

Patch

PR middle-end/92952 - gfortran.dg/lto/pr87689 FAILs

gcc/ChangeLog:

	PR middle-end/92952
	* builtins.c (compute_objsize): Adjust offset by the array low bound.

Index: gcc/builtins.c
===================================================================
--- gcc/builtins.c	(revision 279443)
+++ gcc/builtins.c	(working copy)
@@ -3999,6 +3999,16 @@  compute_objsize (tree dest, int ostype, tree *pdec
 	     above.  */
 	  if (TREE_CODE (dest) == ARRAY_REF)
 	    {
+	      tree lowbnd = array_ref_low_bound (dest);
+	      if (!integer_zerop (lowbnd) && tree_fits_uhwi_p (lowbnd))
+		{
+		  /* Adjust the offset by the low bound of the array
+		     domain (normally zero but 1 in Fortran).  */
+		  unsigned HOST_WIDE_INT lb = tree_to_uhwi (lowbnd);
+		  offrng[0] -= lb;
+		  offrng[1] -= lb;
+		}
+
 	      /* Convert the array index into a byte offset.  */
 	      tree eltype = TREE_TYPE (dest);
 	      tree tpsize = TYPE_SIZE_UNIT (eltype);