diff mbox series

[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) | expand

Commit Message

Martin Sebor Dec. 16, 2019, 10:28 p.m. UTC 
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. UTC | #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. UTC | #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
diff mbox series

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);