History command isn't reliable

[bgirard]

I've been testing the history feature a bit more and I'm finding that it's not very reliable.

Breakpoint 1, breakpointA (i=0) at /home/bgirard/ben/rr-dev/rr/src/test/history.c:6
6     int break_here = 1;
(rr) si
8   }
(rr) back
(rr) frame
#0  breakpointA (i=0) at /home/bgirard/ben/rr-dev/rr/src/test/history.c:8

back should undo the step instruction. AFAIK we're getting pushing a mark properly for history.c:6 on 'si' and then poping it and calling seek_to_mark to on it.

Is the any gotcha or something I should look into to debug this?

[rocallahan]

Nothing springs to mind. RR_LOG=GdbConnection,ReplayTimeline should help. The first question is whether we're trying to restore to the mark and failing, or not even trying.

[bgirard]

Worked this bug into the history test and did a bit of debugging.

current_mark()'s ip points to where it should after seek_to_mark after several iteration of replay_step_to_mark that seem to be working (hitting the right strategies). It looks like ReplayTimeline advances to where it should but GDB is left at the original state. Like the two are in a different state somehow. Still looking.

[bgirard]

I got things to work accidentally by debugging. I've minimized my changes down to:

diff --git a/src/ReplayTimeline.cc b/src/ReplayTimeline.cc
index f161597..590583c 100644
--- a/src/ReplayTimeline.cc
+++ b/src/ReplayTimeline.cc
@@ -449,16 +449,19 @@ ReplayResult ReplayTimeline::replay_step_to_mark(
     // If we hit our breakpoint and there is no client breakpoint there,
     // pretend we didn't hit it.
     if (result.break_status.breakpoint_hit &&
         !has_breakpoint_at_address(t, t->ip())) {
       result.break_status.breakpoint_hit = false;
     }
     update_strategy_and_fix_watchpoint_quirk(strategy, constraints, result,
                                              before);
+    uintptr_t a = mark_addr.register_value();
+    uintptr_t b = 0;
+    printf("ip break %p -> %p\n", (void*)a, (void*)b);
     return result;
   }

   // At required IP, but not in the correct state. Singlestep over this IP.
   // We need the FAST_FORWARD option in case the mark state occurs after
   // many iterations of a string instruction at this address.
   ReplaySession::StepConstraints constraints(RUN_SINGLESTEP_FAST_FORWARD);
   // We don't want to fast-forward past the mark state, so give the mark

With this printf doing a |si, back| combo is passing. This is concerning.

[bgirard]

Nevermind the above. I was tired and didn't realize the gdbserver stdout was being picked up by the test and matched my test regex.

So far everything seems fine. We rewind, set a breakpoint on the target IP and run until we hit it. I notice we rewind the IP after the breakpoint but I haven't gotten to the point where we resync the IP after the breakpoint with GDB (or maybe we don't)?

Looks like a low level RR bug.

[rocallahan]

Actually I don't think it is. If you turn on RR_LOG=GdbConnection then you'll notice that after back, gdb does not re-request register values. Basically I think gdb doesn't know that back has changed the program state, so it just keeps using its cached state.

I'm not sure how to fix this :-(. Maybe there's a way to tell gdb that its cached program state is invalid, but I don't know how to do that. Note that apart from registers, the loaded-shared-libraries list and symbols may need to be updated. Sorry, I should have thought of this during review.

As far as I know, the only way to get gdb to update its state in a gdbserver configuration is by actually issuing a command that executes forward or backward, or by re-running the program via run.

[bgirard]

Thanks I was thinking about it backwards so that's a big help. I'm moving so this might be slow but I'll finish this when I can. I'll see if I can find a good solution since we will want several commands that can change the program state.

[rocallahan]

You might be able to get this to work by adding an rr command that a) tells you the direction you need to go (forward or backward from the current point) and b) sets some internal state so that the next execution command in that direction goes directly to your desired Mark instead of doing the requested command. Then in gdb you run that command and then issue either a "c" or "rc" in the correct direction.

[bgirard]

Still planing on getting back to this in the near future, been really busy with moving.

[rocallahan]

This work caused #1884 so I'm going to push a change that disables saving history markers and adds a test for #1884.