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] Add support for Tilera TILE-Gx processor (part 2/2: gdb)


On 04/20/2012 03:25 AM, Jeff Kenton wrote:
> 

I don't know tile-gx process at all, but some comments on code style.

> 
> diff -r -u -N ./gdb-7.4.50.20120410/gdb/config/tilegx/nm-linux.h
> ./gdb/config/tilegx/nm-linux.h
> --- ./gdb-7.4.50.20120410/gdb/config/tilegx/nm-linux.h  1969-12-31
> 19:00:00.000000000 -0500
> +++ ./gdb/config/tilegx/nm-linux.h      2012-04-19 13:55:56.195492000 -0400
> @@ -0,0 +1,45 @@
> +/* Native-dependent definitions for GNU/Linux on TILE.
> +
> +   Copyright (C) 1986-2012 Free Software Foundation, Inc.
                    ^^^^^^^^^ 2012.

This is a new file, and created in 2012, so it should be "2012" only.
Here and other places.

> diff -r -u -N ./gdb-7.4.50.20120410/gdb/config/tilegx/tm-linux.h
> ./gdb/config/tilegx/tm-linux.h
> --- ./gdb-7.4.50.20120410/gdb/config/tilegx/tm-linux.h  1969-12-31
> 19:00:00.000000000 -0500
> +++ ./gdb/config/tilegx/tm-linux.h      2012-04-19 13:55:56.233503000 -0400
> @@ -0,0 +1,32 @@
> +/* Target-dependent definitions for GNU/Linux TILE.
> +
> +   Copyright (C) 1986-2012 Free Software Foundation, Inc.
> +
> +   This file is part of GDB.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see
> <http://www.gnu.org/licenses/>.  */
> +
> +#ifndef TM_TILELINUX_H
> +#define TM_TILELINUX_H
> +
> +/* Pull in GNU/Linux generic defs.  */
> +#include "config/tm-linux.h"
> +
> +/* Integer capable of holding an instruction bundle.  */
> +typedef unsigned long long t_bundle;
> +
> +/* We do single stepping in ptrace. */
> +/* (which looks like hardware rather than software).  */

What is this?

> +
> +#endif /* TM_TILELINUX_H */
> diff -r -u -N ./gdb-7.4.50.20120410/gdb/gdbserver/linux-tile-low.c
> ./gdb/gdbserver/linux-tile-low.c
> --- ./gdb-7.4.50.20120410/gdb/gdbserver/linux-tile-low.c       
> 1969-12-31 19:00:00.000000000 -0500
> +++ ./gdb/gdbserver/linux-tile-low.c    2012-04-19 13:55:56.328492000 -0400
> @@ -0,0 +1,123 @@
> +#include "server.h"
> +#include "linux-low.h"
> +
> +#include <sys/ptrace.h>
> +
> +/* Defined in auto-generated file reg-tile.c.  */
> +void init_registers_tile (void);
> +
> +#define tile_num_regs 65
> +
> +static int tile_regmap[] =
> +{
> +   0,  1,  2,  3,  4,  5,  6,  7,
> +   8,  9, 10, 11, 12, 13, 14, 15,
> +  16, 17, 18, 19, 20, 21, 22, 23,
> +  24, 25, 26, 27, 28, 29, 30, 31,
> +  32, 33, 34, 35, 36, 37, 38, 39,
> +  40, 41, 42, 43, 44, 45, 46, 47,
> +  48, 49, 50, 51, 52, 53, 54, 55,
> +  -1, -1, -1, -1, -1, -1, -1, -1,
> +  56
> +};
> +
> +static int
> +tile_cannot_fetch_register (int regno)
> +{
> +  if (regno >= 0 && regno < 56)
> +    return 0;
> +  else if (regno == 64)
> +    return 0;
> +  else
> +    return 1;
> +}
> +
> +static int
> +tile_cannot_store_register (int regno)
> +{
> +  if (regno >= 0 && regno < 56)
> +    return 0;
> +  else if (regno == 64)
> +    return 0;
> +  else
> +    return 1;
> +}
> +
> +static CORE_ADDR
> +tile_get_pc (struct regcache *regcache)
> +{
> +  unsigned long pc;

Need a blank line here.

> +  collect_register_by_name (regcache, "pc", &pc);
> +  return pc;
> +}
> +
> +static void
> +tile_set_pc (struct regcache *regcache, CORE_ADDR pc)
> +{
> +  unsigned long newpc = pc;

and here.

> +  supply_register_by_name (regcache, "pc", &newpc);
> +}
> +
> +static unsigned long long tile_breakpoint = 0x400b3cae70166000ULL;
> +#define tile_breakpoint_len 8
> +
> +static int
> +tile_breakpoint_at (CORE_ADDR where)
> +{
> +  unsigned long long insn;
> +

Looks like sized type `uint64_t' may be better here for `insn'.

> +  (*the_target->read_memory) (where, (unsigned char *) &insn, 8);
> +  if (insn == tile_breakpoint)
> +    return 1;
> +
> +  /* If necessary, recognize more trap instructions here.  GDB only
> uses the
> +     one.  */
> +  return 0;
> +}
> +
> +static void
> +tile_fill_gregset (struct regcache *regcache, void *buf)
> +{
> +  int i;
> +
> +  for (i = 0; i < tile_num_regs; i++)
> +    if (tile_regmap[i] != -1)
> +      collect_register (regcache, i, ((unsigned int *) buf) +
> tile_regmap[i]);
> +}
> +

