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: [RFA/RFC] fix problems with unwinder on mips-irix



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



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