fix frame_unwind_id assertion

fix frame_unwind_id assertion

[Pedro Alves]

[-- Attachment #1: Type: text/plain, Size: 3604 bytes --]

This patch fixes the gdb_assert hit I found here:

http://sourceware.org/ml/gdb-patches/2008-05/msg00516.html

The assert being hit happens here, when calling
frame_unwind_id (get_current_frame ()) right after
deciding we have an event to handle in handle_inferior_event:

static void
frame_cleanup_after_sniffer (void *arg)
{
...
  /* No sniffer should extend the frame chain; sniff based on what is
     already certain.  */
  gdb_assert (!frame->prev_p);


#0  internal_error (file=0x7558a1 "../../src/gdb/frame.c", line=1808, 
string=0x755946 "%s: Assertion `%s' failed.")
    at ../../src/gdb/utils.c:779
#1  0x00000000005b5b6a in frame_cleanup_after_sniffer (arg=0xafbd40) 
at ../../src/gdb/frame.c:1808
#2  0x00000000004570ed in do_my_cleanups (pmy_chain=0xa9b900, 
old_chain=0xb13d10) at ../../src/gdb/utils.c:320
#3  0x000000000045709d in do_cleanups (old_chain=0xb13d10) 
at ../../src/gdb/utils.c:303
#4  0x00000000005b62d3 in frame_unwind_find_by_frame (this_frame=0xafbd40, 
this_cache=0xafbd48)
    at ../../src/gdb/frame-unwind.c:105
#5  0x00000000005b2e7f in get_frame_id (fi=0xafbd40) 
at ../../src/gdb/frame.c:258
#6  0x00000000005b4c6a in get_prev_frame_1 (this_frame=0xafbd40) 
at ../../src/gdb/frame.c:1188
#7  0x00000000005b2f72 in frame_unwind_id (next_frame=0xafbd40) 
at ../../src/gdb/frame.c:279
#8  0x0000000000503b00 in handle_inferior_event (ecs=0x7fffd6bca510) 
at ../../src/gdb/infrun.c:2605
#9  0x00000000005016cb in wait_for_inferior (treat_exec_as_sigtrap=0) 
at ../../src/gdb/infrun.c:1482
#10 0x000000000050142d in proceed (addr=18446744073709551615, 
siggnal=TARGET_SIGNAL_DEFAULT, step=1)
    at ../../src/gdb/infrun.c:1273
#11 0x00000000004fd64f in step_1 (skip_subroutines=1, single_inst=0, 
count_string=0x0) at ../../src/gdb/infcmd.c:784
#12 0x00000000004fd32e in next_command (count_string=0x0, from_tty=1) 
at ../../src/gdb/infcmd.c:678
...

The problem triggers whenever the first thing we do with a
frame is to call something that calls get_prev_frame_1, before
the frame having an unwinder and a frame id set.

We weren't seeing this happen, because normally we're calling
get_frame_id before frame_unwind_id:

 /* Check for subroutine calls.  The check for the current frame
     equalling the step ID is not necessary - the check of the
     previous frame's ID is sufficient - but it is a common case and
     cheaper than checking the previous frame's ID.

     NOTE: frame_id_eq will never report two invalid frame IDs as
     being equal, so to get into this block, both the current and
     previous frame must have valid frame IDs.  */
  if (!frame_id_eq (get_frame_id (get_current_frame ()), step_frame_id)
      && frame_id_eq (frame_unwind_id (get_current_frame ()), step_frame_id))
    {
      CORE_ADDR real_stop_pc;

It happened that my new call to frame_unwind_id was earlier than that bit.

Since get_frame_id only does the work once,

struct frame_id
get_frame_id (struct frame_info *fi)
{
  if (fi == NULL)
    {
      return null_frame_id;
    }
  if (!fi->this_id.p)
    {

      ...

      fi->this_id.p = 1;
    }
  return fi->this_id.value;
}

... this sequence was safe:

static struct frame_info *
get_prev_frame_1 (struct frame_info *this_frame)
{
  this_frame->prev_p = 1;
  this_frame->stop_reason = UNWIND_NO_REASON;

  /* Check that this frame's ID was valid.  If it wasn't, don't try to
     unwind to the prev frame.  Be careful to not apply this test to
     the sentinel frame.  */
  this_id = get_frame_id (this_frame); <<<< hits already built id.

Tested on x86_64-unknown-linux-gnu, no regressions.

OK?

-- 
Pedro Alves

[-- Attachment #2: frame_id.diff --]
[-- Type: text/x-diff, Size: 1061 bytes --]

2008-05-20  Pedro Alves  <pedro@codesourcery.com>

	* frame.c (get_prev_frame_1): Build frame id before setting
	this_frame->prev_p, not after.

---
 gdb/frame.c |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Index: src/gdb/frame.c
===================================================================
--- src.orig/gdb/frame.c	2008-05-20 22:41:07.000000000 +0100
+++ src/gdb/frame.c	2008-05-20 22:49:59.000000000 +0100
@@ -1179,13 +1179,17 @@ get_prev_frame_1 (struct frame_info *thi
 	}
       return this_frame->prev;
     }
+
+  /* If the frame id hasn't been built yet, it must be done before
+     setting a stop reason.  */
+  this_id = get_frame_id (this_frame);
+
   this_frame->prev_p = 1;
   this_frame->stop_reason = UNWIND_NO_REASON;
 
   /* Check that this frame's ID was valid.  If it wasn't, don't try to
      unwind to the prev frame.  Be careful to not apply this test to
      the sentinel frame.  */
-  this_id = get_frame_id (this_frame);
   if (this_frame->level >= 0 && !frame_id_p (this_id))
     {
       if (frame_debug)

Re: fix frame_unwind_id assertion

[Daniel Jacobowitz]

On Tue, May 20, 2008 at 11:12:03PM +0100, Pedro Alves wrote:
> 2008-05-20  Pedro Alves  <pedro@codesourcery.com>
> 
> 	* frame.c (get_prev_frame_1): Build frame id before setting
> 	this_frame->prev_p, not after.

OK.

-- 
Daniel Jacobowitz
CodeSourcery

Re: fix frame_unwind_id assertion

[Pedro Alves]

A Tuesday 20 May 2008 23:14:43, Daniel Jacobowitz wrote:
> On Tue, May 20, 2008 at 11:12:03PM +0100, Pedro Alves wrote:
> > 2008-05-20  Pedro Alves  <pedro@codesourcery.com>
> >
> > 	* frame.c (get_prev_frame_1): Build frame id before setting
> > 	this_frame->prev_p, not after.
>
> OK.

Whoa, that was fast.  Thanks!

Checked in.

-- 
Pedro Alves