Re: [PATCH 04/23] vla: make dynamic fortran arrays functional.

[Jan Kratochvil <jan dot kratochvil at redhat dot com>]

On Tue, 17 Jun 2014 15:48:19 +0200, Keven Boell wrote:
> Am 16.06.2014 23:02, schrieb Jan Kratochvil:
> > On Wed, 04 Jun 2014 07:54:07 +0200, Keven Boell wrote:
> >> diff --git a/gdb/valarith.c b/gdb/valarith.c
> >> index 8e863e3..bddb9db 100644
> >> --- a/gdb/valarith.c
> >> +++ b/gdb/valarith.c
> >> @@ -200,7 +200,14 @@ value_subscripted_rvalue (struct value *array, LONGEST index, int lowerbound)
> >>  
> >>    if (index < lowerbound || (!TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED (array_type)
> >>  			     && elt_offs >= TYPE_LENGTH (array_type)))
> >> -    error (_("no such vector element"));
> >> +    {
> >> +      if (TYPE_NOT_ASSOCIATED (array_type))
> >> +        error (_("no such vector element because not associated"));
> >> +      else if (TYPE_NOT_ALLOCATED (array_type))
> >> +        error (_("no such vector element because not allocated"));
> >> +      else
> >> +        error (_("no such vector element"));
> >> +    }
> >>  
> >>    if (VALUE_LVAL (array) == lval_memory && value_lazy (array))
> >>      v = allocate_value_lazy (elt_type);
> > 
> > I find here the patch is incomplete.  Earlier this function has:
> >   unsigned int elt_offs = elt_size * longest_to_int (index - lowerbound);
> > 
> > and in some cases - specifically with the 64-bit inferior objects patch, the
> > *bitpos* patches from:
> > 	http://pkgs.fedoraproject.org/cgit/gdb.git/tree/
> > one occasionally gets for TYPE_NOT_ALLOCATED inferior variables:
> > 	Value out of range.
> > instead of the more correct:
> > 	no such vector element because not allocated
> > 
> > Because for TYPE_NOT_ALLOCATED inferior variable the LOWERBOUND value read
> > from the inferior is bogus and 'index - lowerbound' will not fit in 'int'.
> 
> This change just adds some more information for the already existing
> "no such vector element" message. To me the change you are describing
> sounds more like a generic fix for this function or am I wrong?

I am not sure if the fix I suggest belongs to this patch series part but it
definitely belongs to this patch series.  Beforehand if this function
value_subscripted_rvalue was called the parameter 'lowerbound' had to be
always valid.  But by introducting the TYPE_NOT_ALLOCATED functionality
'lowerbound' can be passed invalid and so GDB should not touch it if the
array/type is TYPE_NOT_ALLOCATED.

Otherwise this patchset has a regression against my former VLA patchset on
unallocated Fortran arrays where instead of expected
	no such vector element because not allocated
it sometimes - depending on what is read from uninitialized inferior memory,
therefore not always - prints instead:
	Value out of range.

I had to apply the attached fix.

I am not completely sure this is the right fix - IMO when the array is not
allocated GDB should not try to read the array bound(s) from inferior memory
as there is an undefined value that time.


Jan

--- ./gdb/valarith.c	2014-06-17 19:05:09.286336188 +0200
+++ ./gdb/valarith.c	2014-06-17 19:14:46.752072463 +0200
@@ -195,10 +195,19 @@ value_subscripted_rvalue (struct value *
   struct type *array_type = check_typedef (value_type (array));
   struct type *elt_type = check_typedef (TYPE_TARGET_TYPE (array_type));
   unsigned int elt_size = TYPE_LENGTH (elt_type);
-  unsigned int elt_offs = longest_to_int (index - lowerbound);
+  unsigned int elt_offs;
   LONGEST elt_stride = TYPE_BYTE_STRIDE (TYPE_INDEX_TYPE (array_type));
   struct value *v;
 
+  if (TYPE_NOT_ASSOCIATED (array_type))
+    error (_("no such vector element because not associated"));
+  if (TYPE_NOT_ALLOCATED (array_type))
+    error (_("no such vector element because not allocated"));
+
+  if (index < lowerbound || (int) (index - lowerbound) != index - lowerbound)
+    error (_("no such vector element"));
+  elt_offs = longest_to_int (index - lowerbound);
+
   if (elt_stride > 0)
     elt_offs *= elt_stride;
   else if (elt_stride < 0)
@@ -210,16 +219,9 @@ value_subscripted_rvalue (struct value *
   else
     elt_offs *= elt_size;
 
-  if (index < lowerbound || (!TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED (array_type)
-			     && elt_offs >= TYPE_LENGTH (array_type)))
-    {
-      if (TYPE_NOT_ASSOCIATED (array_type))
-        error (_("no such vector element because not associated"));
-      else if (TYPE_NOT_ALLOCATED (array_type))
-        error (_("no such vector element because not allocated"));
-      else
-        error (_("no such vector element"));
-    }
+  if (!TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED (array_type)
+      && elt_offs >= TYPE_LENGTH (array_type))
+    error (_("no such vector element"));
 
   if (VALUE_LVAL (array) == lval_memory && value_lazy (array))
     v = allocate_value_lazy (elt_type);