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

Jan Kratochvil
2014-06-17T17:26:23Z

[-- Attachment #1: Type: text/plain, Size: 2879 bytes --]

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

[-- Attachment #2: intelvlarvalue.patch --]
[-- Type: text/plain, Size: 1805 bytes --]

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