public inbox for gdb@sourceware.org
 help / color / mirror / Atom feed
* frame_register_unwind(): "frame != NULL" assertion failure
@ 2003-02-13 21:24 Kevin Buettner
  2003-02-13 21:29 ` Daniel Jacobowitz
  0 siblings, 1 reply; 16+ messages in thread
From: Kevin Buettner @ 2003-02-13 21:24 UTC (permalink / raw)
  To: Andrew Cagney; +Cc: gdb

Andrew,

Any ideas about what to do about this assertion failure?  This is
occurring in a mips-linux native gdb built using current sources. 
(Okay, they're a few hours old by now...) I was debugging gdb with
itself and was running to a breakpoint placed on main().

#0  internal_error (
    file=0x7a1a60 "/home/devel/kevinb/sourceware-mips64/src.baseline/gdb/frame.c", line=187, string=0x7a1aa0 "%s%sAssertion `%s' failed.")
    at /home/devel/kevinb/sourceware-mips64/src.baseline/gdb/utils.c:800
#1  0x00577c94 in frame_register_unwind (frame=0x0, regnum=187, 
    optimizedp=0x7a1b20, lvalp=0x7fff68d4, addrp=0x7fff68d8, 
    realnump=0x7fff68e0, bufferp=0x7fff68c0)
    at /home/devel/kevinb/sourceware-mips64/src.baseline/gdb/frame.c:187
#2  0x004f8534 in read_next_frame_reg (fi=0x7fff68c0, regno=29)
    at /home/devel/kevinb/sourceware-mips64/src.baseline/gdb/mips-tdep.c:1592
#3  0x004f9e08 in heuristic_proc_desc (start_pc=715987376, limit_pc=715987376, 
    next_frame=0x0, cur_frame=1)
    at /home/devel/kevinb/sourceware-mips64/src.baseline/gdb/mips-tdep.c:2140
#4  0x004fac8c in find_proc_desc (pc=715987376, next_frame=0x0, cur_frame=1)
    at /home/devel/kevinb/sourceware-mips64/src.baseline/gdb/mips-tdep.c:2378
#5  0x004fb1c4 in mips_init_extra_frame_info (fromleaf=8002144, fci=0x10057370)
    at /home/devel/kevinb/sourceware-mips64/src.baseline/gdb/mips-tdep.c:2477
#6  0x004e1bb4 in gdbarch_init_extra_frame_info (gdbarch=0x10061738, 
    fromleaf=0, frame=0x10057370)
    at /home/devel/kevinb/sourceware-mips64/src.baseline/gdb/gdbarch.c:4436
#7  0x0057ad74 in get_prev_frame (next_frame=0x10057308)
    at /home/devel/kevinb/sourceware-mips64/src.baseline/gdb/frame.c:1321
#8  0x00578ed4 in unwind_to_current_frame (ui_out=0x7a1a60, args=0x7a1a60)
    at /home/devel/kevinb/sourceware-mips64/src.baseline/gdb/frame.c:496
#9  0x0056a500 in catcher (func=0x578ea4 <unwind_to_current_frame>, 
    func_uiout=0x10059530, func_args=0x10057308, func_val=0x7fff6c00, 
    func_caught=0x7fff6c04, errstring=0x0, mask=2)
    at /home/devel/kevinb/sourceware-mips64/src.baseline/gdb/top.c:424
#10 0x0056a664 in catch_exceptions (uiout=0x10059530, 
    func=0x578ea4 <unwind_to_current_frame>, func_args=0x10057308, 
    errstring=0x0, mask=2)
    at /home/devel/kevinb/sourceware-mips64/src.baseline/gdb/top.c:474
#11 0x00579048 in get_current_frame ()
    at /home/devel/kevinb/sourceware-mips64/src.baseline/gdb/frame.c:518
#12 0x0057a40c in reinit_frame_cache ()
    at /home/devel/kevinb/sourceware-mips64/src.baseline/gdb/frame.c:950
#13 0x00509ed0 in solib_add (pattern=0x0, from_tty=0, target=0x0, readsyms=1)
    at /home/devel/kevinb/sourceware-mips64/src.baseline/gdb/solib.c:591
#14 0x004bba48 in handle_inferior_event (ecs=0x7fff6db8)
    at /home/devel/kevinb/sourceware-mips64/src.baseline/gdb/infrun.c:2078
#15 0x004b91c8 in wait_for_inferior ()
    at /home/devel/kevinb/sourceware-mips64/src.baseline/gdb/infrun.c:1006
#16 0x004b8dc0 in proceed (addr=4294967295, siggnal=TARGET_SIGNAL_0, step=0)
    at /home/devel/kevinb/sourceware-mips64/src.baseline/gdb/infrun.c:804
...

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: frame_register_unwind(): "frame != NULL" assertion failure
  2003-02-13 21:24 frame_register_unwind(): "frame != NULL" assertion failure Kevin Buettner
@ 2003-02-13 21:29 ` Daniel Jacobowitz
  2003-02-13 21:35   ` Kevin Buettner
  0 siblings, 1 reply; 16+ messages in thread
From: Daniel Jacobowitz @ 2003-02-13 21:29 UTC (permalink / raw)
  To: Kevin Buettner; +Cc: Andrew Cagney, gdb

