[rfa/ppc] De double compensate rs6000_frame_chain()

[rfa/ppc] De double compensate rs6000_frame_chain()

[Andrew Cagney]

[-- Attachment #1: Type: text/plain, Size: 2331 bytes --]

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.

[-- Attachment #2: diffs --]
[-- Type: text/plain, Size: 1254 bytes --]

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;
 }
 

Re: [rfa/ppc] De double compensate rs6000_frame_chain()

[Kevin Buettner]

On Sep 26, 11:31am, Andrew Cagney wrote:

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

Yes!

Thanks for including the prefatory text in your email message describing
the problem.

Kevin

Re: [rfa/ppc] De double compensate rs6000_frame_chain()

[Andrew Cagney]

> On Sep 26, 11:31am, Andrew Cagney wrote:
> 
> 
>> Ok to commit?
>> My GNU/Linux PPC shows no regressions.
> 
> 
> Yes!
> 
> Thanks for including the prefatory text in your email message describing
> the problem.

Committed,
Andrew