Variable Length Arrays (VLA) proposal

Variable Length Arrays (VLA) proposal

[Agovic, Sanimir]

Hello,

FSF GDB is currently not able to evaluate Variable Length Arrays (VLA) for 
various languages e.g. Fortran and C. The root cause are dynamic values of 
attributes like length and bounds of array types (see dwarf4:2.19).

On the archer project [1] VLA support was implemented by Jan Kratochvil and is 
known to be shipped with Fedora. This implementation hooks into check_typedef 
and resolves the dynamic values to existing static attributes. However, this 
approach has some drawbacks, which are addressed in Jan's big plan for VLA [1].

We've created three proposals, how VLA support can be added to FSF GDB.


(1)
Resolving check_typedef:

This proposal is mainly based on Jan's proposal [2].

The function check_typedef does primary two things: it updates the passed type 
structure in terms of dynamic attributes, e.g. the size, bounds, etc. and it 
creates a deep copy of the passed type, completes it and returns it to the 
caller. The function check_typedef will be called in the GDB code base before 
attributes of the type structure will be accessed. This should guarantee that 
the type is up-to-date. However, the GDB code base is known that this function 
call is also missed in some places, which may result in flaky behavior. The type
evaluation may or may not work. As check_typedef is a big monolithic function it
should be split into smaller parts, which are responsible for very specific 
tasks, e.g. calculating and updating the size of a type. By just splitting the 
function into several parts will not solve the primary check_typedef problem. 
To solve this, almost all TYPE_* macros, which access potential dynamic 
attributes, need to be changed in order to call the respective functions to 
calculate dynamic attributes instead of just accessing them statically. For 
calculating sizes and bounds the inferior memory is required. Thus the parameter
of most of the TYPE_* macros need to be changed from "struct type*" to 
"struct value*", as "struct value*" contains the memory of the inferior as well 
as "struct type*". By changing the semantic of the TYPE_* macros, so called 
writer macros need to be introduced as some TYPE_* macros occur on the left 
side. E.g:
  TYPE_LENGTH(type) = 1;
However, there is much risk for breaking things by changing most of the TYPE_* 
macros in combination with splitting check_typedef. By going this way the GDB 
mainline needs to be freezed at least for some days to get the changes into the 
repository once they are ready to avoid ending up in a merging disaster. This is 
because the changes are spread over the whole GDB source base.


(2)
Type normalization:

This proposal is similar to Jan's approach [1] in transforming dynamic values 
of attributes into static ones:

 1) resolve all dynamic attributes
 2) create a copy of the type and assign the resolved attributes from step 1)

This proposal doesn't require to change check_typdef completely nor changing,
the TYPE_* macros, which makes this solution very isolated and lean.
Instead of hijacking check_typedef we hook into value_type() to normalize the 
type. Since inferior types depend on a context - namely an address - to be 
resolvable this seems a good place to hook into. In addition all expression 
evaluations are routed via parse_and_eval() which returns a value, which may be 
associated with an address. Also the TYPE_* macros in gdbtypes.h could be left 
as-is. As a side effect gdb is now constantly creating types during the 
normalization process and will increase the memory consumption constantly as 
well. To avoid this behavior a garbage collector would be needed, which frees 
the memory again when required. This could be done for example when GDB returns 
to its main loop. Nevertheless, such a garbage collector can result in a 
performance overhead, which is expected to be very small.


(3)
Split struct type:

The idea behind this proposal is to reflect the difference of types (dynamic/
static attributes) in the type system used in GDB. At the moment we consider the 
following types:

    - struct type
      Serves as a base. Certain properties are exposed using function pointers 
      instead of raw attribute access e.g. length, bounds.

    - struct static_type
      Reassembles the functionality of the current struct type. It is used for 
      types with statically values of attributes.

    - struct dynamic type
      Allows to express the dynamic values of attributes. Computation of length
      and bounds might be done lazily.

    - struct typedef_type
      A simple proxy type. Calls to length and bounds are forwarded to the 
      underlying type.

An implementation details which is left out is whether one would implement it in 
a OO style similar to breakpoint_ops.

The main advantage is the increased maintainability of the type system itself as 
there would be a clear differentiation between static and dynamic types. Also 
extensions and future requirements could be better addressed by simply 
refactoring struct members into function pointers. The check_type function could 
be shortened, as function pointers will do the dynamic calculation work, and in 
future steps completely removed by adding a function peel(), which would 
recursively peel of any typedefs. This change would be rather large and would 
affect many areas of GDB, like proposal (1).


Summary:
Proposal (1) would be very expensive and dangerous as the side effects of 
resolving and removing check_typedef in one shot are not known. As a lot of 
macros have to be changed as well, the whole GDB code base will be affected. 
The implementation of proposal (2) would be very isolated and simple, but will 
leave the static attributes in struct type, which are actually dynamic. 
Proposal (3) will make the type system of GDB more flexible, as differentiating 
between static and dynamic types. Also it will calculate dynamic attributes when
they will be requested by the caller. However, also this change will be large as
lots of GDB code need to be touched but it is the preferred proposal from our
side.