On Thu, Feb 13, 2003 at 02:23:50PM -0700, Kevin Buettner wrote:
> Andrew,
> 
> Any ideas about what to do about this assertion failure?  This is
> occurring in a mips-linux native gdb built using current sources. 
> (Okay, they're a few hours old by now...) I was debugging gdb with
> itself and was running to a breakpoint placed on main().
> 
> #0  internal_error (
>     file=0x7a1a60 "/home/devel/kevinb/sourceware-mips64/src.baseline/gdb/frame.c", line=187, string=0x7a1aa0 "%s%sAssertion `%s' failed.")
>     at /home/devel/kevinb/sourceware-mips64/src.baseline/gdb/utils.c:800
> #1  0x00577c94 in frame_register_unwind (frame=0x0, regnum=187, 
>     optimizedp=0x7a1b20, lvalp=0x7fff68d4, addrp=0x7fff68d8, 
>     realnump=0x7fff68e0, bufferp=0x7fff68c0)
>     at /home/devel/kevinb/sourceware-mips64/src.baseline/gdb/frame.c:187
> #2  0x004f8534 in read_next_frame_reg (fi=0x7fff68c0, regno=29)
>     at /home/devel/kevinb/sourceware-mips64/src.baseline/gdb/mips-tdep.c:1592

That backtrace must be inaccurate?  read_next_frame_reg just passes fi
to frame_register_unwind...


-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: frame_register_unwind(): "frame != NULL" assertion failure
  2003-02-13 21:29 ` Daniel Jacobowitz
@ 2003-02-13 21:35   ` Kevin Buettner
  2003-02-13 21:48     ` Kevin Buettner
  0 siblings, 1 reply; 16+ messages in thread
From: Kevin Buettner @ 2003-02-13 21:35 UTC (permalink / raw)
  To: Daniel Jacobowitz, Kevin Buettner; +Cc: Andrew Cagney, gdb

On Feb 13,  4:29pm, Daniel Jacobowitz wrote:
> Subject: Re: frame_register_unwind(): "frame != NULL" assertion failure
> On Thu, Feb 13, 2003 at 02:23:50PM -0700, Kevin Buettner wrote:
> > Andrew,
> > 
> > Any ideas about what to do about this assertion failure?  This is
> > occurring in a mips-linux native gdb built using current sources. 
> > (Okay, they're a few hours old by now...) I was debugging gdb with
> > itself and was running to a breakpoint placed on main().
> > 
> > #0  internal_error (
> >     file=0x7a1a60 "/home/devel/kevinb/sourceware-mips64/src.baseline/gdb/frame.c", line=187, string=0x7a1aa0 "%s%sAssertion `%s' failed.")
> >     at /home/devel/kevinb/sourceware-mips64/src.baseline/gdb/utils.c:800
> > #1  0x00577c94 in frame_register_unwind (frame=0x0, regnum=187, 
> >     optimizedp=0x7a1b20, lvalp=0x7fff68d4, addrp=0x7fff68d8, 
> >     realnump=0x7fff68e0, bufferp=0x7fff68c0)
> >     at /home/devel/kevinb/sourceware-mips64/src.baseline/gdb/frame.c:187
> > #2  0x004f8534 in read_next_frame_reg (fi=0x7fff68c0, regno=29)
> >     at /home/devel/kevinb/sourceware-mips64/src.baseline/gdb/mips-tdep.c:1592
> 
> That backtrace must be inaccurate?  read_next_frame_reg just passes fi
> to frame_register_unwind...

I'll try to get a better one.  gdb was compiled with -O2 and I see:

outer-gdb> down
#2  0x004f8534 in read_next_frame_reg (fi=0x7fff68c0, regno=29)
    at /home/devel/kevinb/sourceware-mips64/src.baseline/gdb/mips-tdep.c:1592
1592      frame_register_unwind (fi, regno, &optimized, &lval, &addr, &realnum,
outer-gdb> info address fi
Symbol "fi" is an argument in register s1.

I'll recompile without -O2 and post a new backtrace...

Kevin

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: frame_register_unwind(): "frame != NULL" assertion failure
  2003-02-13 21:35   ` Kevin Buettner
@ 2003-02-13 21:48     ` Kevin Buettner
  2003-02-13 23:27       ` Kevin Buettner
  0 siblings, 1 reply; 16+ messages in thread
From: Kevin Buettner @ 2003-02-13 21:48 UTC (permalink / raw)
  To: Daniel Jacobowitz; +Cc: Andrew Cagney, gdb

On Feb 13,  2:35pm, Kevin Buettner wrote:

> > That backtrace must be inaccurate?  read_next_frame_reg just passes fi
> > to frame_register_unwind...
> 
> I'll try to get a better one.  [...]

Okay, here's a better one.  This one shows that we're passing a NULL
frame starting from mips_init_extra_frame_info().

outer-gdb> bt
#0  internal_error (
    file=0x84abe0 "/home/devel/kevinb/sourceware-mips64/src.baseline/gdb/frame.c", line=187, string=0x84ac20 "%s%sAssertion `%s' failed.")
    at /home/devel/kevinb/sourceware-mips64/src.baseline/gdb/utils.c:800
#1  0x005e4ba4 in frame_register_unwind (frame=0x0, regnum=29, 
    optimizedp=0x7fff68d0, lvalp=0x7fff68e4, addrp=0x7fff68d8, 
    realnump=0x7fff68e0, bufferp=0x7fff68c0)
    at /home/devel/kevinb/sourceware-mips64/src.baseline/gdb/frame.c:187
#2  0x00536708 in read_next_frame_reg (fi=0x0, regno=29)
    at /home/devel/kevinb/sourceware-mips64/src.baseline/gdb/mips-tdep.c:1592
#3  0x00538c44 in heuristic_proc_desc (start_pc=715987376, limit_pc=715987376, 
    next_frame=0x0, cur_frame=1)
    at /home/devel/kevinb/sourceware-mips64/src.baseline/gdb/mips-tdep.c:2140
#4  0x0053a484 in find_proc_desc (pc=715987376, next_frame=0x0, cur_frame=1)
    at /home/devel/kevinb/sourceware-mips64/src.baseline/gdb/mips-tdep.c:2378
#5  0x0053ab58 in mips_init_extra_frame_info (fromleaf=0, fci=0x100573d0)
    at /home/devel/kevinb/sourceware-mips64/src.baseline/gdb/mips-tdep.c:2477
