Re: [PATCH][gdb/exp] Fix assert when adding ptr to imaginary unit

[Tom de Vries <tdevries@suse.de>]

On 1/29/21 8:04 AM, Tom de Vries wrote:
> On 1/28/21 3:08 PM, Tom Tromey wrote:
>>>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
>> Tom> 2021-01-28  Tom de Vries  <tdevries@suse.de>
>>
>> Tom> 	PR exp/27265
>> Tom> 	* valarith.c (scalar_binop): Don't call complex_binop unless
>> Tom> 	arguments are either complex or can be casted to complex.
>>
>> Thanks for doing this.
>>
>> Tom> +  if ((type1->code () == TYPE_CODE_COMPLEX
>> Tom> +       && (type2->code () == TYPE_CODE_COMPLEX
>> Tom> +	   || type2->code () == TYPE_CODE_INT
>> Tom> +	   || type2->code () == TYPE_CODE_FLT))
>> Tom> +      || ((type2->code () == TYPE_CODE_COMPLEX
>> Tom> +	   && (type1->code () == TYPE_CODE_COMPLEX
>> Tom> +	       || type1->code () == TYPE_CODE_INT
>> Tom> +	       || type1->code () == TYPE_CODE_FLT))))
>> Tom>      return complex_binop (arg1, arg2, op);
>>
>> Maybe it should use is_floating_type || is_integral_type instead?
>> is_scalar_type sounds like it should be right but does something really
>> different instead.  That function looks suspect to me.
>>
>> This approach would still rule out fixed point.  I'm not sure if that's
>> ok to do though.
>>
>> Also weirdly, is_fixed_point_type looks through TYPE_CODE_RANGE (seems
>> reasonable) but is_integral_type does not (seems wrong).
>>
> I managed to create an example using TYPE_CODE_BOOL that used to work
> but was broken by this patch, added it to the tests in the patch.
> 
> I've now gone a different route: instead of trying to narrow down when
> complex_binop can be called, detect the error situation in complex_bin
> and throw an error.
> 
> That avoids speculation about what type is left after promotion.
> Instead, we just check the type after promotion.
> 
> WDYT?
> 

I've pushed this now.