> +
> +struct regset_info target_regsets[] = {
> +  { PTRACE_GETREGS, PTRACE_SETREGS, 0, tile_num_regs * 4,
> +    GENERAL_REGS, tile_fill_gregset, tile_store_gregset },
> +  { 0, 0, 0, -1, -1, NULL, NULL }
> +};
> +
> +struct linux_target_ops the_low_target = {
> +  init_registers_tile,
> +  tile_num_regs,
> +  tile_regmap,
> +  NULL,
> +  tile_cannot_fetch_register,
> +  tile_cannot_store_register,
> +  NULL,
> +  tile_get_pc,
> +  tile_set_pc,
> +  (const unsigned char *) &tile_breakpoint,
> +  tile_breakpoint_len,
> +  NULL,
> +  0,
> +  tile_breakpoint_at,
> +};

> diff -r -u -N ./gdb-7.4.50.20120410/gdb/tilegx-linux-nat.c
> ./gdb/tilegx-linux-nat.c
> --- ./gdb-7.4.50.20120410/gdb/tilegx-linux-nat.c        1969-12-31
> 19:00:00.000000000 -0500
> +++ ./gdb/tilegx-linux-nat.c    2012-04-19 13:55:56.376489000 -0400
> @@ -0,0 +1,188 @@
> +/* Native-dependent code for GNU/Linux Tile
> +
> +   Copyright (C) 1986-2012 Free Software Foundation, Inc.
> +
> +   This file is part of GDB.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see
> <http://www.gnu.org/licenses/>.  */
> +
> +#include "defs.h"
> +#include "inferior.h"
> +#include "gdbcore.h"
> +#include "regcache.h"
> +#include "linux-nat.h"
> +
> +#include <sys/ptrace.h>
> +
> +#include "gdb_assert.h"
> +#include "gdb_string.h"
> +#include "nm-linux.h"
> +
> +#include <sys/procfs.h>
> +
> +/* Prototypes for supply_gregset etc.  */
> +#include "gregset.h"
> +
> +/* Defines ps_err_e, struct ps_prochandle.  */
> +#include "gdb_proc_service.h"
> +
> +
> +/* The register sets used in GNU/Linux ELF core-dumps are identical to
> +   the register sets in `struct user' that is used for a.out
> +   core-dumps, and is also used by `ptrace'.  The corresponding types
> +   are `elf_gregset_t' for the general-purpose registers (with
> +   `elf_greg_t' the type of a single GP register) and `elf_fpregset_t'
> +   for the floating-point registers.
> +
> +   Those types used to be available under the names `gregset_t' and
> +   `fpregset_t' too, and this file used those names in the past.  But
> +   those names are now used for the register sets used in the
> +   `mcontext_t' type, and have a different size and layout.  */
> +
> +/* Mapping between the general-purpose registers in `struct user'
> +   format and GDB's register array layout. Note that we map the
> +   first 56 registers (0 thru 55) one-to-one. GDB maps the pc to
> +   slot 64, but ptrace returns it in slot 56.  */
> +static const int regmap[] =
> +{
> +   0,  1,  2,  3,  4,  5,  6,  7,
> +   8,  9, 10, 11, 12, 13, 14, 15,
> +  16, 17, 18, 19, 20, 21, 22, 23,
> +  24, 25, 26, 27, 28, 29, 30, 31,
> +  32, 33, 34, 35, 36, 37, 38, 39,
> +  40, 41, 42, 43, 44, 45, 46, 47,
> +  48, 49, 50, 51, 52, 53, 54, 55,
> +  -1, -1, -1, -1, -1, -1, -1, -1,
> +  56
> +};
> +
> +
> +/* Transfering the general-purpose registers between GDB, inferiors
> +   and core files.  */
> +
> +/* Fill GDB's register array with the general-purpose register values
s> +   in *GREGSETP.  */
> +
> +void
> +supply_gregset (struct regcache* regcache,
> +               const elf_gregset_t *gregsetp)

