From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 30490 invoked by alias); 5 May 2011 18:38:30 -0000 Received: (qmail 30418 invoked by uid 22791); 5 May 2011 18:38:29 -0000 X-SWARE-Spam-Status: No, hits=-6.9 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,SPF_HELO_PASS,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 05 May 2011 18:38:09 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id p45Ic7qV006675 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Thu, 5 May 2011 14:38:07 -0400 Received: from ns3.rdu.redhat.com (ns3.rdu.redhat.com [10.11.255.199]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id p45Ic7Jf025497; Thu, 5 May 2011 14:38:07 -0400 Received: from opsy.redhat.com (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by ns3.rdu.redhat.com (8.13.8/8.13.8) with ESMTP id p45Ic6NB021864; Thu, 5 May 2011 14:38:06 -0400 Received: by opsy.redhat.com (Postfix, from userid 500) id 229B63784C3; Thu, 5 May 2011 12:38:05 -0600 (MDT) From: Tom Tromey To: "Ulrich Weigand" Cc: gdb-patches@sourceware.org Subject: Re: RFC: implement typed DWARF stack References: <201105051807.p45I7F7C009704@d06av02.portsmouth.uk.ibm.com> Date: Thu, 05 May 2011 18:38:00 -0000 In-Reply-To: <201105051807.p45I7F7C009704@d06av02.portsmouth.uk.ibm.com> (Ulrich Weigand's message of "Thu, 5 May 2011 20:07:15 +0200 (CEST)") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii 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/msg00154.txt.bz2 >>>>> "Ulrich" == Ulrich Weigand writes: Tom> This patch converts the DWARF expression evaluator to use GDB's value Tom> types. This approach made it easy to support floating point and also Tom> decimal floating point; and also paves the way for any future Tom> improvements. Ulrich> Huh, so value_binop is back after I eliminated it :-) Ulrich> http://sourceware.org/ml/gdb-patches/2010-06/msg00514.html Yeah, I actually referred back to this while writing the patch :) Ulrich> Due to the use of value_as_address in dwarf_expr_fetch_address, this Ulrich> patch actually ought to still work on the SPU ... I'll do a test. Thank you. Tom> There is some ugliness involving signed and unsigned types; this arises Tom> because "old-style" untyped DWARF values don't have a consistent type. Tom> Also I needed a little bit of special code to handle logical right Tom> shifts. Ulrich> Yes, I'm wondering whether old-style values are handled correctly. We Ulrich> used to make sure all arithmetic is performed in ctx->addr_size bits Ulrich> (which is taken from the DWARF section headers). With your patch, Ulrich> we now always use gdbarch_dwarf2_addr_size -- I'm not sure this is Ulrich> always the same. Good point; I am not sure either. Another option would be to use ctx->addr_size to choose an arch type (e.g., builtin_uint32), and then also carry along a flag indicating whether the value is "untyped". I think this is needed because these untyped values must be treated differently in a couple spots :-( Yet another idea would be to lazily instantiate these special-to-DWARF types and make struct dwarf_gdbarch_types a little bigger. I think I like this idea the best. I think in practice we only need to support 3 such types (and if we run into more in the wild we can easily add them). Ulrich> Also, what is the reason for handling the conversion to unsigned so Ulrich> differently in the DW_OP_mod vs. DW_OP_shr cases? There's no reason, I will clean this up. Ulrich> [ In fact, maybe we don't need the whole value_cast business and Ulrich> we could just operate on ULONGEST without involving value_binop, Ulrich> since both cases only support integers anyway ... ] I agree, for DW_OP_shr. I will do that. Unfortunately I think DW_OP_mod still needs special magic. Ulrich> I'd rather see a dwarf_expr_push_address to keep the Ulrich> address-type abstraction local to dwarf2expr.c ... Will do. Ulrich> Does DW_OP_bra really require an integral type on the stack? Ulrich> The standard wording isn't 100% clear to me here ... A couple of oddities were clarified in this thread: http://gcc.gnu.org/ml/gcc-patches/2011-05/msg00333.html There, Cary said that DW_OP_bra should require an integral type and this was just an oversight in the spec. Ulrich> I guess a non_lval value would still seem cleaner here (just as done Ulrich> below for DW_OP_GNU_reinterpret --- maybe this could be abstracted Ulrich> into a new value_from_contents helper). Will do. Tom> @@ -182,7 +189,7 @@ struct dwarf_expr_piece Tom> Tom> /* The piece's register number or literal value, for Tom> DWARF_VALUE_REGISTER or DWARF_VALUE_STACK pieces. */ Tom> - ULONGEST value; Tom> + struct value *value; Ulrich> Maybe now it would be cleaner to split this into two union members, Ulrich> a plain "int regnum" for DWARF_VALUE_REGISTER, and the struct value Ulrich> for DWARF_VALUE_STACK ... That does seem better, I will do that too. Ulrich> Otherwise this looks good to me. Thanks very much for the review. I'll post a new patch when I've made the needed changes. Tom