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

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

[Walfred Tedeschi]

Incorporated comments from Eli.

Thanks a lot for your review!

Thanks and regards,
-Fred


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.

2015-04-20  Walfred Tedeschi  <walfred.tedeschi@intel.com>
            Mircea Gherzan  <mircea.gherzan@intel.com>

	* i386-tdep.c (MPX_BASE_MASK, MPX_BD_MASK, MPX_BT_MASK, MPX_BD_MASK_32,
	MPX_BT_MASK_32): New macros.
	(i386_mpx_set_bounds): New function that implements
	the command "set-mpx-bound".
	(i386_mpx_enabled) Helper function to test MPX availability.
	(i386_mpx_bd_base) Helper function to calculate the base directory
	address. (i386_mpx_get_bt_entry) Helper function to access a bound
	table entry. (i386_mpx_print_bounds) Effectively display bound
	information. (_initialize_i386_tdep): Qdd new commands
	to commands "set mpx" and "show mpx". (_initialize_i386_tdep):
	Add "bound" to the commands "show mpx" and "set mpx" commands.
	(mpx_set_cmdlist and mpx_show_cmdlist):
	list for the new prefixed "set mpx" and "show mpx" commands.
	* NEWS: List new commands for MPX support.

testsuite:

	* gdb.arch/i386-mpx-map.c: New file.
	* gdb.arch/i386-mpx-map.exp: New File.

doc:
	* gdb.texinfo (i386): Add documentation about "show mpx bound"
	and "set mpx bound".
---
 gdb/NEWS                                |   4 +
 gdb/doc/gdb.texinfo                     |  20 ++-
 gdb/i386-tdep.c                         | 273 ++++++++++++++++++++++++++++++++
 gdb/testsuite/gdb.arch/i386-mpx-map.c   |  93 +++++++++++
 gdb/testsuite/gdb.arch/i386-mpx-map.exp |  76 +++++++++
 5 files changed, 465 insertions(+), 1 deletion(-)
 create mode 100644 gdb/testsuite/gdb.arch/i386-mpx-map.c
 create mode 100644 gdb/testsuite/gdb.arch/i386-mpx-map.exp

diff --git a/gdb/NEWS b/gdb/NEWS
index bbfb55d..11ccd4a 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -90,6 +90,10 @@ tui enable
 tui disable
   Explicit commands for enabling and disabling tui mode.
 
+show mpx bound
+set mpx bound on i386 and amd64
+   Support for bound table investigation on Intel(R) MPX enabled applications.
+
 * New options
 
 set debug dwarf-die
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 9ea846a..9e9138b 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -22127,7 +22127,7 @@ be returned in a register.
 @kindex show struct-convention
 Show the current setting of the convention to return @code{struct}s
 from functions.
-@end table
+
 
 @subsubsection Intel(R) @dfn{Memory Protection Extensions} (MPX).
 @cindex Intel(R) Memory Protection Extensions (MPX).
@@ -22162,6 +22162,24 @@ counterpart.  When the bnd0@dots{}bnd3 registers are displayed via
 Python, the display includes the memory size, in bits, accessible to
 the pointer.
 
+Bounds can also be stored in bounds tables, which are stored in
+application memory.  These tables store bounds for pointers by specifying
+the bounds pointer's value along with its bounds.  Evaluating and changing
+bounds located in bound tables is therefore interesting while investigating
+bugs on MPX context.  @value{GDBN} provides commands for this purpose:
+
+@item show mpx bound @var{pointer}
+@kindex show mpx bound
+Display bounds of the given @var{pointer}.
+
+@item set mpx bound @var{pointer}, @var{lbound}, @var{ubound}
+@kindex  set mpx bound
+Set the bounds of a pointer in the bound table.
+This command takes three parameters: @var{pointer} is the pointers
+whose bounds are to be changed, @var{lbound} and @var{ubound} are new values
+for lower and upper bounds respectively.
+@end table
+
 @node Alpha
 @subsection Alpha
 
diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index b4ffe1b..73e9dfd 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -8619,6 +8619,251 @@ i386_target_description (uint64_t xcr0)
     }
 }
 