Can be `static'?  Function name "tilegx_supply_gregset" is better.

> +{
> +  elf_greg_t *regp = (elf_greg_t *) gregsetp;
> +  int i;
> +
> +  for (i = 0; i < sizeof(regmap)/sizeof(regmap[0]); i++)
> +    if (regmap[i] >= 0)
> +      regcache_raw_supply (regcache, i, regp + regmap[i]);
> +}
> +
> +/* Fill registers in *GREGSETPS with the values in GDB's
> +   register array.  */
> +
> +void
> +fill_gregset (const struct regcache* regcache,
> +             elf_gregset_t *gregsetp, int regno)

Likewise.

> +{
> +  elf_greg_t *regp = (elf_greg_t *) gregsetp;
> +  int i;
> +
> +  for (i = 0; i < sizeof(regmap)/sizeof(regmap[0]); i++)
> +    if (regmap[i] >= 0)
> +      regcache_raw_collect (regcache, i, regp + regmap[i]);
> +}
> +
> +/* Transfering floating-point registers between GDB, inferiors and
> cores.  */
> +
> +/* Fill GDB's register array with the floating-point register values in
> +   *FPREGSETP.  */
> +
> +void
> +supply_fpregset (struct regcache *regcache,
> +                const elf_fpregset_t *fpregsetp)

Likewise.

> +{
> +  /* NOTE: There are no floating-point registers for TILE-Gx.  */
> +}
> +
> +/* Fill register REGNO (if it is a floating-point register) in
> +   *FPREGSETP with the value in GDB's register array.  If REGNO is -1,
> +   do this for all registers.  */
> +
> +void
> +fill_fpregset (const struct regcache *regcache,
> +              elf_fpregset_t *fpregsetp, int regno)

Likewise.

> +{
> +  /* NOTE: There are no floating-point registers for TILE-Gx.  */
> +}
> +
> +
> +/* Fetch register REGNUM from the inferior.  If REGNUM is -1, do this
> +   for all registers.  */
> +
> +static void
> +fetch_inferior_registers (struct target_ops *ops,
> +                          struct regcache *regcache, int regnum)
> +{
> +  elf_gregset_t regs;
> +  int tid;
> +
> +  tid = ptid_get_lwp (inferior_ptid);
> +  if (tid == 0)
> +    tid = ptid_get_pid (inferior_ptid);
> +
> +  if (ptrace (PTRACE_GETREGS, tid, 0, (PTRACE_TYPE_ARG3) &regs) < 0)
> +    perror_with_name (_("Couldn't get registers"));
> +
> +  supply_gregset (regcache, (const elf_gregset_t *)&regs);
> +}
> +
> +/* Store register REGNUM back into the inferior.  If REGNUM is -1, do
> +   this for all registers.  */
> +
> +static void
> +store_inferior_registers (struct target_ops *ops,
> +                          struct regcache *regcache, int regnum)
> +{
> +  elf_gregset_t regs;
> +  int tid;
> +
> +  tid = ptid_get_lwp (inferior_ptid);
> +  if (tid == 0)
> +    tid = ptid_get_pid (inferior_ptid);
> +
> +  if (ptrace (PTRACE_GETREGS, tid, 0, (PTRACE_TYPE_ARG3) &regs) < 0)
> +    perror_with_name (_("Couldn't get registers"));
> +
> +  fill_gregset (regcache, &regs, regnum);
> +
> +  if (ptrace (PTRACE_SETREGS, tid, 0, (PTRACE_TYPE_ARG3) &regs) < 0)
> +    perror_with_name (_("Couldn't write registers"));
> +}
> +
> +
> +void _initialize_tile_linux_nat (void);
> +

extern initialize_file_ftype _initialize_tile_linux_nat;

> +void
> +_initialize_tile_linux_nat (void)
> +{
> +  struct target_ops *t;
> +
> +  /* Fill in the generic GNU/Linux methods.  */
> +  t = linux_target ();
> +
> +  /* Add our register access methods.  */
> +  t->to_fetch_registers = fetch_inferior_registers;
> +  t->to_store_registers = store_inferior_registers;
> +
> +  /* Register the target.  */
> +  linux_nat_add_target (t);
> +}
> diff -r -u -N ./gdb-7.4.50.20120410/gdb/tilegx-tdep.c ./gdb/tilegx-tdep.c
> --- ./gdb-7.4.50.20120410/gdb/tilegx-tdep.c     1969-12-31
> 19:00:00.000000000 -0500
> +++ ./gdb/tilegx-tdep.c 2012-04-19 14:41:19.259375000 -0400
> @@ -0,0 +1,1201 @@
> +/* Target-dependent code for the Tilera TILE-Gx processor.
> +
> +   Copyright (C) 1986-2012 Free Software Foundation, Inc.
> +
> +   This file is part of GDB.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see
> <http://www.gnu.org/licenses/>.  */
> +

> +
> +
> +enum { tilegx_reg_size = 8 };
> +
> +/* Function: tilegx_register_name
> +   Returns the name of the standard TILE-Gx register N.  */
> +

Usually, the comment of gdbarch hook method is in this style:

"/* This is the implementation of gdbarch method register_name.  */"

> +static const char *
> +tilegx_register_name (struct gdbarch *gdbarch, int regnum)
> +{
> +  static const char *const register_names[E_NUM_REGS] =
> +    {
> +      "r0",   "r1",   "r2",   "r3",   "r4",   "r5",   "r6",   "r7",
> +      "r8",   "r9",   "r10",  "r11",  "r12",  "r13",  "r14",  "r15",
> +      "r16",  "r17",  "r18",  "r19",  "r20",  "r21",  "r22",  "r23",
> +      "r24",  "r25",  "r26",  "r27",  "r28",  "r29",  "r30",  "r31",
> +      "r32",  "r33",  "r34",  "r35",  "r36",  "r37",  "r38",  "r39",
> +      "r40",  "r41",  "r42",  "r43",  "r44",  "r45",  "r46",  "r47",
> +      "r48",  "r49",  "r50",  "r51",  "r52",  "tp",   "sp",   "lr",
> +      "sn",   "idn0", "idn1", "udn0", "udn1", "udn2", "udn3", "zero",
> +      "pc"
> +    };
> +
> +  if (regnum < 0 || regnum >= E_NUM_REGS)
> +    internal_error (__FILE__, __LINE__,
> +                    "tilegx_register_name: illegal register number %d",
> +                    regnum);
> +
> +  return register_names[regnum];
> +}
> +

/* This is the implementation of gdbarch method register_type.  */

> +static struct type *
> +tilegx_register_type (struct gdbarch *gdbarch, int regnum)
> +{
> +  return builtin_type (gdbarch)->builtin_uint64;

Don't we handle register PC here?  Usually, the type of PC register
should be builtin_func_ptr.

> +}
> +
> +static int
> +tilegx_dwarf2_reg_to_regnum (struct gdbarch *gdbarch, int num)
> +{
> +  return num;
> +}
> +
> +
> +/* Function: tilegx_type_is_scalar

This line of comment is unnecessary.  Here and other places.

> +   Makes the decision if a given type is a scalar types.  Scalar
> +   types are returned in the registers r2-r11 as they fit.  */
> +
> +static int
> +tilegx_type_is_scalar (struct type *t)
> +{
> +  return (TYPE_CODE(t) != TYPE_CODE_STRUCT
> + && TYPE_CODE(t) != TYPE_CODE_UNION
> + && TYPE_CODE(t) != TYPE_CODE_ARRAY);
    ^^
Indentation looks incorrect.

> +}
> +
> +/* Function: tilegx_use_struct_convention
> +   Returns non-zero if the given struct type will be returned using
> +   a special convention, rather than the normal function return method.
> +   Used in the contexts of the "return" command, and of
> +   target function calls from the debugger.  */
> +
> +static int
> +tilegx_use_struct_convention (struct type *type)
> +{
> +  /* Only scalars which fit in R0 - R9 can be returned in registers.
> +     Otherwise, they are returned via a pointer passed in R0.  */
> +  return !tilegx_type_is_scalar (type)
> + && (TYPE_LENGTH (type) > (1 + E_R9_REGNUM - E_R0_REGNUM) *
> tilegx_reg_size);

Indentation is incorrect.

> +}
> +
> +/* Function: tilegx_extract_return_value
> +   Find a function's return value in the appropriate registers (in
> +   regbuf), and copy it into valbuf.  */
> +
> +static void
> +tilegx_extract_return_value (struct type *type, struct regcache *regcache,
> +                             void *valbuf)
> +{
> +  int len = TYPE_LENGTH (type);
> +  int i, regnum = E_R0_REGNUM;
> +
> +  for (i = 0; i < len; i += tilegx_reg_size)
> +    regcache_raw_read (regcache, regnum++, (char *) valbuf + i);
> +}
> +
> +/* Function: tilegx_store_return_value
> +   Copy the function return value from VALBUF into the
> +   proper location for a function return.
> +   Called only in the context of the "return" command.  */
> +
> +static void
> +tilegx_store_return_value (struct type *type, struct regcache *regcache,
> +                           const void *valbuf)
> +{
> +  if (TYPE_LENGTH (type) < tilegx_reg_size)
> +    {
> +      /* Add leading zeros to the value.  */
> +      char buf[tilegx_reg_size];

A blank line is needed here.

> +      memset (buf, 0, tilegx_reg_size);
> +      memcpy (buf, valbuf, TYPE_LENGTH (type));
> +      regcache_raw_write (regcache, E_R0_REGNUM, buf);
> +    }
> +  else
> +    {
> +      int len = TYPE_LENGTH (type);
> +      int i, regnum = E_R0_REGNUM;
> +
> +      for (i = 0; i < len; i += tilegx_reg_size)
> +        regcache_raw_write (regcache, regnum++, (char *) valbuf + i);
> +    }
> +}
> +
> +

> +
> +
> +/* Function: tilegx_push_dummy_call
> +   Setup the function arguments for GDB to call a function in the
> inferior.
> +   Called only in the context of a target function call from the debugger.
> +   Returns the value of the SP register after the args are pushed.  */
> +
> +static CORE_ADDR
> +tilegx_push_dummy_call (struct gdbarch *gdbarch,
> +                        struct value *function,
> +                        struct regcache *regcache,
> +                        CORE_ADDR bp_addr, int nargs,
> +                        struct value **args,
> +                        CORE_ADDR sp, int struct_return,
> +                        CORE_ADDR struct_addr)
> +{
> +  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
> +  CORE_ADDR stack_dest = sp;
> +  int argreg = E_R0_REGNUM;
> +  int i, j;
> +  int typelen, slacklen, alignlen;
> +  static const int zero_words[2] = { 0, 0 };
> +
> +  /* If struct_return is 1, then the struct return address will
> +     consume one argument-passing register.  */
> +  if (struct_return)
> +    {
> +      regcache_cooked_write_unsigned (regcache, argreg++, struct_addr);
> +    }

"{" and "}" is not needed.

> +
> +  /* Arguments are passed in R0 - R9, and as soon as an argument will not
> +     fit completely in the remaining registers, then it, and all remaining
> +     arguments, are put on the stack.  */
> +  for (i = 0; i < nargs && argreg <= E_R9_REGNUM; i++)
> +    {
> +      const char *val;
> +      typelen = TYPE_LENGTH (value_enclosing_type (args[i]));
> +
> +      if (typelen > (E_R9_REGNUM - argreg + 1) * tilegx_reg_size)
> +        break;
> +
> +      /* Put argument into registers wordwise.  */
> +      val = value_contents (args[i]);
> +      for (j = 0; j < typelen; j += tilegx_reg_size)
> +        {
> +          /* ISSUE: Why special handling (only) for "typelen = 4x + 1"?
> +             I am unable to induce any "typelen" values except 4 and
> 8.  */
> +          int n = (typelen - j == 1) ? 1 : tilegx_reg_size;
> +          ULONGEST w = extract_unsigned_integer(val + j, n, byte_order);
> +          regcache_cooked_write_unsigned (regcache, argreg++, w);
> +        }

I am wondering if this loop is necessary.  Usually, if the length of a
type exceeds 8, it will be passed by reference.  If your ABI is similar,
"typelen" should be less than or equal to 8, and this loop is not needed.

> +    }
> +
> +  /* Align SP.  */
> +  stack_dest = tilegx_frame_align (gdbarch, stack_dest);
> +
> +  /* Loop backwards through remaining arguments to determine stack
> alignment  */
           ^ Missing ".".

> +  alignlen = 0;
> +
> +  for (j = nargs - 1; j >= i; j--)
> +    {
> +      typelen = TYPE_LENGTH (value_enclosing_type (args[j]));
> +      alignlen += (typelen + 3) & (~3);
> +    }
> +
> +  if (alignlen & 0x4)
> +    stack_dest -= 4;
> +
> +  /* Loop backwards through remaining arguments and push them on the
> stack,
> +     wordaligned.  */
> +  for (j = nargs - 1; j >= i; j--)
> +    {
> +      char *val;
> +      typelen = TYPE_LENGTH (value_enclosing_type (args[j]));
> +      slacklen = ((typelen + 3) & (~3)) - typelen;
> +      val = alloca (typelen + slacklen);
> +      memcpy (val, value_contents (args[j]), typelen);
> +      memset (val + typelen, 0, slacklen);
> +
> +      /* Now write this data to the stack. The stack grows downwards.  */
> +      stack_dest -= typelen + slacklen;
> +      write_memory (stack_dest, val, typelen + slacklen);
> +    }
> +
> +  /* Add 2 words for linkage space to the stack.  */
> +  stack_dest = stack_dest - 8;
> +  write_memory (stack_dest, (void*)zero_words, 8);
> +
> +  /* Update stack pointer.  */
> +  regcache_cooked_write_unsigned (regcache, E_SP_REGNUM, stack_dest);
> +
> +  /* Set the return address register to point to the entry point of
> +     the program, where a breakpoint lies in wait.  */
> +  regcache_cooked_write_unsigned (regcache, E_LR_REGNUM, bp_addr);
> +
> +  return stack_dest;
> +}
> +
> +

> +
> +/* The epilogue is defined here as the area at the end of a function,
> +   either on the `ret' instruction itself or after an instruction which
> +   destroys the function's stack frame.  */
> +static int
> +tilegx_in_function_epilogue_p (struct gdbarch *gdbarch, CORE_ADDR pc)
> +{
> +  /* ISSUE: Who calls this function?  */
> +

It is called through gdbarch_in_function_epilogue_p.

> +  CORE_ADDR func_addr = 0, func_end = 0;
> +
> +  if (find_pc_partial_function (pc, NULL, &func_addr, &func_end))
> +    {
> +      ULONGEST inst, inst2;
> +      CORE_ADDR addr = func_end - TILEGX_BUNDLE_SIZE_IN_BYTES;
> +
> +      /* FIXME: Find the actual epilogue.  */
> +      /* HACK: Just assume the final bundle is the "ret" instruction".  */
> +      if (pc > addr)
> +        return 1;
> +    }
> +  return 0;
> +}
> +
> +
> +static const unsigned char *
> +tilegx_breakpoint_from_pc (struct gdbarch *gdbarch,
> +                           CORE_ADDR *pcptr, int *lenptr)
> +{
> +  static unsigned char breakpoint[sizeof(tilegx_bundle_bits)];
> +  static char breakpoint_initialized = 0;
> +  if (!breakpoint_initialized)
> +    {
> +      tilegx_bundle_bits bits = TILEGX_BPT_BUNDLE;
> +      unsigned int i;
> +      for (i = 0; i < sizeof(breakpoint); i++)
> +        {
> +          breakpoint[i] = (unsigned char)bits;
> +          bits >>= 8;
> +        }
> +      breakpoint_initialized = 1;

the content of breakpoint insn is unchanged.  You can hard-code it in code.

> +    }
> +  *lenptr = sizeof (breakpoint);
> +  return breakpoint;
> +}
> +
> +
> +/* ISSUE: Does this need an implementation?
> +   NOTE: This function is called by "handle_inferior_event", when stepping
> +   into a subroutine, but NOT a PLT stub (as that is handled
> specially).  */
> +static CORE_ADDR
> +tilegx_skip_trampoline_code (struct frame_info *frame, CORE_ADDR pc)
> +{
> +  return 0;
> +}

