[rfc, frame] Always check for unsaved PC

[rfc, frame] Always check for unsaved PC

[Daniel Jacobowitz]

This patch, combined with my previous two frame patches for today,
fixes the infinite backtrace Olav Zarges reported on gdb@.

Infinite backtraces are a funny phenomenon, which I've seen on a lot of
platforms in the time I've been working on GDB.  Consider a function
prologue which creates a stack frame, but does not store the PC into
the frame - for instance, because the compiler knows it will never
return.  Then it calls some other function, clobbering the return
register.  A hapless prologue unwinder may report "the stack frame size
is 200 bytes, and the saved PC still lives in LR" not realizing that
LR's been clobbered.  If there were no stack frame, this would trigger
the duplicate frame ID check, but there is; the function remains the
same but the CFA moves continually down, and the frame repeats
endlessly.

Here's a simple way to detect this.  Suppose we have two normal frames
in a row.  Unwind PC_REGNUM from both of them.  Two functions having
the same return address isn't a problem.  But two functions sharing a
stack slot for the return address is a big problem; that never happens
normally, since the first one would pop it.  So this indicates that the
older frame didn't actually save it.

This isn't a perfect check, since it relies on PC_REGNUM, as opposed to
gdbarch_unwind_pc and frame_unwind's prev_pc, but it's still safe; if
the PC_REGNUM does not map onto a real and saved register, then it
won't end up with lval_memory.

There's one wart in this patch.  frame_register_unwind doesn't tell us
where a register was ultimately stored; it recurses for the register
value, but fills in *optimizedp, *lvalp, *realnump, *addrp for where
_this frame_ saved it.  Which is reasonable and useful behavior.  So,
we need to iterate ourselves in order to find the final location.

There's an associated FIXME; I discovered that three targets don't do
it this way.  That doesn't break this patch, but it's inconsistent, and
does a bit of extra computation.  So if there's general agreement that
this patch is a good idea, I'll go through and fix them, and try to
document more clearly that you aren't supposed to do it that way.
Then I can remove the FIXME.

Performance-wise: for frames past the current frame, this causes us to
do an additional two register unwinds.  That's not too expensive, and
we're usually backtracing anyway by that point.  I couldn't measure a
performance impact.  I don't think we really cache this information,
but if this code ever becomes a performance issue somehow, that's
how it should be fixed: per-frame unwound register caching.

Any thoughts on this patch?

-- 
Daniel Jacobowitz
CodeSourcery

2006-08-19  Daniel Jacobowitz  <dan@codesourcery.com>

	* frame.c (frame_register_unwind_location): New function.
	(get_prev_frame_1): Check for UNWIND_NO_SAVED_PC.
	(frame_stop_reason_string): Handle UNWIND_NO_SAVED_PC.
	* frame.h (enum unwind_stop_reason): Add UNWIND_NO_SAVED_PC.

---
 gdb/frame.c |   67 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 gdb/frame.h |    4 +++
 2 files changed, 71 insertions(+)

Index: src/gdb/frame.c
===================================================================
--- src.orig/gdb/frame.c	2006-08-19 11:33:33.000000000 -0400
+++ src/gdb/frame.c	2006-08-19 11:38:43.000000000 -0400
@@ -1023,6 +1023,34 @@ reinit_frame_cache (void)
     }
 }
 
