[PATCH v3] Add support for bound table in the Intel MPX context.

[Eli Zaretskii]

> From: Walfred Tedeschi <walfred.tedeschi@intel.com>
> Cc: gdb-patches@sourceware.org, Walfred Tedeschi <walfred.tedeschi@intel.com>
> Date: Mon,  8 Jun 2015 18:06:28 +0200
> 
> Intel(R) Memory protection bound information are located in register
> to be tested using the MPX new instructions. Since the number of
> bound registers are limited a table is used to provide storage for
> bounds during run-time.
> 
> In order to investigate the contents of the MPX bound table two new
> commands are added to GDB.  "show mpx bound" and "set mpx bound" are
> used to display and set values on the MPX bound table.

Thanks.

> doc:
> 	* gdb.texinfo: Add documentation about "show mpx bound"
> 	and "set mpx bound".

This log entry should mention the name(s) of the node(s) in which you
made changes.

> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -90,6 +90,11 @@ tui enable
>  tui disable
>    Explicit commands for enabling and disabling tui mode.
>  
> +show mpx bound
> +set mpx bound on i386 and amd64
> +   for Intel(R) MPX.  Support for bound table investigation on
> +   Intel(R) MPX enabled applications.

The "for Intel(R) MPX." part is redundant, and is also an incomplete
sentence.  I suggest to remove it.

> +Bounds can also be stored in bounds tables, which are stored in
> +application memory.  Pointers bounds are stored into the bounds table
> +by means of the pointers location address.

I suggest to rephrase the last sentence, so as to avoid the use of
passive tense:

  These tables store bounds for pointers by specifying an address of a
  buffer along with its bounds.

Also, please make sure to leave 2 spaces between sentences, not 1.

>                                             Evaluating and changing bounds
> +located in bound tables is therefore interesting while investigating bugs
> +on MPX context. New commands are then added for this purpose:        ^^^^
   ^^^^^^^^^^^^^^
"bugs related to MPX" sounds better, I think.

Also, these "new commands" will cease being new with time, so I
suggest to say simply "@value{GDBN} provides commands for this
purpose."

> +@item show mpx bound @var{pointer}
> +@kindex show mpx bound
> +Displays the bounds of a pointer @var{pointer}.
   ^^^^^^^^
"Display", to be consistent with our style of describing commands.

Also, you can simply say "Display the bounds of the given
@var{pointer}.", there's no need to repeat the word.

> +@item set mpx bound @var{pointer}, @var{lbound}, @var{ubound}
> +@kindex  set mpx bound
> +Set the bounds of a pointer in the map.

What map?  You didn't mention any maps before.

> +This command takes three parameters: @var{pointer} is the pointers

"@var{pointer} is the pointer whose bounds are to be changed"

> +value, @var{lbound} and @var{ubound}, are new values for lower
> +and upper bounds respectively.      ^

No need for that comma.

The documentation parts are okay with those fixes.

> +  stat = regcache_raw_read_unsigned (rcache, tdep->bndcfgu_regnum, &ret);
> +
> +  if (stat != REG_VALID)
> +    error (_("BNDCFGU register invalid, read status %d."), stat);

Is it wise to call a variable by the name of a widely used function?
Some peculiar environments might have a macro that redirects the call
to a replacement function (e.g., Gnulib might use rpl_stat), which
might cause trouble here.  IMO, it is safer to use a different name.

> +    error (_("Wrong number of arguments; Missing lower and upper bound."));

Please use a colon ':' rather than a semi-colon ';'.  Also, "Missing"
shouldn't be capitalized.

> +  add_cmd ("bound", no_class, i386_mpx_info_bounds,
> +	   "show the memory bounds for a given array/pointer storage\

"Show", with a capital S.

> +  add_cmd ("bound", no_class, i386_mpx_set_bounds,
> +	   "set the memory bounds for a given array/pointer storage\

"Set", likewise.

Thanks.