Re: [RFA] Reverse debugging, part 2/3: core interface

[Michael Snyder <msnyder@redhat.com>]

Daniel Jacobowitz wrote:
> On Mon, Apr 17, 2006 at 04:42:02PM -0700, Michael Snyder wrote:
> 
>>@@ -1371,6 +1384,62 @@
>>       do_cleanups (old_chain);
>>     }
>> }
>>+
>>+static void
>>+finish_backwards (struct symbol *function)
>>+{
>>+  struct symtab_and_line sal;
>>+  struct breakpoint *breakpoint;
>>+  struct cleanup *old_chain;
>>+  CORE_ADDR func_addr;
>>+
>>+  if (find_pc_partial_function (get_frame_pc (get_current_frame ()),
>>+				NULL, &func_addr, NULL) == 0)
>>+    internal_error (__FILE__, __LINE__, 
>>+		    "Finish: couldn't find function.");
>>+
>>+  sal = find_pc_line (func_addr, 0);
>>+
>>+  /* Let's cheat and not worry about async until later.  */
> 
> 
> :-(
> > 
> What problems does it present?  Need to use completions?

I have no idea, that's why I have deferred thinking about it.

> Async operation has been getting a bit musty.  I don't think it's fair
> to insist you straighten that out before this goes in.  But, it would
> be nice to issue an error here, instead of doing something quite
> possibly wrong.

Yes, I agree.  I'll undertake to do it.

>>+  /* We don't need a return value.  */
>>+  proceed_to_finish = 0;
>>+  /* Special case: if we're sitting at the function entry point, 
>>+     then all we need to do is take a reverse singlestep.  We
>>+     don't need to set a breakpoint, and indeed it would do us
>>+     no good to do so.
>>+
>>+     Note that this can only happen at frame #0, since there's
>>+     no way that a function up the stack can have a return address
>>+     that's equal to its entry point.  */
>>+
>>+  if (sal.pc != read_pc ())
> 
> Above you used get_frame_pc, here you use read_pc. 

Good catch.  It should probably be get_frame_pc.
You agree?


> More importantly...
> 
> 
>>+    {
>>+      /* Set breakpoint and continue.  */
>>+      breakpoint = 
>>+	set_momentary_breakpoint (sal, 
>>+				  get_frame_id (get_selected_frame (NULL)),
>>+				  bp_breakpoint);
> 
> ...above you used get_current_frame, here you used get_selected_frame.
> Which is it?  I think that it should be the selected frame in all three
> cases, by analogy with finish, and there should probably be a similar
> error for cases without a caller.

Oh man, you're making my head hurt.  Which is, I suppose, a good thing.

What's the difference, now?  Is 'current_frame' the execution frame,
and 'selected_frame' the displayed one?  (ie. the "up/down" one?)

If that's the case, then I think you're right, it should be
the selected frame.

I have to break off and go meet someone.  This is all good
feedback, I'll pick up the rest of your email this afternoon.