If this gdbarch hook is not implemented, generic_skip_trampoline_code
will be called, which returns 0 as well.  You don't have to implement it.

> +
> +
> +/* For now, we'll just do a direct translation, but soon we may
> + * include an ASID and/or tilegx field in addresses, which doesn't
> + * appear in pointers -- so we need to keep the translation
> + * function around.
> + */
> +
> +static CORE_ADDR
> +tilegx_pointer_to_address (struct gdbarch *gdbarch,
> +                           struct type *type, const gdb_byte *buf)
> +{
> +  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
> +  return extract_unsigned_integer (buf, TYPE_LENGTH (type), byte_order);
> +}

Your implementation is exactly the same as its default function
`unsigned_pointer_to_address', so you don't have to implement it.

> +
> +static void
> +tilegx_address_to_pointer (struct gdbarch *gdbarch,
> +                           struct type *type, gdb_byte *buf, CORE_ADDR
> addr)
> +{
> +  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
> +  store_unsigned_integer (buf, TYPE_LENGTH (type), byte_order, addr);
> +}

Likewise.

> +/* Function: tilegx_gdbarch_init
> +   Initializer function for the tilegx gdbarch vector.
> +   Called by gdbarch.  Sets up the gdbarch vector(s) for this target.  */
> +
> +static struct gdbarch *
> +tilegx_gdbarch_init (struct gdbarch_info info, struct gdbarch_list
> *arches)
> +{
> +  struct gdbarch *gdbarch;
> +  int arch_size = 64;
> +
> +  /* Handle arch_size == 32 or 64.  Default to 64.  */
> +  if (info.abfd)
> +    arch_size = bfd_get_arch_size(info.abfd);
> +
> +  if (arch_size != 32)
> +    arch_size = 64;
> +
> +  /* try to find a pre-existing architecture  */
> +  for (arches = gdbarch_list_lookup_by_info (arches, &info);
> +       arches != NULL;
> +       arches = gdbarch_list_lookup_by_info (arches->next, &info))
> +    {
> +      /* We only have two flavors -- just make sure arch_size matches.  */
> +      if (gdbarch_ptr_bit(arches->gdbarch) == arch_size)
> +        return (arches->gdbarch);
> +    }
> +
> +  gdbarch = gdbarch_alloc (&info, NULL);
> +
> +  /*
> +   * Basic register fields and methods, datatype sizes and stuff.
> +   */

Incorrect comment style.

> +
> +  /* There are 64 physical registers, which can be referenced by
> instructions
> +     (although only 56 of them can actually be debugged) and 1 magic
> register
> +     (the PC).  The other three magic registers (ex1, syscall, orig_r0)
> which
> +     are known to "ptrace" are ignored by "gdb".  Note that we simply
> pretend
> +     that there are 65 registers, and no "pseudo registers".  */
> +  set_gdbarch_num_regs (gdbarch, E_NUM_REGS);
> +  set_gdbarch_num_pseudo_regs (gdbarch, 0);
> +
> +  set_gdbarch_sp_regnum (gdbarch, E_SP_REGNUM);
> +  set_gdbarch_pc_regnum (gdbarch, E_PC_REGNUM);
> +
> +  set_gdbarch_register_name (gdbarch, tilegx_register_name);
> +  set_gdbarch_register_type (gdbarch, tilegx_register_type);
> +
> +  set_gdbarch_char_signed (gdbarch, 0);
> +  set_gdbarch_short_bit (gdbarch, 2 * TARGET_CHAR_BIT);
> +  set_gdbarch_int_bit (gdbarch, 4 * TARGET_CHAR_BIT);
> +  set_gdbarch_long_bit (gdbarch, arch_size);
> +  set_gdbarch_long_long_bit (gdbarch, 8 * TARGET_CHAR_BIT);
> +
> +  set_gdbarch_float_bit (gdbarch, 4 * TARGET_CHAR_BIT);
> +  set_gdbarch_double_bit (gdbarch, 8 * TARGET_CHAR_BIT);
> +  set_gdbarch_long_double_bit (gdbarch, 8 * TARGET_CHAR_BIT);
> +
> +  set_gdbarch_ptr_bit (gdbarch, arch_size);
> +  set_gdbarch_addr_bit (gdbarch, arch_size);
> +
> +  set_gdbarch_address_to_pointer (gdbarch, tilegx_address_to_pointer);
> +  set_gdbarch_pointer_to_address (gdbarch, tilegx_pointer_to_address);
> +
> +  set_gdbarch_cannot_fetch_register (gdbarch,
> tilegx_cannot_reference_register);
> +  set_gdbarch_cannot_store_register (gdbarch,
> tilegx_cannot_reference_register);
> +
> +  set_gdbarch_regset_from_core_section (gdbarch,
> +          tilegx_regset_from_core_section);
> +
> +  /* Stack grows down.  */
> +  set_gdbarch_inner_than (gdbarch, core_addr_lessthan);
> +
> +  /*
> +   * Frame Info
> +   */
> +  set_gdbarch_unwind_sp (gdbarch, tilegx_unwind_sp);
> +  set_gdbarch_unwind_pc (gdbarch, tilegx_unwind_pc);
> +  set_gdbarch_dummy_id (gdbarch, tilegx_unwind_dummy_id);
> +  set_gdbarch_frame_align (gdbarch, tilegx_frame_align);
> +  frame_base_set_default (gdbarch, &tilegx_frame_base);
> +
> +  set_gdbarch_skip_prologue (gdbarch, tilegx_skip_prologue);
> +
> +  set_gdbarch_in_function_epilogue_p (gdbarch,
> +              tilegx_in_function_epilogue_p);
                 ^^ Indentation problem.
> +
> +  /* Map debug registers into internal register numbers.  */
> +  set_gdbarch_dwarf2_reg_to_regnum (gdbarch, tilegx_dwarf2_reg_to_regnum);
> +
> +  /* These values and methods are used when gdb calls a target
> function.  */
> +  set_gdbarch_push_dummy_call (gdbarch, tilegx_push_dummy_call);
> +  set_gdbarch_breakpoint_from_pc (gdbarch, tilegx_breakpoint_from_pc);
> +  set_gdbarch_return_value (gdbarch, tilegx_return_value);
> +
> +  set_gdbarch_skip_trampoline_code (gdbarch, tilegx_skip_trampoline_code);
> +
> +  set_gdbarch_print_insn (gdbarch, print_insn_tilegx);
> +
> +  /* GNU/Linux uses SVR4-style shared libraries.  */
> +  if (arch_size == 32)
> +    set_solib_svr4_fetch_link_map_offsets
> +      (gdbarch, svr4_ilp32_fetch_link_map_offsets);
> +  else
> +    set_solib_svr4_fetch_link_map_offsets
> +      (gdbarch, svr4_lp64_fetch_link_map_offsets);
         ^^^ newline in the wrong place.

       set_solib_svr4_fetch_link_map_offsets (gdbarch,
					      svr4_lp64_fetch_link_map_offsets);

> +
> +  /* Enable TLS support.  */
> +  set_gdbarch_fetch_tls_load_module_address (gdbarch,
> +                                             svr4_fetch_objfile_link_map);
> +
> +  gdbarch_init_osabi (info, gdbarch);
> +
> +  dwarf2_append_unwinders(gdbarch);
> +  frame_unwind_append_unwinder (gdbarch, &tilegx_frame_unwind);
> +
> +  return gdbarch;
> +}
> +
> +static void
> +tilegx_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
> +{
> +  /* We may not need anything here, but I think we need the function.  */

Strctly speaking, this function should be renamed to
tilegx_linux_init_abi, and moved to tilegx-linux-tdep.c.

You have to call "linux_init_abi (info, gdbarch);" in this function.

> +}
> +
> +
> +/* Function: _initialize_tilegx_tdep
> +   Initializer function for the Tilera TILE-Gx chip.
> +   Called by gdb at start-up.  */
> +
> +extern initialize_file_ftype _initialize_tilegx_tdep; /*
> -Wmissing-prototypes  */
> +
> +void
> +_initialize_tilegx_tdep (void)
> +{
> +  /* tile-sim is so slow it needs a longer timeout.  */
> +  remote_timeout = 60;
> +
> +  register_gdbarch_init (bfd_arch_tilegx, tilegx_gdbarch_init);
> +
> +  gdbarch_register_osabi (bfd_arch_tilegx, 0, GDB_OSABI_LINUX,
> tilegx_init_abi);

This should be called in _initialize_tilegx_linux_tdep defined in
tilegx-linux-tdep.c.

Last but no least, it is great if you can post the gdb testsuite result.

-- 
Yao (éå)


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