This is the mail archive of the gdb-patches@sources.redhat.com mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: ppc_linux_init_extra_frame_info


On Feb 21, 1:14am, Andreas Schwab wrote:


ppc_linux_init_extra_frame_info is clobbering the frame type which
causes dummy_frame_pop to bail out.  The frame type is now set before
INIT_FRAME_INFO is called.


Not always, it would appear.  In create_new_frame(), the type is
set before INIT_EXTRA_FRAME_INFO() is called.  However, in
legacy_get_prev_frame(), the setting of the type occurs both
before and possibly after the call to INIT_EXTRA_FRAME_INFO().

In fact, the "after" part has me concerned.  I suspect that it could
cause a signal handler's frame to be marked as a sigtramp frame.
Andrew even comments on this:

      /* FIXME: cagney/2002-11-10: This should be moved to before the
	 INIT code above so that the INIT code knows what the frame's
	 type is (in fact, for a [generic] dummy-frame, the type can
	 be set and then the entire initialization can be skipped.
	 Unforunatly, its the INIT code that sets the PC (Hmm, catch
	 22).  */

Hmm,


ac131313 at lulu$ frep deprecated_set_frame_type
frame.c:1388:deprecated_set_frame_type (struct frame_info *frame, enum frame_type type)
frame.h:251:extern void deprecated_set_frame_type (struct frame_info *,
i386-interix-tdep.c:166: deprecated_set_frame_type (frame, SIGTRAMP_FRAME);
i386-interix-tdep.c:174: deprecated_set_frame_type (frame, SIGTRAMP_FRAME);
ppc-linux-tdep.c:376: deprecated_set_frame_type (fi, SIGTRAMP_FRAME);
ppc-linux-tdep.c:380: deprecated_set_frame_type (fi, NORMAL_FRAME);
rs6000-tdep.c:181: deprecated_set_frame_type (fi, SIGTRAMP_FRAME);


(Joel, I'm going to ignore interix :-/)

So it's solely PPC GNU/Linux that is the cause of all the problems. An addition to the comment;

  /* FIXME: cagney/2002-11-18: Should be setting the frame's type
     here, before anything else, and not last.  Various INIT functions
     are full of work-arounds for the frames type not being set
     correctly from the word go.  Ulgh!  */
  prev->type = NORMAL_FRAME;

is that, for legacy targets, you don't know at this point what the PC is.

Why not also modify the nasty ppc-linux hack to read:

	if (get_frame_type () == NORMAL)
	  if (pc in sigtramp)
	   deprecated set frame type ()

Andrew


-   behavior is ever fixed.)
-
-   PC_IN_SIGTRAMP is called from blockframe.c as well in order to set
-   the frame's type (if a SIGTRAMP_FRAME).  Because of our strange
-   definition of in_sigtramp below, we can't rely on the frame's type
-   getting set correctly from within blockframe.c.  This is why we
-   take pains to set it in init_extra_frame_info().
-
-   NOTE: cagney/2002-11-10: I suspect the real problem here is that
-   the get_prev_frame() only initializes the frame's type after the
-   call to INIT_FRAME_INFO.  get_prev_frame() should be fixed, this
-   code shouldn't be working its way around a bug :-(.  */
+   behavior is ever fixed.)  */


I'd prefer to have these comments left in place.

I don't agree with Andrew's comment BTW.  My memory on this is rather
hazy, but I think that the problem is that we really want different
functions for PC_IN_SIGTRAMP as called from infrun.c vs frame.c.

If we could have the frame.c PC_IN_SIGRAMP calls call
ppc_linux_at_sigtramp_return_path() and the infrun.c calls call
ppc_linux_in_sigtramp(), things would work right.  This suggests
that we really need two different methods.

(Take my current remarks with a grain of salt.  It's been a while
since I've worked on this code...)

If you'd like to add you own notes to this comment block (or anywhere
else), I have no problem with that.


 int
 ppc_linux_in_sigtramp (CORE_ADDR pc, char *func_name)
@@ -374,10 +363,6 @@ ppc_linux_init_extra_frame_info (int fro
          at trampoline code */
       if (ppc_linux_at_sigtramp_return_path (fi->pc))
 	deprecated_set_frame_type (fi, SIGTRAMP_FRAME);
-      else
-	/* FIXME: cagney/2002-11-10: Is this double bogus?  What
-           happens if the frame has previously been marked as a dummy?  */
-	deprecated_set_frame_type (fi, NORMAL_FRAME);
     }
 }


This part is fine. (I.e, I think this part should be committed.)

Thanks,

Kevin




Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]