+#define MPX_BASE_MASK (~(ULONGEST) 0xfff)
+
+/* Find the bound directory base address.  */
+
+static unsigned long
+i386_mpx_bd_base (void)
+{
+  struct regcache *rcache;
+  struct gdbarch_tdep *tdep;
+  ULONGEST ret;
+  enum register_status regstatus;
+  struct gdb_exception except;
+
+  rcache = get_current_regcache ();
+  tdep = gdbarch_tdep (get_regcache_arch (rcache));
+
+  regstatus = regcache_raw_read_unsigned (rcache, tdep->bndcfgu_regnum, &ret);
+
+  if (regstatus != REG_VALID)
+    error (_("BNDCFGU register invalid, read status %d."), regstatus);
+
+  return ret & MPX_BASE_MASK;
+}
+
+/* Check if the current target is MPX enabled.  */
+
+static int
+i386_mpx_enabled (void)
+{
+  const struct gdbarch_tdep *tdep = gdbarch_tdep (get_current_arch ());
+  const struct target_desc *tdesc = tdep->tdesc;
+
+  return (tdesc_find_feature (tdesc, "org.gnu.gdb.i386.mpx") != NULL);
+}
+
+#define MPX_BD_MASK     0xfffffff00000ULL	/* select bits [47:20]  */
+#define MPX_BT_MASK     0x0000000ffff8	        /* select bits [19:3]   */
+#define MPX_BD_MASK_32  0xfffff000	        /* select bits [31:12]  */
+#define MPX_BT_MASK_32  0x00000ffc	        /* select bits [11:2]   */
+
+/* Find the bound table entry given the pointer location and the base
+   address of the table.  */
+
+static CORE_ADDR
+i386_mpx_get_bt_entry (CORE_ADDR ptr, CORE_ADDR bd_base)
+{
+  CORE_ADDR offset1;
+  CORE_ADDR offset2;
+  CORE_ADDR mpx_bd_mask, bd_ptr_r_shift, bd_ptr_l_shift;
+  CORE_ADDR bt_mask, bt_select_r_shift, bt_select_l_shift;
+  CORE_ADDR bd_entry_addr;
+  CORE_ADDR bt_addr;
+  CORE_ADDR bd_entry;
+  struct gdbarch *gdbarch = get_current_arch ();
+  struct type *data_ptr_type = builtin_type (gdbarch)->builtin_data_ptr;
+
+
+  if (gdbarch_ptr_bit (gdbarch) == 64)
+    {
+      mpx_bd_mask = MPX_BD_MASK;
+      bd_ptr_r_shift = 20;
+      bd_ptr_l_shift = 3;
+      bt_select_r_shift = 3;
+      bt_select_l_shift = 5;
+      bt_mask = MPX_BT_MASK;
+    }
+  else
+    {
+      mpx_bd_mask = MPX_BD_MASK_32;
+      bd_ptr_r_shift = 12;
+      bd_ptr_l_shift = 2;
+      bt_select_r_shift = 2;
+      bt_select_l_shift = 4;
+      bt_mask = MPX_BT_MASK_32;
+    }
+
+  offset1 = ((ptr & mpx_bd_mask) >> bd_ptr_r_shift) << bd_ptr_l_shift;
+  bd_entry_addr = bd_base + offset1;
+  bd_entry = read_memory_typed_address (bd_entry_addr, data_ptr_type);
+
+  if ((bd_entry & 0x1) == 0)
+    error (_("Invalid bounds directory entry at %s."),
+	   paddress (get_current_arch (), bd_entry_addr));
+
+  /* Clearing status bit.  */
+  bd_entry--;
+  bt_addr = bd_entry & ~bt_select_r_shift;
+  offset2 = ((ptr & bt_mask) >> bt_select_r_shift) << bt_select_l_shift;
+
+  return bt_addr + offset2;
+}
+
+/* Print routine for the mpx bounds.  */
+
+static void
+i386_mpx_print_bounds (const CORE_ADDR bt_entry[])
+{
+  struct ui_out *uiout = current_uiout;
+  long long int size;
+  struct gdbarch *gdbarch = get_current_arch ();
+  CORE_ADDR onecompl = ~((CORE_ADDR) 0);
+  int bounds_in_map = ((~bt_entry[1] == 0 && bt_entry[0] == onecompl) ? 1 : 0);
+
+  if (bounds_in_map == 1)
+    {
+      ui_out_text (uiout, "Null bounds on map:");
+      ui_out_text (uiout, " pointer value = ");
+      ui_out_field_core_addr (uiout, "pointer-value", gdbarch, bt_entry[2]);
+      ui_out_text (uiout, ".");
+      ui_out_text (uiout, "\n");
+    }
+  else
+    {
+      ui_out_text (uiout, "{lbound = ");
+      ui_out_field_core_addr (uiout, "lower-bound", gdbarch, bt_entry[0]);
+      ui_out_text (uiout, ", ubound = ");
+
+      /* The upper bound is stored in 1's complement.  */
+      ui_out_field_core_addr (uiout, "upper-bound", gdbarch, ~bt_entry[1]);
+      ui_out_text (uiout, "}: pointer value = ");
+      ui_out_field_core_addr (uiout, "pointer-value", gdbarch, bt_entry[2]);
+
+      if (gdbarch_ptr_bit (gdbarch) == 64)
+	size = ( (~(int64_t) bt_entry[1]) - (int64_t) bt_entry[0]);
+      else
+	size = ( ~((int32_t) bt_entry[1]) - (int32_t) bt_entry[0]);
+
+      /* In case the bounds are 0x0 and 0xffff... the difference will be -1.
+	 -1 represents in this sense full memory access, and there is no need
+	 one to the size.  */
+
+      size = (size > -1 ? size + 1 : size);
+      ui_out_text (uiout, ", size = ");
+      ui_out_field_fmt (uiout, "size", "%lld", size);
+
+      ui_out_text (uiout, ", metadata = ");
+      ui_out_field_core_addr (uiout, "metadata", gdbarch, bt_entry[3]);
+      ui_out_text (uiout, "\n");
+    }
+}
+
+/* Implement the command "show mpx bound".  */
+
+static void
+i386_mpx_info_bounds (char *args, int from_tty)
+{
+  CORE_ADDR bd_base = 0;
+  CORE_ADDR addr;
+  CORE_ADDR bt_entry_addr = 0;
+  CORE_ADDR bt_entry[4];
+  int i;
+  struct gdbarch *gdbarch = get_current_arch ();
+  struct type *data_ptr_type = builtin_type (gdbarch)->builtin_data_ptr;
+
+  if (!i386_mpx_enabled ())
+    error (_("Intel(R) Memory Protection Extensions not\
+ supported on this target."));
+
+  if (args == NULL)
+    error (_("Address of pointer variable expected."));
+
+  addr = parse_and_eval_address (args);
+
+  bd_base = i386_mpx_bd_base ();
+  bt_entry_addr = i386_mpx_get_bt_entry (addr, bd_base);
+
+  memset (bt_entry, 0, sizeof (bt_entry));
+
+  for (i = 0; i < 4; i++)
+    bt_entry[i] = read_memory_typed_address (bt_entry_addr
+					     + i * data_ptr_type->length,
+					     data_ptr_type);
+
+  i386_mpx_print_bounds (bt_entry);
+}
+
+/* Implement the command "set mpx bound".  */
+
+static void
+i386_mpx_set_bounds (char *args, int from_tty)
+{
+  CORE_ADDR bd_base = 0;
+  CORE_ADDR addr, lower, upper;
+  CORE_ADDR bt_entry_addr = 0;
+  CORE_ADDR bt_entry[2];
+  const char *input = args;
+  int i;
+  struct gdbarch *gdbarch = get_current_arch ();
+  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
+  struct type *data_ptr_type = builtin_type (gdbarch)->builtin_data_ptr;
+
+  if (!i386_mpx_enabled ())
+    error (_("Intel(R) Memory Protection Extensions not supported\
+ on this target."));
+
+  if (args == NULL)
+    error (_("Pointer value expected."));
+
+  addr = value_as_address (parse_to_comma_and_eval (&input));
+
+  if (input[0] == ',')
+    ++input;
+  if (input[0] == '\0')
+    error (_("wrong number of arguments: missing lower and upper bound."));
+  lower = value_as_address (parse_to_comma_and_eval (&input));
+
+  if (input[0] == ',')
+    ++input;
+  if (input[0] == '\0')
+    error (_("Wrong number of arguments; Missing upper bound."));
+  upper = value_as_address (parse_to_comma_and_eval (&input));
+
+  bd_base = i386_mpx_bd_base ();
+  bt_entry_addr = i386_mpx_get_bt_entry (addr, bd_base);
+  for (i = 0; i < 2; i++)
+    bt_entry[i] = read_memory_typed_address (bt_entry_addr
+					     + i * data_ptr_type->length,
+					     data_ptr_type);
+  bt_entry[0] = (uint64_t) lower;
+  bt_entry[1] = ~(uint64_t) upper;
+
+  for (i = 0; i < 2; i++)
+    write_memory_unsigned_integer (bt_entry_addr + i * data_ptr_type->length,
+				   data_ptr_type->length, byte_order,
+				   bt_entry[i]);
+}
+
+static struct cmd_list_element *mpx_set_cmdlist, *mpx_show_cmdlist;
+
+/* Helper function for the CLI commands.  */
+
+static void
+set_mpx_cmd (char *args, int from_tty)
+{
+  help_list (mpx_set_cmdlist, "set mpx", all_commands, gdb_stdout);
+}
+
+/* Helper function for the CLI commands.  */
+
+static void
+show_mpx_cmd (char *args, int from_tty)
+{
+  cmd_show_list (mpx_show_cmdlist, from_tty, "");
+}
+
 /* Provide a prototype to silence -Wmissing-prototypes.  */
 void _initialize_i386_tdep (void);
 
