diff mbox

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

Message ID 20140617172611.GA5176@host2.jankratochvil.net
State Under Review
Headers show

Commit Message

Jan Kratochvil June 17, 2014, 5:26 p.m. UTC 
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
diff mbox

Patch

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