+/* Find where a register is saved (in memory or another register).
+   The result of frame_register_unwind is just where it is saved
+   relative to this particular frame.
+
+   FIXME: alpha, m32c, and h8300 actually do the transitive operation
+   themselves.  */
+
+static void
+frame_register_unwind_location (struct frame_info *this_frame, int regnum,
+				int *optimizedp, enum lval_type *lvalp,
+				CORE_ADDR *addrp, int *realnump)
+{
+  while (this_frame->level >= 0)
+    {
+      frame_register_unwind (this_frame, regnum, optimizedp, lvalp,
+			     addrp, realnump, NULL);
+
+      if (*optimizedp)
+	break;
+
+      if (*lvalp != lval_register)
+	break;
+
+      regnum = *realnump;
+      this_frame = get_next_frame (this_frame);
+    }
+}
+
 /* Return a "struct frame_info" corresponding to the frame that called
    THIS_FRAME.  Returns NULL if there is no such frame.
 
@@ -1077,6 +1105,42 @@ get_prev_frame_1 (struct frame_info *thi
       return NULL;
     }
 
+  /* Check that this and the next frame do not unwind the PC register
+     to the same memory location.  If they do, then even though they
+     have different frame IDs, the new frame will be bogus; two
+     functions can't share a register save slot for the PC.  This can
+     happen when the prologue analyzer finds a stack adjustment, but
+     no PC save.  This check does assume that the "PC register" is
+     roughly a traditional PC, even if the gdbarch_unwind_pc method
+     frobs it.  */
+  if (this_frame->level > 0
+      && get_frame_type (this_frame) == NORMAL_FRAME
+      && get_frame_type (this_frame->next) == NORMAL_FRAME)
+    {
+      int optimized, realnum;
+      enum lval_type lval, nlval;
+      CORE_ADDR addr, naddr;
+
+      frame_register_unwind_location (this_frame, PC_REGNUM, &optimized,
+				      &lval, &addr, &realnum);
+      frame_register_unwind_location (get_next_frame (this_frame), PC_REGNUM,
+				      &optimized, &nlval, &naddr, &realnum);
+
+      if (lval == lval_memory && lval == nlval && addr == naddr)
+	{
+	  if (frame_debug)
+	    {
+	      fprintf_unfiltered (gdb_stdlog, "-> ");
+	      fprint_frame (gdb_stdlog, NULL);
+	      fprintf_unfiltered (gdb_stdlog, " // no saved PC }\n");
+	    }
+
+	  this_frame->stop_reason = UNWIND_NO_SAVED_PC;
+	  this_frame->prev = NULL;
+	  return NULL;
+	}
+    }
+
   /* Allocate the new frame but do not wire it in to the frame chain.
      Some (bad) code in INIT_FRAME_EXTRA_INFO tries to look along
      frame->next to pull some fancy tricks (of course such code is, by
@@ -1623,6 +1687,9 @@ frame_stop_reason_string (enum unwind_st
     case UNWIND_SAME_ID:
       return _("previous frame identical to this frame (corrupt stack?)");
 
+    case UNWIND_NO_SAVED_PC:
+      return _("frame did not save the PC");
+
     case UNWIND_NO_REASON:
     case UNWIND_FIRST_ERROR:
     default:
Index: src/gdb/frame.h
===================================================================
--- src.orig/gdb/frame.h	2006-08-19 11:29:33.000000000 -0400
+++ src/gdb/frame.h	2006-08-19 11:35:41.000000000 -0400
@@ -428,6 +428,10 @@ enum unwind_stop_reason
        this is a sign of unwinder failure.  It could also indicate
        stack corruption.  */
     UNWIND_SAME_ID,
+
+    /* The frame unwinder didn't find any saved PC, but we needed
+       one to unwind further.  */
+    UNWIND_NO_SAVED_PC,
   };
 
 /* Return the reason why we can't unwind past this frame.  */

Re: [rfc, frame] Always check for unsaved PC

[Mark Kettenis]

> Date: Sat, 19 Aug 2006 12:11:39 -0400
> From: Daniel Jacobowitz <drow@false.org>
> 
> This patch, combined with my previous two frame patches for today,
> fixes the infinite backtrace Olav Zarges reported on gdb@.
> 
> Infinite backtraces are a funny phenomenon, which I've seen on a lot of
> platforms in the time I've been working on GDB.  Consider a function
> prologue which creates a stack frame, but does not store the PC into
> the frame - for instance, because the compiler knows it will never
> return.  Then it calls some other function, clobbering the return
> register.  A hapless prologue unwinder may report "the stack frame size
> is 200 bytes, and the saved PC still lives in LR" not realizing that
> LR's been clobbered.

Yeah, we're probably never going to be able to rule this case out
completely for the prologues-scanning unwinders.

> If there were no stack frame, this would trigger
> the duplicate frame ID check, but there is; the function remains the
> same but the CFA moves continually down, and the frame repeats
> endlessly.
> 
> Here's a simple way to detect this.  Suppose we have two normal frames
> in a row.  Unwind PC_REGNUM from both of them.  Two functions having
> the same return address isn't a problem.  But two functions sharing a
> stack slot for the return address is a big problem; that never happens
> normally, since the first one would pop it.  So this indicates that the
> older frame didn't actually save it.

This doesn't fly for architectures where the return address is
computed though, like on sparc.

> This isn't a perfect check, since it relies on PC_REGNUM, as opposed to
> gdbarch_unwind_pc and frame_unwind's prev_pc, but it's still safe; if
> the PC_REGNUM does not map onto a real and saved register, then it
> won't end up with lval_memory.

