Re: [PING] [PATCH v3] gdb: ADI support

[Yao Qi <qiyaoltc at gmail dot com>]

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.

-- 
Yao (齐尧)