This is the mail archive of the gdb-patches@sources.redhat.com mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [cagney_convert-20030606-branch] Add value to REGISTER_TO_VALUEet.al.


   Date: Fri, 06 Jun 2003 14:12:23 -0400
   From: Andrew Cagney <ac131313@redhat.com>

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



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]