@@ -8649,6 +8894,34 @@ is \"default\"."),
 			NULL, /* FIXME: i18n: */
 			&setlist, &showlist);
 
+  /* Add "mpx" prefix for the set commands.  */
+
+  add_prefix_cmd ("mpx", class_support, set_mpx_cmd, _("\
+Set Intel(R) Memory Protection Extensions specific variables."),
+		  &mpx_set_cmdlist, "set tdesc ",
+		  0 /* allow-unknown */, &setlist);
+
+  /* Add "mpx" prefix for the show commands.  */
+
+  add_prefix_cmd ("mpx", class_support, show_mpx_cmd, _("\
+Show Intel(R) Memory Protection Extensions specific variables."),
+		  &mpx_show_cmdlist, "show mpx ",
+		  0 /* allow-unknown */, &showlist);
+
+  /* Add "bound" command for the show mpx commands list.  */
+
+  add_cmd ("bound", no_class, i386_mpx_info_bounds,
+	   "Show the memory bounds for a given array/pointer storage\
+ in the bound table.",
+	   &mpx_show_cmdlist);
+
+  /* Add "bound" command for the set mpx commands list.  */
+
+  add_cmd ("bound", no_class, i386_mpx_set_bounds,
+	   "Set the memory bounds for a given array/pointer storage\
+ in the bound table.",
+	   &mpx_set_cmdlist);
+
   gdbarch_register_osabi_sniffer (bfd_arch_i386, bfd_target_coff_flavour,
 				  i386_coff_osabi_sniffer);
 
