[PING,v3] gdb: ADI support

  The M7 processor supports an Application Data Integrity (ADI) feature
   that detects invalid data accesses.  When software allocates data, it
   chooses a 4-bit version number, sets the version in the upper 4 bits
   of the 64-bit pointer to that data, and stores the 4-bit version in
   every cacheline of the object.  Hardware saves the latter in spare
   bits in the cache and memory hierarchy. On each load and store, the
   processor compares the upper 4 VA (virtual address) bits to the
   cacheline's version. If there is a mismatch, the processor generates
   a version mismatch trap which can be either precise or disrupting.
   The trap is an error condition which the kernel delivers to the process
   as a SIGSEGV signal.

   The upper 4 bits of the VA represent a version and are not part of the
   true address.  The processor clears these bits and sign extends bit 59
   to generate the true address.

   Note that 32-bit applications cannot use ADI.

   This patch adds ADI support in gdb which allows the user to examine
   current version tags and assign new version tags in the program.
   It also catches and reports precise or disrupting memory corruption
   traps.

Tested in sparc64-linux-gnu. No regressions.

gdb/ChangeLog:
2017-06-26  Weimin Pan  <weimin.pan@oracle.com>
	* sparc64-tdep.h: (adi_normalize_address): New export.
	* sparc-nat.h: (open_adi_tag_fd): New export.
	* sparc64-linux-nat.c: (open_adi_tag_fd): New function.
	* sparc64-linux-tdep.c:
	(SEGV_ACCADI, SEGV_ADIDERR, SEGV_ADIPERR) New defines.
	(sparc64_linux_handle_segmentation_fault): New function.
	(sparc64_linux_init_abi): Register
	sparc64_linux_handle_segmentation_fault
	* sparc64-tdep.c: Include cli-utils.h,gdbcmd.h,auxv.h,sparc-nat.h.
	(sparc64_pstate_type): Replace PID1 with MCDE.
	(sparc64_addr_bits_remove): New function.
	(sparc64_init_abi): Register sparc64_addr_bits_remove.
	(MAX_PROC_NAME_SIZE): New macro.
	(AT_ADI_BLKSZ, AT_ADI_NBITS, AT_ADI_UEONADI) New defines.
	(sparc64adilist): New variable.
	(adi_proc_list): New variable.
	(find_adi_info): New function.
	(add_adi_info): New function.
	(get_adi_info_proc): New function.
	(get_adi_info): New function.
	(info_adi_command): New function.
	(read_maps_entry): New function.
	(adi_available): New function.
	(adi_normalize_address): New function.
	(adi_align_address): New function.
	(adi_convert_byte_count): New function.
	(adi_tag_fd): New function.
	(adi_is_addr_mapped): New function.
	(adi_read_versions): New function.
	(adi_write_versions): New function.
	(adi_print_versions): New function.
	(do_examine): New function.
	(do_assign): New function.
	(adi_examine_command): New function.
	(adi_assign_command): New function.
	(_initialize_sparc64_tdep): New function.

gdb/doc/ChangeLog:
2017-06-26  Weimin Pan  <weimin.pan@oracle.com>
	* gdb.texinfo (Architectures): Add new Sparc64 section to document
	ADI support.
	* NEWS: Add "adi examine" and "adi assign" commands.
---
 gdb/ChangeLog            |   39 ++++
 gdb/doc/ChangeLog        |    5 +
 gdb/doc/gdb.texinfo      |   85 ++++++++
 gdb/sparc-nat.h          |    2 +
 gdb/sparc64-linux-nat.c  |   14 ++
 gdb/sparc64-linux-tdep.c |   68 ++++++
 gdb/sparc64-tdep.c       |  508 +++++++++++++++++++++++++++++++++++++++++++++-
 gdb/sparc64-tdep.h       |    2 +
 8 files changed, 722 insertions(+), 1 deletions(-)
  

Weimin Pan <weimin.pan@oracle.com> writes:

> Tested in sparc64-linux-gnu. No regressions.
>

Did you build a cross gdb on x86-linux?  At least it breaks the build
for all targets, I configure gdb this way,

"../binutils-gdb/configure --enable-sim --disable-binutils
--disable-gprof --disable-gold --disable-gas --disable-ld
--enable-targets=all --enable-64-bit-bfd"

sparc64-tdep.o: In function `adi_tag_fd()':
build-gdb/gdb/../../binutils-gdb/gdb/sparc64-tdep.c:1915: undefined reference to `open_adi_tag_fd(int)'

because sparc64-tdep.o references function defined in sparc64-linux-nat.c.

> gdb/ChangeLog:
> 2017-06-26  Weimin Pan  <weimin.pan@oracle.com>

A blank line is needed here.

> 	* sparc64-tdep.h: (adi_normalize_address): New export.
> 	* sparc-nat.h: (open_adi_tag_fd): New export.
> 	* sparc64-linux-nat.c: (open_adi_tag_fd): New function.
> 	* sparc64-linux-tdep.c:
> 	(SEGV_ACCADI, SEGV_ADIDERR, SEGV_ADIPERR) New defines.
> 	(sparc64_linux_handle_segmentation_fault): New function.
> 	(sparc64_linux_init_abi): Register
> 	sparc64_linux_handle_segmentation_fault
> 	* sparc64-tdep.c: Include cli-utils.h,gdbcmd.h,auxv.h,sparc-nat.h.
> 	(sparc64_pstate_type): Replace PID1 with MCDE.
> 	(sparc64_addr_bits_remove): New function.
> 	(sparc64_init_abi): Register sparc64_addr_bits_remove.
> 	(MAX_PROC_NAME_SIZE): New macro.
> 	(AT_ADI_BLKSZ, AT_ADI_NBITS, AT_ADI_UEONADI) New defines.
> 	(sparc64adilist): New variable.
> 	(adi_proc_list): New variable.
> 	(find_adi_info): New function.
> 	(add_adi_info): New function.
> 	(get_adi_info_proc): New function.
> 	(get_adi_info): New function.
> 	(info_adi_command): New function.
> 	(read_maps_entry): New function.
> 	(adi_available): New function.
> 	(adi_normalize_address): New function.
> 	(adi_align_address): New function.
> 	(adi_convert_byte_count): New function.
> 	(adi_tag_fd): New function.
> 	(adi_is_addr_mapped): New function.
> 	(adi_read_versions): New function.
> 	(adi_write_versions): New function.
> 	(adi_print_versions): New function.
> 	(do_examine): New function.
> 	(do_assign): New function.
> 	(adi_examine_command): New function.
> 	(adi_assign_command): New function.
> 	(_initialize_sparc64_tdep): New function.
>
> gdb/doc/ChangeLog:
> 2017-06-26  Weimin Pan  <weimin.pan@oracle.com>
> 	* gdb.texinfo (Architectures): Add new Sparc64 section to document
> 	ADI support.
> 	* NEWS: Add "adi examine" and "adi assign" commands.

Can you add a test case?

> +
> +Below is an example of displaying ADI versions of variable "shmaddr".
> +
> +@smallexample
> +(@value{GDBP}) adi x/100 shmaddr
> +   0xfff800010002c000:     0 0
> +@end smallexample

Why "0" is printed twice?

> +void
> +sparc64_linux_handle_segmentation_fault (struct gdbarch *gdbarch,
> +				      struct ui_out *uiout)
> +{
> +  if (gdbarch_bfd_arch_info (gdbarch)->bits_per_word != 64)
> +    return;
> +
> +  CORE_ADDR addr = 0;
> +  long si_code = 0;
> +
> +  TRY
> +    {
> +      /* Evaluate si_code to see if the segfault is ADI related.  */
> +      si_code = parse_and_eval_long ("$_siginfo.si_code\n");
> +
> +      if (si_code >= SEGV_ACCADI && si_code <= SEGV_ADIPERR)
> +        addr = parse_and_eval_long ("$_siginfo._sifields._sigfault.si_addr");
> +    }
> +  CATCH (exception, RETURN_MASK_ALL)
> +    {
> +      return;
> +    }
> +  END_CATCH
> +
> +  /* Print out ADI event based on sig_code value */
> +  switch (si_code)
> +    {
> +    case SEGV_ACCADI:	/* adi not enabled */
> +      uiout->text ("\n");
> +      uiout->field_string ("sigcode-meaning", _("ADI disabled"));
> +      uiout->text (_(" while accessing address "));
> +      uiout->field_fmt ("bound-access", "%s", paddress (gdbarch, addr));
> +      break;
> +    case SEGV_ADIDERR:	/* disrupting mismatch */
> +      uiout->text ("\n");
> +      uiout->field_string ("sigcode-meaning", _("ADI deferred mismatch"));
> +      uiout->text (_(" while accessing address "));
> +      uiout->field_fmt ("bound-access", "%s", paddress (gdbarch, addr));
> +      break;
> +    case SEGV_ADIPERR:	/* precise mismatch */
> +      uiout->text ("\n");
> +      uiout->field_string ("sigcode-meaning", _("ADI precise mismatch"));
> +      uiout->text (_(" while accessing address "));
> +      uiout->field_fmt ("bound-access", "%s", paddress (gdbarch, addr));
> +      break;
> +    default:
> +      break;
> +    }

It is good to have a test to check the results and outputs here.

> +}
> +
>  
>  /* Return the address of a system call's alternative return
>     address.  */
> @@ -338,6 +404,8 @@ sparc64_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
>    set_xml_syscall_file_name (gdbarch, XML_SYSCALL_FILENAME_SPARC64);
>    set_gdbarch_get_syscall_number (gdbarch,
>                                    sparc64_linux_get_syscall_number);
> +  set_gdbarch_handle_segmentation_fault (gdbarch,
> +					 sparc64_linux_handle_segmentation_fault);
>  }
>  
>  
> diff --git a/gdb/sparc64-tdep.c b/gdb/sparc64-tdep.c
> index 9e0e6b5..9fa8e13 100644
> --- a/gdb/sparc64-tdep.c
> +++ b/gdb/sparc64-tdep.c
> @@ -35,7 +35,11 @@
>  #include "target.h"
>  #include "value.h"
>  
> +#include "cli/cli-utils.h"
> +#include "gdbcmd.h"
> +#include "auxv.h"
>  #include "sparc64-tdep.h"
> +#include "sparc-nat.h"
>  

We can't include "nat.h" in "tdep.c".