#6  0x00516d80 in gdbarch_init_extra_frame_info (gdbarch=0x10061798, 
    fromleaf=0, frame=0x100573d0)
    at /home/devel/kevinb/sourceware-mips64/src.baseline/gdb/gdbarch.c:4436
#7  0x005e88bc in get_prev_frame (next_frame=0x10057368)
    at /home/devel/kevinb/sourceware-mips64/src.baseline/gdb/frame.c:1321
#8  0x005e6298 in unwind_to_current_frame (ui_out=0x10059590, args=0x10057368)
    at /home/devel/kevinb/sourceware-mips64/src.baseline/gdb/frame.c:496
#9  0x005d423c in catcher (func=0x5e6258 <unwind_to_current_frame>, 
    func_uiout=0x10059590, func_args=0x10057368, func_val=0x7fff6c38, 
    func_caught=0x7fff6c3c, errstring=0x0, mask=2)
    at /home/devel/kevinb/sourceware-mips64/src.baseline/gdb/top.c:424
#10 0x005d43dc in catch_exceptions (uiout=0x10059590, 
    func=0x5e6258 <unwind_to_current_frame>, func_args=0x10057368, 
    errstring=0x0, mask=2)
    at /home/devel/kevinb/sourceware-mips64/src.baseline/gdb/top.c:474
#11 0x005e643c in get_current_frame ()
    at /home/devel/kevinb/sourceware-mips64/src.baseline/gdb/frame.c:518
#12 0x005e7c5c in reinit_frame_cache ()
    at /home/devel/kevinb/sourceware-mips64/src.baseline/gdb/frame.c:950
#13 0x0054d000 in solib_add (pattern=0x0, from_tty=0, target=0x0, readsyms=1)
    at /home/devel/kevinb/sourceware-mips64/src.baseline/gdb/solib.c:591
#14 0x004e68ec in handle_inferior_event (ecs=0x7fff6e58)
    at /home/devel/kevinb/sourceware-mips64/src.baseline/gdb/infrun.c:2078
#15 0x004e3388 in wait_for_inferior ()
    at /home/devel/kevinb/sourceware-mips64/src.baseline/gdb/infrun.c:1006
#16 0x004e2ec0 in proceed (addr=18446744073709551615, siggnal=TARGET_SIGNAL_0, 
    step=0)
    at /home/devel/kevinb/sourceware-mips64/src.baseline/gdb/infrun.c:804
...

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: frame_register_unwind(): "frame != NULL" assertion failure
  2003-02-13 21:48     ` Kevin Buettner
@ 2003-02-13 23:27       ` Kevin Buettner
  2003-02-14 14:58         ` Andrew Cagney
  0 siblings, 1 reply; 16+ messages in thread
From: Kevin Buettner @ 2003-02-13 23:27 UTC (permalink / raw)
  To: Andrew Cagney; +Cc: gdb

Here's one possible "fix" for the assertion failure.  I've come to the
conclusion that the real culprit is get_next_frame(), but, unfortunately,
the rest of gdb is not yet ready for it to return the sentinel frame.
(See the comment in the patch for more info.)  So, until it is, I propose
that we use this hack...

	* frame.c (create_sentinel_frame): Make static.  Add forward
	declaration.
	(frame_register_unwind): Add hack for converting NULL frames
	into a sentinel frame.

Index: frame.c
===================================================================
RCS file: /cvs/src/src/gdb/frame.c,v
retrieving revision 1.66
diff -u -p -r1.66 frame.c
--- frame.c	2 Feb 2003 20:31:43 -0000	1.66
+++ frame.c	13 Feb 2003 23:20:22 -0000
@@ -39,6 +39,8 @@
 #include "command.h"
 #include "gdbcmd.h"
 
+static struct frame_info * create_sentinel_frame (struct regcache *regcache);
+
 /* Flag to indicate whether backtraces should stop at main.  */
 
 static int backtrace_below_main;
@@ -180,6 +182,15 @@ frame_register_unwind (struct frame_info
   gdb_assert (realnump != NULL);
   /* gdb_assert (bufferp != NULL); */
 
+  /* Note: kevinb/2003-02-13: This is a hack.  The problem is that
+     get_next_frame() can return NULL when it really ought to be
+     returning the sentinel frame.  So, when we detect frame == NULL,
+     just use the sentinel frame instead.  
+     FIXME: Remove this hack once get_next_frame() has been fixed
+     to never return NULL.  */
+  if (frame == NULL)
+    frame = create_sentinel_frame (current_regcache);
+
   /* NOTE: cagney/2002-11-27: A program trying to unwind a NULL frame
      is broken.  There is always a frame.  If there, for some reason,
      isn't, there is some pretty busted code as it should have
@@ -429,7 +440,7 @@ frame_map_regnum_to_name (int regnum)
 
 /* Create a sentinel frame.  */
 
-struct frame_info *
+static struct frame_info *
 create_sentinel_frame (struct regcache *regcache)
 {
   struct frame_info *frame = FRAME_OBSTACK_ZALLOC (struct frame_info);

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: frame_register_unwind(): "frame != NULL" assertion failure
  2003-02-13 23:27       ` Kevin Buettner
@ 2003-02-14 14:58         ` Andrew Cagney
  2003-02-14 15:14           ` Daniel Jacobowitz
  0 siblings, 1 reply; 16+ messages in thread
From: Andrew Cagney @ 2003-02-14 14:58 UTC (permalink / raw)
  To: Kevin Buettner; +Cc: gdb