We would appreciate your feedback and thoughts about the different proposals.


-- Keven & Sanimir 


[1] http://sourceware.org/gdb/wiki/ProjectArcher
    (branch archer-jankratochvil-vla)
[2] http://sourceware.org/ml/gdb/2012-11/msg00094.html
Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052

Re: Variable Length Arrays (VLA) proposal

[Chris January]

Hello Sanimir,

On Fri, 2013-06-28 at 13:01 +0000, Agovic, Sanimir wrote:
> (1)
> Resolving check_typedef:

> (2)
> Type normalization:

> (3)
> Split struct type:

Just to add another possibility, we implemented VLA for Fortran by
wrapping read_var_value and then adding a call to f_fixup_value which
'fixed up' the type of the variable (filled in the array bounds, etc.)
by modifying the original type. (It also auto-dereferences pointers).

Regards,
Chris January - VP Engineering - Allinea Software Ltd.

Re: Variable Length Arrays (VLA) proposal

[Joel Brobecker]

> Just to add another possibility, we implemented VLA for Fortran by
> wrapping read_var_value and then adding a call to f_fixup_value which
> 'fixed up' the type of the variable (filled in the array bounds, etc.)
> by modifying the original type. (It also auto-dereferences pointers).

One of the questions that need to be asked is whether it's OK to modify
the type in place like that. For Ada, we create new types with the
bounds fixed-up. I think you might run into problems with cached
values, such as the values accessible from the history (Eg: "print $2").

-- 
Joel

Re: Variable Length Arrays (VLA) proposal

[Chris January]

On Sun, 2013-06-30 at 18:54 -0700, Joel Brobecker wrote:
> > Just to add another possibility, we implemented VLA for Fortran by
> > wrapping read_var_value and then adding a call to f_fixup_value which
> > 'fixed up' the type of the variable (filled in the array bounds, etc.)
> > by modifying the original type. (It also auto-dereferences pointers).
> 
> One of the questions that need to be asked is whether it's OK to modify
> the type in place like that. For Ada, we create new types with the
> bounds fixed-up. I think you might run into problems with cached
> values, such as the values accessible from the history (Eg: "print $2").

Do you mean in this scenario (excuse the mixed Fortran / GDB commands)?

ALLOCATE(array(10, 10))
(gdb) print array
$1 = (...)
DEALLOCATE(array)
ALLOCATE(array(20,20))
(gdb) print $1

Then no, modifying the original type does not work in that case (it
breaks $1 as you say).

Chris

RE: Variable Length Arrays (VLA) proposal

[Agovic, Sanimir]

Hello Chris,

> Just to add another possibility, we implemented VLA for Fortran by
> wrapping read_var_value and then adding a call to f_fixup_value which
> 'fixed up' the type of the variable (filled in the array bounds, etc.)
> by modifying the original type. (It also auto-dereferences pointers).
> 
Interesting approach & thanks for your feedback.

We are going to add your proposal to http://sourceware.org/gdb/wiki/FortranVLA. 
This allows us to track the various approaches in a better way.
Maybe we should drop the Fortran prefix as we are aiming for generic vla
support in FSF GDB.

 -Sanimir

Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052

Re: Variable Length Arrays (VLA) proposal

[Jan Kratochvil]

Hello,

On Fri, 28 Jun 2013 15:01:25 +0200, Agovic, Sanimir wrote:
> We've created three proposals, how VLA support can be added to FSF GDB.

FYI this plan is dealing various ways only with item (1) of my "plan"
	plan: VLA (Variable Length Arrays) and Fortran dynamic types
	http://sourceware.org/ml/gdb/2012-11/msg00094.html

While archer-jankratochvil-vla also only implemented the item (1) due to a bit
unclear implementation of values/types one hits the other issues trying to
implement the item (1).  The items (2)..(7) were therefore proposals learned
from (1) which GDB cleanups would make the item (1) implementation easier and
less fragile.


> (1)
> Resolving check_typedef:
[...]
> calculating sizes and bounds the inferior memory is required. Thus the parameter
> of most of the TYPE_* macros need to be changed from "struct type*" to 
> "struct value*", as "struct value*" contains the memory of the inferior as well 
> as "struct type*".

I agree that is the difficult part of this proposal...


> (2)
> Type normalization:
> 
> This proposal is similar to Jan's approach [1] in transforming dynamic values 
> of attributes into static ones:
> 
>  1) resolve all dynamic attributes
>  2) create a copy of the type and assign the resolved attributes from step 1)
> 
> This proposal doesn't require to change check_typdef completely nor changing,
> the TYPE_* macros, which makes this solution very isolated and lean.
> Instead of hijacking check_typedef we hook into value_type() to normalize the 
> type. Since inferior types depend on a context - namely an address - to be 
> resolvable this seems a good place to hook into. In addition all expression 
> evaluations are routed via parse_and_eval() which returns a value, which may be 
> associated with an address. Also the TYPE_* macros in gdbtypes.h could be left 
> as-is. As a side effect gdb is now constantly creating types during the 
> normalization process and will increase the memory consumption constantly as 
> well. To avoid this behavior a garbage collector would be needed, which frees 
> the memory again when required. This could be done for example when GDB returns 
> to its main loop. Nevertheless, such a garbage collector can result in a 
> performance overhead, which is expected to be very small.