Thanks,
- Tom
> 
> 0004-gdb-exp-Fix-assert-when-adding-ptr-to-imaginary-unit.patch
> 
> [gdb/exp] Fix assert when adding ptr to imaginary unit
> 
> I'm running into this assertion failure:
> ...
> $ gdb -batch -ex "p (void *)0 - 5i"
> gdbtypes.c:3430: internal-error: \
>   type* init_complex_type(const char*,   type*): Assertion \
>   `target_type->code () == TYPE_CODE_INT \
>    || target_type->code () == TYPE_CODE_FLT' failed.
> A problem internal to GDB has been detected,
> further debugging may prove unreliable.
> ...
> 
> This is a regression since commit c34e8714662 "Implement complex arithmetic".
> Before that commit we had:
> ...
> (gdb) p (void *)0 - 5i
> Argument to arithmetic operation not a number or boolean.
> ...
> 
> Fix this in complex_binop by throwing an error, such that we have:
> ...
> (gdb) print (void *)0 - 5i
> Argument to complex arithmetic operation not supported.
> ...
> 
> Tested on x86_64-linux.
> 
> gdb/ChangeLog:
> 
> 2021-01-28  Tom de Vries  <tdevries@suse.de>
> 
> 	PR exp/27265
> 	* valarith.c (complex_binop): Throw an error if complex type can't
> 	be created.
> 
> gdb/testsuite/ChangeLog:
> 
> 2021-01-28  Tom de Vries  <tdevries@suse.de>
> 
> 	PR exp/27265
> 	* gdb.base/complex-parts.exp: Add tests.
> 
> ---
>  gdb/gdbtypes.c                           | 12 ++++++++++--
>  gdb/gdbtypes.h                           |  1 +
>  gdb/testsuite/gdb.base/complex-parts.exp | 11 +++++++++++
>  gdb/valarith.c                           |  3 +++
>  4 files changed, 25 insertions(+), 2 deletions(-)
> 
> diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
> index 4dd1a6a64ec..c736dff2ca8 100644
> --- a/gdb/gdbtypes.c
> +++ b/gdb/gdbtypes.c
> @@ -3413,6 +3413,15 @@ init_decfloat_type (struct objfile *objfile, int bit, const char *name)
>    return t;
>  }
>  
> +/* Return true if init_complex_type can be called with TARGET_TYPE.  */
> +
> +bool
> +can_create_complex_type (struct type *target_type)
> +{
> +  return (target_type->code () == TYPE_CODE_INT
> +	  || target_type->code () == TYPE_CODE_FLT);
> +}
> +
>  /* Allocate a TYPE_CODE_COMPLEX type structure.  NAME is the type
>     name.  TARGET_TYPE is the component type.  */
>  
> @@ -3421,8 +3430,7 @@ init_complex_type (const char *name, struct type *target_type)
>  {
>    struct type *t;
>  
> -  gdb_assert (target_type->code () == TYPE_CODE_INT
> -	      || target_type->code () == TYPE_CODE_FLT);
> +  gdb_assert (can_create_complex_type (target_type));
>  
>    if (TYPE_MAIN_TYPE (target_type)->flds_bnds.complex_type == nullptr)
>      {
> diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
> index 40b1aed031e..45014a2b3e8 100644
> --- a/gdb/gdbtypes.h
> +++ b/gdb/gdbtypes.h
> @@ -2289,6 +2289,7 @@ extern struct type *init_float_type (struct objfile *, int, const char *,
>  				     const struct floatformat **,
>  				     enum bfd_endian = BFD_ENDIAN_UNKNOWN);
>  extern struct type *init_decfloat_type (struct objfile *, int, const char *);
> +extern bool can_create_complex_type (struct type *);
>  extern struct type *init_complex_type (const char *, struct type *);
>  extern struct type *init_pointer_type (struct objfile *, int, const char *,
>  				       struct type *);
> diff --git a/gdb/testsuite/gdb.base/complex-parts.exp b/gdb/testsuite/gdb.base/complex-parts.exp
> index 3677c05aa1d..6385752a2f0 100644
> --- a/gdb/testsuite/gdb.base/complex-parts.exp
> +++ b/gdb/testsuite/gdb.base/complex-parts.exp
> @@ -103,3 +103,14 @@ gdb_test "print (_Complex int) 4" " = 4 \\+ 0i"
>  gdb_test "print (_Complex float) 4.5" " = 4.5 \\+ 0i"
>  gdb_test "ptype __complex__ short" " = _Complex short"
>  gdb_test "print (_Complex int) (23.75 + 8.88i)" " = 23 \\+ 8i"
> +
> +set re_reject_arg "Argument to complex arithmetic operation not supported\\."
> +gdb_test "print (void *)0 + 5i" $re_reject_arg
> +gdb_test "print (_Decimal32)0 + 5i" $re_reject_arg
> +
> +# Set language to c++.  Avoid warning by not having current frame.
> +clean_restart
> +gdb_test_no_output "set language c++"
> +
> +# C++ type tests.
> +gdb_test "print (bool)1 + 1i" " = 1 \\+ 1i"
> diff --git a/gdb/valarith.c b/gdb/valarith.c
> index 315030988f4..299a99f4703 100644
> --- a/gdb/valarith.c
> +++ b/gdb/valarith.c
> @@ -1076,6 +1076,9 @@ complex_binop (struct value *arg1, struct value *arg2, enum exp_opcode op)
>  
>    struct type *comp_type = promotion_type (value_type (arg1_real),
>  					   value_type (arg2_real));
> +  if (!can_create_complex_type (comp_type))
> +    error (_("Argument to complex arithmetic operation not supported."));
> +
>    arg1_real = value_cast (comp_type, arg1_real);
>    arg1_imag = value_cast (comp_type, arg1_imag);
>    arg2_real = value_cast (comp_type, arg2_real);
>