From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 25909 invoked by alias); 5 May 2011 18:07:34 -0000 Received: (qmail 25900 invoked by uid 22791); 5 May 2011 18:07:33 -0000 X-SWARE-Spam-Status: No, hits=-1.3 required=5.0 tests=AWL,BAYES_00,MSGID_FROM_MTA_HEADER,SPF_SOFTFAIL,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mtagate6.uk.ibm.com (HELO mtagate6.uk.ibm.com) (194.196.100.166) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 05 May 2011 18:07:18 +0000 Received: from d06nrmr1507.portsmouth.uk.ibm.com (d06nrmr1507.portsmouth.uk.ibm.com [9.149.38.233]) by mtagate6.uk.ibm.com (8.13.1/8.13.1) with ESMTP id p45I7HQx027779 for ; Thu, 5 May 2011 18:07:17 GMT Received: from d06av02.portsmouth.uk.ibm.com (d06av02.portsmouth.uk.ibm.com [9.149.37.228]) by d06nrmr1507.portsmouth.uk.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id p45I8TF22605248 for ; Thu, 5 May 2011 19:08:29 +0100 Received: from d06av02.portsmouth.uk.ibm.com (loopback [127.0.0.1]) by d06av02.portsmouth.uk.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id p45I7GHR009708 for ; Thu, 5 May 2011 12:07:16 -0600 Received: from tuxmaker.boeblingen.de.ibm.com (tuxmaker.boeblingen.de.ibm.com [9.152.85.9]) by d06av02.portsmouth.uk.ibm.com (8.14.4/8.13.1/NCO v10.0 AVin) with SMTP id p45I7F7C009704; Thu, 5 May 2011 12:07:15 -0600 Message-Id: <201105051807.p45I7F7C009704@d06av02.portsmouth.uk.ibm.com> Received: by tuxmaker.boeblingen.de.ibm.com (sSMTP sendmail emulation); Thu, 05 May 2011 20:07:15 +0200 Subject: Re: RFC: implement typed DWARF stack To: tromey@redhat.com (Tom Tromey) Date: Thu, 05 May 2011 18:07:00 -0000 From: "Ulrich Weigand" Cc: gdb-patches@sourceware.org In-Reply-To: from "Tom Tromey" at May 04, 2011 02:47:21 PM MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2011-05/txt/msg00151.txt.bz2 Tom Tromey wrote: > This patch converts the DWARF expression evaluator to use GDB's value > types. This approach made it easy to support floating point and also > decimal floating point; and also paves the way for any future > improvements. Huh, so value_binop is back after I eliminated it :-) http://sourceware.org/ml/gdb-patches/2010-06/msg00514.html Due to the use of value_as_address in dwarf_expr_fetch_address, this patch actually ought to still work on the SPU ... I'll do a test. > There is some ugliness involving signed and unsigned types; this arises > because "old-style" untyped DWARF values don't have a consistent type. > Also I needed a little bit of special code to handle logical right > shifts. Yes, I'm wondering whether old-style values are handled correctly. We used to make sure all arithmetic is performed in ctx->addr_size bits (which is taken from the DWARF section headers). With your patch, we now always use gdbarch_dwarf2_addr_size -- I'm not sure this is always the same. Also, what is the reason for handling the conversion to unsigned so differently in the DW_OP_mod vs. DW_OP_shr cases? + /* We have to special-case "old-style" untyped values + -- these must have mod computed using unsigned + math. */ + if (value_type (first) == address_type) + { + first = value_cast (uaddress_type, first); + second = value_cast (uaddress_type, second); + } vs. + dwarf_require_integral (value_type (first)); + dwarf_require_integral (value_type (second)); + if (!TYPE_UNSIGNED (value_type (first))) + { + struct type *utype + = get_unsigned_type (ctx->gdbarch, value_type (first)); + + first = value_cast (utype, first); + } [ In fact, maybe we don't need the whole value_cast business and we could just operate on ULONGEST without involving value_binop, since both cases only support integers anyway ... ] > - dwarf_expr_push (ctx, initial, initial_in_stack_memory); > + dwarf_expr_push (ctx, value_from_ulongest (dwarf_expr_address_type (ctx, 1), > + initial), > + initial_in_stack_memory); I'd rather see a dwarf_expr_push_address to keep the address-type abstraction local to dwarf2expr.c ... > case DW_OP_bra: > - offset = extract_signed_integer (op_ptr, 2, byte_order); > - op_ptr += 2; > - if (dwarf_expr_fetch (ctx, 0) != 0) > - op_ptr += offset; > - dwarf_expr_pop (ctx); > + { > + struct value *val; > + > + offset = extract_signed_integer (op_ptr, 2, byte_order); > + op_ptr += 2; > + val = dwarf_expr_fetch (ctx, 0); > + dwarf_require_integral (value_type (val)); Does DW_OP_bra really require an integral type on the stack? The standard wording isn't 100% clear to me here ... > + case DW_OP_GNU_const_type: > + { > + ULONGEST type_die; > + int n; > + const gdb_byte *data; > + struct type *type; > + > + op_ptr = read_uleb128 (op_ptr, op_end, &type_die); > + n = *op_ptr++; > + data = op_ptr; > + op_ptr += n; > + > + type = dwarf_get_base_type (ctx, type_die, n); > + > + /* Note that the address does not matter, since there is > + no way to fetch it. */ > + result_val = value_from_contents_and_address (type, data, 0); I guess a non_lval value would still seem cleaner here (just as done below for DW_OP_GNU_reinterpret --- maybe this could be abstracted into a new value_from_contents helper). > @@ -182,7 +189,7 @@ struct dwarf_expr_piece > > /* The piece's register number or literal value, for > DWARF_VALUE_REGISTER or DWARF_VALUE_STACK pieces. */ > - ULONGEST value; > + struct value *value; Maybe now it would be cleaner to split this into two union members, a plain "int regnum" for DWARF_VALUE_REGISTER, and the struct value for DWARF_VALUE_STACK ... Otherwise this looks good to me. Bye, Ulrich -- Dr. Ulrich Weigand GNU Toolchain for Linux on System z and Cell BE Ulrich.Weigand@de.ibm.com