Re: [RFA/RFC] fix problems with unwinder on mips-irix

[Andrew Cagney <cagney@gnu.org>]

> But you want a "major overhaul" of the mips unwinder as a precondition
> to fixing the problem at hand. Could you explain a bit more what
> overhaul you are interested in? I don't see what needs to be done.
> 
> In particular, you said:
> 
> 
>>> Two key things to know:
>>> 
>>> - with three unwinders handling three different cases previously handled 
>>> by one, there's a lot of dead code paths.  For instance, 
>>> mips32_heuristic_proc_desc is now only called by mips_insn32_frame_cache 
>>> and hence can be inlined there, making it possible for your problem to 
>>> be solved more locally.
> 
> 
> mips32_heuristic_proc_desc is called by heuristic_proc_desc. (And
> frankly I find inlining often counter productive, as we end up
> with giant function just as we did with decode_line_1).

Yes, sometimes inlineing doesn't help, here it does.  There's really no 
value in trying to preserve this code so be brutal.

If find_proc_desc is inlined:

> static mips_extra_func_info_t
> find_proc_desc (CORE_ADDR pc, struct frame_info *next_frame, int cur_frame)
> {
>   mips_extra_func_info_t proc_desc;
>   CORE_ADDR startaddr = 0;

then everything from here ....

>   proc_desc = non_heuristic_proc_desc (pc, &startaddr);
> 
>   if (proc_desc)
>     {
>       /* IF this is the topmost frame AND
>        * (this proc does not have debugging information OR
>        * the PC is in the procedure prologue)
>        * THEN create a "heuristic" proc_desc (by analyzing
>        * the actual code) to replace the "official" proc_desc.
>        */
>       if (next_frame == NULL)
> 	{
> 	  struct symtab_and_line val;
> 	  struct symbol *proc_symbol =
> 	    PROC_DESC_IS_DUMMY (proc_desc) ? 0 : PROC_SYMBOL (proc_desc);
> 
> 	  if (proc_symbol)
> 	    {
> 	      val = find_pc_line (BLOCK_START
> 				  (SYMBOL_BLOCK_VALUE (proc_symbol)), 0);
> 	      val.pc = val.end ? val.end : pc;
> 	    }
> 	  if (!proc_symbol || pc < val.pc)
> 	    {
> 	      mips_extra_func_info_t found_heuristic =
> 		heuristic_proc_desc (PROC_LOW_ADDR (proc_desc),
> 				     pc, next_frame, cur_frame);
> 	      if (found_heuristic)
> 		proc_desc = found_heuristic;
> 	    }
> 	}
>     }

>   else
>     {
>       /* Is linked_proc_desc_table really necessary?  It only seems to be used
>          by procedure call dummys.  However, the procedures being called ought
>          to have their own proc_descs, and even if they don't,
>          heuristic_proc_desc knows how to create them! */
> 
>       struct linked_proc_info *link;
> 
>       for (link = linked_proc_desc_table; link; link = link->next)
> 	if (PROC_LOW_ADDR (&link->info) <= pc
> 	    && PROC_HIGH_ADDR (&link->info) > pc)
> 	  return &link->info;

... to here is simply dead.  That is all for the non-heuristic case. 
That leaves:

>       if (startaddr == 0)
> 	startaddr = heuristic_proc_start (pc);
> 
>       proc_desc = heuristic_proc_desc (startaddr, pc, next_frame, cur_frame);
>     }
>   return proc_desc;
> }

If heuristic_proc_descr is then inlined we find:

> static mips_extra_func_info_t
> heuristic_proc_desc (CORE_ADDR start_pc, CORE_ADDR limit_pc,
> 		     struct frame_info *next_frame, int cur_frame)
> {
>   CORE_ADDR sp;
> 
>   if (cur_frame)
>     sp = read_next_frame_reg (next_frame, NUM_REGS + MIPS_SP_REGNUM);
>   else
>     sp = 0;
 >
>   if (start_pc == 0)
>     return NULL;
>   memset (&temp_proc_desc, '\0', sizeof (temp_proc_desc));
>   temp_saved_regs = xrealloc (temp_saved_regs, SIZEOF_FRAME_SAVED_REGS);
>   memset (temp_saved_regs, '\0', SIZEOF_FRAME_SAVED_REGS);
>   PROC_LOW_ADDR (&temp_proc_desc) = start_pc;
>   PROC_FRAME_REG (&temp_proc_desc) = MIPS_SP_REGNUM;
>   PROC_PC_REG (&temp_proc_desc) = RA_REGNUM;
>   if (start_pc + 200 < limit_pc)
>     limit_pc = start_pc + 200;
>   if (pc_is_mips16 (start_pc))

this mips16 call is dead.

>     mips16_heuristic_proc_desc (start_pc, limit_pc, next_frame, sp);
>   else
>     mips32_heuristic_proc_desc (start_pc, limit_pc, next_frame, sp);
>   return &temp_proc_desc;
> }

