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]

[rfa/ppc] De double compensate rs6000_frame_chain()


Hello,

[Note for architecture maintainers: this post fixes a couple implementation problems in the rs6000. Suggest checking your own architecture for similar.]

frame_chain() is ment to return ->frame for the previous frame. With the RS6000, a dummy frame always contains a correctly constructed chain (FP points at saved previous FP) so you would expect rs6000 to always return the frame address of the caller vis:
add(): frame chain -> dummy
dummy: frame chain -> main()
main(): ...
Unfortunatly it doesn't.

When frame_chain() encountered what it thinks is a dummy frame, it chains right through it vis:

@@ -1758,13 +1760,6 @@
fp = FRAME_FP (thisframe);
else
fp = read_memory_addr ((thisframe)->frame, wordsize);
-
- lr = read_register (gdbarch_tdep (current_gdbarch)->ppc_lr_regnum);
- if (lr == entry_point_address ())
- if (fp != 0 && (fpp = read_memory_addr (fp, wordsize)) != 0)
- if (PC_IN_CALL_DUMMY (lr, fpp, fpp))
- return fpp;
-
return fp;
}

This excess is then compensated for, by not chaining at all when chaining from the dummy frame vis:

if (PC_IN_CALL_DUMMY (thisframe->pc, thisframe->frame, thisframe->frame))
- return thisframe->frame; /* dummy frame same as caller's frame */

Why is this a problem?

- The way the code finds LR is technically flawed. It would need to use FRAME_SAVED_PC(). But that is a side issue :-)

- Tt doesn't meet the spec. frame_chain() should return the frame of the caller, not the callers caller.

- Tt creates the chain:
add(): frame chain -> main()
dummy: frame chain -> main()
main(): ...
and this just happens to work because generic_find_dummy_frame() is very generous about what it will match for a dummy frame (SP, FP or TOS). Unfortunatly, per my MIPS generic dummy frame posts, that test is too generous when there are frameless functions. If the test is tightend, rs6000 starts getting failures. Other architectures may be exposed to the same problem.

The attached, delets a good sized chunk of rs6000_frame_chain() and in the process brings it into spec.

Ok to commit?
My GNU/Linux PPC shows no regressions.

Andrew

PS: I went looking. The code is very old so didn't have access any of the current framework so I'm not suprised by the ``bugs''. It pre-dates the FSF cvs repository.
2002-09-26  Andrew Cagney  <cagney@redhat.com>

	* rs6000-tdep.c (rs6000_frame_chain): Don't chain past the dummy
	frame.

Index: rs6000-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/rs6000-tdep.c,v
retrieving revision 1.83
diff -u -r1.83 rs6000-tdep.c
--- rs6000-tdep.c	25 Sep 2002 13:34:53 -0000	1.83
+++ rs6000-tdep.c	26 Sep 2002 14:59:52 -0000
@@ -1741,7 +1741,9 @@
   int wordsize = gdbarch_tdep (current_gdbarch)->wordsize;
 
   if (PC_IN_CALL_DUMMY (thisframe->pc, thisframe->frame, thisframe->frame))
-    return thisframe->frame;	/* dummy frame same as caller's frame */
+    /* A dummy frame always correctly chains back to the previous
+       frame.  */
+    return read_memory_addr ((thisframe)->frame, wordsize);
 
   if (inside_entry_file (thisframe->pc) ||
       thisframe->pc == entry_point_address ())
@@ -1758,13 +1760,6 @@
     fp = FRAME_FP (thisframe);
   else
     fp = read_memory_addr ((thisframe)->frame, wordsize);
-
-  lr = read_register (gdbarch_tdep (current_gdbarch)->ppc_lr_regnum);
-  if (lr == entry_point_address ())
-    if (fp != 0 && (fpp = read_memory_addr (fp, wordsize)) != 0)
-      if (PC_IN_CALL_DUMMY (lr, fpp, fpp))
-	return fpp;
-
   return fp;
 }
 

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