Re: [PATCH v4] gdb: ADI support

[Yao Qi <qiyaoltc at gmail dot com>]

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

> +/* 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;
> +};

Use C++ STL list.  I suggested it on the v3 review.

> +
> +/* Normalize a versioned address - a VA with ADI bits (63-60) set.  */
> +
> +static 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.  */

This line is too long.

> +
> +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;
> +}
> +

> +
> +/* 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);

These two lines are too long.

> +    }
> +
> +  int target_errno;
> +  return target_fileio_pread (fd, tags, size, vaddr, &target_errno);
> +}
> +
> +/* 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);
> +    }
> +
> +  int target_errno;
> +  return target_fileio_pwrite (fd, tags, size, vaddr, &target_errno);
> +}
> +
> +/* Print ADI version tag value in "tags" for memory locations starting
> +   at "vaddr" with number of "cnt".  */
> +

Replace "tags" with TAGS, "cnt' with CNT, "vaddr" with VADDR.

> +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
> +adi_assign_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"));
> +
> +  char *exp = args;
> +  if (exp == 0)
> +    error_no_arg (_("Usage: adi assign|a[/count] <addr> = <version>"));
> +
> +  char *q = (char *) strchr (exp, '=');
> +  if (q)
> +    *q++ = 0;
> +  else
> +    error (_("Usage: adi assign|a[/count] <addr> = <version>"));
> +
> +  size_t cnt = 1;
> +  char *p = args;
> +  if (exp && *exp == '/')
> +    {
> +      p = exp + 1;
> +      cnt = get_number (&p);
> +    }
> +
> +  CORE_ADDR next_address = 0;
> +  if (p != 0 && *p != 0)
> +    next_address = parse_and_eval_address (p);
> +  else
> +    error (_("Usage: adi assign|a[/count] <addr> = <version>"));
> +
> +  int version = 0;
> +  if (q)           /* parse version tag */

if (q != NULL)

> +    {
> +      struct adi_stat_t adi_stat = get_adi_info (ptid_get_pid (inferior_ptid));

The ling is too long.

> +      version = parse_and_eval_long (q);
> +      if (version < 0 || version > adi_stat.max_version)
> +        error (_("Invalid ADI version tag %d"), version);
> +    }
> +
> +  do_assign(next_address, cnt, version);
> +}
> +
> +void
> +_initialize_sparc64_adi_tdep (void)
> +{
> +
> +  add_prefix_cmd ("adi", class_support, info_adi_command,
> +                  _("ADI version related commands."),
> +                  &sparc64adilist, "adi ", 0, &cmdlist);
> +  add_cmd ("examine", class_support, adi_examine_command,
> +           _("Examine ADI versions."), &sparc64adilist);
> +  add_alias_cmd ("x", "examine", no_class, 1, &sparc64adilist);
> +  add_cmd ("assign", class_support, adi_assign_command,
> +           _("Assign ADI versions."), &sparc64adilist);
> +
> +}
> +
> +
>  /* The functions on this page are intended to be used to classify
>     function arguments.  */
>  
> @@ -1290,6 +1805,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);
> +}
> +
>  void
>  sparc64_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
>  {
> @@ -1342,6 +1865,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 +2191,4 @@ const struct sparc_fpregmap sparc64_bsd_fpregmap =
>    0 * 8,			/* %f0 */
>    32 * 8,			/* %fsr */
>  };
> +
> diff --git a/gdb/sparc64-tdep.h b/gdb/sparc64-tdep.h
> index 324778e..da61a82 100644
> --- a/gdb/sparc64-tdep.h
> +++ b/gdb/sparc64-tdep.h
> @@ -138,4 +138,6 @@ extern struct trad_frame_saved_reg *
>  
>  extern const struct sparc_fpregmap sparc64_bsd_fpregmap;
>  
> +extern void sparc64_forget_process (pid_t pid);
> +
>  #endif /* sparc64-tdep.h */
> diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
> index 6b22498..5f8837f 100644
> --- a/gdb/testsuite/ChangeLog
> +++ b/gdb/testsuite/ChangeLog
> @@ -1,3 +1,7 @@
> +2017-07-18  Weimin Pan  <weimin.pan@oracle.com>
> +
> +	* gdb.arch/sparc64-adi.exp: New file.