Indeed on sparc unwinding PC_REGNUM will always end up with bot_lval.

> There's one wart in this patch.  frame_register_unwind doesn't tell us
> where a register was ultimately stored; it recurses for the register
> value, but fills in *optimizedp, *lvalp, *realnump, *addrp for where
> _this frame_ saved it.  Which is reasonable and useful behavior.  So,
> we need to iterate ourselves in order to find the final location.

Don't we have that code already for evaluating variables?  Can't that
code be re-used?

> There's an associated FIXME; I discovered that three targets don't do
> it this way.  That doesn't break this patch, but it's inconsistent, and
> does a bit of extra computation.  So if there's general agreement that
> this patch is a good idea, I'll go through and fix them, and try to
> document more clearly that you aren't supposed to do it that way.
> Then I can remove the FIXME.

I'll have a go at alpha if you don't mind.

> Performance-wise: for frames past the current frame, this causes us to
> do an additional two register unwinds.  That's not too expensive, and
> we're usually backtracing anyway by that point.  I couldn't measure a
> performance impact.  I don't think we really cache this information,
> but if this code ever becomes a performance issue somehow, that's
> how it should be fixed: per-frame unwound register caching.
> 
> Any thoughts on this patch?

I think the error message should be clear about which frame did not
save the PC.

Oh, and this patch doesn't really depend on your "backtrace stop
reasons" patch does it?  The code could just call error().

Mark

> 2006-08-19  Daniel Jacobowitz  <dan@codesourcery.com>
> 
> 	* frame.c (frame_register_unwind_location): New function.
> 	(get_prev_frame_1): Check for UNWIND_NO_SAVED_PC.
> 	(frame_stop_reason_string): Handle UNWIND_NO_SAVED_PC.
> 	* frame.h (enum unwind_stop_reason): Add UNWIND_NO_SAVED_PC.

Re: [rfc, frame] Always check for unsaved PC

[Daniel Jacobowitz]

On Sun, Aug 20, 2006 at 04:27:49PM +0200, Mark Kettenis wrote:
> > If there were no stack frame, this would trigger
> > the duplicate frame ID check, but there is; the function remains the
> > same but the CFA moves continually down, and the frame repeats
> > endlessly.
> > 
> > Here's a simple way to detect this.  Suppose we have two normal frames
> > in a row.  Unwind PC_REGNUM from both of them.  Two functions having
> > the same return address isn't a problem.  But two functions sharing a
> > stack slot for the return address is a big problem; that never happens
> > normally, since the first one would pop it.  So this indicates that the
> > older frame didn't actually save it.
> 
> This doesn't fly for architectures where the return address is
> computed though, like on sparc.

Right.  I don't think we need to make this generic check work
everywhere; what I'd like to do instead would be to allow the prologue
unwinder to return its own "this is why I think unwinding is done"
reason.  The check doesn't really apply to SPARC in the same form.
However, a frameless normal frame where the next frame is also a normal
frame has the same problem, and could use the same unwind stop reason.

I see that gdb doesn't use dwarf2 unwind tables on sparc.  Does GCC
generate them?  How does GCC deal with the extra +8?  It looks like
it doesn't; it uses %o7 to handle unwinding and just looks at the
location of the call site.

> > There's one wart in this patch.  frame_register_unwind doesn't tell us
> > where a register was ultimately stored; it recurses for the register
> > value, but fills in *optimizedp, *lvalp, *realnump, *addrp for where
> > _this frame_ saved it.  Which is reasonable and useful behavior.  So,
> > we need to iterate ourselves in order to find the final location.
> 
> Don't we have that code already for evaluating variables?  Can't that
> code be re-used?

We do have some similar code, but it can't readily be reused here.  I
want the frame prev_register to keep returning the non-transitive
values; that's how we generate a useful "info frame" which says which
registers were saved in this frame, instead of which registers were
saved in any frame.

> > There's an associated FIXME; I discovered that three targets don't do
> > it this way.  That doesn't break this patch, but it's inconsistent, and
> > does a bit of extra computation.  So if there's general agreement that
> > this patch is a good idea, I'll go through and fix them, and try to
> > document more clearly that you aren't supposed to do it that way.
> > Then I can remove the FIXME.
> 
> I'll have a go at alpha if you don't mind.

Sure.  It's simple: look for calls to frame_register and
frame_register_unwind in the tdep files (alpha-tdep.c and
alpha-mdebug-tdep.c), and compare them to the more common
version which uses frame_unwind_register (I hate that naming...).
The alpha version has the next frame fill in *lvalp and
always recurses.  The i386 version has the current frame fill in *lvalp
and only recurses if we need the value.

