This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFA/RFC] Restore old handling of multi-register variables
On Tuesday 25 October 2011 21:30:22, Joel Brobecker wrote:
> Hi Pedro,
>
> Thanks for the review. One question:
>
> > > +/* VALUE must be an lval_register value. If regnum is the value's
> > > + associated register number, and len the length of the values type,
> > > + read one or more registers in FRAME, starting with register REGNUM,
> > > + until we've read LEN bytes. */
> > > +
> > > +void
> > > +read_frame_register_value (struct value *value, struct frame_info *frame)
> >
> > I think this should be in frame.c instead. value.c is for core
> > struct value stuff.
>
> That's what I thought originally too. The reason why I didn't put
> that function there is because I thought that the only way to access
> some of the fields was by using the deprecated_[...]_hack functions.
> So I thought we weren't supposed to be able to access those components
> of a struct value. But looking closer, I think I get the reason why
> it's called a hack and deprecated - it's to allow the previous usage
> of using the VALUE_something macros to change the value of the
> associated component. So I'm assuming that...
>
> regnum = VALUE_REGNUM (val)
>
> ... is OK. While...
>
> VALUE_REGNUM (val) = regnum
>
> ... is definitely frowned upon.
Correct. Ideally we'd replace those hacks by separate
setter and a getter, or better yet, see if we can get
rid of the need to have a setter. VALUE_REGNUM as an
rval is definitely okay, as an lval, not so okay.
> I will make that change if you agree.
>
> > > + const int len = TYPE_LENGTH (value_type (value));
> >
> > Do we need check_typedefs here?
>
> I haven't faced a situation where this might make a difference,
> but I think you are right. When taking the length of a type,
> it should never be a typedef.
I'd think we'd reach here with typedef'ed variables that lives in
a registers, but maybe we're stripping typedefs earlier per chance.
> One might even wonder if it would
> make sense to adjust TYPE_LENGTH to to a check_typedef systematically...
Maybe. I guess that we'd need profiling to make sure that wouldn't
regress performance. I don't know if we have paths were we'd
needlessly call check_typedef more times more.
--
Pedro Alves