From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 23443 invoked by alias); 16 Nov 2011 18:23:14 -0000 Received: (qmail 23431 invoked by uid 22791); 16 Nov 2011 18:23:12 -0000 X-SWARE-Spam-Status: No, hits=-2.3 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, 16 Nov 2011 18:22:55 +0000 Received: from d06nrmr1707.portsmouth.uk.ibm.com (d06nrmr1707.portsmouth.uk.ibm.com [9.149.39.225]) by mtagate2.uk.ibm.com (8.13.1/8.13.1) with ESMTP id pAGIMrG7021860 for ; Wed, 16 Nov 2011 18:22:53 GMT Received: from d06av02.portsmouth.uk.ibm.com (d06av02.portsmouth.uk.ibm.com [9.149.37.228]) by d06nrmr1707.portsmouth.uk.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id pAGIMrYV2224294 for ; Wed, 16 Nov 2011 18:22:53 GMT 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 pAGIMrHA015052 for ; Wed, 16 Nov 2011 11:22:53 -0700 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 pAGIMpx5015021; Wed, 16 Nov 2011 11:22:51 -0700 Message-Id: <201111161822.pAGIMpx5015021@d06av02.portsmouth.uk.ibm.com> Received: by tuxmaker.boeblingen.de.ibm.com (sSMTP sendmail emulation); Wed, 16 Nov 2011 19:22:51 +0100 Subject: Re: Checked in: [RFA] read_frame_register_value and big endian arches To: brobecker@adacore.com (Joel Brobecker) Date: Wed, 16 Nov 2011 18:23:00 -0000 From: "Ulrich Weigand" Cc: gdb-patches@sourceware.org, tromey@redhat.com In-Reply-To: <20111110171526.GC5390@adacore.com> from "Joel Brobecker" at Nov 10, 2011 09:15:26 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-11/txt/msg00437.txt.bz2 Joel Brobecker wrote: > > > gdb/ChangeLog: > > > > > > * findvar.c (read_frame_register_value): Read the correct bytes > > > from registers on big-endian architectures. > > > > Looks good to me. > > Now checked in. Thanks again for the review... It seems this completely broke reading of register variables on SPU. These are in the high-order element of the (always vector) registers, even though we have a big-endian architecture. It seems the underlying problem is that your new read_frame_register_value routine completely ignores the value_offset of the lval_register value. This is the place where this information is encoded: note that for big-endian architectures, value_offset is already correct, so if you'd respect it, you wouldn't need any special-purpose big-endian code. Also, if you'd respect value_offset, the SPU special cases would work. In general, if V is a value with value_lval (V) == lval_register, its contents must be retrieved by reading the register(s) starting with number VALUE_REGNUM (V) in their "natural" sizes, skipping the first value_offset (V) bytes of the concatenated result, and using the next TYPE_LENGTH (value_type (V)) bytes as the value contents. If you look at the original code your patch: http://www.sourceware.org/ml/gdb-patches/2011-10/msg00713.html replaced, it does take value_offset into account: /* Get the data. */ v2 = get_frame_register_value (frame, regnum); value_contents_copy (v, 0, v2, value_offset (v), len); While get_frame_register_value always reads the full register, the value_contents_copy call then extracts only the part starting at value_offset. Note that to handle multi-register values, you may actually need to skip full registers if value_offset starts out bigger than the size of a register. See the get_frame_register_bytes routine how this should be handled. Note that before Tom's patch here: http://sourceware.org/ml/gdb-patches/2011-07/msg00351.html the value_from_register routine actually called get_frame_register_bytes to handle all those situations ... The only problematic thing with get_frame_register_bytes is that it doesn't create a value as output, and therefore does not propagate precise availability information. The new read_frame_register_value should probably implement this (or maybe even replace get_frame_register_bytes). Bye, Ulrich -- Dr. Ulrich Weigand GNU Toolchain for Linux on System z and Cell BE Ulrich.Weigand@de.ibm.com