Re: [RFA] h8300-tdep.c: Rewrite frame code, de-deprecate, ...

[Daniel Jacobowitz <drow at false dot org>]

On Thu, Apr 28, 2005 at 05:32:32PM +0200, Corinna Vinschen wrote:
> Hi,
> 
> the below patch is not exactly a rewrite of the h8300-tdep code, but it's 
> pretty similar to one.  None of the deprecated stuff is left.  I reworked
> a lot of the code, especially the frame code and the prologue analysis. 
> The dwarf2 frame sniffer is also hooked in now.
> 
> I also took the freedom to add a h8300.inc to the gdb.asm testsuite.
> 
> 
> Ok to apply?

It looks generally sane, but there are a couple of minor issues.

> ChangeLog:
> 
> 	* h8300-tdep.c: Undeprecate completely, rewrite frame code, rework
> 	prologue analysis and prologue skipping, fix return value and
> 	struct return handling.

I know it's a pain, but please write a real changelog entry.

> @@ -90,17 +69,105 @@ enum gdb_regnum
>  #define E_PSEUDO_CCR_REGNUM (NUM_REGS)
>  #define E_PSEUDO_EXR_REGNUM (NUM_REGS+1)
>  
> -#define UNSIGNED_SHORT(X) ((X) & 0xffff)
> +struct h8300_frame_cache
> +{
> +  /* Base address.  */
> +  CORE_ADDR base;
> +  CORE_ADDR sp_offset;
> +  CORE_ADDR pc;
> +
> +  /* Flag showing that a frame has been created in the prologue code. */
> +  int uses_fp;
> +
> +  /* Saved registers.  */
> +  CORE_ADDR saved_regs[H8300_MAX_NUM_REGS];
> +  CORE_ADDR saved_sp;
> +};

Can you use trad-frame.h instead of an array of CORE_ADDRs?

> +#define BINWORD (is_h8300hmode (current_gdbarch) && \
> +		  !is_h8300_normal_mode (current_gdbarch) ? h8300h_reg_size : h8300_reg_size)

You're just moving this around in the file, but while you're touching
it, please fix the formatting.  && on the new line.

> @@ -535,13 +294,135 @@ h8300_analyze_prologue (CORE_ADDR pc, CO
>  			struct h8300_frame_cache *cache)
>  {
>    unsigned int op;
> +  int regno, i, spill_size;
> +
> +  /* For frameless frames, %pc is stored at 0(%fp).  */
> +  //cache->saved_regs[E_PC_REGNUM] = 0;
> +  cache->sp_offset = 0;

Probably meant to delete the comment and commented out line?

> +static CORE_ADDR
> +h8300_skip_prologue (CORE_ADDR pc)
> +{
> +  CORE_ADDR func_addr = 0 , func_end = 0;
> +
> +  if (find_pc_partial_function (pc, NULL, &func_addr, &func_end))
> +    {
> +      struct symtab_and_line sal;
> +      struct h8300_frame_cache cache;
> +
> +      /* Found a function.  */
> +      sal = find_pc_line (func_addr, 0);
> +      if (sal.end && sal.end < func_end)
> +        /* Found a line number, use it as end of prologue.  */
> +        return sal.end;
> +
> +      /* No useable line symbol.  Use prologue parsing method.  */
> +      h8300_init_frame_cache (&cache);
> +      return h8300_analyze_prologue (func_addr, func_end, &cache);
> +    }
> +
> +  /* No function symbol -- just return the PC.  */
> +  return (CORE_ADDR) pc;
> +}
> +

Hmm, exactly the same as the iq2000's.  At least it's consistent if I
ever get around to fixing the other one :-)

> @@ -955,11 +900,52 @@ h8300h_store_return_value (struct type *
>        val = extract_unsigned_integer (valbuf, len);
>        regcache_cooked_write_unsigned (regcache, E_RET0_REGNUM, val);
>        break;
> -    case 8:			/* long long, double and long double are all defined
> -				   as 4 byte types so far so this shouldn't happen.  */
> -      error ("I don't know how to return an 8 byte value.");
> +    case 8:
> +      val = extract_unsigned_integer (valbuf, len);
> +      regcache_cooked_write_unsigned (regcache, E_RET0_REGNUM,
> +				      (val >> 32) & 0xffffffff);
> +      regcache_cooked_write_unsigned (regcache, E_RET1_REGNUM, val & 0xffffffff);
>        break;
> +      break;
> +    }
> +}

Spare "break" there.

> Index: testsuite/gdb.asm/asm-source.exp
> ===================================================================
> RCS file: /cvs/src/src/gdb/testsuite/gdb.asm/asm-source.exp,v
> retrieving revision 1.55
> diff -u -p -r1.55 asm-source.exp
> --- testsuite/gdb.asm/asm-source.exp	8 Mar 2005 08:59:16 -0000	1.55
> +++ testsuite/gdb.asm/asm-source.exp	28 Apr 2005 15:28:18 -0000
> @@ -92,7 +92,12 @@ switch -glob -- [istarget] {
>      "powerpc*-*" {
>          set asm-arch powerpc
>      }
> -    "sh*-*-*" {
> +    "sh64-*-*" {
> +        set asm-arch sh64
> +        set asm-flags "-I${srcdir}/${subdir} -I${objdir}/${subdir}"
> +	set debug-flags "-gdwarf-2"
> +    }
> +    "sh-*-*" {
>          set asm-arch sh
>          set asm-flags "-I${srcdir}/${subdir} -I${objdir}/${subdir}"
>  	set debug-flags "-gdwarf-2"

That looks like a different patch.


-- 
Daniel Jacobowitz
CodeSourcery, LLC