I find this proposal interesting as hooking into value_type may be the right
spot where one still has the address for DW_OP_push_object_address and where
one can create appropriate static type.  Current archer-jankratochvil-vla
really usually calls object_address_set() close to value_type() anyway.
This way one can avoid the difficulty of one global object address variable in
archer-jankratochvil-vla when dealing with multiple variables at once.

Implementation may not be so simple as for example
dwarf2_evaluate_loc_desc_full case DWARF_VALUE_MEMORY sets value address
added with its element offset which is wrong when one deals with
DW_AT_push_object_address.  In general in GDB there is value_address,
value_raw_address, value_embedded_offset and value_pointed_to_offset which may
the value address more difficult to get and set.

When hooking to value_type() you then need yet another new kind of
value_address().  archer-jankratochvil-vla uses its object_address_get_data()
to convert value_raw_address (pointing to Fortran array descriptor) into
another addresses pointing to the real array data.  Maybe it was intended that
value_raw_address() would be returning descriptor address vs. value_address()
returning real data address.  But these differences are not clear to me in
GDB.  This is why I was rather suggesting item (6) of my "plan" to clean this
up first.

This your proposal (2) avoids fixing the fragile check_typedef usage but
I understand that is a different bug from the Fortran VLA-types bug.

The archer-jankratochvil-vla differentiates "whatis" vs. "ptype" so that for
dynamic types "whatis" still prints that the type is dynamic.  This would not
be possible with the value_type hook but sure that is not a problem:
	+gdb_test "whatis temp1" "type = char \\\[variable\\\]" "second: whatis temp1"
	+gdb_test "ptype temp1" "type = char \\\[78\\\]" "second: ptype temp1"

The memory leakage of types currently exists even in archer-jankratochvil-vla.
There is unmaintained #if 0-ed garbage collector in archer-jankratochvil-vla.
Described in item (2) of my "plan".

One needs to be careful about item (1) of my "plan": val_print / c_val_print
/ LA_VAL_PRINT passing address and type passed separately.  But it may work.
Current archer-jankratochvil-vla has some hack for it in pascal_val_print;
FYI pascal (fpc) needs similar dynamic types for its strings.


> (3)
> Split struct type:
> 
> The idea behind this proposal is to reflect the difference of types (dynamic/
> static attributes) in the type system used in GDB. At the moment we consider the 
> following types:
> 
>     - struct type
>     - struct static_type
>     - struct dynamic type
>     - struct typedef_type

This seems to me as an implementation variant of the proposal (1).  The
current inferior type system is horrible but I did not consider its
refactorization meaningful before GDB starts to use C++; which currently does
not seem to happen soon.


Regards,
Jan

Re: Variable Length Arrays (VLA) proposal

[Jan Kratochvil]

On Tue, 02 Jul 2013 15:37:12 +0200, Jan Kratochvil wrote:
> > (2)
> > Type normalization:
> > 
> > This proposal is similar to Jan's approach [1] in transforming dynamic values 
> > of attributes into static ones:
> > 
> >  1) resolve all dynamic attributes
> >  2) create a copy of the type and assign the resolved attributes from step 1)
> > 
> > This proposal doesn't require to change check_typdef completely nor changing,
> > the TYPE_* macros, which makes this solution very isolated and lean.
> > Instead of hijacking check_typedef we hook into value_type() to normalize the 
> > type. Since inferior types depend on a context - namely an address - to be 
> > resolvable this seems a good place to hook into. In addition all expression 
> > evaluations are routed via parse_and_eval() which returns a value, which may be 
> > associated with an address. Also the TYPE_* macros in gdbtypes.h could be left 
> > as-is. As a side effect gdb is now constantly creating types during the 
> > normalization process and will increase the memory consumption constantly as 
> > well. To avoid this behavior a garbage collector would be needed, which frees 
> > the memory again when required. This could be done for example when GDB returns 
> > to its main loop. Nevertheless, such a garbage collector can result in a 
> > performance overhead, which is expected to be very small.
> 
> I find this proposal interesting as hooking into value_type may be the right
> spot where one still has the address for DW_OP_push_object_address and where
> one can create appropriate static type.  Current archer-jankratochvil-vla
> really usually calls object_address_set() close to value_type() anyway.
> This way one can avoid the difficulty of one global object address variable in
> archer-jankratochvil-vla when dealing with multiple variables at once.
> 
> Implementation may not be so simple as for example
> dwarf2_evaluate_loc_desc_full case DWARF_VALUE_MEMORY sets value address
> added with its element offset which is wrong when one deals with
> DW_AT_push_object_address.  In general in GDB there is value_address,
> value_raw_address, value_embedded_offset and value_pointed_to_offset which may
> the value address more difficult to get and set.
[...]
> One needs to be careful about item (1) of my "plan": val_print / c_val_print
> / LA_VAL_PRINT passing address and type passed separately.  But it may work.
> Current archer-jankratochvil-vla has some hack for it in pascal_val_print;
> FYI pascal (fpc) needs similar dynamic types for its strings.

