[RFA/mips(commit?)] Unwinding from noreturn function

[Joel Brobecker <brobecker@adacore.com>]

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

Hello,

I'm starting to work on bringing mips-irix back to life. One of the issues
I've noticed is that unwinding is generally broken, particularly from
noreturn functions. I normally have commit priviledges for mips, but
in this case I wouldn't mind an extra opinion on the first issue I found.
Since I don't have access to a mips16 machine, I can't test the change
there...

Consider the following example:

        procedure A is
        begin
           raise Constraint_Error;
        end A;

Compile it with the following command:

        % gnatmake -g a

Then try the following:

        % gdb a
        (gdb) start
        (gdb) b __gnat_debug_raise_exception 
        Breakpoint 2 at 0x9947160: file s-except.adb, line 44.
        (gdb) cont
        Continuing.
        
        Breakpoint 2, <__gnat_debug_raise_exception> (e=0x9a33168)
        [...]
        (gdb) bt
        #0  <__gnat_debug_raise_exception> (e=0x9a33168) at s-except.adb:44
        #1  0x097b82e8 in <__gnat_raise_nodefer_with_msg> (e=0x9a33168)
            at a-except.adb:830
        #2  0x097b82e8 in <__gnat_raise_nodefer_with_msg> (e=0x9a33168)
            at a-except.adb:830
        Backtrace stopped: previous frame identical to this frame (corrupt stack?)

I actually found two issues:

  1. One major: mips_pc_is_mips16 that returns non-zero if bit 0
     of the address it is given is set.

        /* If bit 0 of the address is set, assume this is a MIPS16 address. */
        if (is_mips16_addr (memaddr))
          return 1;

     However, this doesn't work very well in our case, especially
     in this situation:

        static const struct frame_unwind *
        mips_insn16_frame_sniffer (struct frame_info *next_frame)
        {
          CORE_ADDR pc = frame_unwind_address_in_block (next_frame, NORMAL_FRAME);
          if (mips_pc_is_mips16 (pc))
            return &mips_insn16_frame_unwind;
          return NULL;
        }

     In this case, frame_unwind_address_in_block will return the
     frame return address *minus one*, thus accidently triggering
     the check above.

     As a consequence, we ended up using the mips16 unwinder instead
     of the mips32 one.

     It seems to me that the above check is only an optimization,
     and I've spotted at least one instance where I cannot see an
     obvious guaranty that the address has not been decremented
     by one of the _in_block functions... So the decision I made
     was to remove that check.

  2. One minor: There was a confusion in the unwinder between
     the return address and the address of the instruction calling us.
     So I replaced frame_pc_unwind calls by their associated
     frame_unwind_address_in_block.

With the two fixes above, I managed to get the full backtrace:

        (gdb) bt
        #0  <__gnat_debug_raise_exception> (e=0x9a33168) at s-except.adb:44
        #1  0x097b82e8 in <__gnat_raise_nodefer_with_msg> (e=0x9a33168)
            at a-except.adb:830
        #2  0x097b8990 in ada.exceptions.raise_with_location_and_msg (
            e=0x9a33168, f=(system.address) 0xf, l=1717986919, 
            m=(system.address) 0x31) at a-except.adb:995
        #3  0x097b82a0 in <__gnat_raise_constraint_error_msg> (
            file=(system.address) 0x9a33168, line=161761434, 
            msg=(system.address) 0x31) at a-except.adb:795
        #4  0x097b8b58 in <__gnat_rcheck_04> (
            file=(system.address) 0x9a3b240, line=15) at a-except.adb:1043
        #5  0x100027f0 in a () at a.adb:3

2007-03-07  Joel Brobecker  <brobecker@adacore.com>

        * mips-tdep.c (mips_pc_is_mips16): Remove check for bit zero of
        the given address.
        (mips_insn16_frame_cache): Use the proper function to get the
        address of the caller instruction.
        (mips_insn32_frame_cache): Likewise.

Tested on mips-irix. Fixes many many many tests (about 500 or so).
OK to apply?

Thanks,
-- 
Joel

[-- Attachment #2: mips16.diff --]
[-- Type: text/plain, Size: 1382 bytes --]

Index: mips-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/mips-tdep.c,v
retrieving revision 1.404
diff -u -p -r1.404 mips-tdep.c
--- mips-tdep.c	27 Feb 2007 20:17:19 -0000	1.404
+++ mips-tdep.c	7 Mar 2007 04:02:06 -0000
@@ -808,10 +808,6 @@ mips_pc_is_mips16 (CORE_ADDR memaddr)
 {
   struct minimal_symbol *sym;
 
-  /* If bit 0 of the address is set, assume this is a MIPS16 address. */
-  if (is_mips16_addr (memaddr))
-    return 1;
-
   /* A flag indicating that this is a MIPS16 function is stored by elfread.c in
      the high bit of the info field.  Use this to decide if the function is
      MIPS16 or normal MIPS.  */
@@ -1640,7 +1636,8 @@ mips_insn16_frame_cache (struct frame_in
 
   /* Analyze the function prologue.  */
   {
-    const CORE_ADDR pc = frame_pc_unwind (next_frame);
+    const CORE_ADDR pc =
+      frame_unwind_address_in_block (next_frame, NORMAL_FRAME);
     CORE_ADDR start_addr;
 
     find_pc_partial_function (pc, NULL, &start_addr, NULL);
@@ -1961,7 +1958,8 @@ mips_insn32_frame_cache (struct frame_in
 
   /* Analyze the function prologue.  */
   {
-    const CORE_ADDR pc = frame_pc_unwind (next_frame);
+    const CORE_ADDR pc =
+      frame_unwind_address_in_block (next_frame, NORMAL_FRAME);
     CORE_ADDR start_addr;
 
     find_pc_partial_function (pc, NULL, &start_addr, NULL);