Re: [PATCH v4] gdb: ADI support

Yao Qi
2017-07-25T15:16:06Z

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);
> +
> +}
> +\f
> +
> /* 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);
> }
> \f
>
> @@ -1666,3 +2191,4 @@ const struct sparc_fpregmap sparc64_bsd_fpregmap =
> 0 * 8, /* %f0 */
> 32 * 8, /* %fsr */
> };
> +\f
> 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 (齐尧)