and gdb.arch/sparc64-adi.c.

> +
>  2017-07-09  Tom Tromey  <tom@tromey.com>
>  
>  	* gdb.dwarf2/shortpiece.exp: New file.
> diff --git a/gdb/testsuite/gdb.arch/sparc64-adi.c b/gdb/testsuite/gdb.arch/sparc64-adi.c
> new file mode 100755
> index 0000000..a5557ef
> --- /dev/null
> +++ b/gdb/testsuite/gdb.arch/sparc64-adi.c
> @@ -0,0 +1,135 @@
> +/* Application Data Integrity (ADI) test in sparc64.
> +
> +   Copyright 2017 Free Software Foundation, Inc.
> +
> +   This file is part of GDB.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +#include "sparc64-adilib.h"
> +
> +#define MAPSIZE 8192
> +#define SHMSIZE 102400
> +#ifndef PROT_ADI
> +#define PROT_ADI 0x10
> +#endif
> +
> +static int
> +memory_fill(char *addr, size_t size, int pattern)
> +{

In general, the test case should follow the GNU coding standard too.

> +	long	*aligned_addr = (long *) addr;
> +	long	i;
> +	for (i = 0; i < size / sizeof (long); i += ONEKB) {
> +		*aligned_addr = pattern;
> +		aligned_addr = aligned_addr + ONEKB;
> +	}
> +	return (0);
> +}
> +
> +int main()
> +{
> +	char	*haddr;
> +	caddr_t	vaddr;
> +	int	version;
> +
> +	// test ISM
> +	int shmid = shmget(IPC_PRIVATE, SHMSIZE, IPC_CREAT | 0666);
> +	if (shmid == -1) {
> +		printf ("ERROR: shmget failed with an error:%d\n", errno);
> +		exit(1);

The test case is run by GDB testsuite, rather than by human, so these
printing isn't useful.

> +	}
> +	char *shmaddr = (char *)shmat(shmid, NULL, 0x666 | SHM_RND);
> +	if (shmaddr == (char *)-1) {
> +		printf("ERROR: shmat failed with an error:%d\n", errno);
> +		shmctl(shmid, IPC_RMID, NULL);
> +		exit(1);
> +	}
> +	// enable ADI on ISM segment
> +	if (mprotect(shmaddr, SHMSIZE, PROT_READ|PROT_WRITE|PROT_ADI)) {
> +		perror("mprotect failed");
> +		goto err_out;
> +	}
> +	if (memory_fill(shmaddr, SHMSIZE, 0xdeadbeaf) != 0) { /* line breakpoint here */
> +		printf("ERROR: ISM cannot fill memory\n");
> +		exit(1);
> +	}
> +	adi_clr_version(shmaddr, SHMSIZE);
> +	caddr_t vshmaddr = adi_set_version(shmaddr, SHMSIZE, 0x8);
> +	if (vshmaddr == 0) {
> +		printf("ERROR: ISM cannot set version\n");
> +		exit(1);
> +	}
> +	// test mmap
> +	int fd = open("/dev/zero", O_RDWR);
> +	if (fd < 0){
> +		printf("ERROR:MMAP cannot open \n");
> +		exit(1);
> +	}
> +	char *maddr = (char *)mmap(NULL,MAPSIZE,PROT_READ|PROT_WRITE, MAP_PRIVATE, fd, 0);
> +	if (maddr == (char *)-1) {
> +			printf("ERROR: mmap failed with an error:%d\n", errno);
> +			exit(1);
> +	}
> +	// enable ADI
> +	if (mprotect(shmaddr, MAPSIZE, PROT_READ|PROT_WRITE|PROT_ADI)) {
> +		perror("mprotect failed");
> +		goto err_out;
> +	}
> +	if (memory_fill(maddr, MAPSIZE, 0xdeadbeaf) != 0) {
> +		printf("ERROR:MMAP cannot fill memory\n");
> +		exit(1);
> +	}
> +	caddr_t vmaddr = adi_set_version(maddr, MAPSIZE, 0x8);
> +	if (vmaddr == 0)
> +		printf ("ERROR: MMAP cannot set version\n");
> +
> +	// test heap
> +	haddr = (char*) memalign(MAPSIZE, MAPSIZE);
> +	if (!haddr) {
> +		printf ("ERROR: HEAP cannot memalign\n");
> +	}
> +	// enable ADI
> +	if (mprotect(shmaddr, MAPSIZE, PROT_READ|PROT_WRITE|PROT_ADI)) {
> +		perror("mprotect failed");
> +		goto err_out;
> +	}
> +
> +	if (memory_fill(haddr, MAPSIZE, 0xdeadbeaf) != 0) {
> +		printf("ERROR:HEAP cannot fill memory\n");
> +		exit(1);
> +	}
> +	adi_clr_version(haddr, MAPSIZE);
> +	/* Set some ADP version number */
> +	caddr_t vaddr1, vaddr2, vaddr3, vaddr4;
> +	vaddr = adi_set_version(haddr, 64*2, 0x8);
> +	vaddr1 = adi_set_version(haddr+64*2, 64*2, 0x9);
> +	vaddr2 = adi_clr_version(haddr+64*4, 64*2);
> +	vaddr3 = adi_set_version(haddr+64*6, 64*2, 0xa);
> +	vaddr4 = adi_set_version(haddr+64*8, 64*10, 0x3);
> +	if (vaddr == 0) {
> +		printf("ERROR: adi_set_version() failed to set version num\n");
> +		exit(1);
> +	}
> +	char *versioned_p = vaddr;
> +	*versioned_p = 'a';
> +	char *uvp = haddr;	// unversioned pointer
> +	*uvp = 'b';		// version mismatch trap
> +
> +	return (0);
> +err_out:
> +        if (shmdt((const void *)shmaddr) != 0)
> +                perror("Detach failure");
> +        shmctl(shmid, IPC_RMID, NULL);
> +        exit(1);
> +}
> diff --git a/gdb/testsuite/gdb.arch/sparc64-adi.exp b/gdb/testsuite/gdb.arch/sparc64-adi.exp
> new file mode 100644
> index 0000000..25ab183
> --- /dev/null
> +++ b/gdb/testsuite/gdb.arch/sparc64-adi.exp
> @@ -0,0 +1,55 @@
> +# Copyright 2017 Free Software Foundation, Inc.
> +
> +# 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/>.
> +
> +# This file is part of the gdb testsuite.
> +
> +# Basic tests of examining/assigning ADI version tags, and reporting precise mismatch.

