From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 12245 invoked by alias); 8 Jun 2003 17:15:37 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 11740 invoked from network); 8 Jun 2003 17:15:24 -0000 Received: from unknown (HELO localhost.redhat.com) (24.157.166.107) by sources.redhat.com with SMTP; 8 Jun 2003 17:15:24 -0000 Received: from redhat.com (localhost [127.0.0.1]) by localhost.redhat.com (Postfix) with ESMTP id 9DCBE2B63; Sun, 8 Jun 2003 13:15:20 -0400 (EDT) Message-ID: <3EE36F28.9000104@redhat.com> Date: Sun, 08 Jun 2003 17:15:00 -0000 From: Andrew Cagney User-Agent: Mozilla/5.0 (X11; U; NetBSD macppc; en-US; rv:1.0.2) Gecko/20030223 X-Accept-Language: en-us, en MIME-Version: 1.0 To: Mark Kettenis Cc: gdb-patches@sources.redhat.com Subject: Re: [cagney_convert-20030606-branch] Add value to REGISTER_TO_VALUE et.al. References: <3EDE4A9E.70403@redhat.com> <8665nl34ao.fsf@elgar.kettenis.dyndns.org> <3EE0D987.6030207@redhat.com> <200306081643.h58GhiTr048427@elgar.kettenis.dyndns.org> Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit X-SW-Source: 2003-06/txt/msg00276.txt.bz2 > Date: Fri, 06 Jun 2003 14:12:23 -0400 > From: Andrew Cagney > > Mark, > > I've created the branch: cagney_convert-20030606-branch and committed > the attached. It's different from the original in the following ways: > > - REGISTER_TO_VALUE and VALUE_TO_REGISTER take the full ``struct value'' > instead of ``struct type'' and a ``void *buf''. Those methods are now > entirely responsible for converting a register to/from the specified value. > > There is defenitely something wrong here. The first thing is that in > valops.c:value_assign() you call CONVERT_REGISTER_P as follows: I'm not suprised, nothing in the testsuite was testing it. > + if (CONVERT_REGISTER_P (VALUE_REGNO (toval), VALUE_TYPE (toval))) > > This is wrong for the lval_reg_frame_relative case. Replacing > `VALUE_REGNO (toval)' with `value_reg' seems the obvious solution, and > indeed, this makes us execute the following line: > > + VALUE_TO_REGISTER (frame, fromval); > > This is strange. Which register should FROMVAL be stored in? That > information is contained in TOVAL. Passing TOVAL here is also wrong, > since the contents of the value are contained in FROMVAL. A possible > solution is passing the register number to VALUE_TO_REGISTER. Another > solution is assigning the contents of FROMVAL to (a copy of) TOVAL and > pass the latter to VALUE_TO_REGISTER. This seems to be what you had > in mind when you wrote the MIPS implementation of VALUE_TO_REGISTER. > > Besides this issue I have a few other comments on these interfaces. > > 1. I think it is a good idea to ask the following question again: Why > do we have both lval_register and lval_reg_frame_relative? lval_memory and lval_register are important when it comes to storing registers, but not when tracking variables. A variable should really only have lval_reg_frame_relative. Tried it though and things ``broke''. > Both seem to deal with values stored in registers. Yes, these can > be frame-relative or not. But wouldn't it be better to let > VALUE_FRAME_ID() indicate this? A value of null_frame_id would > then indicate a global register variable. Yep, ref: Use of lval_register? http://sources.redhat.com/ml/gdb/2003-06/msg00065.html In theory, it's always frame-relative. > I may be mistaken, but I think this has consequences for the > REGISTER_TO_VALUE interface. Instead of an interface that > constructs the complete value, I think we need an interface that > just fetches the contents from the appropriate registers; something > that is similar to what we have now. > > 2. What do we guarantee about the `struct value' that is passed to > VALUE_TO_REGISTER? That it is created by REGISTER_TO_VALUE? The code guarentee that REGISTER_CONVERT_P holds and, by implication, REGISTER_TO_VALUE was used to construct it. > > - Adds the possibly contraversal put_frame_register method. It turned > out that at least three functions contained the code sequence: find > REGNUM's true location, store the value in that location. > > Nothing against the function, but can we try to keep the naming of the > register functions a bit more symmetrical? I do. Its the frame oposites (get_frame_xxxxx) that are the problem and need some savage renaming. > Anyway, I don't think you should check this in just yet. It > definitely needs some more discussion. I was assuming you were going to fix it (or at least the write side :-) The ``obvious'' interfaces were: register_to_value (frame, regnum, type, buffer) value_to_register (frame, regnum, type, buffer) but that tripped up on something (now what ...?). Dig dig. Notice how, to preserve existing behavior, legacy_register_to_value saves the location based on what frame_register returns. We'd have to switch to lval_reg_frame_relative. Andrew