This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] gdb: ADI support



On 6/14/2017 8:23 AM, Eli Zaretskii wrote:
From: Weimin Pan <weimin.pan@oracle.com>
Date: Tue, 13 Jun 2017 19:05:44 -0500

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

	* sparc64-tdep.h: (adi_normalize_address): New export.
	* sparc64-adi-tdep.c: New file.
	* Makefile.in (ALLDEPFILES): Add sparc64-adi-tdep.c.
	* configure.tgt: Add sparc64-adi-tdep.o.
	* sparc64-linux-nat.c:
	(sparc64_linux_watchpoint_addr_within_range): New function.
	(_initialize_sparc64_linux_nat): Register sparc64_linux_watchpoint_addr_within_range.
	* sparc64-linux-tdep.c: Include <adi.h>.
	(sparc64_linux_handle_segmentation_fault): New function.
	(sparc64_linux_init_abi): Register sparc64_linux_handle_segmentation_fault
	* sparc64-tdep.c: (sparc64_pstate_type): Replace PID1 with MCDE.
gdb/doc/ChangeLog:
2017-06-13  Weimin Pan  <weimin.pan@oracle.com>
	* gdb.texinfo (Architectures): Add new Sparc64 section to document ADI support.
Thanks.  See below some comments about the documentation part:

+@node Sparc64
+@subsection Sparc64
+@cindex Sparc64 support
+@subsubsection ADI Support
I'd add an index entry for "Application Data Integrity" here.

Done,  added "@cindex Application Data Integrity".

+The M7 processor supports an Application Data Integrity (ADI) feature that
+detects invalid data accesses. When software allocates memory and enables ADI on the
Please leave two spaces between sentences, here and elsewhere.  Also,
please refill the paragraphs so that they are shorter than 80 cloumns.

Checked and corrected.

+@item adi examine Command
I think you should remove the "Command" part there.

Done for both "adi examine" and "adi assign".

+The adi examine command displays the value of one ADI version tag per cacheline.
        ^^^^^^^^^^^
This should be in @code{..}

Done for both.

+It has the following command syntax:
+
+@cindex examining memory
There's no need for this index entry here, because this part of the
text doesn't describe the commands that examine memory.  (There are
other such index entries in your patch; they should all be removed.)

OK, 2 of such entries removed.

+@table @code
+@kindex x @r{(examine memory)}
+@itemx adi (examine | x) [ / @var{n} ] @var{addr}
I think the @kindex entry here should be removed.  Instead, you should
have a "@kindex adi examine" earlier, where you mention the command
for the first time.

OK, moved it next to "@item adi examine".

+@table @r
+@item @var{n}, the byte count
This should not be a @table, and you don't need the @item.  Instead,
there should be plain text describing the command arguments.

OK, @table and @item removed.

+@item adi assign Command
Please add an index entry here about "adi assign".

+@cindex examining memory
+@table @code
+@kindex x @r{(examine memory)}
+@itemx adi (assign | a) [ / @var{n} ] @var{addr} = @var{tag}
+@end table
+
+@table @r
+@item @var{n}, the byte count
Same comments as above for "adi examine".

Done.

Do we need a NEWS entry for these new commands?

Is there any guideline as to when such an entry should be added?
I can add something like this:

        * NEWS: Add "adi examine" and "adi assign" commands for printing
        and modifying values of ADI version tag on Sparc64 platform.

Thanks for your comments.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]