[PATCH] Fix type casts losing typedefs and reimplement "whatis" typedef stripping
Simon Marchi
simon.marchi@ericsson.com
Mon Nov 20 17:01:00 GMT 2017
On 2017-11-20 11:42 AM, Pedro Alves wrote:
> On 11/18/2017 10:58 PM, Simon Marchi wrote:
>> On 2017-11-18 03:57 PM, Yao Qi wrote:
>>> On Mon, Aug 21, 2017 at 11:38 AM, Pedro Alves <palves@redhat.com> wrote:
>>>
>>> Hi Pedro,
>>> The new tests in gdb.base/whatis-ptype-typedefs.exp fail on 32-bit target.
>>>
>>> https://gdb-build.sergiodj.net/builders/Ubuntu-AArch32-m32/builds/1175/steps/test%20gdb/logs/stdio
>>> https://gdb-build.sergiodj.net/builders/Fedora-i686/builds/6867/steps/test%20gdb/logs/stdio
>>> https://gdb-build.sergiodj.net/builders/Fedora-x86_64-m32/builds/6849/steps/test%20gdb/logs/stdio
>>>
>>> Can you take a look?
>>>
>>
>> I took a quick look. The issue (at least one of them) boils down to the fact
>> that on 64 bits, you can't do this:
>>
>> (gdb) p (float_typedef) v_uchar_array_t_struct_typedef
>> Invalid cast.
>>
>> but on 32 bits you can:
>>
>> (gdb) p (float_typedef) v_uchar_array_t_struct_typedef
>> $1 = 1.16251721e-41
>>
>> The expression basically tries to cast an array (which decays to a pointer) to
>> a float. The cast works on 32 bits (doesn't give Invalid cast) because a float
>> and a pointer are of the same size, and the execution enters this if branch:
>>
>> https://github.com/bminor/binutils-gdb/blob/master/gdb/valops.c#L554
>>
>> On 64 bits, they are not the same size, so it ends up in the invalid cast
>> branch.
>>
>> I don't really know what to do from there. Should we leave the behavior as-is
>> and update the test, or prevent this kind of cast (the compiler doesn't accept
>> that anyway, and I don't see any real use case to this). This function (value_cast)
>> is a bit convoluted, I'm always afraid to touch it...
>
> I'm not 100% sure either. value_cast is documented as:
> ~~~
> More general than a C cast: accepts any two types of the same length,
> and if ARG2 is an lvalue it can be cast into anything at all. */
> ~~~
>
> and I've found this useful on several occasions (though for me it's
> usually more around converting an object to some structure).
>
> The point of the test was to cover as many of code paths in
> value_cast as possible, as a sort of documentation of the current
> behavior:
>
>
> # The main idea here is testing all the different paths in the
> # value casting code in GDB (value_cast), making sure typedefs are
> # preserved.
> ...
> # We try all combinations, even those that don't
> # parse, or are invalid, to catch the case of a
> # regression making them inadvertently valid. For
> # example, these convertions are invalid:
> ...
>
> So in that spirit, I propose starting my making the testcase adjust
> itself, like below, and also test floats of different sizes, leaving
> changing GDB's behavior for a separate consideration/change (using
> the fixed/extended test as baseline).
That's fine with me, it's strange at first that it works on an architecture
but not the other, but it makes sense when you know why.
>
> This passes on x86 both 64-bit and 32-bit.
>
> From 1a02eedaadcb7b62a5990f5838952b0600d4a8cc Mon Sep 17 00:00:00 2001
> From: Pedro Alves <palves@redhat.com>
> Date: Mon, 20 Nov 2017 16:39:58 +0000
> Subject: [PATCH] fix
>
> ---
> gdb/testsuite/gdb.base/whatis-ptype-typedefs.c | 10 +++++++
> gdb/testsuite/gdb.base/whatis-ptype-typedefs.exp | 37 ++++++++++++++++++++++--
> 2 files changed, 44 insertions(+), 3 deletions(-)
>
> diff --git a/gdb/testsuite/gdb.base/whatis-ptype-typedefs.c b/gdb/testsuite/gdb.base/whatis-ptype-typedefs.c
> index 5711a96..35c7279 100644
> --- a/gdb/testsuite/gdb.base/whatis-ptype-typedefs.c
> +++ b/gdb/testsuite/gdb.base/whatis-ptype-typedefs.c
> @@ -56,6 +56,16 @@ DEF (int);
> typedef float float_typedef;
> DEF (float);
>
> +/* Double floats. */
> +
> +typedef double double_typedef;
> +DEF (double);
> +
> +/* Long doubles. */
> +
> +typedef long double long_double_typedef;
> +DEF (long_double);
> +
> /* Enums. */
>
> typedef enum colors {red, green, blue} colors_typedef;
> diff --git a/gdb/testsuite/gdb.base/whatis-ptype-typedefs.exp b/gdb/testsuite/gdb.base/whatis-ptype-typedefs.exp
> index d333d81..c8fa2bd 100644
> --- a/gdb/testsuite/gdb.base/whatis-ptype-typedefs.exp
> +++ b/gdb/testsuite/gdb.base/whatis-ptype-typedefs.exp
> @@ -92,6 +92,16 @@ set table {
> {"v_float_typedef" "float_typedef" "float"}
> {"v_float_typedef2" "float_typedef2" "float"}
>
> + {"double_typedef" "double" "double"}
> + {"double_typedef2" "double_typedef" "double"}
> + {"v_double_typedef" "double_typedef" "double"}
> + {"v_double_typedef2" "double_typedef2" "double"}
> +
> + {"long_double_typedef" "long double" "long double"}
> + {"long_double_typedef2" "long_double_typedef" "long double"}
> + {"v_long_double_typedef" "long_double_typedef" "long double"}
> + {"v_long_double_typedef2" "long_double_typedef2" "long double"}
> +
> {"colors_typedef" "(enum )?colors" "enum colors( : unsigned int)? {red, green, blue}"}
> {"colors_typedef2" "colors_typedef" "enum colors( : unsigned int)? {red, green, blue}"}
> {"v_colors_typedef" "colors_typedef" "enum colors( : unsigned int)? {red, green, blue}"}
> @@ -199,6 +209,20 @@ proc run_tests {lang} {
> }
> }
>
> + # If floats and pointers have he same on this architecture, then
This sentence weird.
> + # casting from array/function to float works, because
> + # arrays/functions first decay to pointers, and then GDB's cast is
> + # more general than a C cast and accepts any two types of the same
> + # length.
> + set float_ptr_same_size \
> + [get_integer_valueof "sizeof (float) == sizeof (void *)" -1]
> +
> + set double_ptr_same_size \
> + [get_integer_valueof "sizeof (double) == sizeof (void *)" -1]
> +
> + set long_double_ptr_same_size \
> + [get_integer_valueof "sizeof (long double) == sizeof (void *)" -1]
> +
> # Test converting/casting all variables in the first column of the
> # table to all types (found in the first column of the table).
> # The aggregates are all defined to be the same size so that
> @@ -230,7 +254,7 @@ proc run_tests {lang} {
> # regression making them inadvertently valid. For
> # example, these convertions are invalid:
Not in this patch, but "convertions".
> #
> - # float <-> array
> + # float <-> array [iff sizeof pointer != sizeof float]
> # array -> function (not function pointer)
> # array -> member_ptr
> #
> @@ -247,8 +271,15 @@ proc run_tests {lang} {
> gdb_test "whatis ($to) $from" "syntax error.*" "whatis ($to) $from (syntax)"
> gdb_test "ptype ($to) $from" "syntax error.*" "ptype ($to) $from (syntax)"
> } elseif {([string match "*float*" $from] && [string match "*array*" $to])
> - || ([string match "float*" $to] && [string match "*array*" $from])
> - || ([string match "float*" $to] && [string match "*method" $from])
> + || (!$float_ptr_same_size
> + && ([string match "float*" $to] && [string match "*array*" $from]
> + || [string match "float*" $to] && [string match "*method" $from]))
> + || (!$double_ptr_same_size
> + && ([string match "double*" $to] && [string match "*array*" $from]
> + || [string match "double*" $to] && [string match "*method" $from]))
> + || (!$long_double_ptr_same_size
> + && ([string match "long_double*" $to] && [string match "*array*" $from]
> + || [string match "long_double*" $to] && [string match "*method" $from]))
> || ([string match "*ftype" $to] && [string match "*array*" $from])
> || ([string match "*ftype2" $to] && [string match "*array*" $from])
> || ([string match "*ftype" $to] && [string match "*method" $from])
>
Otherwise, LGTM. I haven't tested it, but I trust that you did :)
Simon
More information about the Gdb-patches
mailing list