diff --git a/gdb/testsuite/gdb.arch/i386-mpx-map.c b/gdb/testsuite/gdb.arch/i386-mpx-map.c
new file mode 100644
index 0000000..8a9094c
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/i386-mpx-map.c
@@ -0,0 +1,93 @@
+/* Test program for MPX map allocated bounds.
+
+   Copyright 2015 Free Software Foundation, Inc.
+
+   Contributed by Intel Corp. <walfred.tedeschi@intel.com>
+			      <mircea.gherzan@intel.com>
+
+   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 <stdlib.h>
+#include "x86-cpuid.h"
+
+#ifndef NOINLINE
+#define NOINLINE __attribute__ ((noinline))
+#endif
+
+#define SIZE  5
+
+typedef int T;
+
+unsigned int have_mpx (void) NOINLINE;
+
+unsigned int NOINLINE
+have_mpx (void)
+{
+  unsigned int eax, ebx, ecx, edx;
+
+  if (!__get_cpuid (1, &eax, &ebx, &ecx, &edx))
+    return 0;
+
+  if ((ecx & bit_OSXSAVE) == bit_OSXSAVE)
+    {
+      if (__get_cpuid_max (0, NULL) < 7)
+	return 0;
+
+      __cpuid_count (7, 0, eax, ebx, ecx, edx);
+
+      if ((ebx & bit_MPX) == bit_MPX)
+	return 1;
+      else
+	return 0;
+    }
+  return 0;
+}
+
+void
+foo (T *p)
+{
+  T *x;
+
+#if defined  __GNUC__ && !defined __INTEL_COMPILER
+  __bnd_store_ptr_bounds (p, &p);
+#endif
+
+  x = p + SIZE - 1;
+
+#if defined  __GNUC__ && !defined __INTEL_COMPILER
+  __bnd_store_ptr_bounds (x, &x);
+#endif
+
+  return;			/* after-assign */
+}
+
+int
+main (void)
+{
+  if (have_mpx ())
+    {
+      T *a = NULL;
+
+      a = calloc (SIZE, sizeof (T));	/* after-decl */
+#if defined  __GNUC__ && !defined __INTEL_COMPILER
+      __bnd_store_ptr_bounds (a, &a);
+#endif
+
+      foo (a);				/* after-alloc */
+      free (a);
+    }
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.arch/i386-mpx-map.exp b/gdb/testsuite/gdb.arch/i386-mpx-map.exp
new file mode 100644
index 0000000..47efb16
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/i386-mpx-map.exp
@@ -0,0 +1,76 @@
+# Copyright 2015 Free Software Foundation, Inc.
+#
+# Contributed by Intel Corp. <walfred.tedeschi@intel.com>,
+#                            <mircea.gherzan@intel.com>
+#
+# 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/>.
+
+if { ![istarget i?86-*-*] && ![istarget x86_64-*-* ] } {
+    verbose "Skipping x86 MPX tests."
+    return
+}
+
+standard_testfile
+
+set comp_flags "-fmpx -I${srcdir}/../nat/"
+
+if { [prepare_for_testing ${testfile}.exp ${testfile} ${srcfile} \
+ [list debug nowarnings additional_flags=${comp_flags}]] } {
+    return -1
+}
+
+if ![runto_main] {
+	untested "could not run to main"
+	return -1
+}
+
+set supports_mpx 0
+set test "probe MPX support"
+
+gdb_test_multiple "print have_mpx()" $test {
+    -re ".. = 1\r\n$gdb_prompt $" {
+        pass $test
+        set supports_mpx 1
+    }
+    -re ".. = 0\r\n$gdb_prompt $" {
+        pass $test
+    }
+}
+
+if { !$supports_mpx } {
+    unsupported "processor does not support MPX"
+    return
+}
+
+gdb_breakpoint [ gdb_get_line_number "after-decl" ]
+gdb_breakpoint [ gdb_get_line_number "after-alloc" ]
+gdb_breakpoint [ gdb_get_line_number "after-assign" ]
+
+gdb_test "show mpx bound 0x0" "Invalid bounds directory entry at $hex." "NULL address of the pointer"
+
+gdb_continue_to_breakpoint "after-decl" ".*after-decl.*"
+gdb_test "show mpx bound a" "Invalid bounds directory entry at $hex." "pointer instead of pointer address"
+
+gdb_continue_to_breakpoint "after-alloc" ".*after-alloc.*"
+gdb_test "show mpx bound a" "\\\{lbound = $hex, ubound = $hex\\\}: pointer value = $hex, size = \[8, 4\], metadata = 0x0+" "pointer after allocation"
+
+gdb_continue_to_breakpoint "after-assign" ".*after-assign.*"
+gdb_test "show mpx bound x" "\\\{lbound = $hex, ubound = $hex\\\}: pointer value = $hex, size = \[8, 4\], metadata = 0x0+" "pointer after assignment"
+gdb_test "set mpx bound 0x0, 0x1, 0x2" "Invalid bounds directory entry at $hex." "set mpx bound: NULL address of the pointer"
+gdb_test_no_output "set mpx bound x, 0xcafebabe, 0xdeadbeef" "set mpx bound: set bounds for a valid pointer address"
+gdb_test "show mpx bound x" "\\\{lbound = .*cafebabe, ubound = .*deadbeef\\\}: pointer value = $hex, size = $decimal, metadata = 0x0+" "set mpx bound: bounds map entry after set mpx bound"
+
+
+gdb_test "set mpx bound 0x0, 0x1 0x2" "A syntax error in expression.*" "set mpx bound: Controlling syntax error, missing comma "
+gdb_test "set mpx bound 0x0, 0x1" "Wrong number of arguments.*" "set mpx bound: Controlling syntax error, missing argument "
-- 
2.1.4

Re: [PATCH V4] 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: Tue,  9 Jun 2015 15:34:07 +0200
> 
> Incorporated comments from Eli.

This is fine with me, as far as the documentation parts go.

Thanks.

RE: [PATCH V4] Add support for bound table in the Intel MPX context.

[Tedeschi, Walfred]

Eli,

Thanks again!

Best regards,
-Fred

-----Original Message-----
From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-owner@sourceware.org] On Behalf Of Eli Zaretskii
Sent: Tuesday, June 09, 2015 4:33 PM
To: Tedeschi, Walfred
Cc: brobecker@adacore.com; gdb-patches@sourceware.org; Tedeschi, Walfred
Subject: Re: [PATCH V4] Add support for bound table in the Intel MPX context.