> I think the error message should be clear about which frame did not
> save the PC.

Could you give me an example of how to make it clearer?  Here's what
it looks like at the moment, IIRC (mocked up, I don't have easy access
to the test right now):

#0  0xffffe410 in __kernel_vsyscall ()
#1  0xb7e441cb in poll () from /lib/tls/i686/cmov/libc.so.6
#2  0x0810c713 in gdb_do_one_event ()
Backtrace stopped: frame did not save the PC

And:

(gdb) i frame 2
Stack frame at 0xbf8842c0:
 eip = 0x810c713 in gdb_do_one_event; saved eip 0x8109a43
 Stops backtrace: frame did not save the PC
 called by frame at 0xbf8842f0
 Arglist at 0xbf8842b8, args:

Maybe "current frame" would be clearer?

-- 
Daniel Jacobowitz
CodeSourcery

Re: [rfc, frame] Always check for unsaved PC

[Mark Kettenis]

> Date: Sun, 20 Aug 2006 13:05:08 -0400
> From: Daniel Jacobowitz <drow@false.org>
>
> > This doesn't fly for architectures where the return address is
> > computed though, like on sparc.
> 
> I see that gdb doesn't use dwarf2 unwind tables on sparc.  Does GCC
> generate them?  How does GCC deal with the extra +8?  It looks like
> it doesn't; it uses %o7 to handle unwinding and just looks at the
> location of the call site.

We use them on Linux, but I haven't enabled the DWARF CFI frame
unwinder on all SPARC platforms yet because the StackGhost
complication on OpenBSD/sparc and OpenBSD/sparc64.

The extra +8 is handled by sparc{32|64}_dwarf2_frame_init_reg().

Mark

Re: [rfc, frame] Always check for unsaved PC

[Daniel Jacobowitz]

On Tue, Aug 22, 2006 at 11:23:55PM +0200, Mark Kettenis wrote:
> We use them on Linux, but I haven't enabled the DWARF CFI frame
> unwinder on all SPARC platforms yet because the StackGhost
> complication on OpenBSD/sparc and OpenBSD/sparc64.
> 
> The extra +8 is handled by sparc{32|64}_dwarf2_frame_init_reg().

Thanks, I see how now.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [rfc, frame] Always check for unsaved PC

[Daniel Jacobowitz]

On Sun, Aug 20, 2006 at 04:27:49PM +0200, Mark Kettenis wrote:
> > Date: Sat, 19 Aug 2006 12:11:39 -0400
> > From: Daniel Jacobowitz <drow@false.org>
> > 
> > This patch, combined with my previous two frame patches for today,
> > fixes the infinite backtrace Olav Zarges reported on gdb@.

Actually, the second patch (the most controversial of the bunch) wasn't
necessary for that specific problem.  So until someday I have time to
revisit that one, let's just consider this one on its own...

There wasn't any objection in the followup discussion to this patch.
Accordingly, I have committed it, along with a small fix (Michael
Snyder noticed that I had the wrong expectations for get_next_frame's
return value, it never returns the sentinel frame).

> > There's an associated FIXME; I discovered that three targets don't do
> > it this way.  That doesn't break this patch, but it's inconsistent, and
> > does a bit of extra computation.  So if there's general agreement that
> > this patch is a good idea, I'll go through and fix them, and try to
> > document more clearly that you aren't supposed to do it that way.
> > Then I can remove the FIXME.
> 
> I'll have a go at alpha if you don't mind.

That refered to the FIXME above frame_register_unwind_location.  Mark,
would you still like to do this, or shall I do all three affected
targets?  It should be fairly mechanical.

-- 
Daniel Jacobowitz
CodeSourcery

2006-11-10  Daniel Jacobowitz  <dan@codesourcery.com>

	* frame.c (frame_register_unwind_location): New function.
	(get_prev_frame_1): Check for UNWIND_NO_SAVED_PC.
	(frame_stop_reason_string): Handle UNWIND_NO_SAVED_PC.
	* frame.h (enum unwind_stop_reason): Add UNWIND_NO_SAVED_PC.

Index: frame.c
===================================================================
RCS file: /cvs/src/src/gdb/frame.c,v
retrieving revision 1.214
diff -u -p -r1.214 frame.c
--- frame.c	18 Oct 2006 19:52:05 -0000	1.214
+++ frame.c	10 Nov 2006 20:06:05 -0000
@@ -1023,6 +1023,36 @@ reinit_frame_cache (void)
     }
 }
 
