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: [PATCH] Let dwarf2 CFI's execute_stack_op be used outside ofCFI


On 26 Mar 2002, Jim Blandy wrote:

> 
> Daniel Berlin <dan@dberlin.org> writes:
> > On 26 Mar 2002, Jim Blandy wrote:
> > > Actually, Daniel, I'm sorry --- I've re-read the change more
> > > carefully, and I've gotten more confused than I was before.
> > > 
> > > You've changed the Dwarf 2 location expression evaluator to consult
> > > the current register values --- not the values of the registers with
> > > respect to a specific stack frame.  I can't think of any situations in
> > > which this the correct behavior.  Can you explain more about the
> > > contexts in which this change is useful?  It seems to me that it's
> > > wrong in most of the cases I can think of.  It really needs to take a
> > > frame argument, or at the very least, read registers from the selected
> > > frame (although that's kind of gross and global-variable-ish).
> > 
> > Whoops.
> > You're right.
> > I must have merged it while on crack or something.
> > I *meant* to add the frame argument, and only require *either* the 
> > context argument (Which is what it currently takes, a CFA context)  or the 
> > frame, but it looks like I messed up.
> > 
> > What it *should* look like is closer to the one from the WIP i sent. It 
> > should call get_saved_register with the frame argument if the context is 
> > null, or get_reg with the context argument if the context is not null.
> 
> Okay --- that makes more sense.
> 
> I started reviewing the WIP, but I got interrupted.  Do you want to
> extract something from that and post it as a non-WIP patch?

I'm not sure.  Let me explain:
As it stands, the evaluator in the WIP requires a few extra arguments and 
returns a struct value, while the the one in dwarf2cfi does not.
The extra arguments are required because we need to keep track of whether 
the result is an lval or not, and whether the result is in memory or a 
register, so we can set the approriate flags on the value struct
But i'm not sure this is the right approach.

This is why the first patch was meant only to make it external, take 
either a dwarf2 cfa context, or a struct frame info, and do the right 
thing with either, in terms of handing back a location.

I didn't want to deal with the issue of whether we want to return a struct 
value, and the issue of how best to keep track of whether it's in memory 
or a register, without getting an opinion first.

So, in short, if the approach taken by the WIP evaluator is the right way 
to deal with the issue of needing to be able to set the right flags on a 
struct value, than sure, i'll take that part out, and merge it.

Otherwise, I'd rather make sure the version in the repository is 
changed the right way, merging in the other direction to the WIP.

--Dan



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