One issue is that value_type() returns even typedefs.  But the dynamic types
needing to be converted to static form may be hidden after the typedefs.
That should be still easy.

If you have TYPE_CODE_ARRAY -> TYPE_CODE_STRUCT ->
-> one of the fields dynamic TYPE_CODE_ARRAY then you cannot make the inner
TYPE_CODE_ARRAY static but LA_VAL_PRINT will access it without new
value_type() as LA_VAL_PRINT already passes around separate type and separate
address.  You cannot make the inner TYPE_CODE_ARRAY static apparently because
the type differs according to which outer TYPE_CODE_ARRAY element you access.

This is why I was thinking getting rid of things like LA_VAL_PRINT (converting
them to some form of LA_VALUE_PRINT, my item (5)) first may make the VLA task
easier.


Regards,
Jan

Re: Variable Length Arrays (VLA) proposal

[Joel Brobecker]

> This is why I was thinking getting rid of things like LA_VAL_PRINT
> (converting them to some form of LA_VALUE_PRINT, my item (5)) first
> may make the VLA task easier.

I was also looking at that for unrelated reasons (I thought that
this is the way forward in general, and that the val-print routines
were a relic of the past). I looked at converting ada-val-print to
value-print, but it was a little more involved than I thought, so
left it for another day...

-- 
Joel

RE: Variable Length Arrays (VLA) proposal

[Agovic, Sanimir]

Hello Chris,

I tried gdb_dtd with gfortran to debug a vla sample, without success:

$ nl vla.f90
     1  PROGRAM test
     2      INTEGER,           ALLOCATABLE :: vla(:, :, :)
     3      CHARACTER(len=:),  ALLOCATABLE :: str
     4      ALLOCATE(vla (3, 4, 5))
     5      ALLOCATE(character(len=2) :: str)
     6      vla(:,:,:) = 42
     7      str = '42'
     8      call EXIT(0)
     9  END PROGRAM test

gdb-72-ddt4.0-31457 / gfortran (4.7.2 20121109) / fedora 18

$ gdb_dtd vla -q -batch -ex "b 4" -ex run -ex "p vla" -ex "pt vla" -ex next -ex "p vla" -ex "adv 6" -ex "p str" -ex "p str(0)"
Breakpoint 1 at 0x40095e: file vla.f90, line 4.

