From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 19563 invoked by alias); 5 Oct 2011 12:30:24 -0000 Received: (qmail 19553 invoked by uid 22791); 5 Oct 2011 12:30:23 -0000 X-SWARE-Spam-Status: No, hits=-1.9 required=5.0 tests=AWL,BAYES_00,MSGID_FROM_MTA_HEADER,RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mtagate2.uk.ibm.com (HELO mtagate2.uk.ibm.com) (194.196.100.162) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 05 Oct 2011 12:30:05 +0000 Received: from d06nrmr1507.portsmouth.uk.ibm.com (d06nrmr1507.portsmouth.uk.ibm.com [9.149.38.233]) by mtagate2.uk.ibm.com (8.13.1/8.13.1) with ESMTP id p95CU30g008725 for ; Wed, 5 Oct 2011 12:30:03 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 p95CU2Tf2310244 for ; Wed, 5 Oct 2011 13:30:02 +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 p95CU1Nv025258 for ; Wed, 5 Oct 2011 06:30:01 -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 p95CTxEC025115; Wed, 5 Oct 2011 06:30:00 -0600 Message-Id: <201110051230.p95CTxEC025115@d06av02.portsmouth.uk.ibm.com> Received: by tuxmaker.boeblingen.de.ibm.com (sSMTP sendmail emulation); Wed, 05 Oct 2011 14:29:59 +0200 Subject: Re: [RFA] fetch result of locdesc expressions as integer (not address) To: brobecker@adacore.com (Joel Brobecker) Date: Wed, 05 Oct 2011 12:30:00 -0000 From: "Ulrich Weigand" Cc: gdb-patches@sourceware.org, jan.kratochvil@redhat.com (Jan Kratochvil) In-Reply-To: <20111005010843.GN19246@adacore.com> from "Joel Brobecker" at Oct 04, 2011 06:08:43 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-10/txt/msg00131.txt.bz2 Joel Brobecker wrote: > > Well, maybe they should, but right now they don't, and neither does > > your patch add any such correction. The point I was trying to make > > is that therefore, your patch as it is, while fixing one class of > > bugs on some targets, may simultaneously introduce a different class > > of bugs on other targets. I'm not sure this is a good idea ... > > I understand, but I'm just restoring the previous behavior which, > I believe was changed unintentionally. Well, there *is* a change compared to the previous behavior (i.e. before Jan's patch: http://sourceware.org/ml/gdb-patches/2011-07/msg00762.html). Consider the typical case for DW_AT_location for a global variable: just a single DW_OP_addr statement. This used to be handled previously via a call to read_address. Note that read_address attempts to perform target-specific conversions on the address, in particular it will be sign-extended if required. The dwarf_expr_eval machinery, on the other hand, does not use read_address or otherwise attempt to sign-extend DW_OP_addr addresses. The thought here is that performing that step at this point is premature, since subsequent DWARF operations (e.g. to enforce alignment) may destroy the effect of such extensions. Instead, the sign-extension is performed at the very end of expression evaluation, via the architecture's integer_to_address routine, if required. So for example on mips, before Jan's patch, the address of a global variable would have been sign-extended (in read_address); after Jan's patch, it would *still* have been sign-extended (via integer_to_address); but after your patch, it will no longer be. > It's working fine for instance > on AVR, where the distinction makes a difference, so we're not doing > so bad. Additionally, I tested this patch against the testsuite > which does include a test for the problem that Jan wanted to fix. Hmm, looking into this a bit more, I think I understand why this change doesn't affect testing. The only caller of decode_locdesc that actually expects to handle full location descriptions returning an address is add_partial_symbol when handling a DW_TAG_variable. However, the address of a *partial* symbol for a *variable* (as opposed to a function) doesn't appear to be used for anything; and in fact there is even a comment to that effect ... Given that, I withdraw my objection to the patch. It would still be preferable IMO to no longer call decode_locdesc from add_partial_symbol (but instead just handle those cases that this routine cares about, along the lines of what var_decode_location does). But that can then be a follow-on patch. Bye, Ulrich -- Dr. Ulrich Weigand GNU Toolchain for Linux on System z and Cell BE Ulrich.Weigand@de.ibm.com