If we then inline the mips32_heuristic ... call into mips_insn32...:

> static void
> mips32_heuristic_proc_desc (CORE_ADDR start_pc, CORE_ADDR limit_pc,
> 			    struct frame_info *next_frame, CORE_ADDR sp)
> {
>   CORE_ADDR cur_pc;
>   CORE_ADDR frame_addr = 0;	/* Value of $r30. Used by gcc for frame-pointer */
> restart:
>   temp_saved_regs = xrealloc (temp_saved_regs, SIZEOF_FRAME_SAVED_REGS);
>   memset (temp_saved_regs, '\0', SIZEOF_FRAME_SAVED_REGS);
>   PROC_FRAME_OFFSET (&temp_proc_desc) = 0;
>   PROC_FRAME_ADJUST (&temp_proc_desc) = 0;	/* offset of FP from SP */
>   for (cur_pc = start_pc; cur_pc < limit_pc; cur_pc += MIPS_INSTLEN)
>     {
>       unsigned long inst, high_word, low_word;
>       int reg;
> 
>       /* Fetch the instruction.   */
>       inst = (unsigned long) mips_fetch_instruction (cur_pc);
> 
>       /* Save some code by pre-extracting some useful fields.  */
>       high_word = (inst >> 16) & 0xffff;
>       low_word = inst & 0xffff;
>       reg = high_word & 0x1f;
> 
>       if (high_word == 0x27bd	/* addiu $sp,$sp,-i */
> 	  || high_word == 0x23bd	/* addi $sp,$sp,-i */
> 	  || high_word == 0x67bd)	/* daddiu $sp,$sp,-i */
> 	{
> 	  if (low_word & 0x8000)	/* negative stack adjustment? */
> 	    PROC_FRAME_OFFSET (&temp_proc_desc) += 0x10000 - low_word;
> 	  else
> 	    /* Exit loop if a positive stack adjustment is found, which
> 	       usually means that the stack cleanup code in the function
> 	       epilogue is reached.  */
> 	    break;
> 	}
>       else if ((high_word & 0xFFE0) == 0xafa0)	/* sw reg,offset($sp) */
> 	{
> 	  PROC_REG_MASK (&temp_proc_desc) |= 1 << reg;
> 	  set_reg_offset (temp_saved_regs, reg, sp + low_word);
> 	}
>       else if ((high_word & 0xFFE0) == 0xffa0)	/* sd reg,offset($sp) */
> 	{
> 	  /* Irix 6.2 N32 ABI uses sd instructions for saving $gp and
> 	     $ra.  */
> 	  PROC_REG_MASK (&temp_proc_desc) |= 1 << reg;
> 	  set_reg_offset (temp_saved_regs, reg, sp + low_word);
> 	}
>       else if (high_word == 0x27be)	/* addiu $30,$sp,size */
> 	{
> 	  /* Old gcc frame, r30 is virtual frame pointer.  */
> 	  if ((long) low_word != PROC_FRAME_OFFSET (&temp_proc_desc))
> 	    frame_addr = sp + low_word;
> 	  else if (PROC_FRAME_REG (&temp_proc_desc) == MIPS_SP_REGNUM)
> 	    {
> 	      unsigned alloca_adjust;
> 	      PROC_FRAME_REG (&temp_proc_desc) = 30;
> 	      frame_addr = read_next_frame_reg (next_frame, NUM_REGS + 30);
> 	      alloca_adjust = (unsigned) (frame_addr - (sp + low_word));
> 	      if (alloca_adjust > 0)
> 		{
> 		  /* FP > SP + frame_size. This may be because
> 		   * of an alloca or somethings similar.
> 		   * Fix sp to "pre-alloca" value, and try again.
> 		   */
> 		  sp += alloca_adjust;
> 		  goto restart;
> 		}
> 	    }
> 	}
>       /* move $30,$sp.  With different versions of gas this will be either
>          `addu $30,$sp,$zero' or `or $30,$sp,$zero' or `daddu 30,sp,$0'.
>          Accept any one of these.  */
>       else if (inst == 0x03A0F021 || inst == 0x03a0f025 || inst == 0x03a0f02d)
> 	{
> 	  /* New gcc frame, virtual frame pointer is at r30 + frame_size.  */
> 	  if (PROC_FRAME_REG (&temp_proc_desc) == MIPS_SP_REGNUM)
> 	    {
> 	      unsigned alloca_adjust;
> 	      PROC_FRAME_REG (&temp_proc_desc) = 30;
> 	      frame_addr = read_next_frame_reg (next_frame, NUM_REGS + 30);
> 	      alloca_adjust = (unsigned) (frame_addr - sp);
> 	      if (alloca_adjust > 0)
> 		{
> 		  /* FP > SP + frame_size. This may be because
> 		   * of an alloca or somethings similar.
> 		   * Fix sp to "pre-alloca" value, and try again.
> 		   */
> 		  sp += alloca_adjust;
> 		  goto restart;
> 		}
> 	    }
> 	}
>       else if ((high_word & 0xFFE0) == 0xafc0)	/* sw reg,offset($30) */
> 	{
> 	  PROC_REG_MASK (&temp_proc_desc) |= 1 << reg;
> 	  set_reg_offset (temp_saved_regs, reg, frame_addr + low_word);
> 	}
>     }
> }
> 

