Re: [PATCH] Fix type casts losing typedefs and reimplement "whatis" typedef stripping

[Pedro Alves <palves at redhat dot com>]

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

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
+    # 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:
 		#
-		#  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])
-- 
2.5.5