[PATCH/RFC] Work around GCC compiler bugs in frame.c

[Mark Kettenis <kettenis at chello dot nl>]

I've always considered functions return a struct (as opposed to a
pointer to a struct) bad programming style, but now I know why.  The
following code in frame.c triggers a bug in the system compiler on
OpenBSD/vax (which is basically GCC 2.95.3 with some local patches):

      && frame_id_eq (get_frame_id (this_frame),
                      get_frame_id (this_frame->next))

The key here is that get_frame_id() returns a `struct frame_id'.  The
problem is that OpenBSD/vax is one of the GCC targets that define
PCC_STATIC_STRUCT_RETURN, which means that it uses a static buffer to
return the structure.  This static buffer is overwritten on each call.
This is exectly what happens here: the static buffer gets overwritten
by the second call to get_frame_id() before the result of the first
call is pushed on the stack as an argument to frame_id_eq().  The
result is that the frame_id_eq() always returns true :-(.  There are a
couple of other targets in GCC (mostly m68k), that might suffer from
the same problem.

I don't think it will be easy to fix the compiler bug, but it's not
too difficult to work around it in GDB; see the attached patch.
Actually I think the patch is an improvement since it may reduce the
number of calls to get_frame_id().

Oh, by the way, this PCC_STATIC_STRUCT_RETURN convention has
interesting consequences for the structs.exp testsuite; a lot of FAILs
:-(.  Wonder whether we need 

Comments?

Mark


Index: frame.c
===================================================================
RCS file: /cvs/src/src/gdb/frame.c,v
retrieving revision 1.170
diff -u -p -r1.170 frame.c
--- frame.c 2 Apr 2004 22:58:56 -0000 1.170
+++ frame.c 15 Apr 2004 16:05:54 -0000
@@ -1731,6 +1731,7 @@ legacy_get_prev_frame (struct frame_info
 static struct frame_info *
 get_prev_frame_1 (struct frame_info *this_frame)
 {
+  struct frame_id this_id = get_frame_id (this_frame);
   struct frame_info *prev_frame;
 
   gdb_assert (this_frame != NULL);
@@ -1769,7 +1770,7 @@ get_prev_frame_1 (struct frame_info *thi
   /* 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.  */
-  if (this_frame->level >= 0 && !frame_id_p (get_frame_id (this_frame)))
+  if (this_frame->level >= 0 && !frame_id_p (this_id))
     {
       if (frame_debug)
 	{
@@ -1786,16 +1787,14 @@ get_prev_frame_1 (struct frame_info *thi
      go backwards) and sentinel frames (the test is meaningless).  */
   if (this_frame->next->level >= 0
       && this_frame->next->type != SIGTRAMP_FRAME
-      && frame_id_inner (get_frame_id (this_frame),
-			 get_frame_id (this_frame->next)))
+      && frame_id_inner (this_id, get_frame_id (this_frame->next)))
     error ("Previous frame inner to this frame (corrupt stack?)");
 
   /* Check that this and the next frame are not identical.  If they
      are, there is most likely a stack cycle.  As with the inner-than
      test above, avoid comparing the inner-most and sentinel frames.  */
   if (this_frame->level > 0
-      && frame_id_eq (get_frame_id (this_frame),
-		      get_frame_id (this_frame->next)))
+      && frame_id_eq (this_id, get_frame_id (this_frame->next)))
     error ("Previous frame identical to this frame (corrupt stack?)");
 
   /* Allocate the new frame but do not wire it in to the frame chain.