diff mbox

[1/2] arc: New Synopsys ARC port

Message ID 1472633399-32477-1-git-send-email-Anton.Kolesov@synopsys.com
State Superseded
Headers show

Commit Message

Anton Kolesov Aug. 31, 2016, 8:49 a.m. UTC
ARC is a family of licensable processors developed by Synopsys.

This is an initial patch that doesn't yet support some of the features, that
are already available in Synopsys' fork of GDB, namely:

  * longjmp support
  * signal frame handling
  * prologue analysis
  * Linux targets support
  * native Linux support

ARC cores are configurable and extensible, which means from debugger
perspective that some registers and debug capabilities are optional, therefore
it is up to the GDB stub to determine exact list of register available on
target and supply it to GDB via XML target descriptions.  List of registers
that is known to GDB and is required is intentionally kept small to simplify
requirements to GDB stub and implementation of a GDB client.

Since it is expected that 7.12 will be the last release that supports building
GDB with C compiler (https://sourceware.org/ml/gdb/2016-05/msg00016.html),
this patch contain some code that cannot be compiled with older GCC's default
-std=gnu89 for C, in particular variables are declared at the first use,
rather then at the top of the function.

gdb/ChangeLog:

	* Makefile.in (ALL_TARGET_OBS): Add arc-tdep.o.
	(HFILES_NO_SRCDIR): Add arc-tdep.h.
	(ALLDEPFILES): Add arc-tdep.c.
	* NEWS: Mention new ARC port.
	* configure.tgt: Add ARC.
	* arc-tdep.c: New file.
	* arc-tdep.h: New file.
	* features/Makefile (XMLTOC): Add arc-v2.xml and arc-arcompact.xml.
	* features/arc-v2.xml: New file.
	* features/arc-v2.c: New file (generated).
	* features/arc-arcompact.xml: New file.
	* features/arc-arcompact.c: New file (generated).

gdb/doc/ChangeLog:

	* gdb.texinfo (Embedded Processors): Document ARC.
	(Synopsys ARC): New section.
	(Standard Target Features): Document ARC features.
	(ARC Features): New section.

gdb/testsuite/ChangeLog:

	* gdb.xml/tdesc-regs.exp: set core-regs for arc*-*-elf32.
---
 gdb/Makefile.in                      |    4 +-
 gdb/NEWS                             |    4 +
 gdb/arc-tdep.c                       | 1371 ++++++++++++++++++++++++++++++++++
 gdb/arc-tdep.h                       |  133 ++++
 gdb/configure.tgt                    |    5 +
 gdb/doc/gdb.texinfo                  |   89 +++
 gdb/features/Makefile                |    2 +
 gdb/features/arc-arcompact.c         |   77 ++
 gdb/features/arc-arcompact.xml       |   83 ++
 gdb/features/arc-v2.c                |   81 ++
 gdb/features/arc-v2.xml              |   90 +++
 gdb/testsuite/gdb.xml/tdesc-regs.exp |    4 +
 12 files changed, 1942 insertions(+), 1 deletion(-)
 create mode 100644 gdb/arc-tdep.c
 create mode 100644 gdb/arc-tdep.h
 create mode 100644 gdb/features/arc-arcompact.c
 create mode 100644 gdb/features/arc-arcompact.xml
 create mode 100644 gdb/features/arc-v2.c
 create mode 100644 gdb/features/arc-v2.xml

Comments

Pedro Alves Aug. 31, 2016, 1:58 p.m. UTC | #1
On 08/31/2016 09:49 AM, Anton Kolesov wrote:

> Since it is expected that 7.12 will be the last release that supports building
> GDB with C compiler (https://sourceware.org/ml/gdb/2016-05/msg00016.html),
> this patch contain some code that cannot be compiled with older GCC's default
> -std=gnu89 for C, in particular variables are declared at the first use,
> rather then at the top of the function.

This has not happened yet, and we still support building with a
C compiler.  So this can't go in as is until that is resolved...

In general the patch looks pretty good to me, but I have a few
comments below.

>  
>  * GDB and GDBserver now build with a C++ compiler by default.
> diff --git a/gdb/arc-tdep.c b/gdb/arc-tdep.c
> new file mode 100644
> index 0000000..6e0e68d
> --- /dev/null
> +++ b/gdb/arc-tdep.c
> @@ -0,0 +1,1371 @@
> +/* Target dependent code for ARC arhitecture, for GDB.
> +
> +   Copyright 2005-2016 Free Software Foundation, Inc.

Does this file really contain code that is that old?

> +   Contributed by Synopsys 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/>.  */
> +
> +/* GDB header files.  */
> +#include "defs.h"
> +#include "arch-utils.h"
> +#include "dwarf2-frame.h"
> +#include "frame-base.h"
> +#include "frame-unwind.h"
> +#include "gdbcore.h"
> +#include "gdbcmd.h"
> +#include "objfiles.h"
> +#include "trad-frame.h"
> +
> +/* ARC header files.  */
> +#include "opcode/arc.h"
> +#include "arc-tdep.h"
> +
> +/* Default target descriptions.  */
> +#include "features/arc-v2.c"
> +#include "features/arc-arcompact.c"
> +
> +/* The frame unwind cache for the ARC.  Current structure is a stub, because
> +   it should be filled in during the prologue analysis.  */
> +
> +struct arc_unwind_cache
> +  {

Formatting: { goes on column 0, and then reindent the fields as well.

> +    /* The stack pointer at the time this frame was created; i.e. the caller's
> +       stack pointer when this function was called.  It is used to identify this
> +       frame.  */
> +    CORE_ADDR prev_sp;
> +
> +    /* Offsets for each register in the stack frame.  */
> +    struct trad_frame_saved_reg *saved_regs;
> +  };
> +
> +/* Global debug flag.  */
> +
> +int arc_debug;
> +
> +/* XML target description features.  */
> +
> +static const char *const core_v2_feature_name = "org.gnu.gdb.arc.core.v2";
> +static const char *const
> +  core_reduced_v2_feature_name = "org.gnu.gdb.arc.core-reduced.v2";
> +static const char *const
> +  core_arcompact_feature_name = "org.gnu.gdb.arc.core.arcompact";
> +static const char *const
> +  aux_minimal_feature_name = "org.gnu.gdb.arc.aux-minimal";
> +

Do these need to be pointers instead of arrays?

> +
> +   In ARC PC register is a normal register so in most cases setting PC value
> +   is a straightforward process: debugger just writes PC value.  However it
> +   gets trickier in case when current instruction is an instruction in delay
> +   slot.  In this case CPU will execute instruction at current PC value, then
> +   will set PC to the current value of BTA register; also current instruction
> +   cannot be branch/jump and some of the other instruction types.  Thus if
> +   debugger would try to just change PC value in this case, this instruction
> +   will get executed, but then core will "jump" to the original branch target.
> +
> +   Whether current instruction is a delay-slot instruction or not is indicated
> +   by DE bit in STATUS32 register indicates if current instruction is a delay
> +   slot instruction.  This bit is writable by debug host, which allows debug
> +   host to prevent core from jumping after the delay slot instruction.  It
> +   also works in another direction: setting this bit will make core to treat
> +   any current instructions as a delay slot instruction and to set PC to the
> +   current value of BTA register.
> +
> +   To workaround issues with changing PC register while in delay slot
> +   instruction, debugger should check for the STATUS32.DE bit and reset it if
> +   it is set.  No other change is required in this function.  Most common
> +   case, where this function might be required is calling inferior functions
> +   from debugger.  Generic GDB logic handles this pretty well: current values
> +   of registers are stored, value of PC is changed (that is the job of this
> +   function), and after inferior function is executed, GDB restores all
> +   registers, include BTA and STATUS32, which also means that core is returned
> +   to its original state of being halted on delay slot instructions.
> +
> +   This method is useless for ARC 600, because it doesn't have externally
> +   exposed BTA register.  In the case of ARC 600 it is impossible to restore
> +   core to it's state in all occasions thus core should never be halted (from

"to its".

> +   the perspective of debugger host) in the delay slot.  */
> +


> +/* Push dummy call return code sequence.
> +
> +   We don't actually push any code.  We just identify where a breakpoint can
> +   be inserted to which we are can return and the resume address where we
> +   should be called.

Something seems odd with the last sentence.  "to which we are can"?

> +
> +   ARC does not necessarily have an executable stack, so we can't put the
> +   return breakpoint there.  

What actually goes wrong if you put the breakpoint there?  GDB should
manage to figure out a SIGSEGV at that address means the function
returned.  x86-64 GNU/Linux has no executable stack either, for instance,
and:

 (gdb) set debug infrun 1
 (gdb) p return_false ()
 infrun: clear_proceed_status_thread (Thread 0x7ffff7fc8700 (LWP 7333))
 infrun: proceed (addr=0x4008d7, signal=GDB_SIGNAL_0)
 infrun: proceed: resuming Thread 0x7ffff7fc8700 (LWP 7333)
 infrun: resume (step=0, signal=GDB_SIGNAL_0), trap_expected=0, current thread [Thread 0x7ffff7fc8700 (LWP 7333)] at 0x4008d7
 infrun: infrun_async(1)
 infrun: prepare_to_wait
 infrun: target_wait (-1.0.0, status) =
 infrun:   7333.7333.0 [Thread 0x7ffff7fc8700 (LWP 7333)],
 infrun:   status->kind = stopped, signal = GDB_SIGNAL_SEGV
 infrun: Treating signal as SIGTRAP
 infrun: TARGET_WAITKIND_STOPPED
 infrun: stop_pc = 0x7fffffffd75f
 infrun: BPSTAT_WHAT_STOP_SILENT
 infrun: stop_waiting
 infrun: clear_step_over_info
 infrun: stop_all_threads
 infrun: stop_all_threads, pass=0, iterations=0
 infrun:   Thread 0x7ffff7fc8700 (LWP 7333) not executing
 infrun: stop_all_threads, pass=1, iterations=1
 infrun:   Thread 0x7ffff7fc8700 (LWP 7333) not executing
 infrun: stop_all_threads done
 $1 = 0
 (gdb)

Note the GDB_SIGNAL_SEGV.

> Instead we put it at the entry point of the
> +   function.  This means the SP is unchanged.
> +
> +   SP is a current stack pointer FUNADDR is an address of the function to be
> +   called.  ARGS is arguments to pass.  NARGS is a number of args to pass.
> +   VALUE_TYPE is a type of value returned.  REAL_PC is a resume address when
> +   the function is called.  BP_ADDR is an address where breakpoint should be
> +   set.  Returns the updated stack pointer.  */
> +
> +static CORE_ADDR
> +arc_push_dummy_code (struct gdbarch *gdbarch, CORE_ADDR sp, CORE_ADDR funaddr,
> +		     struct value **args, int nargs, struct type *value_type,
> +		     CORE_ADDR *real_pc, CORE_ADDR *bp_addr,
> +		     struct regcache *regcache)
> +{
> +  *real_pc = funaddr;
> +  *bp_addr = entry_point_address ();
> +  return sp;
> +}
> +
> +/* Determine whether a register can be read.
> +
> +   All of the registers in the required set are readable.  There are
> +   write-only registers among the non-required, but since GDB doesn't know
> +   anything about them, access is controlled by the GDB server.  RESERVED and
> +   LIMM are non-existent registers.

What does this mean, "non-existent registers" ?  If they don't
exist, then why do they have names and are handled here?
I'm puzzled on what's the point of a register that can't be
neither read nor written?

> +
> +   Returns TRUE if we _cannot_ read the register.  */
> +
> +static int
> +arc_cannot_fetch_register (struct gdbarch *gdbarch, int regnum)
> +{
> +  /* Assume that register is readable if it is unknown.  */
> +  switch (regnum)
> +    {
> +    case ARC_RESERVED_REGNUM:
> +    case ARC_LIMM_REGNUM:
> +      return TRUE;
> +    default:
> +      return FALSE;
> +    }
> +}
> +
> +/* Determine whether a register can be written.
> +
> +   It is assumed that first 64 regnums are core registers, hence standard ARC
> +   rules for core registers apply and it is assumed that extension registers
> +   are RW.  Then PC has regnum 64, LP_START is 65, LP_END is 66, and STATUS32
> +   is 67.  All of those are RW in baremetal environment.
> +
> +   Returns TRUE if we _cannot_ write the register.  */
> +
> +static int
> +arc_cannot_store_register (struct gdbarch *gdbarch, int regnum)
> +{
> +  /* Assume that register is writable if it is unknown.  */
> +  switch (regnum)
> +    {
> +    case ARC_RESERVED_REGNUM:
> +    case ARC_LIMM_REGNUM:
> +    case ARC_PCL_REGNUM:
> +      return TRUE;
> +    default:
> +      return FALSE;
> +    }
> +}
> +
> +/* Get the return value of a function from the registers/memory used to
> +   return it, according to the convention used by the ABI - 4-bytes values are
> +   in the R0,

s/in the R0/in R0/ ?

>  while 8-byte values are in the R1.

ITYM "are in R0-R1".  The function seems to extract it out
of both.

> +
> +   TODO: This implementation ignores the case of "complex double", where
> +   according to ABI, value is returned in the R0-R3 registers.
> +
> +   TYPE is a returned value's type.  VALBUF is a buffer for the returned
> +   value.  */
> +
> +static void
> +arc_extract_return_value (struct gdbarch *gdbarch, struct type *type,
> +			  struct regcache *regcache, gdb_byte *valbuf)
> +{
> +  unsigned int len = TYPE_LENGTH (type);
> +
> +  if (arc_debug)
> +    debug_printf ("arc: extract_return_value\n");
> +
> +  if (len <= BYTES_IN_REGISTER)

Where is BYTES_IN_REGISTER defined?

> +    {
> +      ULONGEST val;
> +
> +      /* Get the return value from one register.  */
> +      regcache_cooked_read_unsigned (regcache, ARC_R0_REGNUM, &val);
> +      store_unsigned_integer (valbuf, (int) len,
> +			      gdbarch_byte_order (gdbarch), val);
> +
> +      if (arc_debug)
> +	debug_printf ("arc: returning 0x%s\n", phex (val, BYTES_IN_REGISTER));
> +    }
> +  else if (len <= BYTES_IN_REGISTER * 2)
> +    {
> +      ULONGEST low, high;
> +
> +      /* Get the return value from two registers.  */
> +      regcache_cooked_read_unsigned (regcache, ARC_R0_REGNUM, &low);
> +      regcache_cooked_read_unsigned (regcache, ARC_R1_REGNUM, &high);
> +
> +      store_unsigned_integer (valbuf, BYTES_IN_REGISTER,
> +			      gdbarch_byte_order (gdbarch), low);
> +      store_unsigned_integer (valbuf + BYTES_IN_REGISTER,
> +			      (int) len - BYTES_IN_REGISTER,
> +			      gdbarch_byte_order (gdbarch), high);
> +
> +      if (arc_debug)
> +	debug_printf ("arc: returning 0x%s%s\n",
> +		      phex (high, BYTES_IN_REGISTER),
> +		      phex (low, BYTES_IN_REGISTER));
> +    }
> +  else
> +    error (_("arc: extract_return_value: type length %u too large"), len);
> +}
> +
> +
> +/* Store the return value of a function into the registers/memory used to
> +   return it, according to the convention used by the ABI.
> +
> +   TODO: This implementation ignores the case of "complex double", where
> +   according to ABI, value is returned in the R0-R3 registers.
> +
> +   TYPE is a returned value's type.  VALBUF is a buffer with the value to
> +   return.  */
> +
> +static void
> +arc_store_return_value (struct gdbarch *gdbarch, struct type *type,
> +			struct regcache *regcache, const gdb_byte *valbuf)
> +{
> +  unsigned int len = TYPE_LENGTH (type);
> +
> +  if (arc_debug)
> +    debug_printf ("arc: store_return_value\n");
> +
> +  if (len <= BYTES_IN_REGISTER)
> +    {
> +      ULONGEST val;
> +
> +      /* Put the return value into one register.  */
> +      val = extract_unsigned_integer (valbuf, (int) len,
> +				      gdbarch_byte_order (gdbarch));
> +      regcache_cooked_write_unsigned (regcache, ARC_R0_REGNUM, val);
> +
> +      if (arc_debug)
> +	debug_printf ("arc: storing 0x%s\n", phex (val, BYTES_IN_REGISTER));
> +    }
> +  else if (len <= BYTES_IN_REGISTER * 2)
> +    {
> +      ULONGEST low, high;
> +
> +      /* Put the return value into  two registers.  */
> +      low = extract_unsigned_integer (valbuf, BYTES_IN_REGISTER,
> +				      gdbarch_byte_order (gdbarch));
> +      high = extract_unsigned_integer (valbuf + BYTES_IN_REGISTER,
> +				       (int) len - BYTES_IN_REGISTER,
> +				       gdbarch_byte_order (gdbarch));
> +
> +      regcache_cooked_write_unsigned (regcache, ARC_R0_REGNUM, low);
> +      regcache_cooked_write_unsigned (regcache, ARC_R1_REGNUM, high);
> +
> +      if (arc_debug)
> +	debug_printf ("arc: storing 0x%s%s\n",
> +		      phex (high, BYTES_IN_REGISTER),
> +		      phex (low, BYTES_IN_REGISTER));
> +    }
> +  else
> +    error (_("arc_store_return_value: type length too large."));
> +}
> +
> +/* Determine how a result of particular type is returned.
> +
> +   Return the convention used by the ABI for returning a result of the given
> +   type from a function; it may also be required to:
> +
> +   1. Set the return value (this is for the situation where the debugger user
> +      has issued a "return <value>" command to request immediate return from
> +      the current function with the given result; or

Parens opened but never closed.

> +
> +   2. Get the return value ((this is for the situation where the debugger
> +      user has executed a "call <function>" command to execute the specified
> +      function in the target program, and that function has a non-void result
> +      which must be returned to the user.  */

Spurious double-parens, and never closed.

These generic comments would be better placed in gdbarch.sh, and then
here just say:

/* Implement the XXX gdbarch method.  */

Augmented with mention of any ARC specifics if any worth mentioning.


> +
> +static enum return_value_convention
> +arc_return_value (struct gdbarch *gdbarch, struct value *function,
> +		  struct type *valtype, struct regcache *regcache,
> +		  gdb_byte *readbuf, const gdb_byte *writebuf)
> +{
> +  /* If the return type is a struct, or a union, or would occupy more than two
> +     registers, the ABI uses the "struct return convention": the calling
> +     function passes a hidden first parameter to the callee (in R0).  That
> +     parameter is the address at which the value being returned should be
> +     stored.  Otherwise, the result is returned in registers.  */
> +  int is_struct_return = (TYPE_CODE (valtype) == TYPE_CODE_STRUCT
> +			  || TYPE_CODE (valtype) == TYPE_CODE_UNION
> +			  || TYPE_LENGTH (valtype) > 2 * BYTES_IN_REGISTER);
> +
> +  if (arc_debug)
> +    debug_printf ("arc: return_value (readbuf = %p, writebuf = %p)\n",
> +		  readbuf, writebuf);
> +
> +  if (writebuf != NULL)
> +    {
> +      /* Case 1.  GDB should not ask us to set a struct return value: it
> +         should know the struct return location and write the value there
> +         itself.  */
> +      gdb_assert (!is_struct_return);
> +      arc_store_return_value (gdbarch, valtype, regcache, writebuf);
> +    }
> +  else if (readbuf != NULL)
> +    {
> +      /* Case 2.  GDB should not ask us to get a struct return value: it
> +         should know the struct return location and read the value from there
> +         itself.  */
> +      gdb_assert (!is_struct_return);
> +      arc_extract_return_value (gdbarch, valtype, regcache, readbuf);
> +    }
> +
> +  return is_struct_return ? RETURN_VALUE_STRUCT_CONVENTION
> +    : RETURN_VALUE_REGISTER_CONVENTION;

Needs to be wrapped in parens, so that the second line aligns properly
in emacs, per GNU convention:

  return (is_struct_return ? RETURN_VALUE_STRUCT_CONVENTION
	  : RETURN_VALUE_REGISTER_CONVENTION);

Or perhaps better:

  return (is_struct_return
	  ? RETURN_VALUE_STRUCT_CONVENTION
	  : RETURN_VALUE_REGISTER_CONVENTION);

> +}
> +
> +/* Utility function to create a new frame cache structure.  */
> +
> +static struct arc_unwind_cache *
> +arc_create_cache (struct frame_info *this_frame)
> +{
> +  struct arc_unwind_cache *cache
> +    = FRAME_OBSTACK_ZALLOC (struct arc_unwind_cache);
> +
> +  /* Zero all fields.  */

That's already done by ZALLOC.

> +  cache->prev_sp = 0;
> +
> +  /* Allocate space for saved register info.  */
> +  cache->saved_regs = trad_frame_alloc_saved_regs (this_frame);
> +
> +  return cache;
> +}
> +
> +/* Return the base address of the frame.  For ARC, the base address is the
> +   frame pointer.  */
> +
> +static CORE_ADDR
> +arc_frame_base_address (struct frame_info *this_frame, void **prologue_cache)
> +{
> +  return (CORE_ADDR) get_frame_register_unsigned (this_frame, ARC_FP_REGNUM);
> +}
> +
> +/* Skip the prologue for the function at PC.  This is done by checking from
> +   the line information read from the DWARF, if possible; otherwise, we scan
> +   the function prologue to find its end.  */
> +
> +static CORE_ADDR
> +arc_skip_prologue (struct gdbarch *gdbarch, CORE_ADDR pc)
> +{
> +  if (arc_debug)
> +    debug_printf ("arc: skip_prologue\n");
> +
> +  CORE_ADDR func_addr;
> +  const char *func_name;
> +
> +  /* See what the symbol table says.  */
> +  if (find_pc_partial_function (pc, &func_name, &func_addr, NULL))
> +    {
> +      /* Found a function.  */
> +      CORE_ADDR postprologue_pc
> +	= skip_prologue_using_sal (gdbarch, func_addr);
> +
> +      if (postprologue_pc)
> +	return max (pc, postprologue_pc);
> +    }
> +
> +  /* No prologue info in symbol table, have to analyze prologue.  */
> +
> +  /* Find an upper limit on the function prologue using the debug
> +     information.  If the debug information could not be used to provide that
> +     bound, then pass 0 and arc_scan_prologue will estimate value itself.  */
> +  CORE_ADDR limit_pc = skip_prologue_using_sal (gdbarch, pc);
> +  /* We don't have a proper analyze_prologue function yet, but its result
> +     should be returned here.  Currently GDB will just stop at the first
> +     instruction of function if debug information doesn't have prologue info;
> +     and if there is a debug info about prologue - this code path will not be
> +     taken at all.  */
> +  return (limit_pc == 0 ? pc : limit_pc);
> +}
> +
> +/* arc_get_disassembler () may return different functions depending on bfd
> +   type, so it is not possible to pass print_insn directly to
> +   set_gdbarch_print_insn ().  Instead this wrapper function is used.  It also
> +   may be used by other functions to get disassemble_info for address.  It is
> +   important to note, that those print_insn from opcodes always print
> +   instruction to the stream specified in the info, so if that is not desired,
> +   then print_insn in the info should be set to some function that doesn't
> +   print anything, like arc_scream_into_the_void from this file.  */
> +
> +static int
> +arc_delayed_print_insn (bfd_vma addr, struct disassemble_info *info)
> +{
> +  int (*print_insn) (bfd_vma, struct disassemble_info *);
> +  print_insn = arc_get_disassembler (exec_bfd);

Seems like this will crash if there's no exec_bfd ?

> +  gdb_assert (print_insn != NULL);
> +  return print_insn (addr, info);
> +}
> +
> +/** Baremetal breakpoint instructions.

"/*"


> +
> +   ARC supports both big- and little-endian.  However, instructions for
> +   little-endian processors are encoded in the middle-endian: half-words are
> +   in big-endian, while bytes inside the half-words are in little-endian; data
> +   is represented in the "normal" little-endian.  Big-endian processors treat
> +   data and code identically.
> +
> +   Assuming the number 0x01020304, it will be presented this way:
> +
> +   Address            :  N   N+1  N+2  N+3
> +   little-endian      : 0x04 0x03 0x02 0x01
> +   big-endian         : 0x01 0x02 0x03 0x04
> +   ARC middle-endian  : 0x02 0x01 0x04 0x03
> +  */
> +
> +static const unsigned char arc_brk_s_be[] = { 0x7f, 0xff };
> +static const unsigned char arc_brk_s_le[] = { 0xff, 0x7f };
> +static const unsigned char arc_brk_be[] = { 0x25, 0x6f, 0x00, 0x3f };
> +static const unsigned char arc_brk_le[] = { 0x6f, 0x25, 0x3f, 0x00 };

Use gdb_byte.

> +
> +/* Get breakpoint which is appropriate for address at which it is to be set.
> +
> +   For ARC ELF, breakpoint uses the 16-bit BRK_S instruction, which is 0x7fff
> +   (little endian) or 0xff7f (big endian).  We used to insert BRK_S even
> +   instead of 32-bit instructions, which works mostly ok, unless breakpoint is
> +   inserted into delay slot instruction.  In this case if branch is taken
> +   BLINK value will be set to address of instruction after delay slot, however
> +   if we replaced 32-bit instruction in delay slot with 16-bit long BRK_S,
> +   then BLINK value will have an invalid value - it will point to the address
> +   after the BRK_S (which was there at the moment of branch execution) while
> +   it should point to the address after the 32-bit long instruction.  To avoid
> +   such issues this function disassembles instruction at target location and
> +   evaluates it value.
> +
> +   ARC 600 supports only 16-bit BRK_S.
> +
> +   NB: Baremetal GDB uses BRK[_S], while user-space GDB uses TRAP_S.  BRK[_S]
> +   is much better because it doesn't commit unlike TRAP_S, so it can be set in
> +   delay slots; however it cannot be used in user-mode, hence usage of TRAP_S
> +   in GDB for user-space.
> +
> +   PCPTR is a pointer to the PC where we want to place a breakpoint.  LENPTR
> +   is a number of bytes used by the breakpoint.  Returns the byte sequence of
> +   a breakpoint instruction.  */
> +
> +static const unsigned char *
> +arc_breakpoint_from_pc (struct gdbarch *gdbarch, CORE_ADDR *pcptr,
> +			int *lenptr)
> +{
> +  struct disassemble_info di;
> +  arc_initialize_disassembler (gdbarch, &di);
> +  size_t length_with_limm = arc_delayed_print_insn (*pcptr, &di);

Seems to me that if you need to do this instead of being able 
to use gdb_insn_length, then that leaves gdb_insn_length calls in
common code broken.  So, doesn't gdb_insn_length work here?  Why not?

> +
> +  /* Replace 16-bit instruction with BRK_S, replace 32-bit instructions with
> +     BRK.  LIMM is part of instruction length, so it can be either 4 or 8
> +     bytes.  */
> +  if ((length_with_limm == 4 || length_with_limm == 8)
> +      && !arc_mach_is_arc600 (gdbarch))
> +    {
> +      *lenptr = sizeof (arc_brk_le);
> +      return (gdbarch_byte_order (gdbarch) == BFD_ENDIAN_BIG)
> +	? arc_brk_be : arc_brk_le;

Wrap in parens, realign.

> +    }
> +  else
> +    {
> +      *lenptr = sizeof (arc_brk_s_le);
> +      return (gdbarch_byte_order (gdbarch) == BFD_ENDIAN_BIG)
> +	? arc_brk_s_be : arc_brk_s_le;

Wrap in parens, realign.

> +    }
> +}


> +
> +/* Frame unwinder for normal frames.  */
> +
> +static struct arc_unwind_cache *
> +arc_frame_cache (struct frame_info *this_frame)
> +{
> +  if (arc_debug)
> +    debug_printf ("arc: frame_cache\n");
> +
> +  struct gdbarch *gdbarch = get_frame_arch (this_frame);
> +  struct arc_unwind_cache *cache = arc_create_cache (this_frame);
> +
> +  CORE_ADDR block_addr = get_frame_address_in_block (this_frame);
> +  CORE_ADDR prev_pc = get_frame_pc (this_frame);
> +
> +  CORE_ADDR entrypoint, prologue_end;
> +  if (find_pc_partial_function (block_addr, NULL, &entrypoint, &prologue_end))
> +    {
> +      struct symtab_and_line sal = find_pc_line (entrypoint, 0);
> +      if (!sal.line)

Not a boolean, so:

      if (sal.line == 0)


> +	/* No line info so use current PC.  */
> +	prologue_end = prev_pc;
> +      else if (sal.end < prologue_end)
> +	/* The next line begins after the function end.  */
> +	prologue_end = sal.end;
> +
> +      prologue_end = min (prologue_end, prev_pc);
> +    }

Indentation look odd here.


> +
> +/* Unwind a register from a normal frame.  */
> +
> +static struct value *
> +arc_frame_prev_register (struct frame_info *this_frame,
> +			 void **this_cache, int regnum)
> +{
> +  if (arc_debug)
> +    debug_printf ("arc: frame_prev_register (regnum = %d)\n", regnum);
> +
> +  if (*this_cache == NULL)
> +    *this_cache = arc_frame_cache (this_frame);
> +  struct arc_unwind_cache *cache = (struct arc_unwind_cache *) (*this_cache);
> +
> +  struct gdbarch *gdbarch = get_frame_arch (this_frame);
> +
> +  /* If we are asked to unwind the PC, then we need to return BLINK instead:
> +     the saved value of PC points into this frame's function's prologue, not
> +     the next frame's function's resume location.  `frame_unwind_got_constant`
> +     is used to return non_lvalue, because it is not possible to modify PC

s/non_lvalue/not_lval

> +     directly (one can modify BLINK, if they wish).  If
> +     trad_frame_get_prev_register (ARC_BLINK_REGNUM) would be used here, than
> +     it would be possible to modify PC of frame via altering BLINK of
> +     next_frame, or at least I believe that should be possible technically,
> +     however sematically could be confusing, so better to avoid this, like
> +     other arches do.  */

What other archs do this for this purpose?

> +  if (regnum == gdbarch_pc_regnum (gdbarch))
> +    {
> +      CORE_ADDR blink;
> +      blink = frame_unwind_register_unsigned (this_frame, ARC_BLINK_REGNUM);
> +      return frame_unwind_got_constant (this_frame, regnum, blink);
> +    }
> +


> +
> +/* Identify some properties of particular registers for the DWARF unwinder.
> +   */

Formatting.

> +/* Initialize target description for the ARC.
> +
> +   Returns TRUE if input tdesc was valid and in this case it will assign TDESC
> +   and TDESC_DATA output parameters.  */
> +
> +static int
> +arc_tdesc_init (struct gdbarch_info info, const struct target_desc **tdesc,
> +		struct tdesc_arch_data **tdesc_data)
> +{
> +  if (arc_debug)
> +    debug_printf ("arc: Arhitecture initialization.\n");

Typo.

> +
> +  const struct target_desc *tdesc_loc = info.target_desc;
> +
> +  /* Depending on whether this is ARCompact or ARCv2 we will assigned
> +     different default registers sets (which will differ in exactly two core
> +     registers).  GDB will also refuse to accept register feature from invalid
> +     ISA - v2 features can be used only with v2 ARChitecture.  registers of
> +     invalid architecture.  We read bfd_arch_info, which looks like to be a
> +     safe bet here, as it looks like it is always initialized even when we
> +     don't pass any elf file to GDB at all (it uses default arch in this
> +     case).  Also GDB will call this function multiple times, and if XML
> +     target description file contains architecture specifications, then GDB
> +     will set this architecture to info.bfd_arch_info, overriding value from
> +     ELF file if they are different.  That means that, where matters, this
> +     value is always our best guess on what CPU we are debugging.  It has been
> +     noted that that architecture specified in tdesc file has higher
> +     precedence over ELF and even "set architecture" - that is, using "set
> +     architecture" command will have no effect when tdesc has "arch" tag.  */
> +  /* Cannot use arc_mach_is_arcv2 (), because gdbarch is not created yet.  */

Typo "we will assigned".  Uppercase "registers of invalid architecture" sentence
seems stray.

> +  const int is_arcv2 = (info.bfd_arch_info->mach == bfd_mach_arc_arcv2);
> +  int is_reduced_rf;
> +  const char *const *core_regs;
> +  const char *core_feature_name;
> +
> +  /* If target doesn't provide a description - use default one.  */
> +  if (!tdesc_has_registers (tdesc_loc))
> +    {
> +      if (is_arcv2)
> +	{
> +	  tdesc_loc = tdesc_arc_v2;
> +	  if (arc_debug)
> +	    debug_printf ("arc: Using default register set for ARC v2.\n");
> +	}
> +      else
> +	{
> +	  tdesc_loc = tdesc_arc_arcompact;
> +	  if (arc_debug)
> +	    debug_printf ("arc: Using default register set for ARCompact.\n");
> +	}
> +    }
> +  else
> +    {
> +      if (arc_debug)
> +	debug_printf ("arc: Using provided register set.\n");
> +    }
> +  gdb_assert (tdesc_loc);

No implicit boolean coercion, write:

  gdb_assert (tdesc_loc != NULL);

throughout.

> +
> +  /** Now we can search for base registers.  Core registers can be either full
> +     or reduced.  Summary:

Use "/*".

> +
> +     - core.v2 + aux-minimal
> +     - core-reduced.v2 + aux-minimal
> +     - core.arcompact + aux-minimal
> +
> +     NB: It is intirely feaseble to have ARCompact with reduced

"feasible"

> +     core regs, but we ignore that because GCC doesn't support that and at
> +     the same time this architecture version is obsolete.  */

What is obsolete?  arcompact, or arcompact with reduced core regs?
Please clarify the comment.

> +  const struct tdesc_feature *feature
> +    = tdesc_find_feature (tdesc_loc, core_v2_feature_name);
> +  if (feature)

feature != NULL.  Here and multiple other places.

> +    {


> +
> +  struct tdesc_arch_data *tdesc_data_loc = tdesc_data_alloc ();
> +
> +  gdb_assert (feature);
> +  int valid_p = 1;
> +
> +  for (size_t i = ARC_FIRST_CORE_REGNUM; i < ARC_LAST_CORE_REGNUM; i++)
> +    {
> +      /* R61 and R62 do never exist in real hardware, however have to check for
> +         them to preserve compatibility with old gdbserver which reserve a slot
> +         for them in g/G-packets.  */
> +
> +      /* If rf16, then skip extra registers.  */
> +      if (is_reduced_rf && ((i >= ARC_R4_REGNUM && i <= ARC_R9_REGNUM)
> +			    || (i >= ARC_R16_REGNUM && i <= ARC_R25_REGNUM)))
> +	continue;
> +
> +      /* This subtraction is superflouos, because FIRST_CORE_REGNUM is 0, but

"superfluous"

> +         OTOH it adds a level of abstraction between arrays and regnums.  Not
> +         sure if that is a good idea.  */
> +      const char *name = core_regs[i - ARC_FIRST_CORE_REGNUM];

It's common to just assume first register is 0.  But just pick one or the
other way, and drop the comment, which ends up just being a distraction.

> +
> +      valid_p = tdesc_numbered_register (feature, tdesc_data_loc, i, name);
> +
> +      /* - Ignore errors in extension registers - they are optional.
> +         - Ignore missing ILINK because it doesn't make sense for Linux.
> +         - Ignore missing ILINK2 when architecture is ARCompact, because it
> +         doesn't make sense for Linux targets.
> +
> +         In theory those optional registers should be in separate features,
> +         but that's just crazy - features would be tiny and numerous and would
> +         be make for complex maintanence, especially since regnums of
> +         different features would interleve.  */
> +      if (!valid_p && (i <= ARC_SP_REGNUM || i == ARC_BLINK_REGNUM
> +		       || i == ARC_LP_COUNT_REGNUM || i == ARC_PCL_REGNUM
> +		       || (i == ARC_R30_REGNUM && is_arcv2)))
> +	{
> +	  arc_print (_("Error: Cannot find required register `%s' "
> +		       "in feature `%s'.\n"), name, core_feature_name);
> +	  tdesc_data_cleanup (tdesc_data_loc);
> +	  return FALSE;
> +	}
> +    }
> +
> +  /* Mandatory AUX registeres are intentionally few and are common between
> +     ARCompact and ARC v2, so same code can be used for both.  */
> +  feature = tdesc_find_feature (tdesc_loc, aux_minimal_feature_name);
> +  if (!feature)

feature == NULL

> +    {
> +      arc_print (_("Error: Cannot find required feature `%s' in supplied "
> +		   "target description.\n"), aux_minimal_feature_name);
> +      tdesc_data_cleanup (tdesc_data_loc);
> +      return FALSE;
> +    }
> +
> +  for (size_t i = ARC_FIRST_AUX_REGNUM; i < ARC_LAST_AUX_REGNUM; i++)
> +    {
> +      const char *name = aux_minimal_register_names[i - ARC_FIRST_AUX_REGNUM];
> +      valid_p = tdesc_numbered_register (feature, tdesc_data_loc, i, name);
> +      /* LP registers are optional.  */
> +      if (!valid_p && (i != ARC_LP_START_REGNUM) && (i != ARC_LP_END_REGNUM))

Drop unnecessary parens.

> +	{
> +	  arc_print (_("Error: Cannot find required register `%s' "
> +		       "in feature `%s'.\n"),
> +		     name, tdesc_feature_name (feature));
> +	  tdesc_data_cleanup (tdesc_data_loc);
> +	  return FALSE;
> +	}
> +    }
> +
> +  *tdesc = tdesc_loc;
> +  *tdesc_data = tdesc_data_loc;
> +
> +  return TRUE;
> +}
> +
> +static struct gdbarch *
> +arc_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
> +{
> +  const struct target_desc *tdesc;
> +  struct tdesc_arch_data *tdesc_data;
> +  if (!arc_tdesc_init (info, &tdesc, &tdesc_data))
> +    return NULL;
> +
> +  /* Allocate the ARC-private target-dependent information structure, and the
> +     GDB target-independent information structure.  */
> +  struct gdbarch_tdep *tdep = XCNEW (struct gdbarch_tdep);
> +  struct gdbarch *gdbarch = gdbarch_alloc (&info, tdep);
> +
> +  /* Data types.  */
> +  set_gdbarch_short_bit (gdbarch, 16);
> +  set_gdbarch_int_bit (gdbarch, 32);
> +  set_gdbarch_long_bit (gdbarch, 32);
> +  set_gdbarch_long_long_bit (gdbarch, 64);
> +  set_gdbarch_long_long_align_bit (gdbarch, 32);
> +  set_gdbarch_float_bit (gdbarch, 32);
> +  set_gdbarch_float_format (gdbarch, floatformats_ieee_single);
> +  set_gdbarch_double_bit (gdbarch, 64);
> +  set_gdbarch_double_format (gdbarch, floatformats_ieee_double);
> +  set_gdbarch_ptr_bit (gdbarch, 32);
> +  set_gdbarch_addr_bit (gdbarch, 32);
> +  set_gdbarch_char_signed (gdbarch, 0);
> +
> +  set_gdbarch_write_pc (gdbarch, arc_write_pc);
> +
> +  set_gdbarch_virtual_frame_pointer (gdbarch, arc_virtual_frame_pointer);
> +
> +  /* tdesc_use_registers expects gdbarch_num_regs to return number of registers
> +     parsed by gdbarch_init, and then it will add all of the remaining
> +     registers and will increase number of registers.  */
> +  set_gdbarch_num_regs (gdbarch, ARC_LAST_REGNUM + 1);
> +  set_gdbarch_num_pseudo_regs (gdbarch, 0);
> +  set_gdbarch_sp_regnum (gdbarch, ARC_SP_REGNUM);
> +  set_gdbarch_pc_regnum (gdbarch, ARC_PC_REGNUM);
> +  set_gdbarch_ps_regnum (gdbarch, ARC_STATUS32_REGNUM);
> +  set_gdbarch_fp0_regnum (gdbarch, -1);	/* No FPU registers.  */
> +
> +  set_gdbarch_dummy_id (gdbarch, arc_dummy_id);
> +  set_gdbarch_push_dummy_call (gdbarch, arc_push_dummy_call);
> +  set_gdbarch_push_dummy_code (gdbarch, arc_push_dummy_code);
> +
> +  set_gdbarch_cannot_fetch_register (gdbarch, arc_cannot_fetch_register);
> +  set_gdbarch_cannot_store_register (gdbarch, arc_cannot_store_register);
> +
> +  set_gdbarch_believe_pcc_promotion (gdbarch, 1);
> +
> +  set_gdbarch_return_value (gdbarch, arc_return_value);
> +
> +  set_gdbarch_skip_prologue (gdbarch, arc_skip_prologue);
> +  set_gdbarch_inner_than (gdbarch, core_addr_lessthan);
> +
> +  set_gdbarch_breakpoint_from_pc (gdbarch, arc_breakpoint_from_pc);
> +
> +  /* On ARC 600 BRK_S instruction advances PC, unlike other ARC cores.  */
> +  if (!arc_mach_is_arc600 (gdbarch))
> +    set_gdbarch_decr_pc_after_break (gdbarch, 0);
> +  else
> +    set_gdbarch_decr_pc_after_break (gdbarch, 2);
> +
> +  set_gdbarch_unwind_pc (gdbarch, arc_unwind_pc);
> +  set_gdbarch_unwind_sp (gdbarch, arc_unwind_sp);
> +
> +  set_gdbarch_frame_align (gdbarch, arc_frame_align);
> +
> +  set_gdbarch_print_insn (gdbarch, arc_delayed_print_insn);
> +
> +  set_gdbarch_cannot_step_breakpoint (gdbarch, 1);
> +
> +  /* "nonsteppable" wachpoint means that watchpoint triggers before

"watchpoint"

> +     instruction is committed, therefore it is required to remove watchpoint
> +     to step though instruction that triggers it.  ARC watchpoints trigger
> +     only after instruction is committed, thus there is no need to remove
> +     them.  In fact on ARC watchpoint for memory writes may trigger with more
> +     significant delay, like one or two instructions, depending on type of
> +     memory where write is performed (CCM or external) and next instruction
> +     after the memory write.  That value used to be set to "1", but that is
> +     not correct for ARC - that was causing GDB to step one additional
> +     instruction after watchpoint is triggered and that is not needed.  */

I'd drop the last sentence talking about what the value used to be set to.

> +  set_gdbarch_have_nonsteppable_watchpoint (gdbarch, 0);
> +
> +  /* This doesn't include possible long-immediate value.  */
> +  set_gdbarch_max_insn_length (gdbarch, 4);
> +
> +  /* Frame unwinders and sniffers.  */
> +  dwarf2_frame_set_init_reg (gdbarch, arc_dwarf2_frame_init_reg);
> +  dwarf2_append_unwinders (gdbarch);
> +  frame_unwind_append_unwinder (gdbarch, &arc_frame_unwind);
> +  frame_base_set_default (gdbarch, &arc_normal_base);
> +
> +  /* Setup stuff specific to a particular environment (baremetal or Linux).
> +     It can override functions set earlier.  */
> +  gdbarch_init_osabi (info, gdbarch);
> +
> +  tdesc_use_registers (gdbarch, tdesc, tdesc_data);
> +
> +  return gdbarch;
> +}
> +
> +
> +static void
> +arc_dump_tdep (struct gdbarch *gdbarch, struct ui_file *file)
> +{
> +  /* Empty for now.  */
> +}
> +
> +/* Dummy printf-like function.  Used by arc_initialize_disassembler, see
> +   explanation of its purpose there.  */
> +
> +static int
> +arc_scream_into_the_void (void *file, const char *f, ...)
> +{
> +  return 0;
> +}
> +
> +/* Wrapper for the target_read_memory function.  */
> +
> +static int
> +arc_read_memory_for_disassembler (bfd_vma memaddr, bfd_byte *myaddr,
> +				  unsigned int length,
> +				  struct disassemble_info *info)
> +{
> +  return target_read_memory ((CORE_ADDR) memaddr, (gdb_byte *) myaddr,
> +			     (int) length);
> +}
> +
> +
> +void
> +arc_initialize_disassembler (struct gdbarch *gdbarch,
> +			     struct disassemble_info *info)

This isn't used outside this .c file in this patch, so make it
static.  But see comment above about gdb_insn_length.

> +{
> +  /* opcodes disassembler will always print instruction to the stream, which
> +     is not desired when instruction is disassembled for analysis.  Stream
> +     cannot be set to NULL, because it is used unconditionally in the opcodes,
> +     so the only way is to provide dummy printf-like function which will not
> +     actually print anything anywhere.  */
> +  init_disassemble_info (info, gdb_stderr, arc_scream_into_the_void);
> +  info->arch = gdbarch_bfd_arch_info (gdbarch)->arch;
> +  info->mach = gdbarch_bfd_arch_info (gdbarch)->mach;
> +  info->endian = gdbarch_byte_order (gdbarch);
> +  info->read_memory_func = arc_read_memory_for_disassembler;
> +}
> +


> new file mode 100644
> index 0000000..9e47167
> --- /dev/null
> +++ b/gdb/arc-tdep.h
> @@ -0,0 +1,133 @@
> +/* Target dependent code for ARC arhitecture, for GDB.
> +
> +   Copyright 2005-2016 Free Software Foundation, Inc.

Same comment as before.

> +   Contributed by Synopsys 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 ARC_TDEP_H
> +#define ARC_TDEP_H
> +
> +/* Need disassemble_info.  */
> +#include "dis-asm.h"
> +
> +/* Certain limitations are imposed on GDB register numbers to simplify logic
> +   in the GDB.  Those limitations apply to both ARCompact and ARC v2.
> +
> +   1.  ARC can have up to 64 core registers, and each one of them has same GDB
> +   regnum as an architectural number.  So R0 always has regnum 0, LP_COUNT
> +   always has regnum 60.
> +
> +   2.  PC always has regnum 64.  That register is mandatory.
> +
> +   3.  LP_START and LP_END has regnums 65 and 66 respectively, those registers
> +   are optional, and those register numbers shall not be occupied by other
> +   registers when LP_START and LP_END are not present.
> +
> +   4.  STATUS32 always has regnum 67.  */
> +
> +enum arc_regnum
> +  {
> +    /* Core registers.  */
> +    ARC_R0_REGNUM = 0,
> +    ARC_FIRST_CORE_REGNUM = ARC_R0_REGNUM,
> +    ARC_R1_REGNUM = 1,
> +    ARC_R4_REGNUM = 4,
> +    ARC_R7_REGNUM = 7,
> +    ARC_R9_REGNUM = 9,
> +    ARC_R13_REGNUM = 13,
> +    ARC_R16_REGNUM = 16,
> +    ARC_R25_REGNUM = 25,
> +    /* Global data pointer.  */
> +    ARC_GP_REGNUM,
> +    /* Frame pointer.  */
> +    ARC_FP_REGNUM,
> +    /* Stack pointer.  */
> +    ARC_SP_REGNUM,
> +    /* Return address from interrupt.  */
> +    ARC_ILINK_REGNUM,
> +    ARC_R30_REGNUM,
> +    /* Return address from function.  */
> +    ARC_BLINK_REGNUM,
> +    /* Zero-delay loop counter.  */
> +    ARC_LP_COUNT_REGNUM = 60,
> +    /* Reserved.  */
> +    ARC_RESERVED_REGNUM,
> +    /* Long immediate value (not a physical register.  */
> +    ARC_LIMM_REGNUM,
> +    /* Program counter, aligned to 4-bytes, read-only.  */
> +    ARC_PCL_REGNUM,
> +    ARC_LAST_CORE_REGNUM = ARC_PCL_REGNUM,
> +    /* AUX registers.  */
> +    /* Actual program counter.  */
> +    ARC_PC_REGNUM,
> +    ARC_FIRST_AUX_REGNUM = ARC_PC_REGNUM,
> +    /* Zero-delay loop start instruction.  */
> +    ARC_LP_START_REGNUM,
> +    /* Zero-delay loop next-after-last instruction.  */
> +    ARC_LP_END_REGNUM,
> +    /* Status register.  */
> +    ARC_STATUS32_REGNUM,
> +    ARC_LAST_REGNUM = ARC_STATUS32_REGNUM,
> +    ARC_LAST_AUX_REGNUM = ARC_STATUS32_REGNUM,
> +
> +    /* Additional ABI constants.  */
> +    ARC_FIRST_ARG_REGNUM = ARC_R0_REGNUM,
> +    ARC_LAST_ARG_REGNUM = ARC_R7_REGNUM,
> +    ARC_FIRST_CALLEE_SAVED_REGNUM = ARC_R13_REGNUM,
> +    ARC_LAST_CALLEE_SAVED_REGNUM = ARC_R25_REGNUM,
> +  };
> +
> +#define BYTES_IN_REGISTER  4
> +
> +#define arc_print(fmt, args...) fprintf_unfiltered (gdb_stdlog, fmt, ##args)
> +
> +extern int arc_debug;
> +
> +/* Target-dependent information.  */
> +
> +struct gdbarch_tdep
> +  {
> +  };

Formatting is off.  { should on column 0.

Empty structs with no fields is not valid C89 either.

How about just simply dropping these placeholders, and add
them back when they're actually needed?

> +
> +
> +void _initialize_arc_tdep (void);

The _initialize functions are special.  Move this to just above
the definition, and add a -Wmissing-prototypes comment.  You'll
find numerous examples in the tree.


>  @node ARM
>  @subsection ARM
>  
> @@ -40907,6 +40932,7 @@ registers using the capitalization used in the description.
>  
>  @menu
>  * AArch64 Features::
> +* ARC Features::
>  * ARM Features::
>  * i386 Features::
>  * MicroBlaze Features::
> @@ -40932,6 +40958,69 @@ The @samp{org.gnu.gdb.aarch64.fpu} feature is optional.  If present,
>  it should contain registers @samp{v0} through @samp{v31}, @samp{fpsr},
>  and @samp{fpcr}.
>  
> +@node ARC Features
> +@subsection ARC Features
> +@cindex target descriptions, ARC Features
> +
> +ARC processors are highly configurable, so even core registers and their amount

Registers can be counted, so s/amount/number/

> +are not completely predetermined.  In addition flags and PC registers which are

s/are/is/.

> +important to GDB are not "core" registers in ARC.  To simplify internal GDB
> +design some requirements are applied to ARC target descriptions:
> +
> +@itemize @bullet
> +@item
> +One of the core registers features must be present.  At all occasions GDB
> +register number must be equal to architectural number of that register, so for
> +example regnum of @samp{blink} is always 31.
> +
> +@item @samp{org.gnu.gdb.arc.aux-minimal} is mandatory and registers in it has
> +fixed GDB register numbers.
> +
> +@item All other features are optional and cannot have registers with register
> +number less then 68.

GDB should be mapping the tdesc numbers to internal numbers, so I don't
understand why do the register numbers matter in the target description at all?
It should only matter for the fallback descriptions built in gdb, in
order to match the layout of the g/G packets of remote servers that 
don't send in tdescs over the wire, I believe? 

Thanks,
Pedro Alves
Anton Kolesov Sept. 1, 2016, 7:54 p.m. UTC | #2
Hi Pedro,

Thank you for your review.

> 
> > Since it is expected that 7.12 will be the last release that supports building
> > GDB with C compiler (https://sourceware.org/ml/gdb/2016-05/msg00016.html),
> > this patch contain some code that cannot be compiled with older GCC's default
> > -std=gnu89 for C, in particular variables are declared at the first use,
> > rather then at the top of the function.
> 
> This has not happened yet, and we still support building with a
> C compiler.  So this can't go in as is until that is resolved...

Will be fixed.


> 
> In general the patch looks pretty good to me, but I have a few
> comments below.
> 
> >
> >  * GDB and GDBserver now build with a C++ compiler by default.
> > diff --git a/gdb/arc-tdep.c b/gdb/arc-tdep.c
> > new file mode 100644
> > index 0000000..6e0e68d
> > --- /dev/null
> > +++ b/gdb/arc-tdep.c
> > @@ -0,0 +1,1371 @@
> > +/* Target dependent code for ARC arhitecture, for GDB.
> > +
> > +   Copyright 2005-2016 Free Software Foundation, Inc.
> 
> Does this file really contain code that is that old?

This date was there when I've inherited the project, although I don't
know if there is any code remaining from the initial port - there was
lots of work on GDB but not complete rewrites. Linux was initially
ported to ARC somewhere in 2006 and it should be that GDB was ported
at the same timeframe. Nobody from those times is in current team, so
hard to know for sure. And proper commit history of internal project
development has been lost during multiple source control migrations
since that time, so it is hard to figure out exact dates.


> > +/* XML target description features.  */
> > +
> > +static const char *const core_v2_feature_name = "org.gnu.gdb.arc.core.v2";
> > +static const char *const
> > +  core_reduced_v2_feature_name = "org.gnu.gdb.arc.core-reduced.v2";
> > +static const char *const
> > +  core_arcompact_feature_name = "org.gnu.gdb.arc.core.arcompact";
> > +static const char *const
> > +  aux_minimal_feature_name = "org.gnu.gdb.arc.aux-minimal";
> > +
> 
> Do these need to be pointers instead of arrays?

Probably not, I will correct that.


> 
> 
> > +/* Push dummy call return code sequence.
> > +
> > +   We don't actually push any code.  We just identify where a breakpoint can
> > +   be inserted to which we are can return and the resume address where we
> > +   should be called.
> 
> Something seems odd with the last sentence.  "to which we are can"?

"to which we are to return", I think.


> 
> > +
> > +   ARC does not necessarily have an executable stack, so we can't put the
> > +   return breakpoint there.
> 
> What actually goes wrong if you put the breakpoint there?  GDB should
> manage to figure out a SIGSEGV at that address means the function
> returned.  x86-64 GNU/Linux has no executable stack either, for instance,
> and:

SIGSEGV will happen only if we have some OS or another setup to check for
it, like on Linux targets. With simple baremetal applications on error we
just get a CPU exception, which is handled by the application itself or some
sort of an RTOS if one is involved. Our OpenOCD port doesn't know any of
those specifics, so it doesn't intercept the memory error and doesn't report
SIGSEGV to the GDB client.


> > +
> > +/* Determine whether a register can be read.
> > +
> > +   All of the registers in the required set are readable.  There are
> > +   write-only registers among the non-required, but since GDB doesn't know
> > +   anything about them, access is controlled by the GDB server.  RESERVED and
> > +   LIMM are non-existent registers.
> 
> What does this mean, "non-existent registers" ?  If they don't
> exist, then why do they have names and are handled here?
> I'm puzzled on what's the point of a register that can't be
> neither read nor written?

It's not physical registers but addresses in the core register address space.
Address-field in the instruction encoding is 6-bit long, so there can be up to
64 registers. However, in the real CPU out of those:
	1) R0-R31 are real core registers (though, some of them are optional)
	2) R32-R59 are extension registers, which can be added by the CPU designers.
	   For example, FPU or vector registers, which will be treated as a general
	   purpose registers.
	3) R60 is a loop counter for hardware loops
	4) R61 is the "reserved" address, so that we can add a new register to base
	   ISA without breaking compatibility with hardware extensions already
	   developed by our customers. Hence there is no register per se, but there
	   is an address for it. Currently CPU will just have an exception if
	   instruction tries to use that register.
	5) R62 is a "long immediate data indicator". If instruction uses this as an
	   argument, then this means that arguments is not in the register, but in
	   the following 4-bytes of instruction stream.
	6) R63 is a "PCL", which is a read-only 4-byte aligned instruction pointer.

Unfortunately, some of the older gdbserver implementations considered LIMM and
Reserved as a "real" registers - they had their own space in the g/G packet, so
when I was adding support for XML target descriptions, I've left those two as
optional regs in the "core registers" feature, so that GDB would work fine with
those old gdbservers. Current GDB-servers for ARC, that generate XML target
descriptions on their own (OpenOCD for ARC, our proprietary instruction
simulator), never include those registers in the description, since they are
not real. While looking into the target descriptions for your feedback later in
the email, I've started to realize that it might be that I don't need those hacked
"registers" to support old remote servers. For second patch version I'll see if I
can remove them from internal GDB list, while maintaining compatibility.


> > +
> > +/* Get the return value of a function from the registers/memory used to
> > +   return it, according to the convention used by the ABI - 4-bytes values are
> > +   in the R0,
> 
> s/in the R0/in R0/ ?
> 
> >  while 8-byte values are in the R1.
> 
> ITYM "are in R0-R1".  The function seems to extract it out
> of both.

Yes.


> 
> > +
> > +   TODO: This implementation ignores the case of "complex double", where
> > +   according to ABI, value is returned in the R0-R3 registers.
> > +
> > +   TYPE is a returned value's type.  VALBUF is a buffer for the returned
> > +   value.  */
> > +
> > +static void
> > +arc_extract_return_value (struct gdbarch *gdbarch, struct type *type,
> > +			  struct regcache *regcache, gdb_byte *valbuf)
> > +{
> > +  unsigned int len = TYPE_LENGTH (type);
> > +
> > +  if (arc_debug)
> > +    debug_printf ("arc: extract_return_value\n");
> > +
> > +  if (len <= BYTES_IN_REGISTER)
> 
> Where is BYTES_IN_REGISTER defined?

arc-tdep.h. Should I rename it to something like ARC_BYTES_IN_REGISTER or
ARC_REGISTER_SIZE?


> > +
> > +/* Determine how a result of particular type is returned.
> > +
> > +   Return the convention used by the ABI for returning a result of the given
> > +   type from a function; it may also be required to:
> > +
> > +   1. Set the return value (this is for the situation where the debugger user
> > +      has issued a "return <value>" command to request immediate return from
> > +      the current function with the given result; or
> 
> Parens opened but never closed.
> 
> > +
> > +   2. Get the return value ((this is for the situation where the debugger
> > +      user has executed a "call <function>" command to execute the specified
> > +      function in the target program, and that function has a non-void result
> > +      which must be returned to the user.  */
> 
> Spurious double-parens, and never closed.
> 
> These generic comments would be better placed in gdbarch.sh, and then
> here just say:
> 
> /* Implement the XXX gdbarch method.  */
> 
> Augmented with mention of any ARC specifics if any worth mentioning.

Ok.


> > +/* arc_get_disassembler () may return different functions depending on bfd
> > +   type, so it is not possible to pass print_insn directly to
> > +   set_gdbarch_print_insn ().  Instead this wrapper function is used.  It also
> > +   may be used by other functions to get disassemble_info for address.  It is
> > +   important to note, that those print_insn from opcodes always print
> > +   instruction to the stream specified in the info, so if that is not desired,
> > +   then print_insn in the info should be set to some function that doesn't
> > +   print anything, like arc_scream_into_the_void from this file.  */
> > +
> > +static int
> > +arc_delayed_print_insn (bfd_vma addr, struct disassemble_info *info)
> > +{
> > +  int (*print_insn) (bfd_vma, struct disassemble_info *);
> > +  print_insn = arc_get_disassembler (exec_bfd);
> 
> Seems like this will crash if there's no exec_bfd ?

Indeed. I forgot to send my patch to binutils, which will make it gracefully handle
the case of (exec_bfd == NULL).


> > +static const unsigned char *
> > +arc_breakpoint_from_pc (struct gdbarch *gdbarch, CORE_ADDR *pcptr,
> > +			int *lenptr)
> > +{
> > +  struct disassemble_info di;
> > +  arc_initialize_disassembler (gdbarch, &di);
> > +  size_t length_with_limm = arc_delayed_print_insn (*pcptr, &di);
> 
> Seems to me that if you need to do this instead of being able
> to use gdb_insn_length, then that leaves gdb_insn_length calls in
> common code broken.  So, doesn't gdb_insn_length work here?  Why not?

I didn't know about gdb_insn_length. It should work here.


> > +	/* No line info so use current PC.  */
> > +	prologue_end = prev_pc;
> > +      else if (sal.end < prologue_end)
> > +	/* The next line begins after the function end.  */
> > +	prologue_end = sal.end;
> > +
> > +      prologue_end = min (prologue_end, prev_pc);
> > +    }
> 
> Indentation look odd here.

if-else is at level 3, so indented by 6 spaces. Body is at level 4, so indented by
1 tab instead of 8 spaces. Which is far as I understood is the proper way for GNU
projects, for example target.c:target_section_by_addr. So when "> " or "+" are
inserted in front in the patch file, if-else shifts, but the body doesn't shift,
since tabs align to column numbers %8, instead of having a fixed width instead of
having fixed width. So in my response email indentation looks even weirder. Is
there anything I should do differently?


> > +
> > +  /* If we are asked to unwind the PC, then we need to return BLINK instead:
> > +     the saved value of PC points into this frame's function's prologue, not
> > +     the next frame's function's resume location.  `frame_unwind_got_constant`
> > +     is used to return non_lvalue, because it is not possible to modify PC
> 
> s/non_lvalue/not_lval
> 
> > +     directly (one can modify BLINK, if they wish).  If
> > +     trad_frame_get_prev_register (ARC_BLINK_REGNUM) would be used here, than
> > +     it would be possible to modify PC of frame via altering BLINK of
> > +     next_frame, or at least I believe that should be possible technically,
> > +     however sematically could be confusing, so better to avoid this, like
> > +     other arches do.  */
> 
> What other archs do this for this purpose?

Hm. I probably looked at aarch64 when doing that, but it seems that aarch64 doesn't
really have a good reason to do that as well - the code was copy-pasted and modified
from arm-tdep, where it was needed, because PC value was actually a modified value of
LR register, while in AArch64 and ARC it is just a register value as-is. Old ARC code,
which used to be based on GDB 6.8 contained some custom logic of its own, that was
making PC value a not_lval, which I've replaced with standard got_constant here.
So I should just replace REGNUM with BLINK and use standard
trad_frame_get_prev_register, without making it "constant"? Should aarch64 be fixed as
well, or there is some difference I'm missing?


> > +
> > +struct gdbarch_tdep
> > +  {
> > +  };
> 
> Formatting is off.  { should on column 0.
> 
> Empty structs with no fields is not valid C89 either.
> 
> How about just simply dropping these placeholders, and add
> them back when they're actually needed?

Ok.


> 
> > +important to GDB are not "core" registers in ARC.  To simplify internal GDB
> > +design some requirements are applied to ARC target descriptions:
> > +
> > +@itemize @bullet
> > +@item
> > +One of the core registers features must be present.  At all occasions GDB
> > +register number must be equal to architectural number of that register, so for
> > +example regnum of @samp{blink} is always 31.
> > +
> > +@item @samp{org.gnu.gdb.arc.aux-minimal} is mandatory and registers in it has
> > +fixed GDB register numbers.
> > +
> > +@item All other features are optional and cannot have registers with register
> > +number less then 68.
> 
> GDB should be mapping the tdesc numbers to internal numbers, so I don't
> understand why do the register numbers matter in the target description at all?
> It should only matter for the fallback descriptions built in gdb, in
> order to match the layout of the g/G packets of remote servers that
> don't send in tdescs over the wire, I believe?

I see now, that I misunderstood that internal numbers are not related to the
regnums in the XML. So, when invoking the tdesc_numbered_register (), REGNO
should be the internal register number, which must correspond to what I pass to
set_gdbarch_pc_regnum (), and "regnum" attribute in XML can be anything else or
not set at all - that would be a number used by GDB's p/P packet and it will be
unrelated to internal numbers. Is this correct?

I think, it could be that I was confused because gdbarch functions end with 
"_regnum()", so I thought that regnums in the XML must be identical to regnums
I pass to gdbarch.

Anton

> 
> Thanks,
> Pedro Alves
Pedro Alves Sept. 5, 2016, 6:56 p.m. UTC | #3
Hi Anton,

On 09/01/2016 08:54 PM, Anton Kolesov wrote:

>>> Since it is expected that 7.12 will be the last release that supports building
>>> GDB with C compiler (https://sourceware.org/ml/gdb/2016-05/msg00016.html),
>>> this patch contain some code that cannot be compiled with older GCC's default
>>> -std=gnu89 for C, in particular variables are declared at the first use,
>>> rather then at the top of the function.
>>
>> This has not happened yet, and we still support building with a
>> C compiler.  So this can't go in as is until that is resolved...
> 
> Will be fixed.

Actually, this is now resolved:

 https://sourceware.org/ml/gdb/2016-09/msg00024.html

So given this is all new code, there's no need to try to
make it build with a C compiler, if you haven't yet.

[I'll reply to the rest of your email later on.]

Thanks,
Pedro Alves
Pedro Alves Sept. 15, 2016, 11:39 a.m. UTC | #4
Hi Anton,

On 09/01/2016 08:54 PM, Anton Kolesov wrote:

>>>  * GDB and GDBserver now build with a C++ compiler by default.
>>> diff --git a/gdb/arc-tdep.c b/gdb/arc-tdep.c
>>> new file mode 100644
>>> index 0000000..6e0e68d
>>> --- /dev/null
>>> +++ b/gdb/arc-tdep.c
>>> @@ -0,0 +1,1371 @@
>>> +/* Target dependent code for ARC arhitecture, for GDB.
>>> +
>>> +   Copyright 2005-2016 Free Software Foundation, Inc.
>>
>> Does this file really contain code that is that old?
> 
> This date was there when I've inherited the project, although I don't
> know if there is any code remaining from the initial port - there was
> lots of work on GDB but not complete rewrites. Linux was initially
> ported to ARC somewhere in 2006 and it should be that GDB was ported
> at the same timeframe. Nobody from those times is in current team, so
> hard to know for sure. And proper commit history of internal project
> development has been lost during multiple source control migrations
> since that time, so it is hard to figure out exact dates.

OK, I was just curious.  I think the best is to preserve
the dates you already have.

> 
>>
>>> +
>>> +   ARC does not necessarily have an executable stack, so we can't put the
>>> +   return breakpoint there.
>>
>> What actually goes wrong if you put the breakpoint there?  GDB should
>> manage to figure out a SIGSEGV at that address means the function
>> returned.  x86-64 GNU/Linux has no executable stack either, for instance,
>> and:
> 
> SIGSEGV will happen only if we have some OS or another setup to check for
> it, like on Linux targets. With simple baremetal applications on error we
> just get a CPU exception, which is handled by the application itself or some
> sort of an RTOS if one is involved. Our OpenOCD port doesn't know any of
> those specifics, so it doesn't intercept the memory error and doesn't report
> SIGSEGV to the GDB client.

I see.  That's unfortunate.  I think that sooner or later
you'll want to make the openocd port intercept exceptions.
But what you have will do.



> 
> 
>>> +
>>> +/* Determine whether a register can be read.
>>> +
>>> +   All of the registers in the required set are readable.  There are
>>> +   write-only registers among the non-required, but since GDB doesn't know
>>> +   anything about them, access is controlled by the GDB server.  RESERVED and
>>> +   LIMM are non-existent registers.
>>
>> What does this mean, "non-existent registers" ?  If they don't
>> exist, then why do they have names and are handled here?
>> I'm puzzled on what's the point of a register that can't be
>> neither read nor written?
> 
> It's not physical registers but addresses in the core register address space.
> Address-field in the instruction encoding is 6-bit long, so there can be up to
> 64 registers. However, in the real CPU out of those:
> 	1) R0-R31 are real core registers (though, some of them are optional)
> 	2) R32-R59 are extension registers, which can be added by the CPU designers.
> 	   For example, FPU or vector registers, which will be treated as a general
> 	   purpose registers.
> 	3) R60 is a loop counter for hardware loops
> 	4) R61 is the "reserved" address, so that we can add a new register to base
> 	   ISA without breaking compatibility with hardware extensions already
> 	   developed by our customers. Hence there is no register per se, but there
> 	   is an address for it. Currently CPU will just have an exception if
> 	   instruction tries to use that register.
> 	5) R62 is a "long immediate data indicator". If instruction uses this as an
> 	   argument, then this means that arguments is not in the register, but in
> 	   the following 4-bytes of instruction stream.
> 	6) R63 is a "PCL", which is a read-only 4-byte aligned instruction pointer.
> 
> Unfortunately, some of the older gdbserver implementations considered LIMM and
> Reserved as a "real" registers - they had their own space in the g/G packet, so
> when I was adding support for XML target descriptions, I've left those two as
> optional regs in the "core registers" feature, so that GDB would work fine with
> those old gdbservers. Current GDB-servers for ARC, that generate XML target
> descriptions on their own (OpenOCD for ARC, our proprietary instruction
> simulator), never include those registers in the description, since they are
> not real. While looking into the target descriptions for your feedback later in
> the email, I've started to realize that it might be that I don't need those hacked
> "registers" to support old remote servers. For second patch version I'll see if I
> can remove them from internal GDB list, while maintaining compatibility.

Yes, you should not need those in the target descriptions.  If you need to
support old stubs that did _not_ use XML target descriptions, and included
space for those registers in in the g/G packet, then what you need to do
is add a fallback XML target description built into gdb that matches
the layout of the g/G packet of those stubs.  That's what we do
on ARM too -- older stubs used to always include space for the FPA
registers, even if they didn't have them.  See gdb/features/arm-fpu.xml and 
gdb/features/arm-with-m-fpa-layout.xml for example.  And then
we register a "g/G packet size" -> "built-in xml target description" guess.
See arm_register_g_packet_guesses:

/* For backward-compatibility we allow two 'g' packet lengths with
   the remote protocol depending on whether FPA registers are
   supplied.  M-profile targets do not have FPA registers, but some
   stubs already exist in the wild which use a 'g' packet which
   supplies them albeit with dummy values.  The packet format which
   includes FPA registers should be considered deprecated for
   M-profile targets.  */

static void
arm_register_g_packet_guesses (struct gdbarch *gdbarch)
{


> 
>>
>>> +
>>> +   TODO: This implementation ignores the case of "complex double", where
>>> +   according to ABI, value is returned in the R0-R3 registers.
>>> +
>>> +   TYPE is a returned value's type.  VALBUF is a buffer for the returned
>>> +   value.  */
>>> +
>>> +static void
>>> +arc_extract_return_value (struct gdbarch *gdbarch, struct type *type,
>>> +			  struct regcache *regcache, gdb_byte *valbuf)
>>> +{
>>> +  unsigned int len = TYPE_LENGTH (type);
>>> +
>>> +  if (arc_debug)
>>> +    debug_printf ("arc: extract_return_value\n");
>>> +
>>> +  if (len <= BYTES_IN_REGISTER)
>>
>> Where is BYTES_IN_REGISTER defined?
> 
> arc-tdep.h.

Ah.  Somehow that eluded me.

> Should I rename it to something like ARC_BYTES_IN_REGISTER or
> ARC_REGISTER_SIZE?

Yes please.


> 
>>> +/* arc_get_disassembler () may return different functions depending on bfd
>>> +   type, so it is not possible to pass print_insn directly to
>>> +   set_gdbarch_print_insn ().  Instead this wrapper function is used.  It also
>>> +   may be used by other functions to get disassemble_info for address.  It is
>>> +   important to note, that those print_insn from opcodes always print
>>> +   instruction to the stream specified in the info, so if that is not desired,
>>> +   then print_insn in the info should be set to some function that doesn't
>>> +   print anything, like arc_scream_into_the_void from this file.  */
>>> +
>>> +static int
>>> +arc_delayed_print_insn (bfd_vma addr, struct disassemble_info *info)
>>> +{
>>> +  int (*print_insn) (bfd_vma, struct disassemble_info *);
>>> +  print_insn = arc_get_disassembler (exec_bfd);
>>
>> Seems like this will crash if there's no exec_bfd ?
> 
> Indeed. I forgot to send my patch to binutils, which will make it gracefully handle
> the case of (exec_bfd == NULL).

Great, I see that that's now in.  BTW, I see that arc_extension_map
is a global regenerated whenever the binary changes.  At some point
you'll want to make it not be a global, since gdb supports debugging
more than one process at the same time (with extended-remote).

Another think you may consider is that the XML target description
supports an <architecture> element, which is really the bfd target
name.  You could use that to compute the right disassembler
when you don't have a bfd.

> 
>>> +	/* No line info so use current PC.  */
>>> +	prologue_end = prev_pc;
>>> +      else if (sal.end < prologue_end)
>>> +	/* The next line begins after the function end.  */
>>> +	prologue_end = sal.end;
>>> +
>>> +      prologue_end = min (prologue_end, prev_pc);
>>> +    }
>>
>> Indentation look odd here.
> 
> if-else is at level 3, so indented by 6 spaces. Body is at level 4, so indented by
> 1 tab instead of 8 spaces. Which is far as I understood is the proper way for GNU
> projects, for example target.c:target_section_by_addr. So when "> " or "+" are
> inserted in front in the patch file, if-else shifts, but the body doesn't shift,
> since tabs align to column numbers %8, instead of having a fixed width instead of
> having fixed width. So in my response email indentation looks even weirder. Is
> there anything I should do differently?

No.  I'm used to considering that tab/spaces + ">" interaction when reviewing,
but this time I failed to spot it.  My bad.  


>>> +     directly (one can modify BLINK, if they wish).  If
>>> +     trad_frame_get_prev_register (ARC_BLINK_REGNUM) would be used here, than
>>> +     it would be possible to modify PC of frame via altering BLINK of
>>> +     next_frame, or at least I believe that should be possible technically,
>>> +     however sematically could be confusing, so better to avoid this, like
>>> +     other arches do.  */
>>
>> What other archs do this for this purpose?
> 
> Hm. I probably looked at aarch64 when doing that, but it seems that aarch64 doesn't
> really have a good reason to do that as well - the code was copy-pasted and modified
> from arm-tdep, where it was needed, because PC value was actually a modified value of
> LR register, while in AArch64 and ARC it is just a register value as-is. Old ARC code,
> which used to be based on GDB 6.8 contained some custom logic of its own, that was
> making PC value a not_lval, which I've replaced with standard got_constant here.
> So I should just replace REGNUM with BLINK and use standard
> trad_frame_get_prev_register, without making it "constant"? Should aarch64 be fixed as
> well, or there is some difference I'm missing?

If there's no good reason, I'd start by using standard trad_frame_get_prev_register.
There may be a reason, and if we do find one, we can then add a test and comment.
As for aarch64, no idea.

>> GDB should be mapping the tdesc numbers to internal numbers, so I don't
>> understand why do the register numbers matter in the target description at all?
>> It should only matter for the fallback descriptions built in gdb, in
>> order to match the layout of the g/G packets of remote servers that
>> don't send in tdescs over the wire, I believe?
> 
> I see now, that I misunderstood that internal numbers are not related to the
> regnums in the XML. So, when invoking the tdesc_numbered_register (), REGNO
> should be the internal register number, which must correspond to what I pass to
> set_gdbarch_pc_regnum (), and "regnum" attribute in XML can be anything else or
> not set at all - that would be a number used by GDB's p/P packet and it will be
> unrelated to internal numbers. Is this correct?

Correct.  If you haven't found it yet, the "maint print remote-registers"
command is handy to see the number mapping that gdb ended up with at run time,
as well as the register offsets in the g/G  packet.

> 
> I think, it could be that I was confused because gdbarch functions end with 
> "_regnum()", so I thought that regnums in the XML must be identical to regnums
> I pass to gdbarch.

Yeah, they don't.

Thanks,
Pedro Alves
diff mbox

Patch

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 7b2df86..59987f3 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -657,6 +657,7 @@  ALL_64_TARGET_OBS = \
 
 # All other target-dependent objects files (used with --enable-targets=all).
 ALL_TARGET_OBS = \
+	arc-tdep.o \
 	armbsd-tdep.o arm.o arm-linux.o arm-linux-tdep.o \
 	arm-get-next-pcs.o arm-symbian-tdep.o \
 	armnbsd-tdep.o armobsd-tdep.o \
@@ -911,7 +912,7 @@  LINTFILES = $(SFILES) $(YYFILES) $(CONFIG_SRCS) init.c
 HFILES_NO_SRCDIR = \
 common/gdb_signals.h nat/gdb_thread_db.h common/gdb_vecs.h \
 common/x86-xstate.h nat/linux-ptrace.h nat/mips-linux-watch.h \
-proc-utils.h aarch64-tdep.h arm-tdep.h ax-gdb.h ppcfbsd-tdep.h \
+proc-utils.h aarch64-tdep.h arc-tdep.h arm-tdep.h ax-gdb.h ppcfbsd-tdep.h \
 ppcnbsd-tdep.h cli-out.h gdb_expat.h breakpoint.h infcall.h obsd-tdep.h \
 exec.h m32r-tdep.h osabi.h gdbcore.h x86bsd-nat.h \
 i386bsd-nat.h xml-support.h xml-tdesc.h alphabsd-tdep.h gdb_obstack.h \
@@ -1669,6 +1670,7 @@  ALLDEPFILES = \
 	alpha-tdep.c alpha-mdebug-tdep.c \
 	alpha-linux-tdep.c \
 	alphabsd-tdep.c alphafbsd-tdep.c alphanbsd-tdep.c alphaobsd-tdep.c \
+	arc-tdep.c \
 	amd64-nat.c amd64-tdep.c \
 	amd64bsd-nat.c amd64fbsd-nat.c amd64fbsd-tdep.c \
 	amd64nbsd-nat.c amd64nbsd-tdep.c \
diff --git a/gdb/NEWS b/gdb/NEWS
index 6f5feb1..e65444a 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -9,6 +9,10 @@ 
   running on MS-Windows use to assign names to threads in the
   debugger.
 
+* New targets
+
+Synopsys ARC			arc*-*-elf32
+
 *** Changes in GDB 7.12
 
 * GDB and GDBserver now build with a C++ compiler by default.
diff --git a/gdb/arc-tdep.c b/gdb/arc-tdep.c
new file mode 100644
index 0000000..6e0e68d
--- /dev/null
+++ b/gdb/arc-tdep.c
@@ -0,0 +1,1371 @@ 
+/* Target dependent code for ARC arhitecture, for GDB.
+
+   Copyright 2005-2016 Free Software Foundation, Inc.
+   Contributed by Synopsys 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/>.  */
+
+/* GDB header files.  */
+#include "defs.h"
+#include "arch-utils.h"
+#include "dwarf2-frame.h"
+#include "frame-base.h"
+#include "frame-unwind.h"
+#include "gdbcore.h"
+#include "gdbcmd.h"
+#include "objfiles.h"
+#include "trad-frame.h"
+
+/* ARC header files.  */
+#include "opcode/arc.h"
+#include "arc-tdep.h"
+
+/* Default target descriptions.  */
+#include "features/arc-v2.c"
+#include "features/arc-arcompact.c"
+
+/* The frame unwind cache for the ARC.  Current structure is a stub, because
+   it should be filled in during the prologue analysis.  */
+
+struct arc_unwind_cache
+  {
+    /* The stack pointer at the time this frame was created; i.e. the caller's
+       stack pointer when this function was called.  It is used to identify this
+       frame.  */
+    CORE_ADDR prev_sp;
+
+    /* Offsets for each register in the stack frame.  */
+    struct trad_frame_saved_reg *saved_regs;
+  };
+
+/* Global debug flag.  */
+
+int arc_debug;
+
+/* XML target description features.  */
+
+static const char *const core_v2_feature_name = "org.gnu.gdb.arc.core.v2";
+static const char *const
+  core_reduced_v2_feature_name = "org.gnu.gdb.arc.core-reduced.v2";
+static const char *const
+  core_arcompact_feature_name = "org.gnu.gdb.arc.core.arcompact";
+static const char *const
+  aux_minimal_feature_name = "org.gnu.gdb.arc.aux-minimal";
+
+/* XML target description known registers.  */
+
+static const char *const core_v2_register_names[] = {
+  "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", "gp", "fp",
+  "sp", "ilink", "r30", "blink",
+  "r32", "r33", "r34", "r35",
+  "r36", "r37", "r38", "r39",
+  "r40", "r41", "r42", "r43",
+  "r44", "r45", "r46", "r47",
+  "r48", "r49", "r50", "r51",
+  "r52", "r53", "r54", "r55",
+  "r56", "r57", "accl", "acch",
+  "lp_count", "reserved", "limm", "pcl",
+};
+
+static const char *const aux_minimal_register_names[] = {
+  "pc", "lp_start", "lp_end", "status32",
+};
+
+static const char *const core_arcompact_register_names[] = {
+  "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", "gp", "fp",
+  "sp", "ilink1", "ilink2", "blink",
+  "r32", "r33", "r34", "r35",
+  "r36", "r37", "r38", "r39",
+  "r40", "r41", "r42", "r43",
+  "r44", "r45", "r46", "r47",
+  "r48", "r49", "r50", "r51",
+  "r52", "r53", "r54", "r55",
+  "r56", "r57", "r58", "r59",
+  "lp_count", "reserved", "limm", "pcl",
+};
+
+/* Writes a value to PC register.
+
+   In ARC PC register is a normal register so in most cases setting PC value
+   is a straightforward process: debugger just writes PC value.  However it
+   gets trickier in case when current instruction is an instruction in delay
+   slot.  In this case CPU will execute instruction at current PC value, then
+   will set PC to the current value of BTA register; also current instruction
+   cannot be branch/jump and some of the other instruction types.  Thus if
+   debugger would try to just change PC value in this case, this instruction
+   will get executed, but then core will "jump" to the original branch target.
+
+   Whether current instruction is a delay-slot instruction or not is indicated
+   by DE bit in STATUS32 register indicates if current instruction is a delay
+   slot instruction.  This bit is writable by debug host, which allows debug
+   host to prevent core from jumping after the delay slot instruction.  It
+   also works in another direction: setting this bit will make core to treat
+   any current instructions as a delay slot instruction and to set PC to the
+   current value of BTA register.
+
+   To workaround issues with changing PC register while in delay slot
+   instruction, debugger should check for the STATUS32.DE bit and reset it if
+   it is set.  No other change is required in this function.  Most common
+   case, where this function might be required is calling inferior functions
+   from debugger.  Generic GDB logic handles this pretty well: current values
+   of registers are stored, value of PC is changed (that is the job of this
+   function), and after inferior function is executed, GDB restores all
+   registers, include BTA and STATUS32, which also means that core is returned
+   to its original state of being halted on delay slot instructions.
+
+   This method is useless for ARC 600, because it doesn't have externally
+   exposed BTA register.  In the case of ARC 600 it is impossible to restore
+   core to it's state in all occasions thus core should never be halted (from
+   the perspective of debugger host) in the delay slot.  */
+
+static void
+arc_write_pc (struct regcache *regcache, CORE_ADDR new_pc)
+{
+  struct gdbarch *gdbarch = get_regcache_arch (regcache);
+
+  if (arc_debug)
+    debug_printf ("arc: Writing PC, new value=%s\n",
+		  paddress (gdbarch, new_pc));
+
+  regcache_cooked_write_unsigned (regcache, gdbarch_pc_regnum (gdbarch),
+				  new_pc);
+
+  ULONGEST status32;
+  regcache_cooked_read_unsigned (regcache, gdbarch_ps_regnum (gdbarch),
+				 &status32);
+
+  /* Mask for DE bit is 0x40.  */
+  if (status32 & 0x40)
+    {
+      if (arc_debug)
+	{
+	  debug_printf ("arc: Changing PC while in delay slot.  Will "
+			"reset STATUS32.DE bit to zero.  Value of STATUS32 "
+			"register is 0x%s\n",
+			phex (status32, BYTES_IN_REGISTER));
+	}
+
+      /* Reset bit and write to the cache.  */
+      status32 &= ~0x40;
+      regcache_cooked_write_unsigned (regcache, gdbarch_ps_regnum (gdbarch),
+				      status32);
+    }
+}
+
+/* Return the virtual frame pointer.
+
+   According to ABI the FP (r27) is used to point to the middle of the current
+   stack frame, just below the saved FP and before local variables, register
+   spill area and outgoing args.  However for optimization levels above O2 and
+   in any case in leaf functions, the frame pointer is usually not set at all.
+   The exception being when handling nested functions.
+
+   We use this function to return a "virtual" frame pointer, marking the start
+   of the current stack frame as a register-offset pair.  If the FP is not
+   being used, then it should return SP, with an offset of the frame size.
+
+   The current implementation doesn't actually know the frame size, nor
+   whether the FP is actually being used, so for now we just return SP and an
+   offset of zero.  This is no worse than other architectures, but is needed
+   to avoid assertion failures.
+
+   TODO: Can we determine the frame size to get a correct offset?
+
+   PC is a program counter where we need the virtual FP.  REG_PTR is the base
+   register used for the virtual FP.  OFFSET_PTR is the offset used for the
+   virtual FP.  */
+
+static void
+arc_virtual_frame_pointer (struct gdbarch *gdbarch, CORE_ADDR pc,
+			   int *reg_ptr, LONGEST *offset_ptr)
+{
+  *reg_ptr = gdbarch_sp_regnum (gdbarch);
+  *offset_ptr = 0;
+}
+
+/* Return the frame ID for a dummy stack frame.
+
+   Tear down a dummy frame created by arc_push_dummy_call ().  This data has
+   to be constructed manually from the data in our hand.  The stack pointer
+   and program counter can be obtained from the frame info.  */
+
+static struct frame_id
+arc_dummy_id (struct gdbarch *gdbarch, struct frame_info *this_frame)
+{
+  return frame_id_build (get_frame_sp (this_frame),
+			 get_frame_pc (this_frame));
+}
+
+/* Push stack frame for a dummy call.
+
+   Stack Frame Layout
+
+   This shows the layout of the stack frame for the general case of a
+   function call; a given function might not have a variable number of
+   arguments or local variables, or might not save any registers, so it would
+   not have the corresponding frame areas.  Additionally, a leaf function
+   (i.e. one which calls no other functions) does not need to save the
+   contents of the BLINK register (which holds its return address), and a
+   function might not have a frame pointer.
+
+   The stack grows downward, so SP points below FP in memory; SP always
+   points to the last used word on the stack, not the first one.
+
+                      |                       |   |
+                      |      arg word N       |   | caller's
+                      |           :           |   | frame
+                      |      arg word 10      |   |
+                      |      arg word 9       |   |
+          old SP ---> +-----------------------+ --+
+                      |                       |   |
+                      |      callee-saved     |   |
+                      |       registers       |   |
+                      |  including fp, blink  |   |
+                      |                       |   | callee's
+          new FP ---> +-----------------------+   | frame
+                      |                       |   |
+                      |         local         |   |
+                      |       variables       |   |
+                      |                       |   |
+                      |       register        |   |
+                      |      spill area       |   |
+                      |                       |   |
+                      |     outgoing args     |   |
+                      |                       |   |
+          new SP ---> +-----------------------+ --+
+                      |                       |
+                      |         unused        |
+                      |                       |
+                                  |
+                                  |
+                                  V
+                              downwards
+
+   The list of arguments to be passed to a function is considered to be a
+   sequence of _N_ words (as though all the parameters were stored in order in
+   memory with each parameter occupying an integral number of words).  Words
+   1..8 are passed in registers 0..7; if the function has more than 8 words of
+   arguments then words 9..@em N are passed on the stack in the caller's frame.
+
+   If the function has a variable number of arguments, e.g. it has a form such
+   as `function (p1, p2, ...);' and _P_ words are required to hold the values
+   of the named parameters (which are passed in registers 0..@em P -1), then
+   the remaining 8 - _P_ words passed in registers _P_..7 are spilled into the
+   top of the frame so that the anonymous parameter words occupy a continuous
+   region.
+
+   Any arguments are already in target byte order.  We just need to store
+   them!
+
+   BP_ADDR is the return address where breakpoint must be placed.  NARGS is
+   the number of arguments to the function.  ARGS is the arguments values (in
+   target byte order).  SP is the Current value of SP register.  STRUCT_RETURN
+   is TRUE if structures are returned by the function.  STRUCT_ADDR is the
+   hidden address for returning a struct.  Returns SP of a new frame.  */
+
+static CORE_ADDR
+arc_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)
+{
+  if (arc_debug)
+    debug_printf ("arc: push_dummy_call (nargs = %d)\n", nargs);
+
+  int arg_reg = ARC_FIRST_ARG_REGNUM;
+
+  /* Push the return address.  */
+  regcache_cooked_write_unsigned (regcache, ARC_BLINK_REGNUM, bp_addr);
+
+  /* Are we returning a value using a structure return instead of a normal
+     value return?  If so, struct_addr is the address of the reserved space for
+     the return structure to be written on the stack, and that address is
+     passed to that function as a hidden first argument.  */
+  if (struct_return)
+    {
+      /* Pass the return address in the first argument register.  */
+      regcache_cooked_write_unsigned (regcache, arg_reg, struct_addr);
+
+      if (arc_debug)
+	debug_printf ("arc: struct return address %s passed in R%d",
+		      print_core_address (gdbarch, struct_addr), arg_reg);
+
+      arg_reg++;
+    }
+
+  if (nargs > 0)
+    {
+      unsigned int total_space = 0;
+
+      /* How much space do the arguments occupy in total?  Must round each
+         argument's size up to an integral number of words.  */
+      for (int i = 0; i < nargs; i++)
+	{
+	  unsigned int len = TYPE_LENGTH (value_type (args[i]));
+	  unsigned int space = align_up (len, 4);
+
+	  total_space += space;
+
+	  if (arc_debug)
+	    debug_printf ("arc: arg %d: %u bytes -> %u\n", i, len, space);
+	}
+
+      /* Allocate a buffer to hold a memory image of the arguments.  */
+      gdb_byte *memory_image = XCNEWVEC (gdb_byte, total_space);
+
+      /* Now copy all of the arguments into the buffer, correctly aligned.  */
+      gdb_byte *data = memory_image;
+      for (int i = 0; i < nargs; i++)
+	{
+	  unsigned int len = TYPE_LENGTH (value_type (args[i]));
+	  unsigned int space = align_up (len, 4);
+
+	  memcpy (data, value_contents (args[i]), (size_t) len);
+	  if (arc_debug)
+	    debug_printf ("arc: copying arg %d, val 0x%08x, len %d to mem\n",
+			  i, *((int *) value_contents (args[i])), len);
+
+	  data += space;
+	}
+
+      /* Now load as much as possible of the memory image into registers.  */
+      data = memory_image;
+      while (arg_reg <= ARC_LAST_ARG_REGNUM)
+	{
+	  if (arc_debug)
+	    debug_printf ("arc: passing 0x%02x%02x%02x%02x in register R%d\n",
+			  data[0], data[1], data[2], data[3], arg_reg);
+
+	  /* Note we don't use write_unsigned here, since that would convert
+	     the byte order, but we are already in the correct byte order.  */
+	  regcache_cooked_write (regcache, arg_reg, data);
+
+	  data += BYTES_IN_REGISTER;
+	  total_space -= BYTES_IN_REGISTER;
+
+	  /* All the data is now in registers.  */
+	  if (total_space == 0)
+	    break;
+
+	  arg_reg++;
+	}
+
+      /* If there is any data left, push it onto the stack (in a single write
+         operation).  */
+      if (total_space > 0)
+	{
+	  if (arc_debug)
+	    debug_printf ("arc: passing %d bytes on stack\n", total_space);
+
+	  sp -= total_space;
+	  write_memory (sp, data, (int) total_space);
+	}
+
+      xfree (memory_image);
+    }
+
+  /* Finally, update the SP register.  */
+  regcache_cooked_write_unsigned (regcache, gdbarch_sp_regnum (gdbarch), sp);
+
+  return sp;
+}
+
+/* Push dummy call return code sequence.
+
+   We don't actually push any code.  We just identify where a breakpoint can
+   be inserted to which we are can return and the resume address where we
+   should be called.
+
+   ARC does not necessarily have an executable stack, so we can't put the
+   return breakpoint there.  Instead we put it at the entry point of the
+   function.  This means the SP is unchanged.
+
+   SP is a current stack pointer FUNADDR is an address of the function to be
+   called.  ARGS is arguments to pass.  NARGS is a number of args to pass.
+   VALUE_TYPE is a type of value returned.  REAL_PC is a resume address when
+   the function is called.  BP_ADDR is an address where breakpoint should be
+   set.  Returns the updated stack pointer.  */
+
+static CORE_ADDR
+arc_push_dummy_code (struct gdbarch *gdbarch, CORE_ADDR sp, CORE_ADDR funaddr,
+		     struct value **args, int nargs, struct type *value_type,
+		     CORE_ADDR *real_pc, CORE_ADDR *bp_addr,
+		     struct regcache *regcache)
+{
+  *real_pc = funaddr;
+  *bp_addr = entry_point_address ();
+  return sp;
+}
+
+/* Determine whether a register can be read.
+
+   All of the registers in the required set are readable.  There are
+   write-only registers among the non-required, but since GDB doesn't know
+   anything about them, access is controlled by the GDB server.  RESERVED and
+   LIMM are non-existent registers.
+
+   Returns TRUE if we _cannot_ read the register.  */
+
+static int
+arc_cannot_fetch_register (struct gdbarch *gdbarch, int regnum)
+{
+  /* Assume that register is readable if it is unknown.  */
+  switch (regnum)
+    {
+    case ARC_RESERVED_REGNUM:
+    case ARC_LIMM_REGNUM:
+      return TRUE;
+    default:
+      return FALSE;
+    }
+}
+
+/* Determine whether a register can be written.
+
+   It is assumed that first 64 regnums are core registers, hence standard ARC
+   rules for core registers apply and it is assumed that extension registers
+   are RW.  Then PC has regnum 64, LP_START is 65, LP_END is 66, and STATUS32
+   is 67.  All of those are RW in baremetal environment.
+
+   Returns TRUE if we _cannot_ write the register.  */
+
+static int
+arc_cannot_store_register (struct gdbarch *gdbarch, int regnum)
+{
+  /* Assume that register is writable if it is unknown.  */
+  switch (regnum)
+    {
+    case ARC_RESERVED_REGNUM:
+    case ARC_LIMM_REGNUM:
+    case ARC_PCL_REGNUM:
+      return TRUE;
+    default:
+      return FALSE;
+    }
+}
+
+/* Get the return value of a function from the registers/memory used to
+   return it, according to the convention used by the ABI - 4-bytes values are
+   in the R0, while 8-byte values are in the R1.
+
+   TODO: This implementation ignores the case of "complex double", where
+   according to ABI, value is returned in the R0-R3 registers.
+
+   TYPE is a returned value's type.  VALBUF is a buffer for the returned
+   value.  */
+
+static void
+arc_extract_return_value (struct gdbarch *gdbarch, struct type *type,
+			  struct regcache *regcache, gdb_byte *valbuf)
+{
+  unsigned int len = TYPE_LENGTH (type);
+
+  if (arc_debug)
+    debug_printf ("arc: extract_return_value\n");
+
+  if (len <= BYTES_IN_REGISTER)
+    {
+      ULONGEST val;
+
+      /* Get the return value from one register.  */
+      regcache_cooked_read_unsigned (regcache, ARC_R0_REGNUM, &val);
+      store_unsigned_integer (valbuf, (int) len,
+			      gdbarch_byte_order (gdbarch), val);
+
+      if (arc_debug)
+	debug_printf ("arc: returning 0x%s\n", phex (val, BYTES_IN_REGISTER));
+    }
+  else if (len <= BYTES_IN_REGISTER * 2)
+    {
+      ULONGEST low, high;
+
+      /* Get the return value from two registers.  */
+      regcache_cooked_read_unsigned (regcache, ARC_R0_REGNUM, &low);
+      regcache_cooked_read_unsigned (regcache, ARC_R1_REGNUM, &high);
+
+      store_unsigned_integer (valbuf, BYTES_IN_REGISTER,
+			      gdbarch_byte_order (gdbarch), low);
+      store_unsigned_integer (valbuf + BYTES_IN_REGISTER,
+			      (int) len - BYTES_IN_REGISTER,
+			      gdbarch_byte_order (gdbarch), high);
+
+      if (arc_debug)
+	debug_printf ("arc: returning 0x%s%s\n",
+		      phex (high, BYTES_IN_REGISTER),
+		      phex (low, BYTES_IN_REGISTER));
+    }
+  else
+    error (_("arc: extract_return_value: type length %u too large"), len);
+}
+
+
+/* Store the return value of a function into the registers/memory used to
+   return it, according to the convention used by the ABI.
+
+   TODO: This implementation ignores the case of "complex double", where
+   according to ABI, value is returned in the R0-R3 registers.
+
+   TYPE is a returned value's type.  VALBUF is a buffer with the value to
+   return.  */
+
+static void
+arc_store_return_value (struct gdbarch *gdbarch, struct type *type,
+			struct regcache *regcache, const gdb_byte *valbuf)
+{
+  unsigned int len = TYPE_LENGTH (type);
+
+  if (arc_debug)
+    debug_printf ("arc: store_return_value\n");
+
+  if (len <= BYTES_IN_REGISTER)
+    {
+      ULONGEST val;
+
+      /* Put the return value into one register.  */
+      val = extract_unsigned_integer (valbuf, (int) len,
+				      gdbarch_byte_order (gdbarch));
+      regcache_cooked_write_unsigned (regcache, ARC_R0_REGNUM, val);
+
+      if (arc_debug)
+	debug_printf ("arc: storing 0x%s\n", phex (val, BYTES_IN_REGISTER));
+    }
+  else if (len <= BYTES_IN_REGISTER * 2)
+    {
+      ULONGEST low, high;
+
+      /* Put the return value into  two registers.  */
+      low = extract_unsigned_integer (valbuf, BYTES_IN_REGISTER,
+				      gdbarch_byte_order (gdbarch));
+      high = extract_unsigned_integer (valbuf + BYTES_IN_REGISTER,
+				       (int) len - BYTES_IN_REGISTER,
+				       gdbarch_byte_order (gdbarch));
+
+      regcache_cooked_write_unsigned (regcache, ARC_R0_REGNUM, low);
+      regcache_cooked_write_unsigned (regcache, ARC_R1_REGNUM, high);
+
+      if (arc_debug)
+	debug_printf ("arc: storing 0x%s%s\n",
+		      phex (high, BYTES_IN_REGISTER),
+		      phex (low, BYTES_IN_REGISTER));
+    }
+  else
+    error (_("arc_store_return_value: type length too large."));
+}
+
+/* Determine how a result of particular type is returned.
+
+   Return the convention used by the ABI for returning a result of the given
+   type from a function; it may also be required to:
+
+   1. Set the return value (this is for the situation where the debugger user
+      has issued a "return <value>" command to request immediate return from
+      the current function with the given result; or
+
+   2. Get the return value ((this is for the situation where the debugger
+      user has executed a "call <function>" command to execute the specified
+      function in the target program, and that function has a non-void result
+      which must be returned to the user.  */
+
+static enum return_value_convention
+arc_return_value (struct gdbarch *gdbarch, struct value *function,
+		  struct type *valtype, struct regcache *regcache,
+		  gdb_byte *readbuf, const gdb_byte *writebuf)
+{
+  /* If the return type is a struct, or a union, or would occupy more than two
+     registers, the ABI uses the "struct return convention": the calling
+     function passes a hidden first parameter to the callee (in R0).  That
+     parameter is the address at which the value being returned should be
+     stored.  Otherwise, the result is returned in registers.  */
+  int is_struct_return = (TYPE_CODE (valtype) == TYPE_CODE_STRUCT
+			  || TYPE_CODE (valtype) == TYPE_CODE_UNION
+			  || TYPE_LENGTH (valtype) > 2 * BYTES_IN_REGISTER);
+
+  if (arc_debug)
+    debug_printf ("arc: return_value (readbuf = %p, writebuf = %p)\n",
+		  readbuf, writebuf);
+
+  if (writebuf != NULL)
+    {
+      /* Case 1.  GDB should not ask us to set a struct return value: it
+         should know the struct return location and write the value there
+         itself.  */
+      gdb_assert (!is_struct_return);
+      arc_store_return_value (gdbarch, valtype, regcache, writebuf);
+    }
+  else if (readbuf != NULL)
+    {
+      /* Case 2.  GDB should not ask us to get a struct return value: it
+         should know the struct return location and read the value from there
+         itself.  */
+      gdb_assert (!is_struct_return);
+      arc_extract_return_value (gdbarch, valtype, regcache, readbuf);
+    }
+
+  return is_struct_return ? RETURN_VALUE_STRUCT_CONVENTION
+    : RETURN_VALUE_REGISTER_CONVENTION;
+}
+
+/* Utility function to create a new frame cache structure.  */
+
+static struct arc_unwind_cache *
+arc_create_cache (struct frame_info *this_frame)
+{
+  struct arc_unwind_cache *cache
+    = FRAME_OBSTACK_ZALLOC (struct arc_unwind_cache);
+
+  /* Zero all fields.  */
+  cache->prev_sp = 0;
+
+  /* Allocate space for saved register info.  */
+  cache->saved_regs = trad_frame_alloc_saved_regs (this_frame);
+
+  return cache;
+}
+
+/* Return the base address of the frame.  For ARC, the base address is the
+   frame pointer.  */
+
+static CORE_ADDR
+arc_frame_base_address (struct frame_info *this_frame, void **prologue_cache)
+{
+  return (CORE_ADDR) get_frame_register_unsigned (this_frame, ARC_FP_REGNUM);
+}
+
+/* Skip the prologue for the function at PC.  This is done by checking from
+   the line information read from the DWARF, if possible; otherwise, we scan
+   the function prologue to find its end.  */
+
+static CORE_ADDR
+arc_skip_prologue (struct gdbarch *gdbarch, CORE_ADDR pc)
+{
+  if (arc_debug)
+    debug_printf ("arc: skip_prologue\n");
+
+  CORE_ADDR func_addr;
+  const char *func_name;
+
+  /* See what the symbol table says.  */
+  if (find_pc_partial_function (pc, &func_name, &func_addr, NULL))
+    {
+      /* Found a function.  */
+      CORE_ADDR postprologue_pc
+	= skip_prologue_using_sal (gdbarch, func_addr);
+
+      if (postprologue_pc)
+	return max (pc, postprologue_pc);
+    }
+
+  /* No prologue info in symbol table, have to analyze prologue.  */
+
+  /* Find an upper limit on the function prologue using the debug
+     information.  If the debug information could not be used to provide that
+     bound, then pass 0 and arc_scan_prologue will estimate value itself.  */
+  CORE_ADDR limit_pc = skip_prologue_using_sal (gdbarch, pc);
+  /* We don't have a proper analyze_prologue function yet, but its result
+     should be returned here.  Currently GDB will just stop at the first
+     instruction of function if debug information doesn't have prologue info;
+     and if there is a debug info about prologue - this code path will not be
+     taken at all.  */
+  return (limit_pc == 0 ? pc : limit_pc);
+}
+
+/* arc_get_disassembler () may return different functions depending on bfd
+   type, so it is not possible to pass print_insn directly to
+   set_gdbarch_print_insn ().  Instead this wrapper function is used.  It also
+   may be used by other functions to get disassemble_info for address.  It is
+   important to note, that those print_insn from opcodes always print
+   instruction to the stream specified in the info, so if that is not desired,
+   then print_insn in the info should be set to some function that doesn't
+   print anything, like arc_scream_into_the_void from this file.  */
+
+static int
+arc_delayed_print_insn (bfd_vma addr, struct disassemble_info *info)
+{
+  int (*print_insn) (bfd_vma, struct disassemble_info *);
+  print_insn = arc_get_disassembler (exec_bfd);
+  gdb_assert (print_insn != NULL);
+  return print_insn (addr, info);
+}
+
+/** Baremetal breakpoint instructions.
+
+   ARC supports both big- and little-endian.  However, instructions for
+   little-endian processors are encoded in the middle-endian: half-words are
+   in big-endian, while bytes inside the half-words are in little-endian; data
+   is represented in the "normal" little-endian.  Big-endian processors treat
+   data and code identically.
+
+   Assuming the number 0x01020304, it will be presented this way:
+
+   Address            :  N   N+1  N+2  N+3
+   little-endian      : 0x04 0x03 0x02 0x01
+   big-endian         : 0x01 0x02 0x03 0x04
+   ARC middle-endian  : 0x02 0x01 0x04 0x03
+  */
+
+static const unsigned char arc_brk_s_be[] = { 0x7f, 0xff };
+static const unsigned char arc_brk_s_le[] = { 0xff, 0x7f };
+static const unsigned char arc_brk_be[] = { 0x25, 0x6f, 0x00, 0x3f };
+static const unsigned char arc_brk_le[] = { 0x6f, 0x25, 0x3f, 0x00 };
+
+/* Get breakpoint which is appropriate for address at which it is to be set.
+
+   For ARC ELF, breakpoint uses the 16-bit BRK_S instruction, which is 0x7fff
+   (little endian) or 0xff7f (big endian).  We used to insert BRK_S even
+   instead of 32-bit instructions, which works mostly ok, unless breakpoint is
+   inserted into delay slot instruction.  In this case if branch is taken
+   BLINK value will be set to address of instruction after delay slot, however
+   if we replaced 32-bit instruction in delay slot with 16-bit long BRK_S,
+   then BLINK value will have an invalid value - it will point to the address
+   after the BRK_S (which was there at the moment of branch execution) while
+   it should point to the address after the 32-bit long instruction.  To avoid
+   such issues this function disassembles instruction at target location and
+   evaluates it value.
+
+   ARC 600 supports only 16-bit BRK_S.
+
+   NB: Baremetal GDB uses BRK[_S], while user-space GDB uses TRAP_S.  BRK[_S]
+   is much better because it doesn't commit unlike TRAP_S, so it can be set in
+   delay slots; however it cannot be used in user-mode, hence usage of TRAP_S
+   in GDB for user-space.
+
+   PCPTR is a pointer to the PC where we want to place a breakpoint.  LENPTR
+   is a number of bytes used by the breakpoint.  Returns the byte sequence of
+   a breakpoint instruction.  */
+
+static const unsigned char *
+arc_breakpoint_from_pc (struct gdbarch *gdbarch, CORE_ADDR *pcptr,
+			int *lenptr)
+{
+  struct disassemble_info di;
+  arc_initialize_disassembler (gdbarch, &di);
+  size_t length_with_limm = arc_delayed_print_insn (*pcptr, &di);
+
+  /* Replace 16-bit instruction with BRK_S, replace 32-bit instructions with
+     BRK.  LIMM is part of instruction length, so it can be either 4 or 8
+     bytes.  */
+  if ((length_with_limm == 4 || length_with_limm == 8)
+      && !arc_mach_is_arc600 (gdbarch))
+    {
+      *lenptr = sizeof (arc_brk_le);
+      return (gdbarch_byte_order (gdbarch) == BFD_ENDIAN_BIG)
+	? arc_brk_be : arc_brk_le;
+    }
+  else
+    {
+      *lenptr = sizeof (arc_brk_s_le);
+      return (gdbarch_byte_order (gdbarch) == BFD_ENDIAN_BIG)
+	? arc_brk_s_be : arc_brk_s_le;
+    }
+}
+
+/* Returns the value of PC in this frame.  */
+
+static CORE_ADDR
+arc_unwind_pc (struct gdbarch *gdbarch, struct frame_info *next_frame)
+{
+  int pc_regnum = gdbarch_pc_regnum (gdbarch);
+  CORE_ADDR pc = frame_unwind_register_unsigned (next_frame, pc_regnum);
+
+  if (arc_debug)
+    debug_printf ("arc: unwind PC: %s\n", paddress (gdbarch, pc));
+
+  return pc;
+}
+
+/* Returns the value of SP in this frame.  */
+
+static CORE_ADDR
+arc_unwind_sp (struct gdbarch *gdbarch, struct frame_info *next_frame)
+{
+  int sp_regnum = gdbarch_sp_regnum (gdbarch);
+  CORE_ADDR sp = frame_unwind_register_unsigned (next_frame, sp_regnum);
+
+  if (arc_debug)
+    debug_printf ("arc: unwind SP: %s\n", paddress (gdbarch, sp));
+
+  return sp;
+}
+
+/* Adjust the stack pointer to align frame.  */
+
+static CORE_ADDR
+arc_frame_align (struct gdbarch *gdbarch, CORE_ADDR sp)
+{
+  return align_down (sp, 4);
+}
+
+/* Frame unwinder for normal frames.  */
+
+static struct arc_unwind_cache *
+arc_frame_cache (struct frame_info *this_frame)
+{
+  if (arc_debug)
+    debug_printf ("arc: frame_cache\n");
+
+  struct gdbarch *gdbarch = get_frame_arch (this_frame);
+  struct arc_unwind_cache *cache = arc_create_cache (this_frame);
+
+  CORE_ADDR block_addr = get_frame_address_in_block (this_frame);
+  CORE_ADDR prev_pc = get_frame_pc (this_frame);
+
+  CORE_ADDR entrypoint, prologue_end;
+  if (find_pc_partial_function (block_addr, NULL, &entrypoint, &prologue_end))
+    {
+      struct symtab_and_line sal = find_pc_line (entrypoint, 0);
+      if (!sal.line)
+	/* No line info so use current PC.  */
+	prologue_end = prev_pc;
+      else if (sal.end < prologue_end)
+	/* The next line begins after the function end.  */
+	prologue_end = sal.end;
+
+      prologue_end = min (prologue_end, prev_pc);
+    }
+  else
+    {
+      entrypoint = get_frame_register_unsigned (this_frame,
+						gdbarch_pc_regnum (gdbarch));
+      prologue_end = 0;
+    }
+
+  /* Should call analyze_prologue here, when it will be implemented.  */
+
+  return cache;
+}
+
+/* Get the frame_id of a normal frame.  */
+
+static void
+arc_frame_this_id (struct frame_info *this_frame, void **this_cache,
+		   struct frame_id *this_id)
+{
+  if (arc_debug)
+    debug_printf ("arc: frame_this_id\n");
+
+  struct gdbarch *gdbarch = get_frame_arch (this_frame);
+
+  if (*this_cache == NULL)
+    *this_cache = arc_frame_cache (this_frame);
+  struct arc_unwind_cache *cache = (struct arc_unwind_cache *) (*this_cache);
+
+  CORE_ADDR stack_addr = cache->prev_sp;
+
+  /* There are 4 possible situation which decide how frame_id->code_addr is
+     evaluated:
+
+     1) Function is compiled with option -g.  Then frame_id will be created
+     in dwarf_* function and not in this function.  NB: even if target
+     binary is compiled with -g, some std functions like __start and _init
+     are not, so they still will follow one of the following choices.
+
+     2) Function is compiled without -g and binary hasn't been stripped in
+     any way.  In this case GDB still has enough information to evaluate
+     frame code_addr properly.  This case is covered by call to
+     get_frame_func ().
+
+     3) Binary has been striped with option -g (strip debug symbols).  In
+     this case there is still enough symbols for get_frame_func () to work
+     properly, so this case is also covered by it.
+
+     4) Binary has been striped with option -s (strip all symbols).  In this
+     case GDB cannot get function start address properly, so we return current
+     PC value instead.
+   */
+  CORE_ADDR code_addr = get_frame_func (this_frame);
+  if (!code_addr)
+    code_addr = get_frame_register_unsigned (this_frame,
+					     gdbarch_pc_regnum (gdbarch));
+
+  *this_id = frame_id_build (stack_addr, code_addr);
+}
+
+/* Unwind a register from a normal frame.  */
+
+static struct value *
+arc_frame_prev_register (struct frame_info *this_frame,
+			 void **this_cache, int regnum)
+{
+  if (arc_debug)
+    debug_printf ("arc: frame_prev_register (regnum = %d)\n", regnum);
+
+  if (*this_cache == NULL)
+    *this_cache = arc_frame_cache (this_frame);
+  struct arc_unwind_cache *cache = (struct arc_unwind_cache *) (*this_cache);
+
+  struct gdbarch *gdbarch = get_frame_arch (this_frame);
+
+  /* If we are asked to unwind the PC, then we need to return BLINK instead:
+     the saved value of PC points into this frame's function's prologue, not
+     the next frame's function's resume location.  `frame_unwind_got_constant`
+     is used to return non_lvalue, because it is not possible to modify PC
+     directly (one can modify BLINK, if they wish).  If
+     trad_frame_get_prev_register (ARC_BLINK_REGNUM) would be used here, than
+     it would be possible to modify PC of frame via altering BLINK of
+     next_frame, or at least I believe that should be possible technically,
+     however sematically could be confusing, so better to avoid this, like
+     other arches do.  */
+  if (regnum == gdbarch_pc_regnum (gdbarch))
+    {
+      CORE_ADDR blink;
+      blink = frame_unwind_register_unsigned (this_frame, ARC_BLINK_REGNUM);
+      return frame_unwind_got_constant (this_frame, regnum, blink);
+    }
+
+  /* SP is a special case - we should return prev_sp, because
+     trad_frame_get_prev_register will return _current_ SP value.
+     Alternatively we could have stored cache->prev_sp in the cache->saved
+     regs, but here we follow the lead of AArch64, ARM and Xtensa and will
+     leave that logic in this function, instead of prologue analyzers.  That I
+     think is a bit more clear as `saved_regs` should contain saved regs, not
+     computable.
+
+     Similar to PC, constant value is returned, since it is "computable", thus
+     not editable.  */
+
+  if (regnum == gdbarch_sp_regnum (gdbarch))
+    return frame_unwind_got_constant (this_frame, regnum, cache->prev_sp);
+
+  return trad_frame_get_prev_register (this_frame, cache->saved_regs, regnum);
+}
+
+/* Identify some properties of particular registers for the DWARF unwinder.
+   */
+
+static void
+arc_dwarf2_frame_init_reg (struct gdbarch *gdbarch, int regnum,
+			   struct dwarf2_frame_state_reg *reg,
+			   struct frame_info *info)
+{
+  if (regnum == gdbarch_pc_regnum (gdbarch))
+    /* The return address column.  */
+    reg->how = DWARF2_FRAME_REG_RA;
+  else if (regnum == gdbarch_sp_regnum (gdbarch))
+    /* The call frame address.  */
+    reg->how = DWARF2_FRAME_REG_CFA;
+}
+
+/* Structure defining the ARC ordinary frame unwind functions.  Since we are
+   the fallback unwinder, we use the default frame sniffer, which always
+   accepts the frame.  */
+
+static const struct frame_unwind arc_frame_unwind = {
+  NORMAL_FRAME,
+  default_frame_unwind_stop_reason,
+  arc_frame_this_id,
+  arc_frame_prev_register,
+  NULL,
+  default_frame_sniffer,
+  NULL,
+  NULL
+};
+
+
+static const struct frame_base arc_normal_base = {
+  &arc_frame_unwind,
+  arc_frame_base_address,
+  arc_frame_base_address,
+  arc_frame_base_address
+};
+
+/* Initialize target description for the ARC.
+
+   Returns TRUE if input tdesc was valid and in this case it will assign TDESC
+   and TDESC_DATA output parameters.  */
+
+static int
+arc_tdesc_init (struct gdbarch_info info, const struct target_desc **tdesc,
+		struct tdesc_arch_data **tdesc_data)
+{
+  if (arc_debug)
+    debug_printf ("arc: Arhitecture initialization.\n");
+
+  const struct target_desc *tdesc_loc = info.target_desc;
+
+  /* Depending on whether this is ARCompact or ARCv2 we will assigned
+     different default registers sets (which will differ in exactly two core
+     registers).  GDB will also refuse to accept register feature from invalid
+     ISA - v2 features can be used only with v2 ARChitecture.  registers of
+     invalid architecture.  We read bfd_arch_info, which looks like to be a
+     safe bet here, as it looks like it is always initialized even when we
+     don't pass any elf file to GDB at all (it uses default arch in this
+     case).  Also GDB will call this function multiple times, and if XML
+     target description file contains architecture specifications, then GDB
+     will set this architecture to info.bfd_arch_info, overriding value from
+     ELF file if they are different.  That means that, where matters, this
+     value is always our best guess on what CPU we are debugging.  It has been
+     noted that that architecture specified in tdesc file has higher
+     precedence over ELF and even "set architecture" - that is, using "set
+     architecture" command will have no effect when tdesc has "arch" tag.  */
+  /* Cannot use arc_mach_is_arcv2 (), because gdbarch is not created yet.  */
+  const int is_arcv2 = (info.bfd_arch_info->mach == bfd_mach_arc_arcv2);
+  int is_reduced_rf;
+  const char *const *core_regs;
+  const char *core_feature_name;
+
+  /* If target doesn't provide a description - use default one.  */
+  if (!tdesc_has_registers (tdesc_loc))
+    {
+      if (is_arcv2)
+	{
+	  tdesc_loc = tdesc_arc_v2;
+	  if (arc_debug)
+	    debug_printf ("arc: Using default register set for ARC v2.\n");
+	}
+      else
+	{
+	  tdesc_loc = tdesc_arc_arcompact;
+	  if (arc_debug)
+	    debug_printf ("arc: Using default register set for ARCompact.\n");
+	}
+    }
+  else
+    {
+      if (arc_debug)
+	debug_printf ("arc: Using provided register set.\n");
+    }
+  gdb_assert (tdesc_loc);
+
+  /** Now we can search for base registers.  Core registers can be either full
+     or reduced.  Summary:
+
+     - core.v2 + aux-minimal
+     - core-reduced.v2 + aux-minimal
+     - core.arcompact + aux-minimal
+
+     NB: It is intirely feaseble to have ARCompact with reduced
+     core regs, but we ignore that because GCC doesn't support that and at
+     the same time this architecture version is obsolete.  */
+  const struct tdesc_feature *feature
+    = tdesc_find_feature (tdesc_loc, core_v2_feature_name);
+  if (feature)
+    {
+      /* Confirm that register and architecture match, to prevent accidents in
+         some situations.  This code will trigger an error if:
+
+         1. XML tdesc doesn't specify arch explicitly, registers are for arch
+         X, but ELF specifies arch Y.
+
+         2. XML tdesc specifies arch X, but contains registers for arch Y.
+
+         It will not protect from case where XML or ELF specify arch X,
+         registers are for the same arch X, but the real target is arch Y.  To
+         detect this case we need to check IDENTITY register.  */
+      if (!is_arcv2)
+	{
+	  arc_print (_("Error: ARC v2 target description supplied for "
+		       "non-ARCv2 target.\n"));
+	  return FALSE;
+	}
+
+      is_reduced_rf = FALSE;
+      core_feature_name = core_v2_feature_name;
+      core_regs = core_v2_register_names;
+    }
+  else
+    {
+      feature = tdesc_find_feature (tdesc_loc, core_reduced_v2_feature_name);
+      if (feature)
+	{
+	  if (!is_arcv2)
+	    {
+	      arc_print (_("Error: ARC v2 target description supplied for "
+			   "non-ARCv2 target.\n"));
+	      return FALSE;
+	    }
+
+	  is_reduced_rf = TRUE;
+	  core_feature_name = core_reduced_v2_feature_name;
+	  core_regs = core_v2_register_names;
+	}
+      else
+	{
+	  feature = tdesc_find_feature (tdesc_loc,
+					core_arcompact_feature_name);
+	  if (feature)
+	    {
+	      if (is_arcv2)
+		{
+		  arc_print (_("Error: ARCompact target description supplied "
+			       "for non-ARCompact target.\n"));
+		  return FALSE;
+		}
+
+	      is_reduced_rf = FALSE;
+	      core_feature_name = core_arcompact_feature_name;
+	      core_regs = core_arcompact_register_names;
+	    }
+	  else
+	    {
+	      arc_print (_("Error: Couldn't find core register feature in "
+			   "supplied target description."));
+	      return FALSE;
+	    }
+	}
+    }
+
+  struct tdesc_arch_data *tdesc_data_loc = tdesc_data_alloc ();
+
+  gdb_assert (feature);
+  int valid_p = 1;
+
+  for (size_t i = ARC_FIRST_CORE_REGNUM; i < ARC_LAST_CORE_REGNUM; i++)
+    {
+      /* R61 and R62 do never exist in real hardware, however have to check for
+         them to preserve compatibility with old gdbserver which reserve a slot
+         for them in g/G-packets.  */
+
+      /* If rf16, then skip extra registers.  */
+      if (is_reduced_rf && ((i >= ARC_R4_REGNUM && i <= ARC_R9_REGNUM)
+			    || (i >= ARC_R16_REGNUM && i <= ARC_R25_REGNUM)))
+	continue;
+
+      /* This subtraction is superflouos, because FIRST_CORE_REGNUM is 0, but
+         OTOH it adds a level of abstraction between arrays and regnums.  Not
+         sure if that is a good idea.  */
+      const char *name = core_regs[i - ARC_FIRST_CORE_REGNUM];
+
+      valid_p = tdesc_numbered_register (feature, tdesc_data_loc, i, name);
+
+      /* - Ignore errors in extension registers - they are optional.
+         - Ignore missing ILINK because it doesn't make sense for Linux.
+         - Ignore missing ILINK2 when architecture is ARCompact, because it
+         doesn't make sense for Linux targets.
+
+         In theory those optional registers should be in separate features,
+         but that's just crazy - features would be tiny and numerous and would
+         be make for complex maintanence, especially since regnums of
+         different features would interleve.  */
+      if (!valid_p && (i <= ARC_SP_REGNUM || i == ARC_BLINK_REGNUM
+		       || i == ARC_LP_COUNT_REGNUM || i == ARC_PCL_REGNUM
+		       || (i == ARC_R30_REGNUM && is_arcv2)))
+	{
+	  arc_print (_("Error: Cannot find required register `%s' "
+		       "in feature `%s'.\n"), name, core_feature_name);
+	  tdesc_data_cleanup (tdesc_data_loc);
+	  return FALSE;
+	}
+    }
+
+  /* Mandatory AUX registeres are intentionally few and are common between
+     ARCompact and ARC v2, so same code can be used for both.  */
+  feature = tdesc_find_feature (tdesc_loc, aux_minimal_feature_name);
+  if (!feature)
+    {
+      arc_print (_("Error: Cannot find required feature `%s' in supplied "
+		   "target description.\n"), aux_minimal_feature_name);
+      tdesc_data_cleanup (tdesc_data_loc);
+      return FALSE;
+    }
+
+  for (size_t i = ARC_FIRST_AUX_REGNUM; i < ARC_LAST_AUX_REGNUM; i++)
+    {
+      const char *name = aux_minimal_register_names[i - ARC_FIRST_AUX_REGNUM];
+      valid_p = tdesc_numbered_register (feature, tdesc_data_loc, i, name);
+      /* LP registers are optional.  */
+      if (!valid_p && (i != ARC_LP_START_REGNUM) && (i != ARC_LP_END_REGNUM))
+	{
+	  arc_print (_("Error: Cannot find required register `%s' "
+		       "in feature `%s'.\n"),
+		     name, tdesc_feature_name (feature));
+	  tdesc_data_cleanup (tdesc_data_loc);
+	  return FALSE;
+	}
+    }
+
+  *tdesc = tdesc_loc;
+  *tdesc_data = tdesc_data_loc;
+
+  return TRUE;
+}
+
+static struct gdbarch *
+arc_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
+{
+  const struct target_desc *tdesc;
+  struct tdesc_arch_data *tdesc_data;
+  if (!arc_tdesc_init (info, &tdesc, &tdesc_data))
+    return NULL;
+
+  /* Allocate the ARC-private target-dependent information structure, and the
+     GDB target-independent information structure.  */
+  struct gdbarch_tdep *tdep = XCNEW (struct gdbarch_tdep);
+  struct gdbarch *gdbarch = gdbarch_alloc (&info, tdep);
+
+  /* Data types.  */
+  set_gdbarch_short_bit (gdbarch, 16);
+  set_gdbarch_int_bit (gdbarch, 32);
+  set_gdbarch_long_bit (gdbarch, 32);
+  set_gdbarch_long_long_bit (gdbarch, 64);
+  set_gdbarch_long_long_align_bit (gdbarch, 32);
+  set_gdbarch_float_bit (gdbarch, 32);
+  set_gdbarch_float_format (gdbarch, floatformats_ieee_single);
+  set_gdbarch_double_bit (gdbarch, 64);
+  set_gdbarch_double_format (gdbarch, floatformats_ieee_double);
+  set_gdbarch_ptr_bit (gdbarch, 32);
+  set_gdbarch_addr_bit (gdbarch, 32);
+  set_gdbarch_char_signed (gdbarch, 0);
+
+  set_gdbarch_write_pc (gdbarch, arc_write_pc);
+
+  set_gdbarch_virtual_frame_pointer (gdbarch, arc_virtual_frame_pointer);
+
+  /* tdesc_use_registers expects gdbarch_num_regs to return number of registers
+     parsed by gdbarch_init, and then it will add all of the remaining
+     registers and will increase number of registers.  */
+  set_gdbarch_num_regs (gdbarch, ARC_LAST_REGNUM + 1);
+  set_gdbarch_num_pseudo_regs (gdbarch, 0);
+  set_gdbarch_sp_regnum (gdbarch, ARC_SP_REGNUM);
+  set_gdbarch_pc_regnum (gdbarch, ARC_PC_REGNUM);
+  set_gdbarch_ps_regnum (gdbarch, ARC_STATUS32_REGNUM);
+  set_gdbarch_fp0_regnum (gdbarch, -1);	/* No FPU registers.  */
+
+  set_gdbarch_dummy_id (gdbarch, arc_dummy_id);
+  set_gdbarch_push_dummy_call (gdbarch, arc_push_dummy_call);
+  set_gdbarch_push_dummy_code (gdbarch, arc_push_dummy_code);
+
+  set_gdbarch_cannot_fetch_register (gdbarch, arc_cannot_fetch_register);
+  set_gdbarch_cannot_store_register (gdbarch, arc_cannot_store_register);
+
+  set_gdbarch_believe_pcc_promotion (gdbarch, 1);
+
+  set_gdbarch_return_value (gdbarch, arc_return_value);
+
+  set_gdbarch_skip_prologue (gdbarch, arc_skip_prologue);
+  set_gdbarch_inner_than (gdbarch, core_addr_lessthan);
+
+  set_gdbarch_breakpoint_from_pc (gdbarch, arc_breakpoint_from_pc);
+
+  /* On ARC 600 BRK_S instruction advances PC, unlike other ARC cores.  */
+  if (!arc_mach_is_arc600 (gdbarch))
+    set_gdbarch_decr_pc_after_break (gdbarch, 0);
+  else
+    set_gdbarch_decr_pc_after_break (gdbarch, 2);
+
+  set_gdbarch_unwind_pc (gdbarch, arc_unwind_pc);
+  set_gdbarch_unwind_sp (gdbarch, arc_unwind_sp);
+
+  set_gdbarch_frame_align (gdbarch, arc_frame_align);
+
+  set_gdbarch_print_insn (gdbarch, arc_delayed_print_insn);
+
+  set_gdbarch_cannot_step_breakpoint (gdbarch, 1);
+
+  /* "nonsteppable" wachpoint means that watchpoint triggers before
+     instruction is committed, therefore it is required to remove watchpoint
+     to step though instruction that triggers it.  ARC watchpoints trigger
+     only after instruction is committed, thus there is no need to remove
+     them.  In fact on ARC watchpoint for memory writes may trigger with more
+     significant delay, like one or two instructions, depending on type of
+     memory where write is performed (CCM or external) and next instruction
+     after the memory write.  That value used to be set to "1", but that is
+     not correct for ARC - that was causing GDB to step one additional
+     instruction after watchpoint is triggered and that is not needed.  */
+  set_gdbarch_have_nonsteppable_watchpoint (gdbarch, 0);
+
+  /* This doesn't include possible long-immediate value.  */
+  set_gdbarch_max_insn_length (gdbarch, 4);
+
+  /* Frame unwinders and sniffers.  */
+  dwarf2_frame_set_init_reg (gdbarch, arc_dwarf2_frame_init_reg);
+  dwarf2_append_unwinders (gdbarch);
+  frame_unwind_append_unwinder (gdbarch, &arc_frame_unwind);
+  frame_base_set_default (gdbarch, &arc_normal_base);
+
+  /* Setup stuff specific to a particular environment (baremetal or Linux).
+     It can override functions set earlier.  */
+  gdbarch_init_osabi (info, gdbarch);
+
+  tdesc_use_registers (gdbarch, tdesc, tdesc_data);
+
+  return gdbarch;
+}
+
+
+static void
+arc_dump_tdep (struct gdbarch *gdbarch, struct ui_file *file)
+{
+  /* Empty for now.  */
+}
+
+/* Dummy printf-like function.  Used by arc_initialize_disassembler, see
+   explanation of its purpose there.  */
+
+static int
+arc_scream_into_the_void (void *file, const char *f, ...)
+{
+  return 0;
+}
+
+/* Wrapper for the target_read_memory function.  */
+
+static int
+arc_read_memory_for_disassembler (bfd_vma memaddr, bfd_byte *myaddr,
+				  unsigned int length,
+				  struct disassemble_info *info)
+{
+  return target_read_memory ((CORE_ADDR) memaddr, (gdb_byte *) myaddr,
+			     (int) length);
+}
+
+
+void
+arc_initialize_disassembler (struct gdbarch *gdbarch,
+			     struct disassemble_info *info)
+{
+  /* opcodes disassembler will always print instruction to the stream, which
+     is not desired when instruction is disassembled for analysis.  Stream
+     cannot be set to NULL, because it is used unconditionally in the opcodes,
+     so the only way is to provide dummy printf-like function which will not
+     actually print anything anywhere.  */
+  init_disassemble_info (info, gdb_stderr, arc_scream_into_the_void);
+  info->arch = gdbarch_bfd_arch_info (gdbarch)->arch;
+  info->mach = gdbarch_bfd_arch_info (gdbarch)->mach;
+  info->endian = gdbarch_byte_order (gdbarch);
+  info->read_memory_func = arc_read_memory_for_disassembler;
+}
+
+
+void
+_initialize_arc_tdep (void)
+{
+  gdbarch_register (bfd_arch_arc, arc_gdbarch_init, arc_dump_tdep);
+
+  initialize_tdesc_arc_v2 ();
+  initialize_tdesc_arc_arcompact ();
+
+  /* Register ARC-specific commands with gdb.  */
+
+  /* Debug internals for ARC GDB.  */
+  add_setshow_zinteger_cmd ("arc", class_maintenance,
+			    &arc_debug,
+			    _("Set ARC specific debugging."),
+			    _("Show ARC specific debugging."),
+			    _("Non-zero enables ARC specific debugging."),
+			    NULL, NULL, &setdebuglist, &showdebuglist);
+}
diff --git a/gdb/arc-tdep.h b/gdb/arc-tdep.h
new file mode 100644
index 0000000..9e47167
--- /dev/null
+++ b/gdb/arc-tdep.h
@@ -0,0 +1,133 @@ 
+/* Target dependent code for ARC arhitecture, for GDB.
+
+   Copyright 2005-2016 Free Software Foundation, Inc.
+   Contributed by Synopsys 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 ARC_TDEP_H
+#define ARC_TDEP_H
+
+/* Need disassemble_info.  */
+#include "dis-asm.h"
+
+/* Certain limitations are imposed on GDB register numbers to simplify logic
+   in the GDB.  Those limitations apply to both ARCompact and ARC v2.
+
+   1.  ARC can have up to 64 core registers, and each one of them has same GDB
+   regnum as an architectural number.  So R0 always has regnum 0, LP_COUNT
+   always has regnum 60.
+
+   2.  PC always has regnum 64.  That register is mandatory.
+
+   3.  LP_START and LP_END has regnums 65 and 66 respectively, those registers
+   are optional, and those register numbers shall not be occupied by other
+   registers when LP_START and LP_END are not present.
+
+   4.  STATUS32 always has regnum 67.  */
+
+enum arc_regnum
+  {
+    /* Core registers.  */
+    ARC_R0_REGNUM = 0,
+    ARC_FIRST_CORE_REGNUM = ARC_R0_REGNUM,
+    ARC_R1_REGNUM = 1,
+    ARC_R4_REGNUM = 4,
+    ARC_R7_REGNUM = 7,
+    ARC_R9_REGNUM = 9,
+    ARC_R13_REGNUM = 13,
+    ARC_R16_REGNUM = 16,
+    ARC_R25_REGNUM = 25,
+    /* Global data pointer.  */
+    ARC_GP_REGNUM,
+    /* Frame pointer.  */
+    ARC_FP_REGNUM,
+    /* Stack pointer.  */
+    ARC_SP_REGNUM,
+    /* Return address from interrupt.  */
+    ARC_ILINK_REGNUM,
+    ARC_R30_REGNUM,
+    /* Return address from function.  */
+    ARC_BLINK_REGNUM,
+    /* Zero-delay loop counter.  */
+    ARC_LP_COUNT_REGNUM = 60,
+    /* Reserved.  */
+    ARC_RESERVED_REGNUM,
+    /* Long immediate value (not a physical register.  */
+    ARC_LIMM_REGNUM,
+    /* Program counter, aligned to 4-bytes, read-only.  */
+    ARC_PCL_REGNUM,
+    ARC_LAST_CORE_REGNUM = ARC_PCL_REGNUM,
+    /* AUX registers.  */
+    /* Actual program counter.  */
+    ARC_PC_REGNUM,
+    ARC_FIRST_AUX_REGNUM = ARC_PC_REGNUM,
+    /* Zero-delay loop start instruction.  */
+    ARC_LP_START_REGNUM,
+    /* Zero-delay loop next-after-last instruction.  */
+    ARC_LP_END_REGNUM,
+    /* Status register.  */
+    ARC_STATUS32_REGNUM,
+    ARC_LAST_REGNUM = ARC_STATUS32_REGNUM,
+    ARC_LAST_AUX_REGNUM = ARC_STATUS32_REGNUM,
+
+    /* Additional ABI constants.  */
+    ARC_FIRST_ARG_REGNUM = ARC_R0_REGNUM,
+    ARC_LAST_ARG_REGNUM = ARC_R7_REGNUM,
+    ARC_FIRST_CALLEE_SAVED_REGNUM = ARC_R13_REGNUM,
+    ARC_LAST_CALLEE_SAVED_REGNUM = ARC_R25_REGNUM,
+  };
+
+#define BYTES_IN_REGISTER  4
+
+#define arc_print(fmt, args...) fprintf_unfiltered (gdb_stdlog, fmt, ##args)
+
+extern int arc_debug;
+
+/* Target-dependent information.  */
+
+struct gdbarch_tdep
+  {
+  };
+
+
+void _initialize_arc_tdep (void);
+
+/* Utility functions used by other ARC-specific modules.  */
+
+void arc_initialize_disassembler (struct gdbarch *gdbarch,
+				  struct disassemble_info *info);
+
+static inline int
+arc_mach_is_arc600 (struct gdbarch *gdbarch)
+{
+  return gdbarch_bfd_arch_info (gdbarch)->mach == bfd_mach_arc_arc600
+    || gdbarch_bfd_arch_info (gdbarch)->mach == bfd_mach_arc_arc601;
+}
+
+static inline int
+arc_mach_is_arc700 (struct gdbarch *gdbarch)
+{
+  return gdbarch_bfd_arch_info (gdbarch)->mach == bfd_mach_arc_arc700;
+}
+
+static inline int
+arc_mach_is_arcv2 (struct gdbarch *gdbarch)
+{
+  return gdbarch_bfd_arch_info (gdbarch)->mach == bfd_mach_arc_arcv2;
+}
+
+#endif /* ARC_TDEP_H */
diff --git a/gdb/configure.tgt b/gdb/configure.tgt
index 7f1aac3..ef041de 100644
--- a/gdb/configure.tgt
+++ b/gdb/configure.tgt
@@ -83,6 +83,11 @@  am33_2.0*-*-linux*)
 			solib-svr4.o"
 	;;
 
+arc*-*-*)
+	# Target: baremetal ARC elf32 target
+	gdb_target_obs="arc-tdep.o"
+	;;
+
 arm*-wince-pe | arm*-*-mingw32ce*)
 	# Target: ARM based machine running Windows CE (win32)
 	gdb_target_obs="arm.o arm-get-next-pcs.o arm-tdep.o \
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index d1a5e7c..c1edf14 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -22017,6 +22017,7 @@  acceptable commands.
 
 
 @menu
+* ARC::                         Synopsys ARC
 * ARM::                         ARM
 * M68K::                        Motorola M68K
 * MicroBlaze::			Xilinx MicroBlaze
@@ -22027,6 +22028,30 @@  acceptable commands.
 * Super-H::                     Renesas Super-H
 @end menu
 
+@node ARC
+@subsection Synopsys ARC
+@cindex Synopsys ARC
+@cindex ARC specific commands
+@cindex ARC600
+@cindex ARC700
+@cindex ARC EM
+@cindex ARC HS
+
+@value{GDBN} provides the following ARC-specific commands:
+
+@table @code
+@item set debug arc
+@kindex set debug arc
+Control the level of ARC specific debug messages.  Use 0 for no messages (the
+default) and 1 for debug messages.  At present higher values offer no further
+messages.
+
+@item show debug arc
+@kindex show debug arc
+Show the level of ARC specific debugging in operation.
+
+@end table
+
 @node ARM
 @subsection ARM
 
@@ -40907,6 +40932,7 @@  registers using the capitalization used in the description.
 
 @menu
 * AArch64 Features::
+* ARC Features::
 * ARM Features::
 * i386 Features::
 * MicroBlaze Features::
@@ -40932,6 +40958,69 @@  The @samp{org.gnu.gdb.aarch64.fpu} feature is optional.  If present,
 it should contain registers @samp{v0} through @samp{v31}, @samp{fpsr},
 and @samp{fpcr}.
 
+@node ARC Features
+@subsection ARC Features
+@cindex target descriptions, ARC Features
+
+ARC processors are highly configurable, so even core registers and their amount
+are not completely predetermined.  In addition flags and PC registers which are
+important to GDB are not "core" registers in ARC.  To simplify internal GDB
+design some requirements are applied to ARC target descriptions:
+
+@itemize @bullet
+@item
+One of the core registers features must be present.  At all occasions GDB
+register number must be equal to architectural number of that register, so for
+example regnum of @samp{blink} is always 31.
+
+@item @samp{org.gnu.gdb.arc.aux-minimal} is mandatory and registers in it has
+fixed GDB register numbers.
+
+@item All other features are optional and cannot have registers with register
+number less then 68.
+
+@end itemize
+
+The @samp{org.gnu.gdb.arc.core.v2} feature is required for ARC EM and ARC HS
+targets with a normal register file.  It should contain registers @samp{r0}
+through @samp{r25}, @samp{gp}, @samp{fp}, @samp{sp}, @samp{r30}, @samp{blink},
+@samp{lp_count} and @samp{pcl}.  This feature may contain register
+@samp{ilink} and any of extension core registers @samp{r32} through
+@samp{r59/acch}.  That feature may contain @samp{reserved} and @samp{limm}
+registers, however since those are not real registers, it is not possible to
+write or read them.  @samp{ilink} and extension core registers are not
+available to read/write, when debugging Linux applications, thus ilink is made
+optional.
+
+The @samp{org.gnu.gdb.arc.core-reduced.v2} feature is required for ARC EM and
+ARC HS targets with a reduced register file.  It should contain registers
+@samp{r0} through @samp{r3}, @samp{r10} through @samp{r15}, @samp{gp},
+@samp{fp}, @samp{sp}, @samp{r30}, @samp{blink}, @samp{lp_count} and
+@samp{pcl}.  This feature may contain register @samp{ilink} and any of
+extension core registers @samp{r32} through @samp{r59/acch}.  That feature may
+contain @samp{reserved} and @samp{limm} registers, however since those are not
+real registers, it is not possible to write or read them.
+
+The @samp{org.gnu.gdb.arc.core.arcompact} feature is required for ARCompact
+targets with a normal register file.  It should contain registers @samp{r0}
+through @samp{r25}, @samp{gp}, @samp{fp}, @samp{sp}, @samp{r30}, @samp{blink},
+@samp{lp_count} and @samp{pcl}.  This feature may contain registers
+@samp{ilink1}, @samp{ilink2} and any of extension core registers @samp{r32}
+through @samp{r59/acch}.  This feature may contain @samp{reserved} and
+@samp{limm} registers, however since those are not real registers, it is not
+possible to write or read them.  @samp{ilink1} and @samp{ilink2} and extension
+core registers are not available when debugging Linux applications.  The only
+difference with @samp{org.gnu.gdb.arc.core.v2} feature is in the names of
+@samp{ilink1} and @samp{ilink2} registers and that @samp{r30} is mandatory in
+ARC v2, but @samp{ilink2} is optional on ARCompact.
+
+The @samp{org.gnu.gdb.arc.aux-minimal} feature is required for all ARC
+targets.  It should contain registers @samp{pc} and @samp{status32}.  It may
+contain registers @samp{lp_start} and @samp{lp_end}.  For compatibility with
+older GDB-server implementations regnums in this feature are fixed: @samp{pc}
+is always 64, @samp{lp_start} and @samp{lp_end} are 65 and 66 respectively if
+present, @samp{status32} is always 67.
+
 @node ARM Features
 @subsection ARM Features
 @cindex target descriptions, ARM features
diff --git a/gdb/features/Makefile b/gdb/features/Makefile
index 809c811..6b8557b 100644
--- a/gdb/features/Makefile
+++ b/gdb/features/Makefile
@@ -148,6 +148,8 @@  OUTPUTS = $(patsubst %,$(outdir)/%.dat,$(WHICH))
 # to make on the command line.
 XMLTOC = \
 	aarch64.xml \
+	arc/arc.v2.xml \
+	arc/arcompact.xml \
 	arm-with-iwmmxt.xml \
 	arm-with-m-fpa-layout.xml \
 	arm-with-m-vfp-d16.xml \
diff --git a/gdb/features/arc-arcompact.c b/gdb/features/arc-arcompact.c
new file mode 100644
index 0000000..2107f82
--- /dev/null
+++ b/gdb/features/arc-arcompact.c
@@ -0,0 +1,77 @@ 
+/* THIS FILE IS GENERATED.  -*- buffer-read-only: t -*- vi:set ro:
+  Original: arc-arcompact.xml */
+
+#include "defs.h"
+#include "osabi.h"
+#include "target-descriptions.h"
+
+struct target_desc *tdesc_arc_arcompact;
+static void
+initialize_tdesc_arc_arcompact (void)
+{
+  struct target_desc *result = allocate_target_description ();
+  struct tdesc_feature *feature;
+  struct tdesc_type *field_type;
+  struct tdesc_type *type;
+
+  set_tdesc_architecture (result, bfd_scan_arch ("ARC700"));
+
+  feature = tdesc_create_feature (result, "org.gnu.gdb.arc.core.arcompact");
+  tdesc_create_reg (feature, "r0", 0, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r1", 1, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r2", 2, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r3", 3, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r4", 4, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r5", 5, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r6", 6, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r7", 7, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r8", 8, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r9", 9, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r10", 10, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r11", 11, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r12", 12, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r13", 13, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r14", 14, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r15", 15, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r16", 16, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r17", 17, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r18", 18, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r19", 19, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r20", 20, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r21", 21, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r22", 22, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r23", 23, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r24", 24, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r25", 25, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "gp", 26, 1, NULL, 32, "data_ptr");
+  tdesc_create_reg (feature, "fp", 27, 1, NULL, 32, "data_ptr");
+  tdesc_create_reg (feature, "sp", 28, 1, NULL, 32, "data_ptr");
+  tdesc_create_reg (feature, "ilink1", 29, 1, NULL, 32, "code_ptr");
+  tdesc_create_reg (feature, "ilink2", 30, 1, NULL, 32, "code_ptr");
+  tdesc_create_reg (feature, "blink", 31, 1, NULL, 32, "code_ptr");
+  tdesc_create_reg (feature, "lp_count", 60, 1, NULL, 32, "uint32");
+  tdesc_create_reg (feature, "pcl", 63, 1, "", 32, "code_ptr");
+
+  feature = tdesc_create_feature (result, "org.gnu.gdb.arc.aux-minimal");
+  type = tdesc_create_flags (feature, "status32_type", 4);
+  tdesc_add_bitfield (type, "H", 0, 0);
+  tdesc_add_bitfield (type, "E", 1, 2);
+  tdesc_add_bitfield (type, "A", 3, 4);
+  tdesc_add_bitfield (type, "AE", 5, 5);
+  tdesc_add_bitfield (type, "DE", 6, 6);
+  tdesc_add_bitfield (type, "U", 7, 7);
+  tdesc_add_bitfield (type, "V", 8, 8);
+  tdesc_add_bitfield (type, "C", 9, 9);
+  tdesc_add_bitfield (type, "N", 10, 10);
+  tdesc_add_bitfield (type, "Z", 11, 11);
+  tdesc_add_bitfield (type, "L", 12, 12);
+  tdesc_add_bitfield (type, "R", 13, 13);
+  tdesc_add_bitfield (type, "SE", 14, 14);
+
+  tdesc_create_reg (feature, "pc", 64, 1, NULL, 32, "code_ptr");
+  tdesc_create_reg (feature, "lp_start", 65, 1, NULL, 32, "code_ptr");
+  tdesc_create_reg (feature, "lp_end", 66, 1, NULL, 32, "code_ptr");
+  tdesc_create_reg (feature, "status32", 67, 1, NULL, 32, "status32_type");
+
+  tdesc_arc_arcompact = result;
+}
diff --git a/gdb/features/arc-arcompact.xml b/gdb/features/arc-arcompact.xml
new file mode 100644
index 0000000..b6dadc2
--- /dev/null
+++ b/gdb/features/arc-arcompact.xml
@@ -0,0 +1,83 @@ 
+<?xml version="1.0"?>
+<!-- Copyright (C) 2015-2016 Free Software Foundation, Inc.
+
+     Copying and distribution of this file, with or without modification,
+     are permitted in any medium without royalty provided the copyright
+     notice and this notice are preserved.  -->
+
+<!DOCTYPE feature SYSTEM "gdb-target.dtd">
+<target>
+  <architecture>arc:ARC700</architecture>
+  <!-- No OSABI for bare metal -->
+  <!-- No compatibility for ARC -->
+
+  <feature name="org.gnu.gdb.arc.core.arcompact">
+    <reg name="r0"  bitsize="32"/>
+    <reg name="r1"  bitsize="32"/>
+    <reg name="r2"  bitsize="32"/>
+    <reg name="r3"  bitsize="32"/>
+    <reg name="r4"  bitsize="32"/>
+    <reg name="r5"  bitsize="32"/>
+    <reg name="r6"  bitsize="32"/>
+    <reg name="r7"  bitsize="32"/>
+    <reg name="r8"  bitsize="32"/>
+    <reg name="r9"  bitsize="32"/>
+    <reg name="r10" bitsize="32"/>
+    <reg name="r11" bitsize="32"/>
+    <reg name="r12" bitsize="32"/>
+    <reg name="r13" bitsize="32"/>
+    <reg name="r14" bitsize="32"/>
+    <reg name="r15" bitsize="32"/>
+    <reg name="r16" bitsize="32"/>
+    <reg name="r17" bitsize="32"/>
+    <reg name="r18" bitsize="32"/>
+    <reg name="r19" bitsize="32"/>
+    <reg name="r20" bitsize="32"/>
+    <reg name="r21" bitsize="32"/>
+    <reg name="r22" bitsize="32"/>
+    <reg name="r23" bitsize="32"/>
+    <reg name="r24" bitsize="32"/>
+    <reg name="r25" bitsize="32"/>
+
+    <!-- ARC core pointer registers -->
+    <reg name="gp"  bitsize="32" type="data_ptr"/>
+    <reg name="fp"  bitsize="32" type="data_ptr"/>
+    <reg name="sp"  bitsize="32" type="data_ptr"/>
+
+    <!-- Code pointers.  -->
+    <reg name="ilink1" bitsize="32" type="code_ptr"/>
+    <reg name="ilink2" bitsize="32" type="code_ptr"/>
+    <reg name="blink"  bitsize="32" type="code_ptr"/>
+
+    <!-- Here goes extension core registers: r32 - r59 -->
+
+    <!-- More core registers... -->
+    <reg name="lp_count" bitsize="32" type="uint32" regnum="60"/>
+    <!-- r61 is reserved -->
+    <!-- r62 is long immediate value, not a real register -->
+    <reg name="pcl"      bitsize="32" group="" type="code_ptr" regnum="63"/>
+  </feature>
+
+  <feature name="org.gnu.gdb.arc.aux-minimal">
+    <flags id="status32_type" size="4">
+	<field name="H"  start="0" end="0"/>
+	<field name="E"  start="1" end="2"/>
+	<field name="A"  start="3" end="4"/>
+	<field name="AE" start="5" end="5"/>
+	<field name="DE" start="6" end="6"/>
+	<field name="U"  start="7" end="7"/>
+	<field name="V"  start="8" end="8"/>
+	<field name="C"  start="9" end="9"/>
+	<field name="N"  start="10" end="10"/>
+	<field name="Z"  start="11" end="11"/>
+	<field name="L"  start="12" end="12"/>
+	<field name="R"  start="13" end="13"/>
+	<field name="SE" start="14" end="14"/>
+    </flags>
+
+    <reg name="pc"       bitsize="32" regnum="64" type="code_ptr"/>
+    <reg name="lp_start" bitsize="32" regnum="65" type="code_ptr"/>
+    <reg name="lp_end"   bitsize="32" regnum="66" type="code_ptr"/>
+    <reg name="status32" bitsize="32" regnum="67" type="status32_type"/>
+  </feature>
+</target>
diff --git a/gdb/features/arc-v2.c b/gdb/features/arc-v2.c
new file mode 100644
index 0000000..e4aacf1
--- /dev/null
+++ b/gdb/features/arc-v2.c
@@ -0,0 +1,81 @@ 
+/* THIS FILE IS GENERATED.  -*- buffer-read-only: t -*- vi:set ro:
+  Original: arc-v2.xml */
+
+#include "defs.h"
+#include "osabi.h"
+#include "target-descriptions.h"
+
+struct target_desc *tdesc_arc_v2;
+static void
+initialize_tdesc_arc_v2 (void)
+{
+  struct target_desc *result = allocate_target_description ();
+  struct tdesc_feature *feature;
+  struct tdesc_type *field_type;
+  struct tdesc_type *type;
+
+  set_tdesc_architecture (result, bfd_scan_arch ("ARCv2"));
+
+  feature = tdesc_create_feature (result, "org.gnu.gdb.arc.core.v2");
+  tdesc_create_reg (feature, "r0", 0, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r1", 1, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r2", 2, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r3", 3, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r4", 4, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r5", 5, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r6", 6, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r7", 7, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r8", 8, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r9", 9, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r10", 10, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r11", 11, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r12", 12, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r13", 13, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r14", 14, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r15", 15, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r16", 16, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r17", 17, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r18", 18, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r19", 19, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r20", 20, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r21", 21, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r22", 22, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r23", 23, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r24", 24, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r25", 25, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "gp", 26, 1, NULL, 32, "data_ptr");
+  tdesc_create_reg (feature, "fp", 27, 1, NULL, 32, "data_ptr");
+  tdesc_create_reg (feature, "sp", 28, 1, NULL, 32, "data_ptr");
+  tdesc_create_reg (feature, "ilink", 29, 1, NULL, 32, "code_ptr");
+  tdesc_create_reg (feature, "r30", 30, 1, "", 32, "int");
+  tdesc_create_reg (feature, "blink", 31, 1, NULL, 32, "code_ptr");
+  tdesc_create_reg (feature, "lp_count", 60, 1, NULL, 32, "uint32");
+  tdesc_create_reg (feature, "pcl", 63, 1, "", 32, "code_ptr");
+
+  feature = tdesc_create_feature (result, "org.gnu.gdb.arc.aux-minimal");
+  type = tdesc_create_flags (feature, "status32_type", 4);
+  tdesc_add_bitfield (type, "H", 0, 0);
+  tdesc_add_bitfield (type, "E", 1, 4);
+  tdesc_add_bitfield (type, "AE", 5, 5);
+  tdesc_add_bitfield (type, "DE", 6, 6);
+  tdesc_add_bitfield (type, "U", 7, 7);
+  tdesc_add_bitfield (type, "V", 8, 8);
+  tdesc_add_bitfield (type, "C", 9, 9);
+  tdesc_add_bitfield (type, "N", 10, 10);
+  tdesc_add_bitfield (type, "Z", 11, 11);
+  tdesc_add_bitfield (type, "L", 12, 12);
+  tdesc_add_bitfield (type, "DZ", 13, 13);
+  tdesc_add_bitfield (type, "SC", 14, 14);
+  tdesc_add_bitfield (type, "ES", 15, 15);
+  tdesc_add_bitfield (type, "RB", 16, 18);
+  tdesc_add_bitfield (type, "AD", 19, 19);
+  tdesc_add_bitfield (type, "US", 20, 20);
+  tdesc_add_bitfield (type, "IE", 31, 31);
+
+  tdesc_create_reg (feature, "pc", 64, 1, NULL, 32, "code_ptr");
+  tdesc_create_reg (feature, "lp_start", 65, 1, NULL, 32, "code_ptr");
+  tdesc_create_reg (feature, "lp_end", 66, 1, NULL, 32, "code_ptr");
+  tdesc_create_reg (feature, "status32", 67, 1, NULL, 32, "status32_type");
+
+  tdesc_arc_v2 = result;
+}
diff --git a/gdb/features/arc-v2.xml b/gdb/features/arc-v2.xml
new file mode 100644
index 0000000..ed2628b
--- /dev/null
+++ b/gdb/features/arc-v2.xml
@@ -0,0 +1,90 @@ 
+<?xml version="1.0"?>
+<!-- Copyright (C) 2015-2016 Free Software Foundation, Inc.
+
+     Copying and distribution of this file, with or without modification,
+     are permitted in any medium without royalty provided the copyright
+     notice and this notice are preserved.  -->
+
+<!DOCTYPE target SYSTEM "gdb-target.dtd">
+<target>
+  <architecture>arc:ARCv2</architecture>
+  <!-- No OSABI for bare metal -->
+  <!-- No compatibility for ARC -->
+
+  <feature name="org.gnu.gdb.arc.core.v2">
+    <reg name="r0"  bitsize="32"/>
+    <reg name="r1"  bitsize="32"/>
+    <reg name="r2"  bitsize="32"/>
+    <reg name="r3"  bitsize="32"/>
+    <reg name="r4"  bitsize="32"/>
+    <reg name="r5"  bitsize="32"/>
+    <reg name="r6"  bitsize="32"/>
+    <reg name="r7"  bitsize="32"/>
+    <reg name="r8"  bitsize="32"/>
+    <reg name="r9"  bitsize="32"/>
+    <reg name="r10" bitsize="32"/>
+    <reg name="r11" bitsize="32"/>
+    <reg name="r12" bitsize="32"/>
+    <reg name="r13" bitsize="32"/>
+    <reg name="r14" bitsize="32"/>
+    <reg name="r15" bitsize="32"/>
+    <reg name="r16" bitsize="32"/>
+    <reg name="r17" bitsize="32"/>
+    <reg name="r18" bitsize="32"/>
+    <reg name="r19" bitsize="32"/>
+    <reg name="r20" bitsize="32"/>
+    <reg name="r21" bitsize="32"/>
+    <reg name="r22" bitsize="32"/>
+    <reg name="r23" bitsize="32"/>
+    <reg name="r24" bitsize="32"/>
+    <reg name="r25" bitsize="32"/>
+
+    <!-- ARC core pointer registers -->
+    <reg name="gp"  bitsize="32" type="data_ptr"/>
+    <reg name="fp"  bitsize="32" type="data_ptr"/>
+    <reg name="sp"  bitsize="32" type="data_ptr"/>
+
+    <!-- Code pointers.  R30 is general purpose, but it used to be ILINK2 in
+    ARCompact, thus its odd position in between of special purpose registers.
+    GCC does't use this register, so it isn't a member of a general group. -->
+    <reg name="ilink" bitsize="32" type="code_ptr"/>
+    <reg name="r30"   bitsize="32" group=""/>
+    <reg name="blink" bitsize="32" type="code_ptr"/>
+
+    <!-- Here goes extension core registers: r32 - r57 -->
+    <!-- Here goes ACCL/ACCH registers, r58, r59 -->
+
+    <!-- More core registers... -->
+    <reg name="lp_count" bitsize="32" type="uint32" regnum="60"/>
+    <!-- r61 is reserved -->
+    <!-- r62 is long immediate value, not a real register -->
+    <reg name="pcl" bitsize="32" type="code_ptr" regnum="63" group=""/>
+  </feature>
+
+  <feature name="org.gnu.gdb.arc.aux-minimal">
+    <flags id="status32_type" size="4">
+	<field name="H"   start="0" end="0"/>
+	<field name="E"   start="1" end="4"/>
+	<field name="AE"  start="5" end="5"/>
+	<field name="DE"  start="6" end="6"/>
+	<field name="U"   start="7" end="7"/>
+	<field name="V"   start="8" end="8"/>
+	<field name="C"   start="9" end="9"/>
+	<field name="N"   start="10" end="10"/>
+	<field name="Z"   start="11" end="11"/>
+	<field name="L"   start="12" end="12"/>
+	<field name="DZ"  start="13" end="13"/>
+	<field name="SC"  start="14" end="14"/>
+	<field name="ES"  start="15" end="15"/>
+	<field name="RB"  start="16" end="18"/>
+	<field name="AD"  start="19" end="19"/>
+	<field name="US"  start="20" end="20"/>
+	<field name="IE"  start="31" end="31"/>
+    </flags>
+
+    <reg name="pc"       bitsize="32" regnum="64" type="code_ptr"/>
+    <reg name="lp_start" bitsize="32" regnum="65" type="code_ptr"/>
+    <reg name="lp_end"   bitsize="32" regnum="66" type="code_ptr"/>
+    <reg name="status32" bitsize="32" regnum="67" type="status32_type"/>
+  </feature>
+</target>
diff --git a/gdb/testsuite/gdb.xml/tdesc-regs.exp b/gdb/testsuite/gdb.xml/tdesc-regs.exp
index c197e28..311917f 100644
--- a/gdb/testsuite/gdb.xml/tdesc-regs.exp
+++ b/gdb/testsuite/gdb.xml/tdesc-regs.exp
@@ -30,6 +30,10 @@  switch -glob -- [istarget] {
     "aarch64*-*-*" {
 	set core-regs {aarch64-core.xml}
     }
+    "arc*-*-*" {
+	set architecture "arc:ARCv2"
+	set core-regs {arc-v2.xml}
+    }
     "arm*-*-*" {
         set core-regs {arm-core.xml}
     }