From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 32254 invoked by alias); 12 May 2011 00:03:50 -0000 Received: (qmail 32246 invoked by uid 22791); 12 May 2011 00:03:49 -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 mtagate3.uk.ibm.com (HELO mtagate3.uk.ibm.com) (194.196.100.163) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 12 May 2011 00:03:35 +0000 Received: from d06nrmr1507.portsmouth.uk.ibm.com (d06nrmr1507.portsmouth.uk.ibm.com [9.149.38.233]) by mtagate3.uk.ibm.com (8.13.1/8.13.1) with ESMTP id p4C03WMs000903 for ; Thu, 12 May 2011 00:03:32 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 p4C03WI72445514 for ; Thu, 12 May 2011 01:03:32 +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 p4C03WSG022588 for ; Wed, 11 May 2011 18:03:32 -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 p4C03V9u022585; Wed, 11 May 2011 18:03:31 -0600 Message-Id: <201105120003.p4C03V9u022585@d06av02.portsmouth.uk.ibm.com> Received: by tuxmaker.boeblingen.de.ibm.com (sSMTP sendmail emulation); Thu, 12 May 2011 02:03:31 +0200 Subject: Re: RFC: implement typed DWARF stack To: tromey@redhat.com (Tom Tromey) Date: Thu, 12 May 2011 00:03:00 -0000 From: "Ulrich Weigand" Cc: gdb-patches@sourceware.org In-Reply-To: from "Tom Tromey" at May 11, 2011 08:59:16 AM 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/msg00299.txt.bz2 Tom Tromey wrote: > >>>>> "Ulrich" == Ulrich Weigand writes: > Ulrich> Huh, interesting approach. In a sense, that might be OK, since > Ulrich> it mirrors what we're doing in dwarf_expr_read_reg by calling > Ulrich> address_from_register. On the other hand, I'm not sure > Ulrich> value_cast always does the right thing if the size of a pointer > Ulrich> type differs from the size of the DWARF address type ... > > I had not considered that as a possibility. I think the most obviously > safe thing to do is just revert dwarf_expr_fetch_address to (mostly) > resemble its pre-patch state. I will do that and test it. Yes, I agree your latest version of dwarf_expr_fetch_address should be obviously safe, that is, getting the same result as prior to the change. I've tested your patch on Cell/B.E. with no regressions (using both -m32 and -m64 for the PowerPC side). Another option that occurred to me in the meantime would be to ensure that untyped "old-style" DWARF values are represented by an *unsigned* type (either always, and converted to signed for operations that need it, or else just converted to unsigned in dwarf_expr_fetch_address), so that calling value_as_address will then do the right thing ... > Ulrich> Another issue that just occurred to me: your patch creates > Ulrich> possibly many temporary struct value objects. I'm wondering > Ulrich> whether those ought to be released from the value chain at some > Ulrich> point ... > > I considered this but talked myself out of it using the following > reasoning: > > 1. Most DWARF expressions are simple, so in practice not many values > will be released; > 2. The unwinder code is value based but does not seem to call > value_free_to_mark, so it must not be significant there; But it does call release_value; see frame.c:frame_register_unwind: /* Dispose of the new value. This prevents watchpoints from trying to watch the saved frame pointer. */ release_value (value); value_free (value); > 3. In other (expression-evaluation) contexts, some caller is going to > free the values anyway; > 4. The watchpoint code looks at the value stack to determine what > intermediate values to watch, and perhaps the values from the DWARF > expression are relevant (though ... it occurs to me just now that > this approach must be pretty broken in the presence of location > lists). > > I am actually not sure if #4 is an argument for or against. Maybe those > intermediate values confuse things; there is a comment in > value_fetch_lazy indicating that this may be the case. Yes, that is my concern -- that there could be intermediate values that are *not* actually appropriate to watch ... I noticed one more minor buglet in the latest patch: @@ -576,7 +590,7 @@ read_pieced_value (struct value *v) case DWARF_VALUE_REGISTER: { struct gdbarch *arch = get_frame_arch (frame); - int gdb_regnum = gdbarch_dwarf2_reg_to_regnum (arch, p->v.value); + int gdb_regnum = gdbarch_dwarf2_reg_to_regnum (arch, p->v.regno); int reg_offset = source_offset; if (gdbarch_byte_order (arch) == BFD_ENDIAN_BIG @@ -609,7 +623,7 @@ read_pieced_value (struct value *v) else { error (_("Unable to access DWARF register number %s"), - paddress (arch, p->v.value)); + paddress (arch, value_as_long (p->v.value))); That should be p->v.regno here (and at another place a bit farther down). 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