From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 10157 invoked by alias); 4 Oct 2010 21:58:00 -0000 Received: (qmail 10145 invoked by uid 22791); 4 Oct 2010 21:57:59 -0000 X-SWARE-Spam-Status: No, hits=-1.3 required=5.0 tests=AWL,BAYES_00,MSGID_FROM_MTA_HEADER,SPF_SOFTFAIL,TW_CP,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mtagate6.de.ibm.com (HELO mtagate6.de.ibm.com) (195.212.17.166) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 04 Oct 2010 21:57:53 +0000 Received: from d12nrmr1607.megacenter.de.ibm.com (d12nrmr1607.megacenter.de.ibm.com [9.149.167.49]) by mtagate6.de.ibm.com (8.13.1/8.13.1) with ESMTP id o94LvoMd002704 for ; Mon, 4 Oct 2010 21:57:50 GMT Received: from d12av02.megacenter.de.ibm.com (d12av02.megacenter.de.ibm.com [9.149.165.228]) by d12nrmr1607.megacenter.de.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id o94LvnYt4038880 for ; Mon, 4 Oct 2010 23:57:49 +0200 Received: from d12av02.megacenter.de.ibm.com (loopback [127.0.0.1]) by d12av02.megacenter.de.ibm.com (8.12.11.20060308/8.13.3) with ESMTP id o94Lvm20018620 for ; Mon, 4 Oct 2010 23:57:49 +0200 Received: from tuxmaker.boeblingen.de.ibm.com (tuxmaker.boeblingen.de.ibm.com [9.152.85.9]) by d12av02.megacenter.de.ibm.com (8.12.11.20060308/8.12.11) with SMTP id o94LvlhY018611; Mon, 4 Oct 2010 23:57:47 +0200 Message-Id: <201010042157.o94LvlhY018611@d12av02.megacenter.de.ibm.com> Received: by tuxmaker.boeblingen.de.ibm.com (sSMTP sendmail emulation); Mon, 04 Oct 2010 23:57:47 +0200 Subject: Re: [patch] fix pre-/post- in-/decrement To: dan@codesourcery.com (Daniel Jacobowitz) Date: Mon, 04 Oct 2010 21:58:00 -0000 From: "Ulrich Weigand" Cc: ken@linux.vnet.ibm.com (Ken Werner), gdb-patches@sourceware.org, brobecker@adacore.com (Joel Brobecker) In-Reply-To: <20101004204538.GA1052@caradoc.them.org> from "Daniel Jacobowitz" at Oct 04, 2010 04:45:44 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: 2010-10/txt/msg00032.txt.bz2 Daniel Jacobowitz wrote: > Try this - it's easiest to see what's going on with set debug target > 1, or else by using gdbserver and set debug remote 1 (the latter is > easier for me to read). I've edited out some reads relating to the > stack frame. > > (gdb) set $p = (int *) $sp > (gdb) p *$p > Sending packet: $m7fffffffdff0,4#2e...Packet received: 01000000 > $4 = 1 > (gdb) set *$p = 2 > Sending packet: $X7fffffffdff0,4:\002\000\000\000#55...Packet received: OK > (gdb) print *$p = 1 > Sending packet: $X7fffffffdff0,4:\001\000\000\000#54...Packet received: OK > Sending packet: $m7fffffffdff0,4#2e...Packet received: 01000000 > $5 = 1 > (gdb) > > I suspect that your patch will be called to handle the value_assign > for the set command, and it will result in an unnecessary read from > the target. You want the resulting value to be unwritable, but it can > still be lazy; you don't need the value. > > Of course, those two things are not orthogonal in GDB's current > representation. So I don't know how to do this. But don't break the > current behavior, please - it's important both for performance and for > embedded correctness. It would appear that even the current behavior, as shown in your trace, already contains an unnecessary load. There should be no need to perform a memory read to evaluate "print *$p = 1". In fact, value_assign contains code that apparently tries to avoid just that, but it seems this no longer works as expected with lazy values: val = value_copy (toval); memcpy (value_contents_raw (val), value_contents (fromval), TYPE_LENGTH (type)); deprecated_set_value_type (val, type); val = value_change_enclosing_type (val, value_enclosing_type (fromval)); set_value_embedded_offset (val, value_embedded_offset (fromval)); set_value_pointed_to_offset (val, value_pointed_to_offset (fromval)); Note that if "toval" was lazy, "val" is likewise lazy -- but then, its contents are filled in, and are subsequently completely ignored because the value is lazy. The rest of this sequence seems dubious as well. The type of "toval" (and hence "val") will already be "type" -- modulo typedefs, but I don't see why you'd want an assignment operator to remove typedefs. Changing the enclosing type and embedded offset seems just wrong: the constructed "val" contents will have only "toval"'s enclosing contents, not "fromval"'s. (In addition, if the enclosing types are actually different, the value_change_enclosing_type will just completely wipe out "val"'s contents again ...) The pointed_to_offset call looks OK. (I'm not sure whether the whole pointed_to_offset logic is needed at all today, but I guess that's a different topic ...) I think the way to fix this problem would be to add a call to set_value_lazy (val, 0); at this point -- we have filled in the contents, and thus the value indeed is no longer lazy if it ever was. This would -even without Ken's patch fix- the unnecessary read shown above, and would have the additional effect that Ken's patch will introduce no further reads from the target, as all values returned from value_assign will already be non-lazy. Thoughts? Bye, Ulrich -- Dr. Ulrich Weigand GNU Toolchain for Linux on System z and Cell BE Ulrich.Weigand@de.ibm.com