Re: Variable Length Arrays (VLA) proposal

[Keven Boell <keven.boell@linux.intel.com>]

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