> From: Walfred Tedeschi <walfred.tedeschi@intel.com>
> Cc: gdb-patches@sourceware.org, Walfred Tedeschi <walfred.tedeschi@intel.com>
> Date: Tue,  9 Jun 2015 15:34:07 +0200
> 
> Incorporated comments from Eli.

This is fine with me, as far as the documentation parts go.

Thanks.
Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052

Re: [PATCH V4] Add support for bound table in the Intel MPX context.

[Joel Brobecker]

> 2015-04-20  Walfred Tedeschi  <walfred.tedeschi@intel.com>
>             Mircea Gherzan  <mircea.gherzan@intel.com>
> 
> 	* i386-tdep.c (MPX_BASE_MASK, MPX_BD_MASK, MPX_BT_MASK, MPX_BD_MASK_32,
> 	MPX_BT_MASK_32): New macros.
> 	(i386_mpx_set_bounds): New function that implements
> 	the command "set-mpx-bound".
> 	(i386_mpx_enabled) Helper function to test MPX availability.
> 	(i386_mpx_bd_base) Helper function to calculate the base directory
> 	address. (i386_mpx_get_bt_entry) Helper function to access a bound
> 	table entry. (i386_mpx_print_bounds) Effectively display bound
> 	information. (_initialize_i386_tdep): Qdd new commands
> 	to commands "set mpx" and "show mpx". (_initialize_i386_tdep):
> 	Add "bound" to the commands "show mpx" and "set mpx" commands.
> 	(mpx_set_cmdlist and mpx_show_cmdlist):
> 	list for the new prefixed "set mpx" and "show mpx" commands.
> 	* NEWS: List new commands for MPX support.
> 
> testsuite:
> 
> 	* gdb.arch/i386-mpx-map.c: New file.
> 	* gdb.arch/i386-mpx-map.exp: New File.
> 
> doc:
> 	* gdb.texinfo (i386): Add documentation about "show mpx bound"
> 	and "set mpx bound".

> +/* Print routine for the mpx bounds.  */
> +
> +static void
> +i386_mpx_print_bounds (const CORE_ADDR bt_entry[])

I don't think we've been using the [] syntax for parameters, but
I don't see a problem with that per se.

What I'm wondering, however, is the size of the array. In particular,
you'll need, at least, to update the documentation. But if the size
is actually statically known (which it seems is at most 4 elements),
then perhaps it might be worth just saying it in the parameter type
declaration...

Other than that, the patch looks good to me.

-- 
Joel