Breakpoint 1, test () at vla.f90:4
4           ALLOCATE(vla (3, 4, 5))
$1 = <not allocated>
type = integer(kind=4), ALLOCATABLE (0:1,0:1,0:1)
5           ALLOCATE(character(len=2) :: str)
$2 = (( ( 0, 0) ( 0, 0) ) ( ( 0, 0) ( 0, 0) ) )
test () at vla.f90:6
6           vla(:,:,:) = 42
Cannot access memory at address 0x6060c0
warning: array or string index out of range
valarith.c:63: internal-error: find_size_for_pointer_math: Assertion `TYPE_CODE (ptr_type) == TYPE_CODE_PTR' failed.
[core dump]

Upper/lower bounds are always 0:1, allocated character seem not work, character subscripts lead to core dump.

 -Sanimir

> -----Original Message-----
> From: gdb-owner@sourceware.org [mailto:gdb-owner@sourceware.org] On Behalf Of Chris
> January
> Sent: Friday, June 28, 2013 05:41 PM
> To: gdb@sourceware.org
> Subject: Re: Variable Length Arrays (VLA) proposal
> 
> Hello Sanimir,
> 
> On Fri, 2013-06-28 at 13:01 +0000, Agovic, Sanimir wrote:
> > (1)
> > Resolving check_typedef:
> 
> > (2)
> > Type normalization:
> 
> > (3)
> > Split struct type:
> 
> Just to add another possibility, we implemented VLA for Fortran by
> wrapping read_var_value and then adding a call to f_fixup_value which
> 'fixed up' the type of the variable (filled in the array bounds, etc.)
> by modifying the original type. (It also auto-dereferences pointers).
> 
> Regards,
> Chris January - VP Engineering - Allinea Software Ltd.
> 

Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052

RE: Variable Length Arrays (VLA) proposal

[Chris January]

On Thu, 2013-07-04 at 08:17 +0000, Agovic, Sanimir wrote:
> $ nl vla.f90
>      1  PROGRAM test
>      2      INTEGER,           ALLOCATABLE :: vla(:, :, :)
>      3      CHARACTER(len=:),  ALLOCATABLE :: str
>      4      ALLOCATE(vla (3, 4, 5))
>      5      ALLOCATE(character(len=2) :: str)
>      6      vla(:,:,:) = 42
>      7      str = '42'
>      8      call EXIT(0)
>      9  END PROGRAM test

> Breakpoint 1, test () at vla.f90:4
> 4           ALLOCATE(vla (3, 4, 5))
> $1 = <not allocated>
> type = integer(kind=4), ALLOCATABLE (0:1,0:1,0:1)

This highlights another issues implementing VLA. When printing a type
(e.g. in f-typeprint.c) you don't have the value of the variable and
therefore you can't evaluate the DW_AT_lower_bound, DW_AT_upper_bound,
DW_AT_allocated, etc. since they usually uses DW_OP_push_object_address
and we don't have the value address in f_type_print and friends. So to
print reliably print type type of an expression GDB actually needs to
evaluate it, something it hasn't needed to do before.

Regards,
Chris

RE: Variable Length Arrays (VLA) proposal

[Chris January]

On Thu, 2013-07-04 at 09:53 +0100, Chris January wrote:
> On Thu, 2013-07-04 at 08:17 +0000, Agovic, Sanimir wrote:
> > Breakpoint 1, test () at vla.f90:4
> > 4           ALLOCATE(vla (3, 4, 5))
> > $1 = <not allocated>
> > type = integer(kind=4), ALLOCATABLE (0:1,0:1,0:1)
> > 5           ALLOCATE(character(len=2) :: str)
> > $2 = (( ( 0, 0) ( 0, 0) ) ( ( 0, 0) ( 0, 0) ) )
> 
> I have had reports this happens with the Fedora version of gfortran
> 4.7.2 (but not with the Debian version, IIRC) - I will look into it.

GDB 7.2 does not support DWARF 4 (specifically DW_FORM_exprloc in this
case) and that is the cause of this problem.

Chris

RE: Variable Length Arrays (VLA) proposal

[Agovic, Sanimir]

> > Breakpoint 1, test () at vla.f90:4
> > 4           ALLOCATE(vla (3, 4, 5))
> > $1 = <not allocated>
> > type = integer(kind=4), ALLOCATABLE (0:1,0:1,0:1)
> 
> This highlights another issues implementing VLA. When printing a type
> (e.g. in f-typeprint.c) you don't have the value of the variable and
> therefore you can't evaluate the DW_AT_lower_bound, DW_AT_upper_bound,
> DW_AT_allocated, etc. since they usually uses DW_OP_push_object_address
> and we don't have the value address in f_type_print and friends. So to
> print reliably print type type of an expression GDB actually needs to
> evaluate it, something it hasn't needed to do before.

Afaik gdb does a combination of:

const char * exp = [...]
struct expression *expr = parse_expression (exp);
struct value *val = evaluate_type (expr);
[...]

for whatis/ptype therefore we should be fine as we have a value in place.

 -Sanimir
Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052

Re: Variable Length Arrays (VLA) proposal

[Keven Boell]

On 02.07.2013 15:37, Jan Kratochvil wrote:
> Hello,
>
> On Fri, 28 Jun 2013 15:01:25 +0200, Agovic, Sanimir wrote:
>> We've created three proposals, how VLA support can be added to FSF GDB.
>
> FYI this plan is dealing various ways only with item (1) of my "plan"
> 	plan: VLA (Variable Length Arrays) and Fortran dynamic types
> 	http://sourceware.org/ml/gdb/2012-11/msg00094.html
>
> While archer-jankratochvil-vla also only implemented the item (1) due to a bit
> unclear implementation of values/types one hits the other issues trying to
> implement the item (1).  The items (2)..(7) were therefore proposals learned
> from (1) which GDB cleanups would make the item (1) implementation easier and
> less fragile.
>
>
>> (1)
>> Resolving check_typedef:
> [...]
>> calculating sizes and bounds the inferior memory is required. Thus the parameter
>> of most of the TYPE_* macros need to be changed from "struct type*" to
>> "struct value*", as "struct value*" contains the memory of the inferior as well
>> as "struct type*".
>
> I agree that is the difficult part of this proposal...
>
>
>> (2)
>> Type normalization:
>>
>> This proposal is similar to Jan's approach [1] in transforming dynamic values
>> of attributes into static ones:
>>
>>   1) resolve all dynamic attributes
>>   2) create a copy of the type and assign the resolved attributes from step 1)
>>
>> This proposal doesn't require to change check_typdef completely nor changing,
>> the TYPE_* macros, which makes this solution very isolated and lean.
>> Instead of hijacking check_typedef we hook into value_type() to normalize the
>> type. Since inferior types depend on a context - namely an address - to be
>> resolvable this seems a good place to hook into. In addition all expression
>> evaluations are routed via parse_and_eval() which returns a value, which may be
>> associated with an address. Also the TYPE_* macros in gdbtypes.h could be left
>> as-is. As a side effect gdb is now constantly creating types during the
>> normalization process and will increase the memory consumption constantly as
>> well. To avoid this behavior a garbage collector would be needed, which frees
>> the memory again when required. This could be done for example when GDB returns
>> to its main loop. Nevertheless, such a garbage collector can result in a
>> performance overhead, which is expected to be very small.
>
> I find this proposal interesting as hooking into value_type may be the right
> spot where one still has the address for DW_OP_push_object_address and where
> one can create appropriate static type.  Current archer-jankratochvil-vla
> really usually calls object_address_set() close to value_type() anyway.
> This way one can avoid the difficulty of one global object address variable in
> archer-jankratochvil-vla when dealing with multiple variables at once.
>
> Implementation may not be so simple as for example
> dwarf2_evaluate_loc_desc_full case DWARF_VALUE_MEMORY sets value address
> added with its element offset which is wrong when one deals with
> DW_AT_push_object_address.  In general in GDB there is value_address,
> value_raw_address, value_embedded_offset and value_pointed_to_offset which may
> the value address more difficult to get and set.
>
> When hooking to value_type() you then need yet another new kind of
> value_address().  archer-jankratochvil-vla uses its object_address_get_data()
> to convert value_raw_address (pointing to Fortran array descriptor) into
> another addresses pointing to the real array data.  Maybe it was intended that
> value_raw_address() would be returning descriptor address vs. value_address()
> returning real data address.  But these differences are not clear to me in
> GDB.  This is why I was rather suggesting item (6) of my "plan" to clean this
> up first.
>
> This your proposal (2) avoids fixing the fragile check_typedef usage but
> I understand that is a different bug from the Fortran VLA-types bug.
>
> The archer-jankratochvil-vla differentiates "whatis" vs. "ptype" so that for
> dynamic types "whatis" still prints that the type is dynamic.  This would not
> be possible with the value_type hook but sure that is not a problem:
> 	+gdb_test "whatis temp1" "type = char \\\[variable\\\]" "second: whatis temp1"
> 	+gdb_test "ptype temp1" "type = char \\\[78\\\]" "second: ptype temp1"
>
> The memory leakage of types currently exists even in archer-jankratochvil-vla.
> There is unmaintained #if 0-ed garbage collector in archer-jankratochvil-vla.
> Described in item (2) of my "plan".
>
> One needs to be careful about item (1) of my "plan": val_print / c_val_print
> / LA_VAL_PRINT passing address and type passed separately.  But it may work.
> Current archer-jankratochvil-vla has some hack for it in pascal_val_print;
> FYI pascal (fpc) needs similar dynamic types for its strings.
>
>
>> (3)
>> Split struct type:
>>
>> The idea behind this proposal is to reflect the difference of types (dynamic/
>> static attributes) in the type system used in GDB. At the moment we consider the
>> following types:
>>
>>      - struct type
>>      - struct static_type
>>      - struct dynamic type
>>      - struct typedef_type
>
> This seems to me as an implementation variant of the proposal (1).  The
> current inferior type system is horrible but I did not consider its
> refactorization meaningful before GDB starts to use C++; which currently does
> not seem to happen soon.
>
>
> Regards,
> Jan
>
>

Hello Jan,

We've created some tests for the VLA features in Fortran and C in 
advance to test our future implementation against it. We used/split some 
of your tests from archer-jankratochvil-vla and added some more to cover 
more VLA use-cases, we want to fix/enable in GDB. Maybe you can have a 
look at them to see if we agree on the feature set in general, which 
will be available to the user afterwards.

You can find them in our github repository (see the last few commits):
https://github.com/ChristophTWeinmann/GDB/tree/vla-testbase

The tests are covering only Fortran and C at the moment.

gdb/testsuite/gdb.base/vla-datatypes.exp
gdb/testsuite/gdb.base/vla-multi.exp
gdb/testsuite/gdb.base/vla-ptr.exp
gdb/testsuite/gdb.fortran/vla-alloc-assoc.exp
gdb/testsuite/gdb.fortran/vla-datatypes.exp
gdb/testsuite/gdb.fortran/vla-func.exp
gdb/testsuite/gdb.fortran/vla-ptype-sub.exp
gdb/testsuite/gdb.fortran/vla-ptype.exp
gdb/testsuite/gdb.fortran/vla-type.exp
gdb/testsuite/gdb.fortran/vla-value-sub-arbitrary.exp
gdb/testsuite/gdb.fortran/vla-value-sub-finish.exp
gdb/testsuite/gdb.fortran/vla-value-sub.exp
gdb/testsuite/gdb.fortran/vla-value.exp

Thanks,
Keven

RE: Variable Length Arrays (VLA) proposal

[Chris January]

On Thu, 2013-07-04 at 11:49 +0000, Agovic, Sanimir wrote:
> Afaik gdb does a combination of:
> 
> const char * exp = [...]
> struct expression *expr = parse_expression (exp);
> struct value *val = evaluate_type (expr);
> [...]
> 
> for whatis/ptype therefore we should be fine as we have a value in place.

1. The value is not passed through to f_print_type.
2. evaluate_type evaluates expr with no side effects and no memory reads
- but for VLA the type depends on the actual value so you do not to
evaluate with side effects. 

Chris

RE: Variable Length Arrays (VLA) proposal

[Agovic, Sanimir]

> On Thu, 2013-07-04 at 11:49 +0000, Agovic, Sanimir wrote:
> > Afaik gdb does a combination of:
> >
> > const char * exp = [...]
> > struct expression *expr = parse_expression (exp);
> > struct value *val = evaluate_type (expr);
> > [...]
> >
> > for whatis/ptype therefore we should be fine as we have a value in place.
> 
> 1. The value is not passed through to f_print_type.
> 2. evaluate_type evaluates expr with no side effects and no memory reads
> - but for VLA the type depends on the actual value so you do not to
> evaluate with side effects.
>
Chris, thanks for hint. Regarding 2) we may go simply with printing unbound 
dimensions e.g. (: : :). Later we can decide to either relax the no side effects
flag and print the bounds or keep it as-is.

 -Sanimir
Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052

Re: Variable Length Arrays (VLA) proposal

[Jan Kratochvil]

On Fri, 28 Jun 2013 15:01:25 +0200, Agovic, Sanimir wrote:
> (3)
> Split struct type:
> 
> The idea behind this proposal is to reflect the difference of types (dynamic/
> static attributes) in the type system used in GDB. At the moment we consider the 
> following types:
> 
>     - struct type
>       Serves as a base. Certain properties are exposed using function pointers 
>       instead of raw attribute access e.g. length, bounds.
> 
>     - struct static_type
>       Reassembles the functionality of the current struct type. It is used for 
>       types with statically values of attributes.
> 
>     - struct dynamic type
>       Allows to express the dynamic values of attributes. Computation of length
>       and bounds might be done lazily.
> 
>     - struct typedef_type
>       A simple proxy type. Calls to length and bounds are forwarded to the 
>       underlying type.

I think you would need also some:
      - struct opaque_type

to resolve TYPE_IS_OPAQUE/TYPE_STUB/TYPE_TARGET_STUB types.


> Proposal (3) will make the type system of GDB more flexible, as differentiating 
> between static and dynamic types. Also it will calculate dynamic attributes when
> they will be requested by the caller.

If you want to do the calculation any time when TYPE_ARRAY_UPPER_BOUND_VALUE
is called then you do not have the object address available - where will you
get it?  The global pointer archer-jankratochvil-vla is using is really not
acceptable.


> An implementation details which is left out is whether one would implement it in 
> a OO style similar to breakpoint_ops.

In part similar to breakpoint_ops but also similar to SYMBOL_IMPL - to save
the 8 bytes of a pointer - as struct type (main_type) should be very small.
Due to expansion of whole CUs (Compilation Units) together with CU
interdependencies one gets tons of struct type (main_type) instances
with C++ inferiors.


> Also 
> extensions and future requirements could be better addressed by simply 
> refactoring struct members into function pointers. The check_type function could 
> be shortened, as function pointers will do the dynamic calculation work,

I believe you are now talking more about full struct type/main_type rework,
aren't you?  It could be better to provide data definition samples.
Personally I do not think it is worth to start the struct type/main_type
rework in some pseudo-C++, before real C++ gets deployed.


> and in future steps completely removed by adding a function peel(), which
> would recursively peel of any typedefs. This change would be rather large
> and would affect many areas of GDB, like proposal (1).

I do not understand much this paragraph and I find it mostly off-topic here.
Callers of check_typedef currently expect all the typedefs get stripped so
I do not see who would be the callers for peel().


> The implementation of proposal (2) would be very isolated and simple, but will 
> leave the static attributes in struct type, which are actually dynamic. 

After all I find the (2) proposal the most one feasible, hooking into
value_type() together with some cleanups
like my-proposal (5) LA_VAL_PRINT->LA_VALUE_PRINT should make it working.


> However, also this change will be large as lots of GDB code need to be
> touched but it is the preferred proposal from our side.

As I see we found disagreement whether to go the (2) or (3) way I have placed
some question above which should clear it up what are your plans.


Thanks,
Jan

Re: Variable Length Arrays (VLA) proposal

[Jan Kratochvil]

On Thu, 04 Jul 2013 14:32:07 +0200, Keven Boell wrote:
> We've created some tests for the VLA features in Fortran and C in
> advance to test our future implementation against it. We used/split
> some of your tests from archer-jankratochvil-vla and added some more
> to cover more VLA use-cases, we want to fix/enable in GDB. Maybe you
> can have a look at them to see if we agree on the feature set in
> general, which will be available to the user afterwards.
> 
> You can find them in our github repository (see the last few commits):
> https://github.com/ChristophTWeinmann/GDB/tree/vla-testbase

GIT URL: https://github.com/ChristophTWeinmann/GDB.git

> The tests are covering only Fortran and C at the moment.

Some of the files need CRLF->LF conversion.


> gdb/testsuite/gdb.base/vla-datatypes.exp
> gdb/testsuite/gdb.base/vla-multi.exp
> gdb/testsuite/gdb.base/vla-ptr.exp
> gdb/testsuite/gdb.fortran/vla-alloc-assoc.exp
> gdb/testsuite/gdb.fortran/vla-datatypes.exp

type = long [5]
(gdb) FAIL: gdb.base/vla-datatypes.exp: ptype long_vla

Expected "long int [5]", I use gcc-4.8.1-5.fc20.x86_64.
Such minor differences for different compilers are OK and common in GDB
testsuite.


> gdb/testsuite/gdb.fortran/vla-func.exp
> gdb/testsuite/gdb.fortran/vla-ptype-sub.exp
> gdb/testsuite/gdb.fortran/vla-ptype.exp


> gdb/testsuite/gdb.fortran/vla-type.exp

archer-jankratochvil-vla has some FAILs here for more compilated types, that
is a known bug of archer-jankratochvil-vla.


gdb.fortran/vla-value.exp

Why isn't prepare_for_testing used here?


gdb.fortran/vla.f90

Missing copyright header.


I did not check it but I guess these testcases / expect strings work only with
gfortran.  If you are interested it would be sure great if they worked also
with iFort.


In general expect strings in testcases "\\$\\d+ = ..." are commonly simplified
to " = ..." (start of expect strings are not anchored by ^ even in gdb_test).
But it is up to the submitter, "\\$\\d+ = ..." is also fine.

In general 'untested' call is not needed after failed prepare_for_testing.
The same applies to failed 'runto MAIN__'.


The testcases are pre-approved for check-in.  But you will also need to write
stub (just "*: New files." for everything) ChangeLog entry and post it to
gdb-patches.  And if you like to check them in before the real VLA
implementation they would need KFAILs for everything (IMO not worth the work
to check in the testcases before the implementation).


Thanks,
Jan

Re: Variable Length Arrays (VLA) proposal

[Keven Boell]

Thanks for feedback.

On 04.08.2013 21:33, Jan Kratochvil wrote:
> On Thu, 04 Jul 2013 14:32:07 +0200, Keven Boell wrote:
>> We've created some tests for the VLA features in Fortran and C in
>> advance to test our future implementation against it. We used/split
>> some of your tests from archer-jankratochvil-vla and added some more
>> to cover more VLA use-cases, we want to fix/enable in GDB. Maybe you
>> can have a look at them to see if we agree on the feature set in
>> general, which will be available to the user afterwards.
>>
>> You can find them in our github repository (see the last few commits):
>> https://github.com/ChristophTWeinmann/GDB/tree/vla-testbase
>
> GIT URL: https://github.com/ChristophTWeinmann/GDB.git
>
>> The tests are covering only Fortran and C at the moment.
>
> Some of the files need CRLF->LF conversion.
Done.

>
>
>> gdb/testsuite/gdb.base/vla-datatypes.exp
>> gdb/testsuite/gdb.base/vla-multi.exp
>> gdb/testsuite/gdb.base/vla-ptr.exp
>> gdb/testsuite/gdb.fortran/vla-alloc-assoc.exp
>> gdb/testsuite/gdb.fortran/vla-datatypes.exp
>
> type = long [5]
> (gdb) FAIL: gdb.base/vla-datatypes.exp: ptype long_vla
>
> Expected "long int [5]", I use gcc-4.8.1-5.fc20.x86_64.
> Such minor differences for different compilers are OK and common in GDB
> testsuite.
>
I've extended the regex to catch these cases.

>
>> gdb/testsuite/gdb.fortran/vla-func.exp
>> gdb/testsuite/gdb.fortran/vla-ptype-sub.exp
>> gdb/testsuite/gdb.fortran/vla-ptype.exp
>
>
>> gdb/testsuite/gdb.fortran/vla-type.exp
>
> archer-jankratochvil-vla has some FAILs here for more compilated types, that
> is a known bug of archer-jankratochvil-vla.
>
>
> gdb.fortran/vla-value.exp
>
> Why isn't prepare_for_testing used here?
You're right, there is no reason for not having prepare_for_testing.

>
>
> gdb.fortran/vla.f90
>
> Missing copyright header.
Done.

>
>
> I did not check it but I guess these testcases / expect strings work only with
> gfortran.  If you are interested it would be sure great if they worked also
> with iFort.
I've also checked them with ifort and it worked. However, there are some 
more fails but not related to the expect strings.

>
>
> In general expect strings in testcases "\\$\\d+ = ..." are commonly simplified
> to " = ..." (start of expect strings are not anchored by ^ even in gdb_test).
> But it is up to the submitter, "\\$\\d+ = ..." is also fine.
>
> In general 'untested' call is not needed after failed prepare_for_testing.
> The same applies to failed 'runto MAIN__'.
>
Done.

>
> The testcases are pre-approved for check-in.  But you will also need to write
> stub (just "*: New files." for everything) ChangeLog entry and post it to
> gdb-patches.  And if you like to check them in before the real VLA
> implementation they would need KFAILs for everything (IMO not worth the work
> to check in the testcases before the implementation).
I agree, this doesn't make too much sense. We'll submit the tests as 
soon as we have the VLA implementation ready.

Thanks,
Keven