>  /* This file implements the SPARC 64-bit ABI as defined by the
>     section "Low-Level System Information" of the SPARC Compliance
> @@ -165,7 +169,7 @@ sparc64_pstate_type (struct gdbarch *gdbarch)
>        append_flags_type_flag (type, 8, "TLE");
>        append_flags_type_flag (type, 9, "CLE");
>        append_flags_type_flag (type, 10, "PID0");
> -      append_flags_type_flag (type, 11, "PID1");
> +      append_flags_type_flag (type, 11, "MCDE");
>  

I don't understand your reply regarding to this change,

> This change, which indicated that the TSTATE reigster contained the
> "mcde" value, was requested by our kernel engineers.

How is it related to the adi support?  If it is not, split it to another
patch.  In any case, we need a technical reason for the change.

>        tdep->sparc64_pstate_type = type;
>      }
> @@ -1290,6 +1294,14 @@ sparc64_dwarf2_frame_init_reg (struct gdbarch *gdbarch, int regnum,
>      }
>  }
>  
> +/* sparc64_addr_bits_remove - remove useless address bits  */
> +
> +static CORE_ADDR
> +sparc64_addr_bits_remove (struct gdbarch *gdbarch, CORE_ADDR addr)
> +{
> +  return adi_normalize_address (addr);

adi_normalize_address can be a static function, and other .c file can
use it via gdbarch_addr_bits_remove.

> +}
> +
>  void
>  sparc64_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
>  {
> @@ -1342,6 +1354,8 @@ sparc64_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
>  
>    frame_unwind_append_unwinder (gdbarch, &sparc64_frame_unwind);
>    frame_base_set_default (gdbarch, &sparc64_frame_base);
> +
> +  set_gdbarch_addr_bits_remove (gdbarch, sparc64_addr_bits_remove);
>  }
>  
>  
> @@ -1666,3 +1680,495 @@ const struct sparc_fpregmap sparc64_bsd_fpregmap =
>    0 * 8,			/* %f0 */
>    32 * 8,			/* %fsr */
>  };
> +
> +/* The M7 processor supports an Application Data Integrity (ADI) feature
> +   that detects invalid data accesses.  When software allocates memory and 
> +   enables ADI on the allocated memory, it chooses a 4-bit version number, 
> +   sets the version in the upper 4 bits of the 64-bit pointer to that data, 
> +   and stores the 4-bit version in every cacheline of the object.  Hardware 
> +   saves the latter in spare bits in the cache and memory hierarchy. On each 
> +   load and store, the processor compares the upper 4 VA (virtual address) bits 
> +   to the cacheline's version. If there is a mismatch, the processor generates
> +   a version mismatch trap which can be either precise or disrupting.
> +   The trap is an error condition which the kernel delivers to the process
> +   as a SIGSEGV signal.
> +
> +   The upper 4 bits of the VA represent a version and are not part of the
> +   true address.  The processor clears these bits and sign extends bit 59
> +   to generate the true address.
> +
> +   Note that 32-bit applications cannot use ADI. */
> +
> +
> +#define MAX_PROC_NAME_SIZE sizeof("/proc/99999/lwp/9999/adi/lstatus")
> +
> +/* ELF Auxiliary vectors */
> +#ifndef AT_ADI_BLKSZ
> +#define AT_ADI_BLKSZ    34
> +#endif
> +#ifndef AT_ADI_NBITS
> +#define AT_ADI_NBITS    35
> +#endif
> +#ifndef AT_ADI_UEONADI
> +#define AT_ADI_UEONADI  36
> +#endif
> +
> +/* ADI command list.  */
> +static struct cmd_list_element *sparc64adilist = NULL;
> +
> +/* ADI stat settings.  */
> +struct adi_stat_t
> +{
> +  /* The ADI block size.  */
> +  unsigned long blksize;
> +
> +  /* Number of bits used for an ADI version tag which can be
> +   * used together with the shift value for an ADI version tag
> +   * to encode or extract the ADI version value in a pointer.  */
> +  unsigned long nbits;
> +
> +  /* The maximum ADI version tag value supported.  */
> +  int max_version;
> +
> +  /* ADI version tag file.  */
> +  int tag_fd;
> +
> +  /* Last ADI address examined.  */
> +  CORE_ADDR last_vaddr;
> +
> +  /* Last specified examination count.  */
> +  int last_cnt;
> +
> +  /* ADI availability check has been done.  */
> +  bool checked_avail;
> +
> +  /* ADI is available.  */
> +  bool is_avail;
> +
> +};
> +
> +/* Per-process ADI stat info.  */
> +
> +struct sparc64_adi_info
> +{
> +  /* The process identifier.  */
> +  pid_t pid;
> +
> +  /* The ADI stat.  */
> +  struct adi_stat_t stat;
> +
> +  /* Linked list.  */
> +  struct sparc64_adi_info *next;

Can you use C++ STL list?

> +};
> +
> +static struct sparc64_adi_info *adi_proc_list = NULL;
> +
> +/* Find ADI info for process PID.  */
> +
> +static struct sparc64_adi_info *
> +find_adi_info (pid_t pid)
> +{
> +  struct sparc64_adi_info *proc;
> +
> +  for (proc = adi_proc_list; proc; proc = proc->next)
> +    if (proc->pid == pid)
> +      return proc;
> +
> +  return NULL;
> +}
> +
> +/* Add ADI info for process PID.  Returns newly allocated info
> +   object.  */
> +
> +static struct sparc64_adi_info *
> +add_adi_info (pid_t pid)
> +{
> +  struct sparc64_adi_info *proc = XCNEW (struct sparc64_adi_info);
> +
> +  proc->pid = pid;
> +  proc->next = adi_proc_list;
> +  adi_proc_list = proc;
> +  proc->stat.is_avail = false;
> +  proc->stat.checked_avail = false;
> +  proc->stat.tag_fd = 0;
> +
> +  return proc;
> +}
> +
> +/* Get ADI info for process PID, creating one if it doesn't exist.  */
> +
> +static struct sparc64_adi_info * 
> +get_adi_info_proc (pid_t pid)
> +{
> +  struct sparc64_adi_info *proc;
> +
> +  proc = find_adi_info (pid);
> +  if (proc == NULL)
> +    proc = add_adi_info (pid);
> +
> +  return proc;
> +}

I don't see the place you remove elements from adi list.  You probably
need to implement sparc64_forget_process (in which you can delete the
adi element if the process exits), and hook it on via
"linux_nat_set_forget_process (t, sparc64_forget_process);".

> +
> +static struct adi_stat_t 
> +get_adi_info (pid_t pid)
> +{
> +  struct sparc64_adi_info *proc;
> +
> +  proc = get_adi_info_proc (pid);
> +  return proc->stat;
> +}
> +
> +static void
> +info_adi_command (char *args, int from_tty)
> +{
> +  printf_unfiltered ("\"adi\" must be followed by \"examine\" "
> +                     "or \"assign\".\n");
> +  help_list (sparc64adilist, "adi ", all_commands, gdb_stdout);
> +}
> +
> +/* Read attributes of a maps entry in /proc/[pid]/adi/maps.  */
> +
> +static void
> +read_maps_entry (const char *line,
> +              ULONGEST *addr, ULONGEST *endaddr)
> +{
> +  const char *p = line;
> +
> +  *addr = strtoulst (p, &p, 16);
> +  if (*p == '-')
> +    p++;
> +
> +  *endaddr = strtoulst (p, &p, 16);
> +}
> +
> +/* Check if ADI is available.  */
> +
> +static bool
> +adi_available (void)
> +{
> +  pid_t pid = ptid_get_pid (inferior_ptid);
> +  struct sparc64_adi_info *proc = get_adi_info_proc (pid);
> +
> +  if (proc->stat.checked_avail)
> +    return proc->stat.is_avail;
> +
> +  proc->stat.checked_avail = true;
> +  if (target_auxv_search (&current_target, AT_ADI_BLKSZ, &proc->stat.blksize) <= 0)

THis line is too long.

> +    return false;
> +  target_auxv_search (&current_target, AT_ADI_NBITS, &proc->stat.nbits);
> +  proc->stat.max_version = (1 << proc->stat.nbits) - 2;
> +  proc->stat.is_avail = true;
> +
> +  return proc->stat.is_avail;
> +}
> +
> +/* Normalize a versioned address - a VA with ADI bits (63-60) set.  */
> +
> +CORE_ADDR
> +adi_normalize_address (CORE_ADDR addr)
> +{
> +  struct adi_stat_t adi_stat = get_adi_info (ptid_get_pid (inferior_ptid));
> +
> +  if (adi_stat.nbits)
> +    return ((CORE_ADDR)(((long)addr << adi_stat.nbits) >> adi_stat.nbits));
> +  return addr;
> +}
> +
> +/* Align a normalized address - a VA with bit 59 sign extended into ADI bits.  */
> +
> +static CORE_ADDR
> +adi_align_address (CORE_ADDR naddr)
> +{
> +  struct adi_stat_t adi_stat = get_adi_info (ptid_get_pid (inferior_ptid));
> +
> +  return (naddr - (naddr % adi_stat.blksize)) / adi_stat.blksize;
> +}
> +
> +/* Convert a byte count to count at a ratio of 1:adi_blksz.  */
> +
> +static int
> +adi_convert_byte_count (CORE_ADDR naddr, int nbytes, CORE_ADDR locl)
> +{
> +  struct adi_stat_t adi_stat = get_adi_info (ptid_get_pid (inferior_ptid));
> +
> +  return ((naddr + nbytes + adi_stat.blksize - 1) / adi_stat.blksize) - locl;
> +}
> +
> +/* The /proc/[pid]/adi/tags file, which allows gdb to get/set ADI
> +   version in a target process, maps linearly to the address space
> +   of the target process at a ratio of 1:adi_blksz.
> +
> +   A read (or write) at offset K in the file returns (or modifies)
> +   the ADI version tag stored in the cacheline containing address
> +   K * adi_blksz, encoded as 1 version tag per byte.  The allowed
> +   version tag values are between 0 and adi_stat.max_version.  */
> +
> +static int
> +adi_tag_fd (void)
> +{
> +  pid_t pid = ptid_get_pid (inferior_ptid);
> +  struct sparc64_adi_info *proc = get_adi_info_proc (pid);
> +
> +  if (proc->stat.tag_fd != 0)
> +    return proc->stat.tag_fd;
> +
> +  proc->stat.tag_fd = open_adi_tag_fd (pid);
> +  return proc->stat.tag_fd;
> +}
> +
> +/* Check if an address set is ADI enabled, using /proc/[pid]/adi/maps
> +   which was exported by the kernel and contains the currently ADI
> +   mapped memory regions and their access permissions.  */
> +
> +static bool
> +adi_is_addr_mapped (CORE_ADDR vaddr, size_t cnt)
> +{
> +  char filename[MAX_PROC_NAME_SIZE];
> +  size_t i = 0;
> +
> +  pid_t pid = ptid_get_pid (inferior_ptid);
> +  snprintf (filename, sizeof filename, "/proc/%d/adi/maps", pid);
> +  char *data = target_fileio_read_stralloc (NULL, filename);
> +  if (data)
> +    {
> +      struct cleanup *cleanup = make_cleanup (xfree, data);
> +      struct adi_stat_t adi_stat = get_adi_info (pid);
> +      char *line;
> +      for (line = strtok (data, "\n"); line; line = strtok (NULL, "\n"))
> +        {
> +          ULONGEST addr, endaddr;
> +
> +          read_maps_entry (line, &addr, &endaddr);
> +
> +          while (((vaddr + i) * adi_stat.blksize) >= addr
> +                 && ((vaddr + i) * adi_stat.blksize) < endaddr)
> +            {
> +              if (++i == cnt)
> +                {
> +                  do_cleanups (cleanup);
> +                  return true;
> +                }
> +            }
> +        }
> +        do_cleanups (cleanup);
> +      }
> +    else
> +      warning (_("unable to open /proc file '%s'"), filename);
> +
> +  return false;
> +}
> +
> +/* Read ADI version tag value for memory locations starting at "vaddr"
> +   for "size" number of bytes.  */
> +
> +static int
> +adi_read_versions (CORE_ADDR vaddr, size_t size, unsigned char *tags)
> +{
> +  int fd = adi_tag_fd ();
> +  if (fd == -1)
> +    return -1;
> +
> +  if (!adi_is_addr_mapped (vaddr, size))
> +    {
> +      struct adi_stat_t adi_stat = get_adi_info (ptid_get_pid (inferior_ptid));
> +      error(_("Address at 0x%lx is not in ADI maps"), vaddr*adi_stat.blksize);
> +    }
> +
> +  return pread64 (fd, tags, size, vaddr);
> +}