and compare it against these parts of the body of mips_insn32's 
heuristic ..., we also find:

> static struct mips_frame_cache *
> mips_insn32_frame_cache (struct frame_info *next_frame, void **this_cache)
> {
>   mips_extra_func_info_t proc_desc;
>   struct mips_frame_cache *cache;
>   struct gdbarch *gdbarch = get_frame_arch (next_frame);
>   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
>   /* r0 bit means kernel trap */
>   int kernel_trap;
>   /* What registers have been saved?  Bitmasks.  */
>   unsigned long gen_mask, float_mask;
> 
>   if ((*this_cache) != NULL)
>     return (*this_cache);
>   cache = FRAME_OBSTACK_ZALLOC (struct mips_frame_cache);
>   (*this_cache) = cache;
>   cache->saved_regs = trad_frame_alloc_saved_regs (next_frame);

this is has been inlined down to mips32_...

>   /* Get the mdebug proc descriptor.  */
>   proc_desc = find_proc_desc (frame_pc_unwind (next_frame), next_frame, 1);
>   if (proc_desc == NULL)
>     /* I'm not sure how/whether this can happen.  Normally when we
>        can't find a proc_desc, we "synthesize" one using
>        heuristic_proc_desc and set the saved_regs right away.  */
>     return cache;
> 
>   /* Extract the frame's base.  */
>   cache->base = (frame_unwind_register_signed (next_frame, NUM_REGS + PROC_FRAME_REG (proc_desc))
> 		 + PROC_FRAME_OFFSET (proc_desc) - PROC_FRAME_ADJUST (proc_desc));

This kernel_trap stuff isn't applicable here.

>   kernel_trap = PROC_REG_MASK (proc_desc) & 1;
>   gen_mask = kernel_trap ? 0xFFFFFFFF : PROC_REG_MASK (proc_desc);
>   float_mask = kernel_trap ? 0xFFFFFFFF : PROC_FREG_MASK (proc_desc);

THe following is just duplicating that inlined mips32... so the pair can 
be folded into one.

>   /* In any frame other than the innermost or a frame interrupted by a
>      signal, we assume that all registers have been saved.  This
>      assumes that all register saves in a function happen before the
>      first function call.  */
>   if (in_prologue (frame_pc_unwind (next_frame), PROC_LOW_ADDR (proc_desc))
>       /* Not sure exactly what kernel_trap means, but if it means the
> 	 kernel saves the registers without a prologue doing it, we
> 	 better not examine the prologue to see whether registers
> 	 have been saved yet.  */
>       && !kernel_trap)
>     {
>       /* We need to figure out whether the registers that the
>          proc_desc claims are saved have been saved yet.  */
> 
>       CORE_ADDR addr;
> 
>       /* Bitmasks; set if we have found a save for the register.  */
>       unsigned long gen_save_found = 0;
>       unsigned long float_save_found = 0;
>       int mips16;
> 
>       /* If the address is odd, assume this is MIPS16 code.  */
>       addr = PROC_LOW_ADDR (proc_desc);
>       mips16 = pc_is_mips16 (addr);
> 
>       /* Scan through this function's instructions preceding the
>          current PC, and look for those that save registers.  */
>       while (addr < frame_pc_unwind (next_frame))
> 	{
> 	  if (mips16)
> 	    {
> 	      mips16_decode_reg_save (mips16_fetch_instruction (addr),
> 				      &gen_save_found);
> 	      addr += MIPS16_INSTLEN;
> 	    }
> 	  else
> 	    {
> 	      mips32_decode_reg_save (mips32_fetch_instruction (addr),
> 				      &gen_save_found, &float_save_found);
> 	      addr += MIPS_INSTLEN;
> 	    }
> 	}
>       gen_mask = gen_save_found;
>       float_mask = float_save_found;
>     }
> 
>   /* Fill in the offsets for the registers which gen_mask says were
>      saved.  */
>   {
>     CORE_ADDR reg_position = (cache->base
> 			      + PROC_REG_OFFSET (proc_desc));
>     int ireg;
>     for (ireg = MIPS_NUMREGS - 1; gen_mask; --ireg, gen_mask <<= 1)
>       if (gen_mask & 0x80000000)
> 	{
> 	  cache->saved_regs[NUM_REGS + ireg].addr = reg_position;
> 	  reg_position -= mips_abi_regsize (gdbarch);
> 	}
>   }

this is dead ...

>   /* The MIPS16 entry instruction saves $s0 and $s1 in the reverse
>      order of that normally used by gcc.  Therefore, we have to fetch
>      the first instruction of the function, and if it's an entry
>      instruction that saves $s0 or $s1, correct their saved addresses.  */
>   if (pc_is_mips16 (PROC_LOW_ADDR (proc_desc)))
>     {
>       ULONGEST inst = mips16_fetch_instruction (PROC_LOW_ADDR (proc_desc));
>       if ((inst & 0xf81f) == 0xe809 && (inst & 0x700) != 0x700)
> 	/* entry */
> 	{
> 	  int reg;
> 	  int sreg_count = (inst >> 6) & 3;
> 
> 	  /* Check if the ra register was pushed on the stack.  */
> 	  CORE_ADDR reg_position = (cache->base
> 				    + PROC_REG_OFFSET (proc_desc));
> 	  if (inst & 0x20)
> 	    reg_position -= mips_abi_regsize (gdbarch);
> 
> 	  /* Check if the s0 and s1 registers were pushed on the
> 	     stack.  */
> 	  /* NOTE: cagney/2004-02-08: Huh?  This is doing no such
>              check.  */
> 	  for (reg = 16; reg < sreg_count + 16; reg++)
> 	    {
> 	      cache->saved_regs[NUM_REGS + reg].addr = reg_position;
> 	      reg_position -= mips_abi_regsize (gdbarch);
> 	    }
> 	}
>     }

this is redundant (We've got the same loop (or equivalent)) in the 
mips32_heuristic_proc_desc.

>   /* Fill in the offsets for the registers which float_mask says were
>      saved.  */
>   {
>     CORE_ADDR reg_position = (cache->base
> 			      + PROC_FREG_OFFSET (proc_desc));
>     int ireg;
>     /* Fill in the offsets for the float registers which float_mask
>        says were saved.  */
>     for (ireg = MIPS_NUMREGS - 1; float_mask; --ireg, float_mask <<= 1)
>       if (float_mask & 0x80000000)
> 	{
> 	  if (mips_abi_regsize (gdbarch) == 4
> 	      && TARGET_BYTE_ORDER == BFD_ENDIAN_BIG)
> 	    {
> 	      /* On a big endian 32 bit ABI, floating point registers
> 	         are paired to form doubles such that the most
> 	         significant part is in $f[N+1] and the least
> 	         significant in $f[N] vis: $f[N+1] ||| $f[N].  The
> 	         registers are also spilled as a pair and stored as a
> 	         double.
> 
> 	         When little-endian the least significant part is
> 	         stored first leading to the memory order $f[N] and
> 	         then $f[N+1].
> 
> 	         Unfortunately, when big-endian the most significant
> 	         part of the double is stored first, and the least
> 	         significant is stored second.  This leads to the
> 	         registers being ordered in memory as firt $f[N+1] and
> 	         then $f[N].
> 
> 	         For the big-endian case make certain that the
> 	         addresses point at the correct (swapped) locations
> 	         $f[N] and $f[N+1] pair (keep in mind that
> 	         reg_position is decremented each time through the
> 	         loop).  */
> 	      if ((ireg & 1))
> 		cache->saved_regs[NUM_REGS + mips_regnum (current_gdbarch)->fp0 + ireg]
> 		  .addr = reg_position - mips_abi_regsize (gdbarch);
> 	      else
> 		cache->saved_regs[NUM_REGS + mips_regnum (current_gdbarch)->fp0 + ireg]
> 		  .addr = reg_position + mips_abi_regsize (gdbarch);
> 	    }
> 	  else
> 	    cache->saved_regs[NUM_REGS + mips_regnum (current_gdbarch)->fp0 + ireg]
> 	      .addr = reg_position;
> 	  reg_position -= mips_abi_regsize (gdbarch);
> 	}
> 
>     cache->saved_regs[NUM_REGS + mips_regnum (current_gdbarch)->pc]
>       = cache->saved_regs[NUM_REGS + RA_REGNUM];
>   }
> 
>   /* SP_REGNUM, contains the value and not the address.  */
>   trad_frame_set_value (cache->saved_regs, NUM_REGS + MIPS_SP_REGNUM, cache->base);
> 
>   return (*this_cache);
> }

and so it goes

have fun!
Andrew