This is the mail archive of the gdb-patches@sourceware.org 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: [PATCH 4/4] GDB: New target s12z


On Fri, Sep 07, 2018 at 04:03:17PM -0600, Tom Tromey wrote:
     >>>>> "John" == John Darrington <john@darrington.wattle.id.au> writes:
     
     Thank you for the patch.  It seems generally good.  I have a few nits,
     but nothing serious.
     
     John> diff --git a/gdb/Makefile.in b/gdb/Makefile.in
     John> index 118c3c8062..f448d1ee19 100644
     John> --- a/gdb/Makefile.in
     John> +++ b/gdb/Makefile.in
     John> @@ -758,6 +758,7 @@ ALL_TARGET_OBS = \
     John>  	rs6000-lynx178-tdep.o \
     John>  	rs6000-tdep.o \
     John>  	rx-tdep.o \
     John> +	s12z-tdep.o \
     
     This change should be mentioned in the ChangeLog.
     
     John> +
     John> +#define N_PHYSICAL_REGISTERS (S12Z_N_REGISTERS - 2)
     John> +
     John> +static const int reg_perm[N_PHYSICAL_REGISTERS] =
     John> +  {
     
     New functions, comments, and macros should have an explanatory comment.
     Sometimes this can be just a reference to some generic thing, like
     implementations of gdbarch methods can refer to gdbarch.h.  There are a
     number of cases of this in the file.
     
     John> +  /*  registers is declared in opcodes/s12z.h */
     
     GNU comment style is to start with a capital letter and end with a
     period and two spaces.
     
     John> +      if (0 != prologue_end)
     John> +	{
     John> +	  struct symtab_and_line prologue_sal = find_pc_line (start_pc, 0);
     John> +	  struct compunit_symtab *compunit
     John> +	    = SYMTAB_COMPUNIT (prologue_sal.symtab);
     John> +	  const char *debug_format = COMPUNIT_DEBUGFORMAT (compunit);
     John> +
     John> +	  if ((NULL != debug_format)
     John> +	      && (strlen ("dwarf") <= strlen (debug_format))
     John> +	      && (0 == strncasecmp ("dwarf", debug_format, strlen ("dwarf"))))
     John> +	    return (prologue_end > pc) ? prologue_end : pc;
     John> +	}
     
     Is this stuff useful?
     
I suspect not.  In fact, I'm very dubious about the whole
skip_prologue_using_sal function.  It seems to not work very well. 
     
     John> +      /* JPB: 28-Apr-11.  This is a temporary patch, to get round GDB
     John> +	 crashing right at the beginning.  Build the frame ID as best we
     John> +	 can.  */
     John> +      trad_frame_set_id (info, frame_id_build (this_sp, this_pc));
     
     Could you explain this more?

I'm afraid I can't.  This was code I took over from the or1k target.


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