Calling pread64 in -tdep.c makes few sense to me.  If you only want to
support native debugging, move them to -nat.c file.

> +
> +/* Write ADI version tag for memory locations starting at "vaddr" for
> + "size" number of bytes to "tags".  */
> +
> +static int
> +adi_write_versions (CORE_ADDR vaddr, size_t size, unsigned char *tags)
> +{
> +  int fd = adi_tag_fd ();
> +  if (fd == -1)
> +    return -1;
> +
> +  if (!adi_is_addr_mapped (vaddr, size))
> +    {
> +      struct adi_stat_t adi_stat = get_adi_info (ptid_get_pid (inferior_ptid));
> +      error(_("Address at 0x%lx is not in ADI maps"), vaddr*adi_stat.blksize);
> +    }
> +
> +  return pwrite64 (fd, tags, size, vaddr);
> +}
> +
> +/* Print ADI version tag value in "tags" for memory locations starting
> +   at "vaddr" with number of "cnt".  */
> +
> +static void
> +adi_print_versions (CORE_ADDR vaddr, size_t cnt, unsigned char *tags)
> +{
> +  int v_idx = 0;
> +  const int maxelts = 8;  /* # of elements per line */
> +
> +  struct adi_stat_t adi_stat = get_adi_info (ptid_get_pid (inferior_ptid));
> +
> +  while (cnt > 0)
> +    {
> +      QUIT;
> +      printf_filtered ("0x%016lx:\t", vaddr * adi_stat.blksize);
> +      for (int i = maxelts; i > 0 && cnt > 0; i--, cnt--)
> +        {
> +          if (tags[v_idx] == 0xff)    /* no version tag */
> +            printf_filtered ("- ");
> +          else
> +            printf_filtered ("%1X ", tags[v_idx]);
> +          ++v_idx;
> +        }
> +      printf_filtered ("\n");
> +      gdb_flush (gdb_stdout);
> +      vaddr += maxelts;
> +    }
> +}
> +
> +static void
> +do_examine (CORE_ADDR start, int bcnt)
> +{
> +  CORE_ADDR vaddr = adi_normalize_address (start);
> +  struct cleanup *cleanup;
> +
> +  CORE_ADDR vstart = adi_align_address (vaddr);
> +  int cnt = adi_convert_byte_count (vaddr, bcnt, vstart);
> +  unsigned char *buf = (unsigned char *) xmalloc (cnt);
> +  cleanup = make_cleanup (xfree, buf);
> +  int read_cnt = adi_read_versions (vstart, cnt, buf);
> +  if (read_cnt == -1)
> +    error (_("No ADI information"));
> +  else if (read_cnt < cnt)
> +    error(_("No ADI information at 0x%lx"), vaddr);
> +
> +  adi_print_versions (vstart, cnt, buf);
> +
> +  do_cleanups (cleanup);
> +}
> +
> +static void
> +do_assign (CORE_ADDR start, size_t bcnt, int version)
> +{
> +  CORE_ADDR vaddr = adi_normalize_address (start);
> +
> +  CORE_ADDR vstart = adi_align_address (vaddr);
> +  int cnt = adi_convert_byte_count (vaddr, bcnt, vstart);
> +  unsigned char *buf = (unsigned char *) xmalloc (cnt);

Use std::vector<gdb_byte>? so that you don't need memset and xfree.

> +  memset(buf, version, cnt);
> +
> +  int set_cnt = adi_write_versions (vstart, cnt, buf);
> +  xfree (buf);
> +
> +  if (set_cnt == -1)
> +    error (_("No ADI information"));
> +  else if (set_cnt < cnt)
> +    error(_("No ADI information at 0x%lx"), vaddr);
> +
> +}
> +
> +/* ADI examine version tag command.
> +
> +   Command syntax:
> +
> +     adi (examine|x)/count <addr> */
> +
> +static void
> +adi_examine_command (char *args, int from_tty)
> +{
> +  /* make sure program is active and adi is available */
> +  if (!target_has_execution)
> +    error (_("ADI command requires a live process/thread"));
> +
> +  if (!adi_available ())
> +    error (_("No ADI information"));
> +
> +  pid_t pid = ptid_get_pid (inferior_ptid);
> +  struct sparc64_adi_info *proc = get_adi_info_proc (pid);
> +  int cnt = proc->stat.last_cnt? proc->stat.last_cnt : 1;
> +  char *p = args;
> +  if (p && *p == '/')
> +    {
> +      p++;
> +      cnt = get_number (&p);
> +    }
> +
> +  CORE_ADDR next_address = proc->stat.last_vaddr ? proc->stat.last_vaddr : 0;
> +  if (p != 0 && *p != 0)
> +    next_address = parse_and_eval_address (p);
> +  else if (!proc->stat.last_cnt)
> +    error (_("Usage: adi examine|x[/count] <addr>"));

IIUC, "address" in this command is mandatory, and it makes sense to do
so.  We don't need .last_vaddr.  I don't like such variable making the
command stateful.
  

With your comments like:
  * You can't access /proc in *-tdep.c file, because it is also compiled for
    cross-debugger (from previous review);
  * We can't include "nat.h" in "tdep.c";
  * Calling pread64 in -tdep.c makes few sense to me.  If you only want to
    support native debugging, move them to -nat.c file.

I now believe that the ADI support should be in sparc6 4-linux-nat.c,
not sparc64-tdep.c. Doing so also makes cross gdb build a non-issue.

Our QA team has been working on an ADI test case which they will submit
upstream separately.  Hope that's OK.

Will address your other comments in separate emails. Thanks.


On 7/10/2017 3:37 AM, Yao Qi wrote:
> Weimin Pan <weimin.pan@oracle.com> writes:
>
>> Tested in sparc64-linux-gnu. No regressions.
>>
> Did you build a cross gdb on x86-linux?  At least it breaks the build
> for all targets, I configure gdb this way,
>
> "../binutils-gdb/configure --enable-sim --disable-binutils
> --disable-gprof --disable-gold --disable-gas --disable-ld
> --enable-targets=all --enable-64-bit-bfd"
>
> sparc64-tdep.o: In function `adi_tag_fd()':
> build-gdb/gdb/../../binutils-gdb/gdb/sparc64-tdep.c:1915: undefined reference to `open_adi_tag_fd(int)'
>
> because sparc64-tdep.o references function defined in sparc64-linux-nat.c.
>
>> gdb/ChangeLog:
>> 2017-06-26  Weimin Pan  <weimin.pan@oracle.com>
> A blank line is needed here.
>
>> 	* sparc64-tdep.h: (adi_normalize_address): New export.
>> 	* sparc-nat.h: (open_adi_tag_fd): New export.
>> 	* sparc64-linux-nat.c: (open_adi_tag_fd): New function.
>> 	* sparc64-linux-tdep.c:
>> 	(SEGV_ACCADI, SEGV_ADIDERR, SEGV_ADIPERR) New defines.
>> 	(sparc64_linux_handle_segmentation_fault): New function.
>> 	(sparc64_linux_init_abi): Register
>> 	sparc64_linux_handle_segmentation_fault
>> 	* sparc64-tdep.c: Include cli-utils.h,gdbcmd.h,auxv.h,sparc-nat.h.
>> 	(sparc64_pstate_type): Replace PID1 with MCDE.
>> 	(sparc64_addr_bits_remove): New function.
>> 	(sparc64_init_abi): Register sparc64_addr_bits_remove.
>> 	(MAX_PROC_NAME_SIZE): New macro.
>> 	(AT_ADI_BLKSZ, AT_ADI_NBITS, AT_ADI_UEONADI) New defines.
>> 	(sparc64adilist): New variable.
>> 	(adi_proc_list): New variable.
>> 	(find_adi_info): New function.
>> 	(add_adi_info): New function.
>> 	(get_adi_info_proc): New function.
>> 	(get_adi_info): New function.
>> 	(info_adi_command): New function.
>> 	(read_maps_entry): New function.
>> 	(adi_available): New function.
>> 	(adi_normalize_address): New function.
>> 	(adi_align_address): New function.
>> 	(adi_convert_byte_count): New function.
>> 	(adi_tag_fd): New function.
>> 	(adi_is_addr_mapped): New function.
>> 	(adi_read_versions): New function.
>> 	(adi_write_versions): New function.
>> 	(adi_print_versions): New function.
>> 	(do_examine): New function.
>> 	(do_assign): New function.
>> 	(adi_examine_command): New function.
>> 	(adi_assign_command): New function.
>> 	(_initialize_sparc64_tdep): New function.
>>
>> gdb/doc/ChangeLog:
>> 2017-06-26  Weimin Pan  <weimin.pan@oracle.com>
>> 	* gdb.texinfo (Architectures): Add new Sparc64 section to document
>> 	ADI support.
>> 	* NEWS: Add "adi examine" and "adi assign" commands.
> Can you add a test case?
>
>> +
>> +Below is an example of displaying ADI versions of variable "shmaddr".
>> +
>> +@smallexample
>> +(@value{GDBP}) adi x/100 shmaddr
>> +   0xfff800010002c000:     0 0
>> +@end smallexample
> Why "0" is printed twice?
>
>> +void
>> +sparc64_linux_handle_segmentation_fault (struct gdbarch *gdbarch,
>> +				      struct ui_out *uiout)
>> +{
>> +  if (gdbarch_bfd_arch_info (gdbarch)->bits_per_word != 64)
>> +    return;
>> +
>> +  CORE_ADDR addr = 0;
>> +  long si_code = 0;
>> +
>> +  TRY
>> +    {
>> +      /* Evaluate si_code to see if the segfault is ADI related.  */
>> +      si_code = parse_and_eval_long ("$_siginfo.si_code\n");
>> +
>> +      if (si_code >= SEGV_ACCADI && si_code <= SEGV_ADIPERR)
>> +        addr = parse_and_eval_long ("$_siginfo._sifields._sigfault.si_addr");
>> +    }
>> +  CATCH (exception, RETURN_MASK_ALL)
>> +    {
>> +      return;
>> +    }
>> +  END_CATCH
>> +
>> +  /* Print out ADI event based on sig_code value */
>> +  switch (si_code)
>> +    {
>> +    case SEGV_ACCADI:	/* adi not enabled */
>> +      uiout->text ("\n");
>> +      uiout->field_string ("sigcode-meaning", _("ADI disabled"));
>> +      uiout->text (_(" while accessing address "));
>> +      uiout->field_fmt ("bound-access", "%s", paddress (gdbarch, addr));
>> +      break;
>> +    case SEGV_ADIDERR:	/* disrupting mismatch */
>> +      uiout->text ("\n");
>> +      uiout->field_string ("sigcode-meaning", _("ADI deferred mismatch"));
>> +      uiout->text (_(" while accessing address "));
>> +      uiout->field_fmt ("bound-access", "%s", paddress (gdbarch, addr));
>> +      break;
>> +    case SEGV_ADIPERR:	/* precise mismatch */
>> +      uiout->text ("\n");
>> +      uiout->field_string ("sigcode-meaning", _("ADI precise mismatch"));
>> +      uiout->text (_(" while accessing address "));
>> +      uiout->field_fmt ("bound-access", "%s", paddress (gdbarch, addr));
>> +      break;
>> +    default:
>> +      break;
>> +    }
> It is good to have a test to check the results and outputs here.
>
>> +}
>> +
>>   
>>   /* Return the address of a system call's alternative return
>>      address.  */
>> @@ -338,6 +404,8 @@ sparc64_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
>>     set_xml_syscall_file_name (gdbarch, XML_SYSCALL_FILENAME_SPARC64);
>>     set_gdbarch_get_syscall_number (gdbarch,
>>                                     sparc64_linux_get_syscall_number);
>> +  set_gdbarch_handle_segmentation_fault (gdbarch,
>> +					 sparc64_linux_handle_segmentation_fault);
>>   }
>>   
>>   
>> diff --git a/gdb/sparc64-tdep.c b/gdb/sparc64-tdep.c
>> index 9e0e6b5..9fa8e13 100644
>> --- a/gdb/sparc64-tdep.c
>> +++ b/gdb/sparc64-tdep.c
>> @@ -35,7 +35,11 @@
>>   #include "target.h"
>>   #include "value.h"
>>   
>> +#include "cli/cli-utils.h"
>> +#include "gdbcmd.h"
>> +#include "auxv.h"
>>   #include "sparc64-tdep.h"
>> +#include "sparc-nat.h"
>>   
> We can't include "nat.h" in "tdep.c".
>
>>   /* This file implements the SPARC 64-bit ABI as defined by the
>>      section "Low-Level System Information" of the SPARC Compliance
>> @@ -165,7 +169,7 @@ sparc64_pstate_type (struct gdbarch *gdbarch)
>>         append_flags_type_flag (type, 8, "TLE");
>>         append_flags_type_flag (type, 9, "CLE");
>>         append_flags_type_flag (type, 10, "PID0");
>> -      append_flags_type_flag (type, 11, "PID1");
>> +      append_flags_type_flag (type, 11, "MCDE");
>>   
> I don't understand your reply regarding to this change,
>
>> This change, which indicated that the TSTATE reigster contained the
>> "mcde" value, was requested by our kernel engineers.
> How is it related to the adi support?  If it is not, split it to another
> patch.  In any case, we need a technical reason for the change.
>
>>         tdep->sparc64_pstate_type = type;
>>       }
>> @@ -1290,6 +1294,14 @@ sparc64_dwarf2_frame_init_reg (struct gdbarch *gdbarch, int regnum,
>>       }
>>   }
>>   
>> +/* sparc64_addr_bits_remove - remove useless address bits  */
>> +
>> +static CORE_ADDR
>> +sparc64_addr_bits_remove (struct gdbarch *gdbarch, CORE_ADDR addr)
>> +{
>> +  return adi_normalize_address (addr);
> adi_normalize_address can be a static function, and other .c file can
> use it via gdbarch_addr_bits_remove.
>
>> +}
>> +
>>   void
>>   sparc64_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
>>   {
>> @@ -1342,6 +1354,8 @@ sparc64_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
>>   
>>     frame_unwind_append_unwinder (gdbarch, &sparc64_frame_unwind);
>>     frame_base_set_default (gdbarch, &sparc64_frame_base);
>> +
>> +  set_gdbarch_addr_bits_remove (gdbarch, sparc64_addr_bits_remove);
>>   }
>>   
>>   
>> @@ -1666,3 +1680,495 @@ const struct sparc_fpregmap sparc64_bsd_fpregmap =
>>     0 * 8,			/* %f0 */
>>     32 * 8,			/* %fsr */
>>   };
>> +
>> +/* The M7 processor supports an Application Data Integrity (ADI) feature
>> +   that detects invalid data accesses.  When software allocates memory and
>> +   enables ADI on the allocated memory, it chooses a 4-bit version number,
>> +   sets the version in the upper 4 bits of the 64-bit pointer to that data,
>> +   and stores the 4-bit version in every cacheline of the object.  Hardware
>> +   saves the latter in spare bits in the cache and memory hierarchy. On each
>> +   load and store, the processor compares the upper 4 VA (virtual address) bits
>> +   to the cacheline's version. If there is a mismatch, the processor generates
>> +   a version mismatch trap which can be either precise or disrupting.
>> +   The trap is an error condition which the kernel delivers to the process
>> +   as a SIGSEGV signal.
>> +
>> +   The upper 4 bits of the VA represent a version and are not part of the
>> +   true address.  The processor clears these bits and sign extends bit 59
>> +   to generate the true address.
>> +
>> +   Note that 32-bit applications cannot use ADI. */
>> +
>> +
>> +#define MAX_PROC_NAME_SIZE sizeof("/proc/99999/lwp/9999/adi/lstatus")
>> +
>> +/* ELF Auxiliary vectors */
>> +#ifndef AT_ADI_BLKSZ
>> +#define AT_ADI_BLKSZ    34
>> +#endif
>> +#ifndef AT_ADI_NBITS
>> +#define AT_ADI_NBITS    35
>> +#endif
>> +#ifndef AT_ADI_UEONADI
>> +#define AT_ADI_UEONADI  36
>> +#endif
>> +
>> +/* ADI command list.  */
>> +static struct cmd_list_element *sparc64adilist = NULL;
>> +
>> +/* ADI stat settings.  */
>> +struct adi_stat_t
>> +{
>> +  /* The ADI block size.  */
>> +  unsigned long blksize;
>> +
>> +  /* Number of bits used for an ADI version tag which can be
>> +   * used together with the shift value for an ADI version tag
>> +   * to encode or extract the ADI version value in a pointer.  */
>> +  unsigned long nbits;
>> +
>> +  /* The maximum ADI version tag value supported.  */
>> +  int max_version;
>> +
>> +  /* ADI version tag file.  */
>> +  int tag_fd;
>> +
>> +  /* Last ADI address examined.  */
>> +  CORE_ADDR last_vaddr;
>> +
>> +  /* Last specified examination count.  */
>> +  int last_cnt;
>> +
>> +  /* ADI availability check has been done.  */
>> +  bool checked_avail;
>> +
>> +  /* ADI is available.  */
>> +  bool is_avail;
>> +
>> +};
>> +
>> +/* Per-process ADI stat info.  */
>> +
>> +struct sparc64_adi_info
>> +{
>> +  /* The process identifier.  */
>> +  pid_t pid;
>> +
>> +  /* The ADI stat.  */
>> +  struct adi_stat_t stat;
>> +
>> +  /* Linked list.  */
>> +  struct sparc64_adi_info *next;
> Can you use C++ STL list?
>
>> +};
>> +
>> +static struct sparc64_adi_info *adi_proc_list = NULL;
>> +
>> +/* Find ADI info for process PID.  */
>> +
>> +static struct sparc64_adi_info *
>> +find_adi_info (pid_t pid)
>> +{
>> +  struct sparc64_adi_info *proc;
>> +
>> +  for (proc = adi_proc_list; proc; proc = proc->next)
>> +    if (proc->pid == pid)
>> +      return proc;
>> +
>> +  return NULL;
>> +}
>> +
>> +/* Add ADI info for process PID.  Returns newly allocated info
>> +   object.  */
>> +
>> +static struct sparc64_adi_info *
>> +add_adi_info (pid_t pid)
>> +{
>> +  struct sparc64_adi_info *proc = XCNEW (struct sparc64_adi_info);
>> +
>> +  proc->pid = pid;
>> +  proc->next = adi_proc_list;
>> +  adi_proc_list = proc;
>> +  proc->stat.is_avail = false;
>> +  proc->stat.checked_avail = false;
>> +  proc->stat.tag_fd = 0;
>> +
>> +  return proc;
>> +}
>> +
>> +/* Get ADI info for process PID, creating one if it doesn't exist.  */
>> +
>> +static struct sparc64_adi_info *
>> +get_adi_info_proc (pid_t pid)
>> +{
>> +  struct sparc64_adi_info *proc;
>> +
>> +  proc = find_adi_info (pid);
>> +  if (proc == NULL)
>> +    proc = add_adi_info (pid);
>> +
>> +  return proc;
>> +}
> I don't see the place you remove elements from adi list.  You probably
> need to implement sparc64_forget_process (in which you can delete the
> adi element if the process exits), and hook it on via
> "linux_nat_set_forget_process (t, sparc64_forget_process);".
>
>> +
>> +static struct adi_stat_t
>> +get_adi_info (pid_t pid)
>> +{
>> +  struct sparc64_adi_info *proc;
>> +
>> +  proc = get_adi_info_proc (pid);
>> +  return proc->stat;
>> +}
>> +
>> +static void
>> +info_adi_command (char *args, int from_tty)
>> +{
>> +  printf_unfiltered ("\"adi\" must be followed by \"examine\" "
>> +                     "or \"assign\".\n");
>> +  help_list (sparc64adilist, "adi ", all_commands, gdb_stdout);
>> +}
>> +
>> +/* Read attributes of a maps entry in /proc/[pid]/adi/maps.  */
>> +
>> +static void
>> +read_maps_entry (const char *line,
>> +              ULONGEST *addr, ULONGEST *endaddr)
>> +{
>> +  const char *p = line;
>> +
>> +  *addr = strtoulst (p, &p, 16);
>> +  if (*p == '-')
>> +    p++;
>> +
>> +  *endaddr = strtoulst (p, &p, 16);
>> +}
>> +
>> +/* Check if ADI is available.  */
>> +
>> +static bool
>> +adi_available (void)
>> +{
>> +  pid_t pid = ptid_get_pid (inferior_ptid);
>> +  struct sparc64_adi_info *proc = get_adi_info_proc (pid);
>> +
>> +  if (proc->stat.checked_avail)
>> +    return proc->stat.is_avail;
>> +
>> +  proc->stat.checked_avail = true;
>> +  if (target_auxv_search (&current_target, AT_ADI_BLKSZ, &proc->stat.blksize) <= 0)
> THis line is too long.
>
>> +    return false;
>> +  target_auxv_search (&current_target, AT_ADI_NBITS, &proc->stat.nbits);
>> +  proc->stat.max_version = (1 << proc->stat.nbits) - 2;
>> +  proc->stat.is_avail = true;
>> +
>> +  return proc->stat.is_avail;
>> +}
>> +
>> +/* Normalize a versioned address - a VA with ADI bits (63-60) set.  */
>> +
>> +CORE_ADDR
>> +adi_normalize_address (CORE_ADDR addr)
>> +{
>> +  struct adi_stat_t adi_stat = get_adi_info (ptid_get_pid (inferior_ptid));
>> +
>> +  if (adi_stat.nbits)
>> +    return ((CORE_ADDR)(((long)addr << adi_stat.nbits) >> adi_stat.nbits));
>> +  return addr;
>> +}
>> +
>> +/* Align a normalized address - a VA with bit 59 sign extended into ADI bits.  */
>> +
>> +static CORE_ADDR
>> +adi_align_address (CORE_ADDR naddr)
>> +{
>> +  struct adi_stat_t adi_stat = get_adi_info (ptid_get_pid (inferior_ptid));
>> +
>> +  return (naddr - (naddr % adi_stat.blksize)) / adi_stat.blksize;
>> +}
>> +
>> +/* Convert a byte count to count at a ratio of 1:adi_blksz.  */
>> +
>> +static int
>> +adi_convert_byte_count (CORE_ADDR naddr, int nbytes, CORE_ADDR locl)
>> +{
>> +  struct adi_stat_t adi_stat = get_adi_info (ptid_get_pid (inferior_ptid));
>> +
>> +  return ((naddr + nbytes + adi_stat.blksize - 1) / adi_stat.blksize) - locl;
>> +}
>> +
>> +/* The /proc/[pid]/adi/tags file, which allows gdb to get/set ADI
>> +   version in a target process, maps linearly to the address space
>> +   of the target process at a ratio of 1:adi_blksz.
>> +
>> +   A read (or write) at offset K in the file returns (or modifies)
>> +   the ADI version tag stored in the cacheline containing address
>> +   K * adi_blksz, encoded as 1 version tag per byte.  The allowed
>> +   version tag values are between 0 and adi_stat.max_version.  */
>> +
>> +static int
>> +adi_tag_fd (void)
>> +{
>> +  pid_t pid = ptid_get_pid (inferior_ptid);
>> +  struct sparc64_adi_info *proc = get_adi_info_proc (pid);
>> +
>> +  if (proc->stat.tag_fd != 0)
>> +    return proc->stat.tag_fd;
>> +
>> +  proc->stat.tag_fd = open_adi_tag_fd (pid);
>> +  return proc->stat.tag_fd;
>> +}
>> +
>> +/* Check if an address set is ADI enabled, using /proc/[pid]/adi/maps
>> +   which was exported by the kernel and contains the currently ADI
>> +   mapped memory regions and their access permissions.  */
>> +
>> +static bool
>> +adi_is_addr_mapped (CORE_ADDR vaddr, size_t cnt)
>> +{
>> +  char filename[MAX_PROC_NAME_SIZE];
>> +  size_t i = 0;
>> +
>> +  pid_t pid = ptid_get_pid (inferior_ptid);
>> +  snprintf (filename, sizeof filename, "/proc/%d/adi/maps", pid);
>> +  char *data = target_fileio_read_stralloc (NULL, filename);
>> +  if (data)
>> +    {
>> +      struct cleanup *cleanup = make_cleanup (xfree, data);
>> +      struct adi_stat_t adi_stat = get_adi_info (pid);
>> +      char *line;
>> +      for (line = strtok (data, "\n"); line; line = strtok (NULL, "\n"))
>> +        {
>> +          ULONGEST addr, endaddr;
>> +
>> +          read_maps_entry (line, &addr, &endaddr);
>> +
>> +          while (((vaddr + i) * adi_stat.blksize) >= addr
>> +                 && ((vaddr + i) * adi_stat.blksize) < endaddr)
>> +            {
>> +              if (++i == cnt)
>> +                {
>> +                  do_cleanups (cleanup);
>> +                  return true;
>> +                }
>> +            }
>> +        }
>> +        do_cleanups (cleanup);
>> +      }
>> +    else
>> +      warning (_("unable to open /proc file '%s'"), filename);
>> +
>> +  return false;
>> +}
>> +
>> +/* Read ADI version tag value for memory locations starting at "vaddr"
>> +   for "size" number of bytes.  */
>> +
>> +static int
>> +adi_read_versions (CORE_ADDR vaddr, size_t size, unsigned char *tags)
>> +{
>> +  int fd = adi_tag_fd ();
>> +  if (fd == -1)
>> +    return -1;
>> +
>> +  if (!adi_is_addr_mapped (vaddr, size))
>> +    {
>> +      struct adi_stat_t adi_stat = get_adi_info (ptid_get_pid (inferior_ptid));
>> +      error(_("Address at 0x%lx is not in ADI maps"), vaddr*adi_stat.blksize);
>> +    }
>> +
>> +  return pread64 (fd, tags, size, vaddr);
>> +}
> Calling pread64 in -tdep.c makes few sense to me.  If you only want to
> support native debugging, move them to -nat.c file.
>
>> +
>> +/* Write ADI version tag for memory locations starting at "vaddr" for
>> + "size" number of bytes to "tags".  */
>> +
>> +static int
>> +adi_write_versions (CORE_ADDR vaddr, size_t size, unsigned char *tags)
>> +{
>> +  int fd = adi_tag_fd ();
>> +  if (fd == -1)
>> +    return -1;
>> +
>> +  if (!adi_is_addr_mapped (vaddr, size))
>> +    {
>> +      struct adi_stat_t adi_stat = get_adi_info (ptid_get_pid (inferior_ptid));
>> +      error(_("Address at 0x%lx is not in ADI maps"), vaddr*adi_stat.blksize);
>> +    }
>> +
>> +  return pwrite64 (fd, tags, size, vaddr);
>> +}
>> +
>> +/* Print ADI version tag value in "tags" for memory locations starting
>> +   at "vaddr" with number of "cnt".  */
>> +
>> +static void
>> +adi_print_versions (CORE_ADDR vaddr, size_t cnt, unsigned char *tags)
>> +{
>> +  int v_idx = 0;
>> +  const int maxelts = 8;  /* # of elements per line */
>> +
>> +  struct adi_stat_t adi_stat = get_adi_info (ptid_get_pid (inferior_ptid));
>> +
>> +  while (cnt > 0)
>> +    {
>> +      QUIT;
>> +      printf_filtered ("0x%016lx:\t", vaddr * adi_stat.blksize);
>> +      for (int i = maxelts; i > 0 && cnt > 0; i--, cnt--)
>> +        {
>> +          if (tags[v_idx] == 0xff)    /* no version tag */
>> +            printf_filtered ("- ");
>> +          else
>> +            printf_filtered ("%1X ", tags[v_idx]);
>> +          ++v_idx;
>> +        }
>> +      printf_filtered ("\n");
>> +      gdb_flush (gdb_stdout);
>> +      vaddr += maxelts;
>> +    }
>> +}
>> +
>> +static void
>> +do_examine (CORE_ADDR start, int bcnt)
>> +{
>> +  CORE_ADDR vaddr = adi_normalize_address (start);
>> +  struct cleanup *cleanup;
>> +
>> +  CORE_ADDR vstart = adi_align_address (vaddr);
>> +  int cnt = adi_convert_byte_count (vaddr, bcnt, vstart);
>> +  unsigned char *buf = (unsigned char *) xmalloc (cnt);
>> +  cleanup = make_cleanup (xfree, buf);
>> +  int read_cnt = adi_read_versions (vstart, cnt, buf);
>> +  if (read_cnt == -1)
>> +    error (_("No ADI information"));
>> +  else if (read_cnt < cnt)
>> +    error(_("No ADI information at 0x%lx"), vaddr);
>> +
>> +  adi_print_versions (vstart, cnt, buf);
>> +
>> +  do_cleanups (cleanup);
>> +}
>> +
>> +static void
>> +do_assign (CORE_ADDR start, size_t bcnt, int version)
>> +{
>> +  CORE_ADDR vaddr = adi_normalize_address (start);
>> +
>> +  CORE_ADDR vstart = adi_align_address (vaddr);
>> +  int cnt = adi_convert_byte_count (vaddr, bcnt, vstart);
>> +  unsigned char *buf = (unsigned char *) xmalloc (cnt);
> Use std::vector<gdb_byte>? so that you don't need memset and xfree.
>
>> +  memset(buf, version, cnt);
>> +
>> +  int set_cnt = adi_write_versions (vstart, cnt, buf);
>> +  xfree (buf);
>> +
>> +  if (set_cnt == -1)
>> +    error (_("No ADI information"));
>> +  else if (set_cnt < cnt)
>> +    error(_("No ADI information at 0x%lx"), vaddr);
>> +
>> +}
>> +
>> +/* ADI examine version tag command.
>> +
>> +   Command syntax:
>> +
>> +     adi (examine|x)/count <addr> */
>> +
>> +static void
>> +adi_examine_command (char *args, int from_tty)
>> +{
>> +  /* make sure program is active and adi is available */
>> +  if (!target_has_execution)
>> +    error (_("ADI command requires a live process/thread"));
>> +
>> +  if (!adi_available ())
>> +    error (_("No ADI information"));
>> +
>> +  pid_t pid = ptid_get_pid (inferior_ptid);
>> +  struct sparc64_adi_info *proc = get_adi_info_proc (pid);
>> +  int cnt = proc->stat.last_cnt? proc->stat.last_cnt : 1;
>> +  char *p = args;
>> +  if (p && *p == '/')
>> +    {
>> +      p++;
>> +      cnt = get_number (&p);
>> +    }
>> +
>> +  CORE_ADDR next_address = proc->stat.last_vaddr ? proc->stat.last_vaddr : 0;
>> +  if (p != 0 && *p != 0)
>> +    next_address = parse_and_eval_address (p);
>> +  else if (!proc->stat.last_cnt)
>> +    error (_("Usage: adi examine|x[/count] <addr>"));
> IIUC, "address" in this command is mandatory, and it makes sense to do
> so.  We don't need .last_vaddr.  I don't like such variable making the
> command stateful.
>
  

Wei-min Pan <weimin.pan@oracle.com> writes:

> Our QA team has been working on an ADI test case which they will submit
> upstream separately.  Hope that's OK.

The test case is some tcl/expect script and could be run with gdb
regression testsuite.  You can take a look at gdb/testsuite/gdb.arch/.

I expect that the new feature and the tests can go together.
  

On 07/11/2017 03:56 AM, Wei-min Pan wrote:
> With your comments like:
>  * You can't access /proc in *-tdep.c file, because it is also compiled for
>    cross-debugger (from previous review);
>  * We can't include "nat.h" in "tdep.c";
>  * Calling pread64 in -tdep.c makes few sense to me.  If you only want to
>    support native debugging, move them to -nat.c file.
> 
> I now believe that the ADI support should be in sparc6 4-linux-nat.c,
> not sparc64-tdep.c. Doing so also makes cross gdb build a non-issue.

Please don't leave remote debugging out of the design.
I.e., remote debugging against gdbserver.

You were reading some /proc files with target_fileio, like:

  snprintf (filename, sizeof filename, "/proc/%d/adi/maps", pid);
  char *data = target_fileio_read_stralloc (NULL, filename);

and that should work against gdbserver. (It'll read the remote
target file when remote debugging.)  So one way would be to
replace open/pread64/close etc calls with calls to corresponding
target_fileio_xxx routines.

Alternatively (or in addition) defining new "enum target_object"
objects, and then using the target_xfer_partial interface may
make sense.  (I really haven't studied the code in any detail
to tell, but it should be a useful pointer, regardless.)

Thanks,
Pedro Alves
  

I have incorporated changes to all your other comments that are not 
listed below.

 > Can you add a test case?

We are working on one which will be part of the next patch.

 > > +Below is an example of displaying ADI versions of variable "shmaddr".
 > > +
 > > +@smallexample
 > > +(@value{GDBP}) adi x/100 shmaddr
 > > +   0xfff800010002c000:     0 0
 > > +@end smallexample

 > Why "0" is printed twice?

It's not printed twice. At the ratio of 1:ADI block size (which is
64 bytes by default), 100 bytes mean dumping 2 version tags.

 > > +void
 > > +sparc64_linux_handle_segmentation_fault (struct gdbarch *gdbarch,
 > > +                                  struct ui_out *uiout)
 > > +{
 > > +  if (gdbarch_bfd_arch_info (gdbarch)->bits_per_word != 64)
 > > +    return;
 > > +
 > > +  CORE_ADDR addr = 0;
 > > +  long si_code = 0;
 > > +
 > > +  TRY
 > > +    {
 > > +      /* Evaluate si_code to see if the segfault is ADI related.  */
 > > +      si_code = parse_and_eval_long ("$_siginfo.si_code\n");
 > > +
 > > +      if (si_code >= SEGV_ACCADI && si_code <= SEGV_ADIPERR)
 > > +        addr = parse_and_eval_long 
("$_siginfo._sifields._sigfault.si_addr");
 > > +    }
 > > +  CATCH (exception, RETURN_MASK_ALL)
 > > +    {
 > > +      return;
 > > +    }
 > > +  END_CATCH
 > > +
 > > +  /* Print out ADI event based on sig_code value */
 > > +  switch (si_code)
 > > +    {
 > > +    case SEGV_ACCADI:       /* adi not enabled */
 > > +      uiout->text ("\n");
 > > +      uiout->field_string ("sigcode-meaning", _("ADI disabled"));
 > > +      uiout->text (_(" while accessing address "));
 > > +      uiout->field_fmt ("bound-access", "%s", paddress (gdbarch, 
addr));
 > > +      break;
 > > +    case SEGV_ADIDERR:      /* disrupting mismatch */
 > > +      uiout->text ("\n");
 > > +      uiout->field_string ("sigcode-meaning", _("ADI deferred 
mismatch"));
 > > +      uiout->text (_(" while accessing address "));
 > > +      uiout->field_fmt ("bound-access", "%s", paddress (gdbarch, 
addr));
 > > +      break;
 > > +    case SEGV_ADIPERR:      /* precise mismatch */
 > > +      uiout->text ("\n");
 > > +      uiout->field_string ("sigcode-meaning", _("ADI precise 
mismatch"));
 > > +      uiout->text (_(" while accessing address "));
 > > +      uiout->field_fmt ("bound-access", "%s", paddress (gdbarch, 
addr));
 > > +      break;
 > > +    default:
 > > +      break;
 > > +    }

 > It is good to have a test to check the results and outputs here.

Do you mean to add a test like "uiout->is_mi_like_p ()"?

 > > This change, which indicated that the TSTATE register contained the
 > > "mcde" value, was requested by our kernel engineers.

 > How is it related to the adi support?  If it is not, split it to another
 > patch.  In any case, we need a technical reason for the change.

I've checked with our kernel engineer who had made the request.
He is fine with making this change separately. Change is removed.

Thanks.


On 7/10/2017 3:37 AM, Yao Qi wrote:
> Weimin Pan <weimin.pan@oracle.com> writes:
>
>> Tested in sparc64-linux-gnu. No regressions.
>>
> Did you build a cross gdb on x86-linux?  At least it breaks the build
> for all targets, I configure gdb this way,
>
> "../binutils-gdb/configure --enable-sim --disable-binutils
> --disable-gprof --disable-gold --disable-gas --disable-ld
> --enable-targets=all --enable-64-bit-bfd"
>
> sparc64-tdep.o: In function `adi_tag_fd()':
> build-gdb/gdb/../../binutils-gdb/gdb/sparc64-tdep.c:1915: undefined reference to `open_adi_tag_fd(int)'
>
> because sparc64-tdep.o references function defined in sparc64-linux-nat.c.
>
>> gdb/ChangeLog:
>> 2017-06-26  Weimin Pan  <weimin.pan@oracle.com>
> A blank line is needed here.
>
>> 	* sparc64-tdep.h: (adi_normalize_address): New export.
>> 	* sparc-nat.h: (open_adi_tag_fd): New export.
>> 	* sparc64-linux-nat.c: (open_adi_tag_fd): New function.
>> 	* sparc64-linux-tdep.c:
>> 	(SEGV_ACCADI, SEGV_ADIDERR, SEGV_ADIPERR) New defines.
>> 	(sparc64_linux_handle_segmentation_fault): New function.
>> 	(sparc64_linux_init_abi): Register
>> 	sparc64_linux_handle_segmentation_fault
>> 	* sparc64-tdep.c: Include cli-utils.h,gdbcmd.h,auxv.h,sparc-nat.h.
>> 	(sparc64_pstate_type): Replace PID1 with MCDE.
>> 	(sparc64_addr_bits_remove): New function.
>> 	(sparc64_init_abi): Register sparc64_addr_bits_remove.
>> 	(MAX_PROC_NAME_SIZE): New macro.
>> 	(AT_ADI_BLKSZ, AT_ADI_NBITS, AT_ADI_UEONADI) New defines.
>> 	(sparc64adilist): New variable.
>> 	(adi_proc_list): New variable.
>> 	(find_adi_info): New function.
>> 	(add_adi_info): New function.
>> 	(get_adi_info_proc): New function.
>> 	(get_adi_info): New function.
>> 	(info_adi_command): New function.
>> 	(read_maps_entry): New function.
>> 	(adi_available): New function.
>> 	(adi_normalize_address): New function.
>> 	(adi_align_address): New function.
>> 	(adi_convert_byte_count): New function.
>> 	(adi_tag_fd): New function.
>> 	(adi_is_addr_mapped): New function.
>> 	(adi_read_versions): New function.
>> 	(adi_write_versions): New function.
>> 	(adi_print_versions): New function.
>> 	(do_examine): New function.
>> 	(do_assign): New function.
>> 	(adi_examine_command): New function.
>> 	(adi_assign_command): New function.
>> 	(_initialize_sparc64_tdep): New function.
>>
>> gdb/doc/ChangeLog:
>> 2017-06-26  Weimin Pan  <weimin.pan@oracle.com>
>> 	* gdb.texinfo (Architectures): Add new Sparc64 section to document
>> 	ADI support.
>> 	* NEWS: Add "adi examine" and "adi assign" commands.
> Can you add a test case?
>
>> +
>> +Below is an example of displaying ADI versions of variable "shmaddr".
>> +
>> +@smallexample
>> +(@value{GDBP}) adi x/100 shmaddr
>> +   0xfff800010002c000:     0 0
>> +@end smallexample
> Why "0" is printed twice?
>
>> +void
>> +sparc64_linux_handle_segmentation_fault (struct gdbarch *gdbarch,
>> +				      struct ui_out *uiout)
>> +{
>> +  if (gdbarch_bfd_arch_info (gdbarch)->bits_per_word != 64)
>> +    return;
>> +
>> +  CORE_ADDR addr = 0;
>> +  long si_code = 0;
>> +
>> +  TRY
>> +    {
>> +      /* Evaluate si_code to see if the segfault is ADI related.  */
>> +      si_code = parse_and_eval_long ("$_siginfo.si_code\n");
>> +
>> +      if (si_code >= SEGV_ACCADI && si_code <= SEGV_ADIPERR)
>> +        addr = parse_and_eval_long ("$_siginfo._sifields._sigfault.si_addr");
>> +    }
>> +  CATCH (exception, RETURN_MASK_ALL)
>> +    {
>> +      return;
>> +    }
>> +  END_CATCH
>> +
>> +  /* Print out ADI event based on sig_code value */
>> +  switch (si_code)
>> +    {
>> +    case SEGV_ACCADI:	/* adi not enabled */
>> +      uiout->text ("\n");
>> +      uiout->field_string ("sigcode-meaning", _("ADI disabled"));
>> +      uiout->text (_(" while accessing address "));
>> +      uiout->field_fmt ("bound-access", "%s", paddress (gdbarch, addr));
>> +      break;
>> +    case SEGV_ADIDERR:	/* disrupting mismatch */
>> +      uiout->text ("\n");
>> +      uiout->field_string ("sigcode-meaning", _("ADI deferred mismatch"));
>> +      uiout->text (_(" while accessing address "));
>> +      uiout->field_fmt ("bound-access", "%s", paddress (gdbarch, addr));
>> +      break;
>> +    case SEGV_ADIPERR:	/* precise mismatch */
>> +      uiout->text ("\n");
>> +      uiout->field_string ("sigcode-meaning", _("ADI precise mismatch"));
>> +      uiout->text (_(" while accessing address "));
>> +      uiout->field_fmt ("bound-access", "%s", paddress (gdbarch, addr));
>> +      break;
>> +    default:
>> +      break;
>> +    }
> It is good to have a test to check the results and outputs here.
>
>> +}
>> +
>>   
>>   /* Return the address of a system call's alternative return
>>      address.  */
>> @@ -338,6 +404,8 @@ sparc64_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
>>     set_xml_syscall_file_name (gdbarch, XML_SYSCALL_FILENAME_SPARC64);
>>     set_gdbarch_get_syscall_number (gdbarch,
>>                                     sparc64_linux_get_syscall_number);
>> +  set_gdbarch_handle_segmentation_fault (gdbarch,
>> +					 sparc64_linux_handle_segmentation_fault);
>>   }
>>   
>>   
>> diff --git a/gdb/sparc64-tdep.c b/gdb/sparc64-tdep.c
>> index 9e0e6b5..9fa8e13 100644
>> --- a/gdb/sparc64-tdep.c
>> +++ b/gdb/sparc64-tdep.c
>> @@ -35,7 +35,11 @@
>>   #include "target.h"
>>   #include "value.h"
>>   
>> +#include "cli/cli-utils.h"
>> +#include "gdbcmd.h"
>> +#include "auxv.h"
>>   #include "sparc64-tdep.h"
>> +#include "sparc-nat.h"
>>   
> We can't include "nat.h" in "tdep.c".
>
>>   /* This file implements the SPARC 64-bit ABI as defined by the
>>      section "Low-Level System Information" of the SPARC Compliance
>> @@ -165,7 +169,7 @@ sparc64_pstate_type (struct gdbarch *gdbarch)
>>         append_flags_type_flag (type, 8, "TLE");
>>         append_flags_type_flag (type, 9, "CLE");
>>         append_flags_type_flag (type, 10, "PID0");
>> -      append_flags_type_flag (type, 11, "PID1");
>> +      append_flags_type_flag (type, 11, "MCDE");
>>   
> I don't understand your reply regarding to this change,
>
>> This change, which indicated that the TSTATE reigster contained the
>> "mcde" value, was requested by our kernel engineers.
> How is it related to the adi support?  If it is not, split it to another
> patch.  In any case, we need a technical reason for the change.
>
>>         tdep->sparc64_pstate_type = type;
>>       }
>> @@ -1290,6 +1294,14 @@ sparc64_dwarf2_frame_init_reg (struct gdbarch *gdbarch, int regnum,
>>       }
>>   }
>>   
>> +/* sparc64_addr_bits_remove - remove useless address bits  */
>> +
>> +static CORE_ADDR
>> +sparc64_addr_bits_remove (struct gdbarch *gdbarch, CORE_ADDR addr)
>> +{
>> +  return adi_normalize_address (addr);
> adi_normalize_address can be a static function, and other .c file can
> use it via gdbarch_addr_bits_remove.
>
>> +}
>> +
>>   void
>>   sparc64_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
>>   {
>> @@ -1342,6 +1354,8 @@ sparc64_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
>>   
>>     frame_unwind_append_unwinder (gdbarch, &sparc64_frame_unwind);
>>     frame_base_set_default (gdbarch, &sparc64_frame_base);
>> +
>> +  set_gdbarch_addr_bits_remove (gdbarch, sparc64_addr_bits_remove);
>>   }
>>   
>>   
>> @@ -1666,3 +1680,495 @@ const struct sparc_fpregmap sparc64_bsd_fpregmap =
>>     0 * 8,			/* %f0 */
>>     32 * 8,			/* %fsr */
>>   };
>> +
>> +/* The M7 processor supports an Application Data Integrity (ADI) feature
>> +   that detects invalid data accesses.  When software allocates memory and
>> +   enables ADI on the allocated memory, it chooses a 4-bit version number,
>> +   sets the version in the upper 4 bits of the 64-bit pointer to that data,
>> +   and stores the 4-bit version in every cacheline of the object.  Hardware
>> +   saves the latter in spare bits in the cache and memory hierarchy. On each
>> +   load and store, the processor compares the upper 4 VA (virtual address) bits
>> +   to the cacheline's version. If there is a mismatch, the processor generates
>> +   a version mismatch trap which can be either precise or disrupting.
>> +   The trap is an error condition which the kernel delivers to the process
>> +   as a SIGSEGV signal.
>> +
>> +   The upper 4 bits of the VA represent a version and are not part of the
>> +   true address.  The processor clears these bits and sign extends bit 59
>> +   to generate the true address.
>> +
>> +   Note that 32-bit applications cannot use ADI. */
>> +
>> +
>> +#define MAX_PROC_NAME_SIZE sizeof("/proc/99999/lwp/9999/adi/lstatus")
>> +
>> +/* ELF Auxiliary vectors */
>> +#ifndef AT_ADI_BLKSZ
>> +#define AT_ADI_BLKSZ    34
>> +#endif
>> +#ifndef AT_ADI_NBITS
>> +#define AT_ADI_NBITS    35
>> +#endif
>> +#ifndef AT_ADI_UEONADI
>> +#define AT_ADI_UEONADI  36
>> +#endif
>> +
>> +/* ADI command list.  */
>> +static struct cmd_list_element *sparc64adilist = NULL;
>> +
>> +/* ADI stat settings.  */
>> +struct adi_stat_t
>> +{
>> +  /* The ADI block size.  */
>> +  unsigned long blksize;
>> +
>> +  /* Number of bits used for an ADI version tag which can be
>> +   * used together with the shift value for an ADI version tag
>> +   * to encode or extract the ADI version value in a pointer.  */
>> +  unsigned long nbits;
>> +
>> +  /* The maximum ADI version tag value supported.  */
>> +  int max_version;
>> +
>> +  /* ADI version tag file.  */
>> +  int tag_fd;
>> +
>> +  /* Last ADI address examined.  */
>> +  CORE_ADDR last_vaddr;
>> +
>> +  /* Last specified examination count.  */
>> +  int last_cnt;
>> +
>> +  /* ADI availability check has been done.  */
>> +  bool checked_avail;
>> +
>> +  /* ADI is available.  */
>> +  bool is_avail;
>> +
>> +};
>> +
>> +/* Per-process ADI stat info.  */
>> +
>> +struct sparc64_adi_info
>> +{
>> +  /* The process identifier.  */
>> +  pid_t pid;
>> +
>> +  /* The ADI stat.  */
>> +  struct adi_stat_t stat;
>> +
>> +  /* Linked list.  */
>> +  struct sparc64_adi_info *next;
> Can you use C++ STL list?
>
>> +};
>> +
>> +static struct sparc64_adi_info *adi_proc_list = NULL;
>> +
>> +/* Find ADI info for process PID.  */
>> +
>> +static struct sparc64_adi_info *
>> +find_adi_info (pid_t pid)
>> +{
>> +  struct sparc64_adi_info *proc;
>> +
>> +  for (proc = adi_proc_list; proc; proc = proc->next)
>> +    if (proc->pid == pid)
>> +      return proc;
>> +
>> +  return NULL;
>> +}
>> +
>> +/* Add ADI info for process PID.  Returns newly allocated info
>> +   object.  */
>> +
>> +static struct sparc64_adi_info *
>> +add_adi_info (pid_t pid)
>> +{
>> +  struct sparc64_adi_info *proc = XCNEW (struct sparc64_adi_info);
>> +
>> +  proc->pid = pid;
>> +  proc->next = adi_proc_list;
>> +  adi_proc_list = proc;
>> +  proc->stat.is_avail = false;
>> +  proc->stat.checked_avail = false;
>> +  proc->stat.tag_fd = 0;
>> +
>> +  return proc;
>> +}
>> +
>> +/* Get ADI info for process PID, creating one if it doesn't exist.  */
>> +
>> +static struct sparc64_adi_info *
>> +get_adi_info_proc (pid_t pid)
>> +{
>> +  struct sparc64_adi_info *proc;
>> +
>> +  proc = find_adi_info (pid);
>> +  if (proc == NULL)
>> +    proc = add_adi_info (pid);
>> +
>> +  return proc;
>> +}
> I don't see the place you remove elements from adi list.  You probably
> need to implement sparc64_forget_process (in which you can delete the
> adi element if the process exits), and hook it on via
> "linux_nat_set_forget_process (t, sparc64_forget_process);".
>
>> +
>> +static struct adi_stat_t
>> +get_adi_info (pid_t pid)
>> +{
>> +  struct sparc64_adi_info *proc;
>> +
>> +  proc = get_adi_info_proc (pid);
>> +  return proc->stat;
>> +}
>> +
>> +static void
>> +info_adi_command (char *args, int from_tty)
>> +{
>> +  printf_unfiltered ("\"adi\" must be followed by \"examine\" "
>> +                     "or \"assign\".\n");
>> +  help_list (sparc64adilist, "adi ", all_commands, gdb_stdout);
>> +}
>> +
>> +/* Read attributes of a maps entry in /proc/[pid]/adi/maps.  */
>> +
>> +static void
>> +read_maps_entry (const char *line,
>> +              ULONGEST *addr, ULONGEST *endaddr)
>> +{
>> +  const char *p = line;
>> +
>> +  *addr = strtoulst (p, &p, 16);
>> +  if (*p == '-')
>> +    p++;
>> +
>> +  *endaddr = strtoulst (p, &p, 16);
>> +}
>> +
>> +/* Check if ADI is available.  */
>> +
>> +static bool
>> +adi_available (void)
>> +{
>> +  pid_t pid = ptid_get_pid (inferior_ptid);
>> +  struct sparc64_adi_info *proc = get_adi_info_proc (pid);
>> +
>> +  if (proc->stat.checked_avail)
>> +    return proc->stat.is_avail;
>> +
>> +  proc->stat.checked_avail = true;
>> +  if (target_auxv_search (&current_target, AT_ADI_BLKSZ, &proc->stat.blksize) <= 0)
> THis line is too long.
>
>> +    return false;
>> +  target_auxv_search (&current_target, AT_ADI_NBITS, &proc->stat.nbits);
>> +  proc->stat.max_version = (1 << proc->stat.nbits) - 2;
>> +  proc->stat.is_avail = true;
>> +
>> +  return proc->stat.is_avail;
>> +}
>> +
>> +/* Normalize a versioned address - a VA with ADI bits (63-60) set.  */
>> +
>> +CORE_ADDR
>> +adi_normalize_address (CORE_ADDR addr)
>> +{
>> +  struct adi_stat_t adi_stat = get_adi_info (ptid_get_pid (inferior_ptid));
>> +
>> +  if (adi_stat.nbits)
>> +    return ((CORE_ADDR)(((long)addr << adi_stat.nbits) >> adi_stat.nbits));
>> +  return addr;
>> +}
>> +
>> +/* Align a normalized address - a VA with bit 59 sign extended into ADI bits.  */
>> +
>> +static CORE_ADDR
>> +adi_align_address (CORE_ADDR naddr)
>> +{
>> +  struct adi_stat_t adi_stat = get_adi_info (ptid_get_pid (inferior_ptid));
>> +
>> +  return (naddr - (naddr % adi_stat.blksize)) / adi_stat.blksize;
>> +}
>> +
>> +/* Convert a byte count to count at a ratio of 1:adi_blksz.  */
>> +
>> +static int
>> +adi_convert_byte_count (CORE_ADDR naddr, int nbytes, CORE_ADDR locl)
>> +{
>> +  struct adi_stat_t adi_stat = get_adi_info (ptid_get_pid (inferior_ptid));
>> +
>> +  return ((naddr + nbytes + adi_stat.blksize - 1) / adi_stat.blksize) - locl;
>> +}
>> +
>> +/* The /proc/[pid]/adi/tags file, which allows gdb to get/set ADI
>> +   version in a target process, maps linearly to the address space
>> +   of the target process at a ratio of 1:adi_blksz.
>> +
>> +   A read (or write) at offset K in the file returns (or modifies)
>> +   the ADI version tag stored in the cacheline containing address
>> +   K * adi_blksz, encoded as 1 version tag per byte.  The allowed
>> +   version tag values are between 0 and adi_stat.max_version.  */
>> +
>> +static int
>> +adi_tag_fd (void)
>> +{
>> +  pid_t pid = ptid_get_pid (inferior_ptid);
>> +  struct sparc64_adi_info *proc = get_adi_info_proc (pid);
>> +
>> +  if (proc->stat.tag_fd != 0)
>> +    return proc->stat.tag_fd;
>> +
>> +  proc->stat.tag_fd = open_adi_tag_fd (pid);
>> +  return proc->stat.tag_fd;
>> +}
>> +
>> +/* Check if an address set is ADI enabled, using /proc/[pid]/adi/maps
>> +   which was exported by the kernel and contains the currently ADI
>> +   mapped memory regions and their access permissions.  */
>> +
>> +static bool
>> +adi_is_addr_mapped (CORE_ADDR vaddr, size_t cnt)
>> +{
>> +  char filename[MAX_PROC_NAME_SIZE];
>> +  size_t i = 0;
>> +
>> +  pid_t pid = ptid_get_pid (inferior_ptid);
>> +  snprintf (filename, sizeof filename, "/proc/%d/adi/maps", pid);
>> +  char *data = target_fileio_read_stralloc (NULL, filename);
>> +  if (data)
>> +    {
>> +      struct cleanup *cleanup = make_cleanup (xfree, data);
>> +      struct adi_stat_t adi_stat = get_adi_info (pid);
>> +      char *line;
>> +      for (line = strtok (data, "\n"); line; line = strtok (NULL, "\n"))
>> +        {
>> +          ULONGEST addr, endaddr;
>> +
>> +          read_maps_entry (line, &addr, &endaddr);
>> +
>> +          while (((vaddr + i) * adi_stat.blksize) >= addr
>> +                 && ((vaddr + i) * adi_stat.blksize) < endaddr)
>> +            {
>> +              if (++i == cnt)
>> +                {
>> +                  do_cleanups (cleanup);
>> +                  return true;
>> +                }
>> +            }
>> +        }
>> +        do_cleanups (cleanup);
>> +      }
>> +    else
>> +      warning (_("unable to open /proc file '%s'"), filename);
>> +
>> +  return false;
>> +}
>> +
>> +/* Read ADI version tag value for memory locations starting at "vaddr"
>> +   for "size" number of bytes.  */
>> +
>> +static int
>> +adi_read_versions (CORE_ADDR vaddr, size_t size, unsigned char *tags)
>> +{
>> +  int fd = adi_tag_fd ();
>> +  if (fd == -1)
>> +    return -1;
>> +
>> +  if (!adi_is_addr_mapped (vaddr, size))
>> +    {
>> +      struct adi_stat_t adi_stat = get_adi_info (ptid_get_pid (inferior_ptid));
>> +      error(_("Address at 0x%lx is not in ADI maps"), vaddr*adi_stat.blksize);
>> +    }
>> +
>> +  return pread64 (fd, tags, size, vaddr);
>> +}
> Calling pread64 in -tdep.c makes few sense to me.  If you only want to
> support native debugging, move them to -nat.c file.
>
>> +
>> +/* Write ADI version tag for memory locations starting at "vaddr" for
>> + "size" number of bytes to "tags".  */
>> +
>> +static int
>> +adi_write_versions (CORE_ADDR vaddr, size_t size, unsigned char *tags)
>> +{
>> +  int fd = adi_tag_fd ();
>> +  if (fd == -1)
>> +    return -1;
>> +
>> +  if (!adi_is_addr_mapped (vaddr, size))
>> +    {
>> +      struct adi_stat_t adi_stat = get_adi_info (ptid_get_pid (inferior_ptid));
>> +      error(_("Address at 0x%lx is not in ADI maps"), vaddr*adi_stat.blksize);
>> +    }
>> +
>> +  return pwrite64 (fd, tags, size, vaddr);
>> +}
>> +
>> +/* Print ADI version tag value in "tags" for memory locations starting
>> +   at "vaddr" with number of "cnt".  */
>> +
>> +static void
>> +adi_print_versions (CORE_ADDR vaddr, size_t cnt, unsigned char *tags)
>> +{
>> +  int v_idx = 0;
>> +  const int maxelts = 8;  /* # of elements per line */
>> +
>> +  struct adi_stat_t adi_stat = get_adi_info (ptid_get_pid (inferior_ptid));
>> +
>> +  while (cnt > 0)
>> +    {
>> +      QUIT;
>> +      printf_filtered ("0x%016lx:\t", vaddr * adi_stat.blksize);
>> +      for (int i = maxelts; i > 0 && cnt > 0; i--, cnt--)
>> +        {
>> +          if (tags[v_idx] == 0xff)    /* no version tag */
>> +            printf_filtered ("- ");
>> +          else
>> +            printf_filtered ("%1X ", tags[v_idx]);
>> +          ++v_idx;
>> +        }
>> +      printf_filtered ("\n");
>> +      gdb_flush (gdb_stdout);
>> +      vaddr += maxelts;
>> +    }
>> +}
>> +
>> +static void
>> +do_examine (CORE_ADDR start, int bcnt)
>> +{
>> +  CORE_ADDR vaddr = adi_normalize_address (start);
>> +  struct cleanup *cleanup;
>> +
>> +  CORE_ADDR vstart = adi_align_address (vaddr);
>> +  int cnt = adi_convert_byte_count (vaddr, bcnt, vstart);
>> +  unsigned char *buf = (unsigned char *) xmalloc (cnt);
>> +  cleanup = make_cleanup (xfree, buf);
>> +  int read_cnt = adi_read_versions (vstart, cnt, buf);
>> +  if (read_cnt == -1)
>> +    error (_("No ADI information"));
>> +  else if (read_cnt < cnt)
>> +    error(_("No ADI information at 0x%lx"), vaddr);
>> +
>> +  adi_print_versions (vstart, cnt, buf);
>> +
>> +  do_cleanups (cleanup);
>> +}
>> +
>> +static void
>> +do_assign (CORE_ADDR start, size_t bcnt, int version)
>> +{
>> +  CORE_ADDR vaddr = adi_normalize_address (start);
>> +
>> +  CORE_ADDR vstart = adi_align_address (vaddr);
>> +  int cnt = adi_convert_byte_count (vaddr, bcnt, vstart);
>> +  unsigned char *buf = (unsigned char *) xmalloc (cnt);
> Use std::vector<gdb_byte>? so that you don't need memset and xfree.
>
>> +  memset(buf, version, cnt);
>> +
>> +  int set_cnt = adi_write_versions (vstart, cnt, buf);
>> +  xfree (buf);
>> +
>> +  if (set_cnt == -1)
>> +    error (_("No ADI information"));
>> +  else if (set_cnt < cnt)
>> +    error(_("No ADI information at 0x%lx"), vaddr);
>> +
>> +}
>> +
>> +/* ADI examine version tag command.
>> +
>> +   Command syntax:
>> +
>> +     adi (examine|x)/count <addr> */
>> +
>> +static void
>> +adi_examine_command (char *args, int from_tty)
>> +{
>> +  /* make sure program is active and adi is available */
>> +  if (!target_has_execution)
>> +    error (_("ADI command requires a live process/thread"));
>> +
>> +  if (!adi_available ())
>> +    error (_("No ADI information"));
>> +
>> +  pid_t pid = ptid_get_pid (inferior_ptid);
>> +  struct sparc64_adi_info *proc = get_adi_info_proc (pid);
>> +  int cnt = proc->stat.last_cnt? proc->stat.last_cnt : 1;
>> +  char *p = args;
>> +  if (p && *p == '/')
>> +    {
>> +      p++;
>> +      cnt = get_number (&p);
>> +    }
>> +
>> +  CORE_ADDR next_address = proc->stat.last_vaddr ? proc->stat.last_vaddr : 0;
>> +  if (p != 0 && *p != 0)
>> +    next_address = parse_and_eval_address (p);
>> +  else if (!proc->stat.last_cnt)
>> +    error (_("Usage: adi examine|x[/count] <addr>"));
> IIUC, "address" in this command is mandatory, and it makes sense to do
> so.  We don't need .last_vaddr.  I don't like such variable making the
> command stateful.
>
  

Wei-min Pan <weimin.pan@oracle.com> writes:

>> It is good to have a test to check the results and outputs here.
>
> Do you mean to add a test like "uiout->is_mi_like_p ()"?

No, what I meant is to add a test case to check/verify the output.