> Here's one possible "fix" for the assertion failure.  I've come to the
> conclusion that the real culprit is get_next_frame(), but, unfortunately,
> the rest of gdb is not yet ready for it to return the sentinel frame.
> (See the comment in the patch for more info.)  So, until it is, I propose
> that we use this hack...
> 
> 	* frame.c (create_sentinel_frame): Make static.  Add forward
> 	declaration.
> 	(frame_register_unwind): Add hack for converting NULL frames
> 	into a sentinel frame.

I think, if there is going to be a hac, then it should be in 
read_next_frame_reg().

However, can you please first investigate modifying the start of 
mips_init_extra_frame_info() where it does:

proc_desc = get_next_frame (fci) ? .....

to, when get_next_frame(fci) is null, call:

	find_proc_desc (get_frame_pc (fci), fci, 1);

that is the current, and not prev frame.

Andrew



> Index: frame.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/frame.c,v
> retrieving revision 1.66
> diff -u -p -r1.66 frame.c
> --- frame.c	2 Feb 2003 20:31:43 -0000	1.66
> +++ frame.c	13 Feb 2003 23:20:22 -0000
> @@ -39,6 +39,8 @@
>  #include "command.h"
>  #include "gdbcmd.h"
>  
> +static struct frame_info * create_sentinel_frame (struct regcache *regcache);
> +
>  /* Flag to indicate whether backtraces should stop at main.  */
>  
>  static int backtrace_below_main;
> @@ -180,6 +182,15 @@ frame_register_unwind (struct frame_info
>    gdb_assert (realnump != NULL);
>    /* gdb_assert (bufferp != NULL); */
>  
> +  /* Note: kevinb/2003-02-13: This is a hack.  The problem is that
> +     get_next_frame() can return NULL when it really ought to be
> +     returning the sentinel frame.  So, when we detect frame == NULL,
> +     just use the sentinel frame instead.  
> +     FIXME: Remove this hack once get_next_frame() has been fixed
> +     to never return NULL.  */
> +  if (frame == NULL)
> +    frame = create_sentinel_frame (current_regcache);
> +
>    /* NOTE: cagney/2002-11-27: A program trying to unwind a NULL frame
>       is broken.  There is always a frame.  If there, for some reason,
>       isn't, there is some pretty busted code as it should have
> @@ -429,7 +440,7 @@ frame_map_regnum_to_name (int regnum)
>  
>  /* Create a sentinel frame.  */
>  
> -struct frame_info *
> +static struct frame_info *
>  create_sentinel_frame (struct regcache *regcache)
>  {
>    struct frame_info *frame = FRAME_OBSTACK_ZALLOC (struct frame_info);
> 
> 


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: frame_register_unwind(): "frame != NULL" assertion failure
  2003-02-14 14:58         ` Andrew Cagney
@ 2003-02-14 15:14           ` Daniel Jacobowitz
  2003-02-14 15:24             ` Kevin Buettner
  2003-02-17 15:37             ` Andrew Cagney
  0 siblings, 2 replies; 16+ messages in thread
From: Daniel Jacobowitz @ 2003-02-14 15:14 UTC (permalink / raw)
  To: Andrew Cagney; +Cc: Kevin Buettner, gdb

On Fri, Feb 14, 2003 at 03:58:55PM +0100, Andrew Cagney wrote:
> >Here's one possible "fix" for the assertion failure.  I've come to the
> >conclusion that the real culprit is get_next_frame(), but, unfortunately,
> >the rest of gdb is not yet ready for it to return the sentinel frame.
> >(See the comment in the patch for more info.)  So, until it is, I propose
> >that we use this hack...
> >
> >	* frame.c (create_sentinel_frame): Make static.  Add forward
> >	declaration.
> >	(frame_register_unwind): Add hack for converting NULL frames
> >	into a sentinel frame.
> 
> I think, if there is going to be a hac, then it should be in 
> read_next_frame_reg().
> 
> However, can you please first investigate modifying the start of 
> mips_init_extra_frame_info() where it does:
> 
> proc_desc = get_next_frame (fci) ? .....
> 
> to, when get_next_frame(fci) is null, call:
> 
> 	find_proc_desc (get_frame_pc (fci), fci, 1);
> 
> that is the current, and not prev frame.

I'm pretty sure that won't work - we're initializing the saved regs for
fci right now, and find_proc_desc wants a frame to read registers out
of.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: frame_register_unwind(): "frame != NULL" assertion failure
  2003-02-14 15:14           ` Daniel Jacobowitz
@ 2003-02-14 15:24             ` Kevin Buettner
  2003-02-17 15:37             ` Andrew Cagney
  1 sibling, 0 replies; 16+ messages in thread
From: Kevin Buettner @ 2003-02-14 15:24 UTC (permalink / raw)
  To: Daniel Jacobowitz, Andrew Cagney; +Cc: Kevin Buettner, gdb

On Feb 14, 10:14am, Daniel Jacobowitz wrote:

> > However, can you please first investigate modifying the start of 
> > mips_init_extra_frame_info() where it does:
> > 
> > proc_desc = get_next_frame (fci) ? .....
> > 
> > to, when get_next_frame(fci) is null, call:
> > 
> > 	find_proc_desc (get_frame_pc (fci), fci, 1);
> > 
> > that is the current, and not prev frame.
> 
> I'm pretty sure that won't work - we're initializing the saved regs for
> fci right now, and find_proc_desc wants a frame to read registers out
> of.

Yes, I agree.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: frame_register_unwind(): "frame != NULL" assertion failure
  2003-02-14 15:14           ` Daniel Jacobowitz
  2003-02-14 15:24             ` Kevin Buettner
@ 2003-02-17 15:37             ` Andrew Cagney
  2003-02-17 23:21               ` Kevin Buettner
  2003-02-18  1:59               ` Kevin Buettner
  1 sibling, 2 replies; 16+ messages in thread
From: Andrew Cagney @ 2003-02-17 15:37 UTC (permalink / raw)
  To: Daniel Jacobowitz, Kevin Buettner; +Cc: gdb

> On Feb 14, 10:14am, Daniel Jacobowitz wrote:
> 
> 
>> > However, can you please first investigate modifying the start of 
>> > mips_init_extra_frame_info() where it does:
>> > 
>> > proc_desc = get_next_frame (fci) ? .....
>> > 
>> > to, when get_next_frame(fci) is null, call:
>> > 
>> > 	find_proc_desc (get_frame_pc (fci), fci, 1);
>> > 
>> > that is the current, and not prev frame.
> 
>> 
>> I'm pretty sure that won't work

Hmm, yes. I took the name `read_next_frame_reg()' at face value :-( 
That leaves us with:

 > - we're initializing the saved regs for
 >> fci right now, and find_proc_desc wants a frame to read registers out
 >> of.

BTW.  This problem is going away.  New targets should no longer need to 
do all those get_next_frame(FCI) calls since the new tdep methods have 
that next frame passed in.

This is also why the comment:

+  /* Note: kevinb/2003-02-13: This is a hack.  The problem is that
+     get_next_frame() can return NULL when it really ought to be
+     returning the sentinel frame.  So, when we detect frame == NULL,
+     just use the sentinel frame instead.
+     FIXME: Remove this hack once get_next_frame() has been fixed
+     to never return NULL.  */

is misleading.  get_next_frame(current_frame), by definition, is NULL. 
The new tdep code uses
	OLD_NEXT_FRAME->unwind()
instead of relying on:
	HALF_INITIALIZED_NEW_PREV_FRAME->next->unwind()
(the sentinal frame unwind still needs to be cleaned up).

The one fly in the ointment is that the sentinel frame unwinder is still 
hardwired.

Anyway, Kevin,

   /* Use proc_desc calculated in frame_chain */
   proc_desc =
     get_next_frame (fci)
     ? cached_proc_desc
     : find_proc_desc (get_frame_pc (fci), get_next_frame (fci), 1);

can you please change the above to be:

     : find_proc_desc (get_frame_pc (fci), NULL, 1);

(with a comment) and modify read_next_frame_reg() to, when NULL, pull a 
value from the register cache.

Andrew


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: frame_register_unwind(): "frame != NULL" assertion failure
  2003-02-17 15:37             ` Andrew Cagney
@ 2003-02-17 23:21               ` Kevin Buettner
  2003-02-18  2:39                 ` Andrew Cagney
  2003-02-18  1:59               ` Kevin Buettner
  1 sibling, 1 reply; 16+ messages in thread
From: Kevin Buettner @ 2003-02-17 23:21 UTC (permalink / raw)
  To: Andrew Cagney, Daniel Jacobowitz, Kevin Buettner; +Cc: gdb

On Feb 17,  4:37pm, Andrew Cagney wrote:

>    /* Use proc_desc calculated in frame_chain */
>    proc_desc =
>      get_next_frame (fci)
>      ? cached_proc_desc
>      : find_proc_desc (get_frame_pc (fci), get_next_frame (fci), 1);
> 
> can you please change the above to be:
> 
>      : find_proc_desc (get_frame_pc (fci), NULL, 1);
> 
> (with a comment) and modify read_next_frame_reg() to, when NULL, pull a 
> value from the register cache.

I will do this, but I really do not think it's the best solution.  (If
we're going to be checking for frame == NULL, then why did you introduce
sentinel frames in the first place?)

Kevin

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: frame_register_unwind(): "frame != NULL" assertion failure
  2003-02-17 15:37             ` Andrew Cagney
  2003-02-17 23:21               ` Kevin Buettner
@ 2003-02-18  1:59               ` Kevin Buettner
  2003-02-18 22:51                 ` Andrew Cagney
  1 sibling, 1 reply; 16+ messages in thread
From: Kevin Buettner @ 2003-02-18  1:59 UTC (permalink / raw)
  To: Andrew Cagney, Daniel Jacobowitz; +Cc: gdb

On Feb 17,  4:37pm, Andrew Cagney wrote:

> Anyway, Kevin,
> 
>    /* Use proc_desc calculated in frame_chain */
>    proc_desc =
>      get_next_frame (fci)
>      ? cached_proc_desc
>      : find_proc_desc (get_frame_pc (fci), get_next_frame (fci), 1);
> 
> can you please change the above to be:
> 
>      : find_proc_desc (get_frame_pc (fci), NULL, 1);
> 
> (with a comment) and modify read_next_frame_reg() to, when NULL, pull a 
> value from the register cache.

I have done this, but I am still seeing the assertion failure.  The reason
is slightly different, however.  Here's a partial backtrace:

outer-gdb> bt
#0  internal_error (file=0x84ae60 "/home/devel/kevinb/sourceware-mips64/src.baseline/gdb/frame.c", line=187, string=0x84aea0 "%s%sAssertion `%s' failed.") at /home/devel/kevinb/sourceware-mips64/src.baseline/gdb/utils.c:800
#1  0x005e4cf4 in frame_register_unwind (frame=0x0, regnum=31, optimizedp=0x7fff6dc8, lvalp=0x7fff6ddc, addrp=0x7fff6dd0, realnump=0x7fff6c80, bufferp=0x7fff6db8) at /home/devel/kevinb/sourceware-mips64/src.baseline/gdb/frame.c:187
During symbol reading, struct/union type gets multiply defined: struct elf_obj_tdata.
#2  0x005479cc in mips_get_saved_register (raw_buffer=0x7fff6db8 "\020\002\177°\177ÿmÈ\020\002\177°\177ÿmÐ", optimizedp=0x7fff6dc8, addrp=0x7fff6dd0, frame=0x100573c8, regnum=31, lvalp=0x7fff6ddc) at /home/devel/kevinb/sourceware-mips64/src.baseline/gdb/mips-tdep.c:5509
#3  0x00513dc4 in gdbarch_get_saved_register (gdbarch=0x10061790, raw_buffer=0x7fff6db8 "\020\002\177°\177ÿmÈ\020\002\177°\177ÿmÐ", optimized=0x7fff6dc8, addrp=0x7fff6dd0, frame=0x100573c8, regnum=31, lval=0x7fff6ddc) at /home/devel/kevinb/sourceware-mips64/src.baseline/gdb/gdbarch.c:3931
#4  0x005e500c in frame_register (frame=0x100573c8, regnum=0, optimizedp=0x7fff6dc8, lvalp=0x7fff6ddc, addrp=0x7fff6dd0, realnump=0x7fff6dd8, bufferp=0x7fff6db8) at /home/devel/kevinb/sourceware-mips64/src.baseline/gdb/frame.c:212
#5  0x005e6eb0 in frame_saved_regs_register_unwind (frame=0x100573c8, cache=0x100573ec, regnum=31, optimizedp=0x7fff6dc8, lvalp=0x7fff6ddc, addrp=0x7fff6dd0, realnump=0x7fff6dd8, bufferp=0x7fff6db8) at /home/devel/kevinb/sourceware-mips64/src.baseline/gdb/frame.c:672
#6  0x005e4d60 in frame_register_unwind (frame=0x100573c8, regnum=31, optimizedp=0x7fff6dc8, lvalp=0x7fff6ddc, addrp=0x7fff6dd0, realnump=0x7fff6dd8, bufferp=0x7fff6db8) at /home/devel/kevinb/sourceware-mips64/src.baseline/gdb/frame.c:190
#7  0x0053687c in read_next_frame_reg (fi=0x100573c8, regno=31) at /home/devel/kevinb/sourceware-mips64/src.baseline/gdb/mips-tdep.c:1599
#8  0x00536ff4 in mips_frame_saved_pc (frame=0x100573c8) at /home/devel/kevinb/sourceware-mips64/src.baseline/gdb/mips-tdep.c:1722
#9  0x00518c3c in gdbarch_frame_saved_pc (gdbarch=0x10061790, fi=0x100573c8) at /home/devel/kevinb/sourceware-mips64/src.baseline/gdb/gdbarch.c:4742
#10 0x0053a958 in mips_frame_chain (frame=0x100573c8) at /home/devel/kevinb/sourceware-mips64/src.baseline/gdb/mips-tdep.c:2434
#11 0x00518710 in gdbarch_frame_chain (gdbarch=0x10061790, frame=0x100573c8) at /home/devel/kevinb/sourceware-mips64/src.baseline/gdb/gdbarch.c:4690
#12 0x005e7fd0 in legacy_get_prev_frame (next_frame=0x100573c8) at /home/devel/kevinb/sourceware-mips64/src.baseline/gdb/frame.c:1012
#13 0x005e87a0 in get_prev_frame (next_frame=0x100573c8) at /home/devel/kevinb/sourceware-mips64/src.baseline/gdb/frame.c:1253
#14 0x004f61f8 in backtrace_command_1 (count_exp=0x0, show_locals=0, from_tty=1) at /home/devel/kevinb/sourceware-mips64/src.baseline/gdb/stack.c:975

If I were to follow your above suggestion, I would also have to add some
explicit regcache fetching code to mips_get_saved_register() too, but I
really can't believe that this is the best approach.

To cleanly solve this problem, I believe that get_next_frame needs to
be able to return the sentinel frame.  But in order to do so, current
usages of get_next_frame() need to be fixed to not check for NULL.

The other approach, less clean, but certainly expeditious, is to use a
hack similar to the one that I've already posted for
frame_register_unwind().

Kevin

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: frame_register_unwind(): "frame != NULL" assertion failure
  2003-02-17 23:21               ` Kevin Buettner
@ 2003-02-18  2:39                 ` Andrew Cagney
  0 siblings, 0 replies; 16+ messages in thread
From: Andrew Cagney @ 2003-02-18  2:39 UTC (permalink / raw)
  To: Kevin Buettner; +Cc: Andrew Cagney, Daniel Jacobowitz, gdb

> On Feb 17,  4:37pm, Andrew Cagney wrote:
> 
> >    /* Use proc_desc calculated in frame_chain */
> >    proc_desc =
> >      get_next_frame (fci)
> >      ? cached_proc_desc
> >      : find_proc_desc (get_frame_pc (fci), get_next_frame (fci), 1);
> > 
> > can you please change the above to be:
> > 
> >      : find_proc_desc (get_frame_pc (fci), NULL, 1);
> > 
> > (with a comment) and modify read_next_frame_reg() to, when NULL, pull a 
> > value from the register cache.

e.g., The mips should be updated to use the new frame unwind
mechanisms and, as part of this, implement a custom sentinel frame
unwinder (which would be passed the sentinal frame as a parameter).
The MIPS specific sentinel frame unwind code could then call
find_proc_desc() with the sentinel frame making all the frame==NULL
tests redundant.  In the mean time, this call to find_proc_desc()
passes in an explicit (rather than implicit) NULL (by definition
current_frame-> == NULL).  This way the task of tracking down if/where
find_proc_desc is called with a NULL frame is made much much easier.

> I will do this, but I really do not think it's the best solution.  (If
> we're going to be checking for frame == NULL, then why did you introduce
> sentinel frames in the first place?)

It is part of the hack.  The `correct fix' (rewrite MIPS to use the unwind
mechanism) would eliminate that call.

Andrew

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: frame_register_unwind(): "frame != NULL" assertion failure
  2003-02-18  1:59               ` Kevin Buettner
@ 2003-02-18 22:51                 ` Andrew Cagney
  2003-02-20 16:18                   ` Kevin Buettner
  0 siblings, 1 reply; 16+ messages in thread
From: Andrew Cagney @ 2003-02-18 22:51 UTC (permalink / raw)
  To: Kevin Buettner; +Cc: Daniel Jacobowitz, gdb

> #2  0x005479cc in mips_get_saved_register (raw_buffer=0x7fff6db8 "\020\002\177°\177ÿmÈ\020\002\177°\177ÿmÐ", optimizedp=0x7fff6dc8, addrp=0x7fff6dd0, frame=0x100573c8, regnum=31, lvalp=0x7fff6ddc) at /home/devel/kevinb/sourceware-mips64/src.baseline/gdb/mips-tdep.c:5509

[...]

 > If I were to follow your above suggestion, I would also have to add some
 > explicit regcache fetching code to mips_get_saved_register() too, but I
 > really can't believe that this is the best approach.

Each case should be examined individually.  Change this call:

   frame_register_unwind (get_next_frame (frame), regnum, optimizedp, lvalp,
                          addrp, &realnum, raw_buffer);

to instead call:

static void
generic_unwind_get_saved_register (char *raw_buffer,
                                    int *optimizedp,
                                    CORE_ADDR *addrp,
                                    struct frame_info *frame,
                                    int regnum,
                                    enum lval_type *lvalp)

(note that the get_next_frame(frame) call isn't needed - that function 
does not have a well chosen name).  The function frame_register() would 
be better but because that knows about old style get_saved_register code 
it would result in infinite recursion :-(

Andrew


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: frame_register_unwind(): "frame != NULL" assertion failure
  2003-02-18 22:51                 ` Andrew Cagney
@ 2003-02-20 16:18                   ` Kevin Buettner
  2003-02-20 16:28                     ` Andrew Cagney
  0 siblings, 1 reply; 16+ messages in thread
From: Kevin Buettner @ 2003-02-20 16:18 UTC (permalink / raw)
  To: Andrew Cagney; +Cc: gdb

On Feb 18,  5:55pm, Andrew Cagney wrote:

> Change this call:
> 
>    frame_register_unwind (get_next_frame (frame), regnum, optimizedp, lvalp,
>                           addrp, &realnum, raw_buffer);
> 
> to instead call:
> 
> static void
> generic_unwind_get_saved_register (char *raw_buffer,
>                                     int *optimizedp,
>                                     CORE_ADDR *addrp,
>                                     struct frame_info *frame,
>                                     int regnum,
>                                     enum lval_type *lvalp)
> 
> (note that the get_next_frame(frame) call isn't needed - that function 
> does not have a well chosen name).  The function frame_register() would 
> be better but because that knows about old style get_saved_register code 
> it would result in infinite recursion :-(

That works.  How does this look?

	* frame.c (generic_unwind_get_saved_register): Make non-static.
	* frame.h (generic_unwind_get_saved_register): Declare.
	* mips-tdep.c (read_next_frame_reg): Fetch register from
	current regcache when frame is NULL.
	(mips_init_extra_frame_info): Pass NULL explicitly for parameter
	that must be NULL.
	(mips_get_saved_register): Call generic_unwind_get_saved_register()
	instead of frame_register_unwind().

Index: frame.c
===================================================================
RCS file: /cvs/src/src/gdb/frame.c,v
retrieving revision 1.66
diff -u -p -r1.66 frame.c
--- frame.c	2 Feb 2003 20:31:43 -0000	1.66
+++ frame.c	20 Feb 2003 16:01:58 -0000
@@ -307,7 +307,7 @@ frame_read_signed_register (struct frame
   frame_unwind_signed_register (frame->next, regnum, val);
 }
 
-static void
+void
 generic_unwind_get_saved_register (char *raw_buffer,
 				   int *optimizedp,
 				   CORE_ADDR *addrp,
Index: frame.h
===================================================================
RCS file: /cvs/src/src/gdb/frame.h,v
retrieving revision 1.66
diff -u -p -r1.66 frame.h
--- frame.h	2 Feb 2003 20:31:43 -0000	1.66
+++ frame.h	20 Feb 2003 16:01:58 -0000
@@ -553,6 +553,13 @@ extern void generic_fix_call_dummy (char
 				    int nargs, struct value **args,
 				    struct type *type, int gcc_p);
 
+void generic_unwind_get_saved_register (char *raw_buffer,
+				        int *optimizedp,
+				        CORE_ADDR *addrp,
+				        struct frame_info *frame,
+				        int regnum,
+				        enum lval_type *lvalp);
+
 /* The function generic_get_saved_register() has been made obsolete.
    GET_SAVED_REGISTER now defaults to the recursive equivalent -
    generic_unwind_get_saved_register() - so there is no need to even
Index: mips-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/mips-tdep.c,v
retrieving revision 1.161
diff -u -p -r1.161 mips-tdep.c
--- mips-tdep.c	28 Jan 2003 16:31:11 -0000	1.161
+++ mips-tdep.c	20 Feb 2003 16:01:59 -0000
@@ -1589,20 +1589,28 @@ read_next_frame_reg (struct frame_info *
   int realnum;
   enum lval_type lval;
   void *raw_buffer = alloca (MAX_REGISTER_RAW_SIZE);
-  frame_register_unwind (fi, regno, &optimized, &lval, &addr, &realnum,
-			 raw_buffer);
-  /* FIXME: cagney/2002-09-13: This is just soooo bad.  The MIPS
-     should have a pseudo register range that correspons to the ABI's,
-     rather than the ISA's, view of registers.  These registers would
-     then implicitly describe their size and hence could be used
-     without the below munging.  */
-  if (lval == lval_memory)
-    {
-      if (regno < 32)
-	{
-	  /* Only MIPS_SAVED_REGSIZE bytes of GP registers are
-	     saved. */
-	  return read_memory_integer (addr, MIPS_SAVED_REGSIZE);
+
+  if (fi == NULL)
+    {
+      regcache_cooked_read (current_regcache, regno, raw_buffer);
+    }
+  else
+    {
+      frame_register_unwind (fi, regno, &optimized, &lval, &addr, &realnum,
+			     raw_buffer);
+      /* FIXME: cagney/2002-09-13: This is just soooo bad.  The MIPS
+	 should have a pseudo register range that correspons to the ABI's,
+	 rather than the ISA's, view of registers.  These registers would
+	 then implicitly describe their size and hence could be used
+	 without the below munging.  */
+      if (lval == lval_memory)
+	{
+	  if (regno < 32)
+	    {
+	      /* Only MIPS_SAVED_REGSIZE bytes of GP registers are
+		 saved. */
+	      return read_memory_integer (addr, MIPS_SAVED_REGSIZE);
+	    }
 	}
     }
 
@@ -2473,11 +2481,16 @@ mips_init_extra_frame_info (int fromleaf
   if (get_frame_type (fci) == DUMMY_FRAME)
     return;
 
-  /* Use proc_desc calculated in frame_chain */
+  /* Use proc_desc calculated in frame_chain.  When there is no
+     next frame, i.e, get_next_frame (fci) == NULL, we call
+     find_proc_desc () to calculate it, passing an explicit
+     NULL as the frame parameter.  */
   proc_desc =
     get_next_frame (fci)
     ? cached_proc_desc
-    : find_proc_desc (get_frame_pc (fci), get_next_frame (fci), 1);
+    : find_proc_desc (get_frame_pc (fci),
+                      NULL /* i.e, get_next_frame (fci) */,
+		      1);
 
   frame_extra_info_zalloc (fci, sizeof (struct frame_extra_info));
 
@@ -5481,7 +5494,6 @@ mips_get_saved_register (char *raw_buffe
   CORE_ADDR addrx;
   enum lval_type lvalx;
   int optimizedx;
-  int realnum;
 
   if (!target_has_registers)
     error ("No registers.");
@@ -5493,8 +5505,8 @@ mips_get_saved_register (char *raw_buffe
     lvalp = &lvalx;
   if (optimizedp == NULL)
     optimizedp = &optimizedx;
-  frame_register_unwind (get_next_frame (frame), regnum, optimizedp, lvalp,
-			 addrp, &realnum, raw_buffer);
+  generic_unwind_get_saved_register (raw_buffer, optimizedp, addrp, frame,
+                                     regnum, lvalp);
   /* FIXME: cagney/2002-09-13: This is just so bad.  The MIPS should
      have a pseudo register range that correspons to the ABI's, rather
      than the ISA's, view of registers.  These registers would then

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: frame_register_unwind(): "frame != NULL" assertion failure
  2003-02-20 16:18                   ` Kevin Buettner
@ 2003-02-20 16:28                     ` Andrew Cagney
  2003-02-20 16:36                       ` Kevin Buettner
  0 siblings, 1 reply; 16+ messages in thread
From: Andrew Cagney @ 2003-02-20 16:28 UTC (permalink / raw)
  To: Kevin Buettner; +Cc: gdb

> 
> That works.  How does this look?
> 
> 	* frame.c (generic_unwind_get_saved_register): Make non-static.
> 	* frame.h (generic_unwind_get_saved_register): Declare.
> 	* mips-tdep.c (read_next_frame_reg): Fetch register from
> 	current regcache when frame is NULL.
> 	(mips_init_extra_frame_info): Pass NULL explicitly for parameter
> 	that must be NULL.
> 	(mips_get_saved_register): Call generic_unwind_get_saved_register()
> 	instead of frame_register_unwind().
> 
yes, ok.


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: frame_register_unwind(): "frame != NULL" assertion failure
  2003-02-20 16:28                     ` Andrew Cagney
@ 2003-02-20 16:36                       ` Kevin Buettner
  0 siblings, 0 replies; 16+ messages in thread
From: Kevin Buettner @ 2003-02-20 16:36 UTC (permalink / raw)
  To: Andrew Cagney; +Cc: gdb

On Feb 20, 11:33am, Andrew Cagney wrote:

> > That works.  How does this look?
> > 
> > 	* frame.c (generic_unwind_get_saved_register): Make non-static.
> > 	* frame.h (generic_unwind_get_saved_register): Declare.
> > 	* mips-tdep.c (read_next_frame_reg): Fetch register from
> > 	current regcache when frame is NULL.
> > 	(mips_init_extra_frame_info): Pass NULL explicitly for parameter
> > 	that must be NULL.
> > 	(mips_get_saved_register): Call generic_unwind_get_saved_register()
> > 	instead of frame_register_unwind().
> > 
> yes, ok.

Committed.

Thanks for the help.

Kevin

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2003-02-20 16:36 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2003-02-13 21:24 frame_register_unwind(): "frame != NULL" assertion failure Kevin Buettner
2003-02-13 21:29 ` Daniel Jacobowitz
2003-02-13 21:35   ` Kevin Buettner
2003-02-13 21:48     ` Kevin Buettner
2003-02-13 23:27       ` Kevin Buettner
2003-02-14 14:58         ` Andrew Cagney
2003-02-14 15:14           ` Daniel Jacobowitz
2003-02-14 15:24             ` Kevin Buettner
2003-02-17 15:37             ` Andrew Cagney
2003-02-17 23:21               ` Kevin Buettner
2003-02-18  2:39                 ` Andrew Cagney
2003-02-18  1:59               ` Kevin Buettner
2003-02-18 22:51                 ` Andrew Cagney
2003-02-20 16:18                   ` Kevin Buettner
2003-02-20 16:28                     ` Andrew Cagney
2003-02-20 16:36                       ` Kevin Buettner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).