Re: [PATCH v4] gdb: ADI support

[Wei-min Pan <weimin dot pan at oracle dot com>]

On 7/25/2017 8:15 AM, Yao Qi wrote:
> 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.

I'm using forward_list to replace the linked list.

> ...
> > +++ 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.

OK, done.

> .....
>
> > +    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?

Here are the results of running this new test:

% make check TESTS="gdb.arch/sparc64-adi.exp"
...
...

                === gdb tests ===

Schedule of variations:
    unix

Running target unix
Using /usr/share/dejagnu/baseboards/unix.exp as board description file 
for target.
Using /usr/share/dejagnu/config/unix.exp as generic interface file for 
target.
Using ~/binutils-gdb/gdb/testsuite/config/unix.exp as 
tool-and-target-specific interface file.
Running ~/binutils-gdb/gdb/testsuite/gdb.arch/sparc64-adi.exp ...

                === gdb Summary ===

# of expected passes            7
...

> > +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?

Merged the header file into sparc64-adi.c.