From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 29303 invoked by alias); 29 Sep 2014 13:30:27 -0000 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 Received: (qmail 29290 invoked by uid 89); 29 Sep 2014 13:30:26 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=0.3 required=5.0 tests=AWL,BAYES_20,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,SPF_PASS,UNSUBSCRIBE_BODY autolearn=no version=3.3.2 X-HELO: mail-we0-f171.google.com Received: from mail-we0-f171.google.com (HELO mail-we0-f171.google.com) (74.125.82.171) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Mon, 29 Sep 2014 13:30:25 +0000 Received: by mail-we0-f171.google.com with SMTP id w61so258894wes.30 for ; Mon, 29 Sep 2014 06:30:22 -0700 (PDT) X-Received: by 10.180.35.134 with SMTP id h6mr48107176wij.0.1411997421830; Mon, 29 Sep 2014 06:30:21 -0700 (PDT) Received: from [192.168.0.102] (bl6-164-206.dsl.telepac.pt. [82.155.164.206]) by mx.google.com with ESMTPSA id cy10sm15736132wjb.21.2014.09.29.06.30.20 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 29 Sep 2014 06:30:21 -0700 (PDT) Message-ID: <54295EEB.30009@gmail.com> Date: Mon, 29 Sep 2014 13:30:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.1.1 MIME-Version: 1.0 To: lgustavo@codesourcery.com, "'gdb-patches@sourceware.org'" Subject: Re: [PATCH] MIPS bit field failures in gdb.base/store.exp References: <5413534F.7000705@codesourcery.com> <541C63DF.8090206@redhat.com> <541C6A43.2000200@codesourcery.com> <54246DAE.6080208@codesourcery.com> <5425835B.609@redhat.com> <5429523F.3000706@codesourcery.com> In-Reply-To: <5429523F.3000706@codesourcery.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2014-09/txt/msg00831.txt.bz2 On 09/29/2014 01:36 PM, Luis Machado wrote: > Hi, > > On 09/26/2014 12:16 PM, Pedro Alves wrote: >> On 09/25/2014 08:31 PM, Luis Machado wrote: >> >>> ping! Any ideas on different approaches suitable for this problem or is >>> the proposed fix ok (with either passing a value struct or a bit size)? >> >> Sorry, it's not easy to have a quick opinion without thinking this >> through... >> >> So, in value_assign, the case in question, we see: >> >> gdbarch = get_frame_arch (frame); >> if (gdbarch_convert_register_p (gdbarch, VALUE_REGNUM (toval), type)) >> { >> /* If TOVAL is a special machine register requiring >> conversion of program values to a special raw >> format. */ >> gdbarch_value_to_register (gdbarch, frame, >> VALUE_REGNUM (toval), type, >> value_contents (fromval)); >> } >> >> Notice how gdbarch_value_to_register takes the fromval's contents >> as a buffer, only, and isn't passed down anything that would make it >> possible to find out whether it's writing to a bitfield, so that >> the implementation could do a read-modify-write itself and >> write to the proper bitfield offset. >> >> So, it seems to me that until we find an arch that needs to handle >> bitfields especially (I'm having trouble imagining why that >> would be necessary), we should just change value_assign's >> lval_register handling from: >> >> if (gdbarch_convert_register_p (gdbarch, VALUE_REGNUM (toval), type)) >> { >> gdbarch_value_to_register (); >> } >> else >> { >> if (value_bitsize (toval)) >> { >> // read-modify-write >> } >> else >> { >> put_frame_register_bytes (); >> } >> } >> >> to: >> >> if (value_bitsize (toval)) >> { >> // read-modify-write >> } >> else >> { >> if (gdbarch_convert_register_p (gdbarch, VALUE_REGNUM (toval), type)) >> { >> gdbarch_value_to_register (); >> } >> else >> { >> put_frame_register_bytes (); >> } >> } > > Though a bit less generic, that also seems to be a reasonable solution > for now, and it fixes the failures i saw for MIPS. The proper solution for an arch that needs to treat bitfields differently might well be to do without gdbarch_convert_register_p and change gdbarch_value_to_register's parameters to 'gdbarch_value_to_register(gdbarch, toval, fromval)', and rename it to gdbarch_register_assign while at it, as it's only called by value_assign. Like: if (gdbarch_register_assign_p (gdbarch)) { gdbarch_register_assign (gdbarch, toval, fromval); } else { // default fallback } (Or install the fallback code as fallback gdbarch_register_assign implementation and then just call gdbarch_register_assign directly.) Seems unnecessary to do until we find a user that wants to treat bitfields differently though. Or viewed another way, we're discussing what that "default fallback" code should look like. :-) > Out of the top of my > head i also don't recall a target that handles bit fields in a special > way. Should i go with this patch for the next submission Yes, please. > or do you want to author it? Nope. Thanks, Pedro Alves