+/* Find where a register is saved (in memory or another register).
+   The result of frame_register_unwind is just where it is saved
+   relative to this particular frame.
+
+   FIXME: alpha, m32c, and h8300 actually do the transitive operation
+   themselves.  */
+
+static void
+frame_register_unwind_location (struct frame_info *this_frame, int regnum,
+				int *optimizedp, enum lval_type *lvalp,
+				CORE_ADDR *addrp, int *realnump)
+{
+  gdb_assert (this_frame == NULL || this_frame->level >= 0);
+
+  while (this_frame != NULL)
+    {
+      frame_register_unwind (this_frame, regnum, optimizedp, lvalp,
+			     addrp, realnump, NULL);
+
+      if (*optimizedp)
+	break;
+
+      if (*lvalp != lval_register)
+	break;
+
+      regnum = *realnump;
+      this_frame = get_next_frame (this_frame);
+    }
+}
+
 /* Return a "struct frame_info" corresponding to the frame that called
    THIS_FRAME.  Returns NULL if there is no such frame.
 
@@ -1111,6 +1141,42 @@ get_prev_frame_1 (struct frame_info *thi
       return NULL;
     }
 
+  /* Check that this and the next frame do not unwind the PC register
+     to the same memory location.  If they do, then even though they
+     have different frame IDs, the new frame will be bogus; two
+     functions can't share a register save slot for the PC.  This can
+     happen when the prologue analyzer finds a stack adjustment, but
+     no PC save.  This check does assume that the "PC register" is
+     roughly a traditional PC, even if the gdbarch_unwind_pc method
+     frobs it.  */
+  if (this_frame->level > 0
+      && get_frame_type (this_frame) == NORMAL_FRAME
+      && get_frame_type (this_frame->next) == NORMAL_FRAME)
+    {
+      int optimized, realnum;
+      enum lval_type lval, nlval;
+      CORE_ADDR addr, naddr;
+
+      frame_register_unwind_location (this_frame, PC_REGNUM, &optimized,
+				      &lval, &addr, &realnum);
+      frame_register_unwind_location (get_next_frame (this_frame), PC_REGNUM,
+				      &optimized, &nlval, &naddr, &realnum);
+
+      if (lval == lval_memory && lval == nlval && addr == naddr)
+	{
+	  if (frame_debug)
+	    {
+	      fprintf_unfiltered (gdb_stdlog, "-> ");
+	      fprint_frame (gdb_stdlog, NULL);
+	      fprintf_unfiltered (gdb_stdlog, " // no saved PC }\n");
+	    }
+
+	  this_frame->stop_reason = UNWIND_NO_SAVED_PC;
+	  this_frame->prev = NULL;
+	  return NULL;
+	}
+    }
+
   /* Allocate the new frame but do not wire it in to the frame chain.
      Some (bad) code in INIT_FRAME_EXTRA_INFO tries to look along
      frame->next to pull some fancy tricks (of course such code is, by
@@ -1611,6 +1677,9 @@ frame_stop_reason_string (enum unwind_st
     case UNWIND_SAME_ID:
       return _("previous frame identical to this frame (corrupt stack?)");
 
+    case UNWIND_NO_SAVED_PC:
+      return _("frame did not save the PC");
+
     case UNWIND_NO_REASON:
     case UNWIND_FIRST_ERROR:
     default:
Index: frame.h
===================================================================
RCS file: /cvs/src/src/gdb/frame.h,v
retrieving revision 1.149
diff -u -p -r1.149 frame.h
--- frame.h	18 Oct 2006 19:52:05 -0000	1.149
+++ frame.h	10 Nov 2006 20:06:05 -0000
@@ -428,6 +428,10 @@ enum unwind_stop_reason
        this is a sign of unwinder failure.  It could also indicate
        stack corruption.  */
     UNWIND_SAME_ID,
+
+    /* The frame unwinder didn't find any saved PC, but we needed
+       one to unwind further.  */
+    UNWIND_NO_SAVED_PC,
   };
 
 /* Return the reason why we can't unwind past this frame.  */

Re: [rfc, frame] Always check for unsaved PC

[Andreas Schwab]

Daniel Jacobowitz <drow@false.org> writes:

> @@ -1111,6 +1141,42 @@ get_prev_frame_1 (struct frame_info *thi
>        return NULL;
>      }
>  
> +  /* Check that this and the next frame do not unwind the PC register
> +     to the same memory location.  If they do, then even though they
> +     have different frame IDs, the new frame will be bogus; two
> +     functions can't share a register save slot for the PC.  This can
> +     happen when the prologue analyzer finds a stack adjustment, but
> +     no PC save.  This check does assume that the "PC register" is
> +     roughly a traditional PC, even if the gdbarch_unwind_pc method
> +     frobs it.  */
> +  if (this_frame->level > 0
> +      && get_frame_type (this_frame) == NORMAL_FRAME
> +      && get_frame_type (this_frame->next) == NORMAL_FRAME)
> +    {
> +      int optimized, realnum;
> +      enum lval_type lval, nlval;
> +      CORE_ADDR addr, naddr;
> +
> +      frame_register_unwind_location (this_frame, PC_REGNUM, &optimized,
> +				      &lval, &addr, &realnum);
> +      frame_register_unwind_location (get_next_frame (this_frame), PC_REGNUM,
> +				      &optimized, &nlval, &naddr, &realnum);
> +

This is broken.  You can't use PC_REGNUM unconditionally without checking
whether that register actually exists.  The ia64 does not have such a
register.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [rfc, frame] Always check for unsaved PC

[Daniel Jacobowitz]

On Thu, Jan 11, 2007 at 05:34:22PM +0100, Andreas Schwab wrote:
> This is broken.  You can't use PC_REGNUM unconditionally without checking
> whether that register actually exists.  The ia64 does not have such a
> register.

I assume it causes trouble there.  Sorry.  Does this help (untested)?

-- 
Daniel Jacobowitz
CodeSourcery

2007-01-11  Daniel Jacobowitz  <dan@codesourcery.com>

	* frame.c (get_prev_frame_1): Check PC_REGNUM before using it.

Index: frame.c
===================================================================
RCS file: /cvs/src/src/gdb/frame.c,v
retrieving revision 1.218
diff -u -p -r1.218 frame.c
--- frame.c	9 Jan 2007 20:19:15 -0000	1.218
+++ frame.c	11 Jan 2007 16:44:40 -0000
@@ -1221,10 +1221,17 @@ get_prev_frame_1 (struct frame_info *thi
      have different frame IDs, the new frame will be bogus; two
      functions can't share a register save slot for the PC.  This can
      happen when the prologue analyzer finds a stack adjustment, but
-     no PC save.  This check does assume that the "PC register" is
-     roughly a traditional PC, even if the gdbarch_unwind_pc method
-     frobs it.  */
+     no PC save.
+
+     This check does assume that the "PC register" is roughly a
+     traditional PC, even if the gdbarch_unwind_pc method adjusts
+     it (we do not rely on the value, only on the unwound PC being
+     dependent on this value).  A potential improvement would be
+     to have the frame prev_pc method and the gdbarch unwind_pc
+     method set the same lval and location information as
+     frame_register_unwind.  */
   if (this_frame->level > 0
+      && PC_REGNUM >= 0
       && get_frame_type (this_frame) == NORMAL_FRAME
       && get_frame_type (this_frame->next) == NORMAL_FRAME)
     {

Re: [rfc, frame] Always check for unsaved PC

[Andreas Schwab]

Daniel Jacobowitz <drow@false.org> writes:

> On Thu, Jan 11, 2007 at 05:34:22PM +0100, Andreas Schwab wrote:
>> This is broken.  You can't use PC_REGNUM unconditionally without checking
>> whether that register actually exists.  The ia64 does not have such a
>> register.
>
> I assume it causes trouble there.  Sorry.  Does this help (untested)?

Yes, thanks, this fixes about 200 regressions.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [rfc, frame] Always check for unsaved PC

[Daniel Jacobowitz]

On Thu, Jan 11, 2007 at 06:15:45PM +0100, Andreas Schwab wrote:
> Daniel Jacobowitz <drow@false.org> writes:
> 
> > On Thu, Jan 11, 2007 at 05:34:22PM +0100, Andreas Schwab wrote:
> >> This is broken.  You can't use PC_REGNUM unconditionally without checking
> >> whether that register actually exists.  The ia64 does not have such a
> >> register.
> >
> > I assume it causes trouble there.  Sorry.  Does this help (untested)?
> 
> Yes, thanks, this fixes about 200 regressions.

Checked in then.  Thank you for the report - and for testing ia64, in
general.

-- 
Daniel Jacobowitz
CodeSourcery