From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id oJfPNJYWHWDlGgAAWB0awg (envelope-from ) for ; Fri, 05 Feb 2021 04:57:42 -0500 Received: by simark.ca (Postfix, from userid 112) id D69AB1EFCB; Fri, 5 Feb 2021 04:57:42 -0500 (EST) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-1.0 required=5.0 tests=MAILING_LIST_MULTI, URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.2 Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id D1C3B1E939 for ; Fri, 5 Feb 2021 04:57:41 -0500 (EST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 537A739F0810; Fri, 5 Feb 2021 09:57:41 +0000 (GMT) Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) by sourceware.org (Postfix) with ESMTPS id 1CACD39F080F for ; Fri, 5 Feb 2021 09:57:39 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 1CACD39F080F Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=suse.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=tdevries@suse.de X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.221.27]) by mx2.suse.de (Postfix) with ESMTP id 3A65AACBA; Fri, 5 Feb 2021 09:57:38 +0000 (UTC) Subject: Re: [PATCH][gdb/exp] Fix assert when adding ptr to imaginary unit From: Tom de Vries To: Tom Tromey References: <20210128130655.GA8529@delia> <87im7hduh9.fsf@tromey.com> <396c65a8-3054-44ab-db5c-bbef7d698b66@suse.de> Message-ID: <641fc956-c259-d39f-7350-68f14ff4daf0@suse.de> Date: Fri, 5 Feb 2021 10:57:37 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.7.0 MIME-Version: 1.0 In-Reply-To: <396c65a8-3054-44ab-db5c-bbef7d698b66@suse.de> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: gdb-patches@sourceware.org Errors-To: gdb-patches-bounces@sourceware.org Sender: "Gdb-patches" 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 writes: >> Tom> 2021-01-28 Tom de Vries >> >> 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 > > 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 > > 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); >