Re: [PATCH v2 1/2] record: set stop_pc in "record goto" command

[Pedro Alves <palves@redhat.com>]

Thanks.

On 07/09/2015 07:08 AM, Markus Metzger wrote:
> When navigating in the recorded execution trace via "record goto", we do not
> set stop_pc.  This may trigger an internal error in infrun.c when stepping
> from that location.  Set it.
> 
> (gdb) rec full
> (gdb) c
> Continuing.
> 
> Breakpoint 1, foo (void) at foo.c:42
> 42             x = y
> (gdb) rn
> foo (void)
>     at foo.c:41
> 41             y = x
> (gdb) rec go end
> Go forward to insn number 98724
>     at foo.c:42
> 42             x = y
> (gdb) n
> infrun.c:2382: internal-error: resume: Assertion `sig != GDB_SIGNAL_0' failed.
> A problem internal to GDB has been detected,
> further debugging may prove unreliable.
> Quit this debugging session? (y or n)

I realized that we had context in our minds from the private
chats that might be missing from the archives/logs.

I think we should extend the commit log a bit to connect the
dots between "stop_pc not set" <-> "internal error".  E.g.,:

~~~
This happens because there's a breakpoint at PC when the "next"
is issued, so that breapoint should be immediately stepped over.
That should have been detected/done by proceed, here:

  if (addr == (CORE_ADDR) -1)
    {
      if (pc == stop_pc
	  && breakpoint_here_p (aspace, pc) == ordinary_breakpoint_here
	  && execution_direction != EXEC_REVERSE)
	/* There is a breakpoint at the address we will resume at,
	   step one instruction before inserting breakpoints so that
	   we do not stop right away (and report a second hit at this
	   breakpoint).

	   Note, we don't do this in reverse, because we won't
	   actually be executing the breakpoint insn anyway.
	   We'll be (un-)executing the previous instruction.  */
	tp->stepping_over_breakpoint = 1;

But since stop_pc was stale, the pc == stop_pc check failed, and left the
breakpont at PC inserted.

~~~~~~

> +set bp [gdb_get_line_number "fun4.3" $srcfile]
> +gdb_breakpoint $srcfile:$bp
> +
> +# record the execution until we hit a breakpoint
> +gdb_test_no_output "record btrace"
> +gdb_continue_to_breakpoint "cont to $bp" ".*$srcfile:$bp.*"
> +

Seems like these put line number in the test message
in gdb.sum.  Please tweak the messages to avoid that,
so that if the test lines change in the future, the test
messages remain stable.

> +# reverse-step, then jump to the end of the trace
> +gdb_test "reverse-next" ".*fun4\.2.*"
> +gdb_test "record goto end" ".*fun4\.3.*"
> +
> +# test that we can continue stepping

Please mention the breakpoint here.  E.g.,:

# test that we can continue stepping, even though
# there's a breakpoint at PC.

> +gdb_test "next" ".*fun4\.4.*"

Fix these and you're good to go.  Please push.

Thanks,
Pedro Alves