This line is too long.

> +
> +if ![istarget "sparc64*-*-linux*"] then {
> +    verbose "Skipping sparc64 ADI test."
> +    return 0
> +}
> +
> +set testfile sparc64-adi
> +set srcfile ${testfile}.c
> +set binfile ${objdir}/${subdir}/${testfile}

Replace them with standard_testfile.

> +
> +set cflags "-g"
> +if { [prepare_for_testing "failed to prepare" $testfile $srcfile \
> +          [list additional_flags=$cflags libs=-ladi]] } {

[list debug libs=-ladi]

> +    return -1
> +}
> +
> +if ![runto_main] then {
> +    untested "could not run to main"
> +    return -1
> +}
> +
> +gdb_test "break [gdb_get_line_number "line breakpoint here"]" \
> +         "Breakpoint .* at .*${srcfile}.*" \
> +         "set line breakpoint in main"
> +gdb_continue_to_breakpoint "continue to line breakpoint in main"
> +
> +##########################################
> +set newadi "7"
> +gdb_test "adi x shmaddr" "" "examine ADI"

Can you match the output?

> +gdb_test "adi a/100 shmaddr=${newadi}" "" "assign ADI"

Likewise.

> +gdb_test "adi x/100 shmaddr" \
> +"0x\[0-9a-f\]+00:.*\t${newadi} ${newadi}.*" "examine new ADI"
> +gdb_test "adi a/100 shmaddr=0x0" "" "reset ADI"
> +gdb_test "continue" \
> +         ".*Program received signal SIGSEGV.*
> +.*ADI precise mismatch while accessing address.*" \
> +	"continue to sigsegv"
> diff --git a/gdb/testsuite/gdb.arch/sparc64-adilib.h b/gdb/testsuite/gdb.arch/sparc64-adilib.h
> new file mode 100755
> index 0000000..add026c
> --- /dev/null
> +++ b/gdb/testsuite/gdb.arch/sparc64-adilib.h
> @@ -0,0 +1,24 @@

Why do you need a header?

-- 
Yao (齐尧)