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

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

[Walfred Tedeschi]

In order to investigate the contents of the MPX boundary table two new commands
are added to GDB.  "mpx-info-bounds" and "mpx-set-bounds" are used to display
and set values on the MPX bound table.

2014-08-12  Mircea Gherzan  <mircea.gherzan@intel.com>
	    Walfred Tedeschi  <walfred.tedeschi@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 "mpx set-bounds" command.
	(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): Add new commands
	to commands list.
	* 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: Add documentation about "mpx-info-bounds"
	and "mpx-set-bounds"

---
 gdb/doc/gdb.texinfo                     |  10 ++
 gdb/i386-tdep.c                         | 227 ++++++++++++++++++++++++++++++++
 gdb/i386-tdep.h                         |   3 +
 gdb/testsuite/gdb.arch/i386-mpx-map.c   |  86 ++++++++++++
 gdb/testsuite/gdb.arch/i386-mpx-map.exp |  80 +++++++++++
 5 files changed, 406 insertions(+)
 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/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 026706a..62283c8 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -21482,6 +21482,16 @@ be returned in a register.
 @kindex show struct-convention
 Show the current setting of the convention to return @code{struct}s
 from functions.
+
+@item mpx-info-bound @var{pointer storage}
+@kindex mpx-info-bound
+Displays the bounds of a pointer given its storage.
+
+@item mpx-set-bound @var{storage} @var{lbound} @var{ubound}
+@kindex mpx-set-bound
+Set the bounds of a pointer in the map given its storage. This command takes
+three parameters @var{storage} is the pointers storage and @var{lbound} and
+@var{ubound} are lower and upper bounds new values respectivelly.
 @end table
 
 @subsubsection Intel(R) @dfn{Memory Protection Extensions} (MPX).
diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index 1e68505..b92139d 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -8619,6 +8619,225 @@ i386_coff_osabi_sniffer (bfd *abfd)
 }
 \f
 
+/* Show the MPX bounds for the given array/pointer.  */
+
+#define MPX_BASE_MASK (~(ULONGEST)0xfff)
+
+static unsigned long
+i386_mpx_bd_base (void)
+{
+  struct regcache *rcache;
+  struct gdbarch_tdep *tdep;
+  ULONGEST ret;
+  enum register_status stat;
+  struct gdb_exception except;
+
+  rcache = get_current_regcache ();
+  tdep = gdbarch_tdep (get_regcache_arch (rcache));
+
+  stat = regcache_raw_read_unsigned (rcache, tdep->bndcfgu_regnum, &ret);
+
+  if (stat != REG_VALID)
+    error (_("BNDCFGU register invalid, read status %d."), stat);
+
+  return ret & MPX_BASE_MASK;
+}
+
+/* Check if the current target is MPX enabled.  */
+
+int
+i386_mpx_enabled (void)
+{
+  const struct gdbarch_tdep *tdep = gdbarch_tdep (get_current_arch ());
+  const struct target_desc *tdesc = tdep->tdesc;
+
+  /* Try MPX registers.  */
+  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]   */
+
+/* Pure pointer math for 64 bit address translation.  */
+static CORE_ADDR
+i386_mpx_get_bt_entry (CORE_ADDR ptr, CORE_ADDR bd_base)
+{
+  ULONGEST offset1;
+  ULONGEST offset2;
+  ULONGEST mpx_bd_mask, bd_ptr_r_shift, bd_ptr_l_shift;
+  ULONGEST 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 ();
+  int ret = 0;
+  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;
+
+  ret = target_read_memory (bd_entry_addr, (gdb_byte *) &bd_entry,
+			    sizeof (bd_entry));
+
+  if (ret)
+    error (_("Cannot read bounds directory entry at 0x%lx."),
+	   (long) bd_entry_addr);
+
+  if ((bd_entry & 0x1) == 0)
+    error (_("Invalid bounds directory entry at 0x%lx."),
+	   (long) bd_entry_addr);
+
+  bt_addr = bd_entry & ~bt_select_r_shift;
+
+  offset2 = ((ptr & bt_mask) >> bt_select_r_shift) << bt_select_l_shift;
+
+  return bt_addr + offset2;
+}
+
+
+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]);
+      ui_out_text (uiout, ", size = ");
+      /* 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 = ((long long int) ~bt_entry[1] - (long long int) bt_entry[0]);
+      size = (size > -1 ? size + 1 : 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");
+    }
+}
+
+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 ret;
+
+  if (!i386_mpx_enabled ())
+    error (_("MPX not supported on this target."));
+
+  if (!args)
+    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);
+
+  ret = target_read_memory (bt_entry_addr, (gdb_byte *) bt_entry,
+			    sizeof (bt_entry));
+  if (ret)
+    error (_("Cannot read bounds table entry at %lx."), bt_entry_addr);
+
+  i386_mpx_print_bounds (bt_entry);
+}
+
+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[4];
+  int ret;
+  char *addr_str, *lower_str, *upper_str, *tmp;
+
+  if (!i386_mpx_enabled ())
+    error ("MPX not supported on this target.");
+
+  if (!args)
+    error ("Address of pointer variable expected.");
+
+  addr_str = strtok (args, " ");
+  if (!addr_str)
+    error ("Missing address of the pointer in the command.");
+
+  lower_str = strtok (NULL, " ");
+  if (!lower_str)
+    error ("Missing lower bound in the command.");
+
+  upper_str = strtok (NULL, " ");
+  if (!upper_str)
+    error ("Missing upper bound in the command.");
+
+  addr = parse_and_eval_address (addr_str);
+  lower = parse_and_eval_address (lower_str);
+  upper = parse_and_eval_address (upper_str);
+
+  bd_base = i386_mpx_bd_base ();
+  bt_entry_addr = i386_mpx_get_bt_entry (addr, bd_base);
+
+  ret = target_read_memory (bt_entry_addr, (gdb_byte *) bt_entry,
+			    sizeof (bt_entry));
+  if (ret)
+    error ("Cannot read bounds table entry at 0x%lx", (long) bt_entry_addr);
+
+  bt_entry[0] = (uint64_t) lower;
+  bt_entry[1] = ~(uint64_t) upper;
+
+  ret = target_write_memory (bt_entry_addr, (gdb_byte *) bt_entry,
+			     sizeof (bt_entry));
+
+  if (ret)
+    error ("Cannot write bounds table entry at 0x%lx", (long) bt_entry_addr);
+  else
+    i386_mpx_print_bounds (bt_entry);
+}
+
+
 /* Provide a prototype to silence -Wmissing-prototypes.  */
 void _initialize_i386_tdep (void);
 
@@ -8649,6 +8868,14 @@ is \"default\"."),
 			NULL, /* FIXME: i18n: */
 			&setlist, &showlist);
 
+  add_cmd ("mpx-info-bounds", no_class, i386_mpx_info_bounds,
+	   "show the memory bounds for a given array/pointer storage.",
+	   &cmdlist);
+
+  add_cmd ("mpx-set-bounds", no_class, i386_mpx_set_bounds,
+	   "set the memory bounds for a given array/pointer storage",
+	   &cmdlist);
+
   gdbarch_register_osabi_sniffer (bfd_arch_i386, bfd_target_coff_flavour,
 				  i386_coff_osabi_sniffer);
 
diff --git a/gdb/i386-tdep.h b/gdb/i386-tdep.h
index e0950a3..42da850 100644
--- a/gdb/i386-tdep.h
+++ b/gdb/i386-tdep.h
@@ -434,4 +434,7 @@ extern int i386_stap_is_single_operand (struct gdbarch *gdbarch,
 extern int i386_stap_parse_special_token (struct gdbarch *gdbarch,
 					  struct stap_parse_info *p);
 
+int
+i386_mpx_enabled (void);
+
 #endif /* i386-tdep.h */
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..441131a
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/i386-mpx-map.c
@@ -0,0 +1,86 @@
+/* Test program for MPX map allocated bounds.
+
+   Copyright 2014 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 <stdio.h>
+#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..539e1dd
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/i386-mpx-map.exp
@@ -0,0 +1,80 @@
+# Copyright 2014 Free Software Foundation, Inc.
+#
+# Contributed by Intel Corp. <mircea.gherzan@intel.com>,
+#                            <walfred.tedeschi@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"
+send_gdb "print have_mpx ()\r"
+
+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 "mpx-info-bounds 0x0" "Invalid bounds directory entry at 0x\[0-9a-fA-F\]+." "NULL address of the pointer"
+
+gdb_continue_to_breakpoint "after-decl" ".*after-decl.*"
+#gdb_test "mpx-info-bounds a" "Invalid bounds directory entry at 0x\[0-9a-fA-F\]+." "pointer instead of pointer address"
+# Fix size
+
+gdb_test "mpx-info-bounds &a" "Null bounds on map: pointer value = 0x0+." "address of a NULL pointer"
+
+gdb_continue_to_breakpoint "after-alloc" ".*after-alloc.*"
+gdb_test "mpx-info-bounds &a" "Null bounds on map: pointer value = 0x\[0-9a-fA-F\]+." "pointer after allocation"
+
+#
+gdb_continue_to_breakpoint "after-assign" ".*after-assign.*"
+gdb_test "mpx-info-bounds &x" "\\\{lbound = 0x\[0-9a-fA-F\]+, ubound = 0x\[0-9a-fA-F\]+\\\}: pointer value = 0x\[0-9a-fA-F\]+, size = -1, metadata = 0x0+." "pointer after assignment"
+gdb_test "mpx-set-bounds 0x0 0x1 0x2" "Invalid bounds directory entry at 0x\[0-9a-fA-F\]+." "mpx-set-bounds: NULL address of the pointer"
+#
+gdb_test "mpx-set-bounds &x 0xcafebabe 0xdeadbeef" "
+\\\{lbound = 0x0*cafebabe, ubound = 0x0*deadbeef\\\}: pointer value = 0x\[0-9a-fA-F\]+, size = 330236978, metadata = 0x0+." "mpx-set-bounds: set bounds for a valid pointer address"
+#
+gdb_test "mpx-info-bounds &x" "\\\{lbound = 0x0*cafebabe, ubound = 0x0*deadbeef\\\}: pointer value = 0x\[0-9a-fA-F\]+, size = 330236978, metadata = 0x0+." "mpx-set-bounds: bounds map entry after mpx-set-bounds"
-- 
1.9.1

Re: [PATCH] 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, 30 Sep 2014 09:29:08 +0200
> 
> In order to investigate the contents of the MPX boundary table two new commands
> are added to GDB.  "mpx-info-bounds" and "mpx-set-bounds" are used to display
> and set values on the MPX bound table.

Thanks.

> 	* NEWS: List new commands for MPX support.

I don't see this part in the changeset you've sent.

> doc:
> 	* gdb.texinfo: Add documentation about "mpx-info-bounds"
> 	and "mpx-set-bounds"

Please state the name of the node in which you make the changes (as if
it were a function).

> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -21482,6 +21482,16 @@ be returned in a register.
>  @kindex show struct-convention
>  Show the current setting of the convention to return @code{struct}s
>  from functions.
> +
> +@item mpx-info-bound @var{pointer storage}

Shouldn't this go into the next subsection, which describes features
specific to the MPX support?

> +@kindex mpx-info-bound
> +Displays the bounds of a pointer given its storage.

What is a "pointer's storage"?  We should explain that here, otherwise
this text is entirely unclear.  I also think that we should say a bit
more about the bounds, so that readers will understand what this
feature is about and how to use it to their advantage.

> +@item mpx-set-bound @var{storage} @var{lbound} @var{ubound}
> +@kindex mpx-set-bound
> +Set the bounds of a pointer in the map given its storage. This command takes
                                                           ^^
Two spaces between sentences, please.

> +three parameters @var{storage} is the pointers storage and @var{lbound} and

Please add a colon ":" after "parameters", and a comma "," between
"storage" and "and".

> +@var{ubound} are lower and upper bounds new values respectivelly.

"are the new values for the lower and the upper bound, respectively"

(Only 1 'l' in "respectively".)

> +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[4];
> +  int ret;
> +  char *addr_str, *lower_str, *upper_str, *tmp;
> +
> +  if (!i386_mpx_enabled ())
> +    error ("MPX not supported on this target.");
> +
> +  if (!args)
> +    error ("Address of pointer variable expected.");
> +
> +  addr_str = strtok (args, " ");
> +  if (!addr_str)
> +    error ("Missing address of the pointer in the command.");
> +
> +  lower_str = strtok (NULL, " ");
> +  if (!lower_str)
> +    error ("Missing lower bound in the command.");
> +
> +  upper_str = strtok (NULL, " ");
> +  if (!upper_str)
> +    error ("Missing upper bound in the command.");
> +
> +  addr = parse_and_eval_address (addr_str);
> +  lower = parse_and_eval_address (lower_str);
> +  upper = parse_and_eval_address (upper_str);
> +
> +  bd_base = i386_mpx_bd_base ();
> +  bt_entry_addr = i386_mpx_get_bt_entry (addr, bd_base);
> +
> +  ret = target_read_memory (bt_entry_addr, (gdb_byte *) bt_entry,
> +			    sizeof (bt_entry));
> +  if (ret)
> +    error ("Cannot read bounds table entry at 0x%lx", (long) bt_entry_addr);
> +
> +  bt_entry[0] = (uint64_t) lower;
> +  bt_entry[1] = ~(uint64_t) upper;
> +
> +  ret = target_write_memory (bt_entry_addr, (gdb_byte *) bt_entry,
> +			     sizeof (bt_entry));
> +
> +  if (ret)
> +    error ("Cannot write bounds table entry at 0x%lx", (long) bt_entry_addr);
> +  else
> +    i386_mpx_print_bounds (bt_entry);
> +}

Why aren't error messages in this function inside _() ?

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

[Walfred Tedeschi]

Am 9/30/2014 4:17 PM, schrieb Eli Zaretskii:
Eli,

Thank you very much for your prompt review.  See my answers below:
>> From: Walfred Tedeschi <walfred.tedeschi@intel.com>
>> Cc: gdb-patches@sourceware.org, Walfred Tedeschi <walfred.tedeschi@intel.com>
>> Date: Tue, 30 Sep 2014 09:29:08 +0200
>>
>> In order to investigate the contents of the MPX boundary table two new commands
>> are added to GDB.  "mpx-info-bounds" and "mpx-set-bounds" are used to display
>> and set values on the MPX bound table.
> Thanks.
>
>> 	* NEWS: List new commands for MPX support.
> I don't see this part in the changeset you've sent.
I think it has got lost will add it!
>> doc:
>> 	* gdb.texinfo: Add documentation about "mpx-info-bounds"
>> 	and "mpx-set-bounds"
> Please state the name of the node in which you make the changes (as if
> it were a function).
Ok!
>> --- a/gdb/doc/gdb.texinfo
>> +++ b/gdb/doc/gdb.texinfo
>> @@ -21482,6 +21482,16 @@ be returned in a register.
>>   @kindex show struct-convention
>>   Show the current setting of the convention to return @code{struct}s
>>   from functions.
>> +
>> +@item mpx-info-bound @var{pointer storage}
> Shouldn't this go into the next subsection, which describes features
> specific to the MPX support?
Yes it should! Will move.
>> +@kindex mpx-info-bound
>> +Displays the bounds of a pointer given its storage.
> What is a "pointer's storage"?  We should explain that here, otherwise
> this text is entirely unclear.  I also think that we should say a bit
> more about the bounds, so that readers will understand what this
> feature is about and how to use it to their advantage.
>
>> +@item mpx-set-bound @var{storage} @var{lbound} @var{ubound}
>> +@kindex mpx-set-bound
>> +Set the bounds of a pointer in the map given its storage. This command takes
>                                                             ^^
> Two spaces between sentences, please.
Sorry!
>> +three parameters @var{storage} is the pointers storage and @var{lbound} and
> Please add a colon ":" after "parameters", and a comma "," between
> "storage" and "and".
>
>> +@var{ubound} are lower and upper bounds new values respectivelly.
> "are the new values for the lower and the upper bound, respectively"
>
> (Only 1 'l' in "respectively".)
>
>> +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[4];
>> +  int ret;
>> +  char *addr_str, *lower_str, *upper_str, *tmp;
>> +
>> +  if (!i386_mpx_enabled ())
>> +    error ("MPX not supported on this target.");
>> +
>> +  if (!args)
>> +    error ("Address of pointer variable expected.");
>> +
>> +  addr_str = strtok (args, " ");
>> +  if (!addr_str)
>> +    error ("Missing address of the pointer in the command.");
>> +
>> +  lower_str = strtok (NULL, " ");
>> +  if (!lower_str)
>> +    error ("Missing lower bound in the command.");
>> +
>> +  upper_str = strtok (NULL, " ");
>> +  if (!upper_str)
>> +    error ("Missing upper bound in the command.");
>> +
>> +  addr = parse_and_eval_address (addr_str);
>> +  lower = parse_and_eval_address (lower_str);
>> +  upper = parse_and_eval_address (upper_str);
>> +
>> +  bd_base = i386_mpx_bd_base ();
>> +  bt_entry_addr = i386_mpx_get_bt_entry (addr, bd_base);
>> +
>> +  ret = target_read_memory (bt_entry_addr, (gdb_byte *) bt_entry,
>> +			    sizeof (bt_entry));
>> +  if (ret)
>> +    error ("Cannot read bounds table entry at 0x%lx", (long) bt_entry_addr);
>> +
>> +  bt_entry[0] = (uint64_t) lower;
>> +  bt_entry[1] = ~(uint64_t) upper;
>> +
>> +  ret = target_write_memory (bt_entry_addr, (gdb_byte *) bt_entry,
>> +			     sizeof (bt_entry));
>> +
>> +  if (ret)
>> +    error ("Cannot write bounds table entry at 0x%lx", (long) bt_entry_addr);
>> +  else
>> +    i386_mpx_print_bounds (bt_entry);
>> +}
> Why aren't error messages in this function inside _() ?
Have to fix it!


Thanks a lot again!
For the next review you will see all those changes addressed.

Best regards,
-Fred
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] Add support for bound table in the Intel MPX context.

[Joel Brobecker]

> In order to investigate the contents of the MPX boundary table two new commands
> are added to GDB.  "mpx-info-bounds" and "mpx-set-bounds" are used to display
> and set values on the MPX bound table.
> 
> 2014-08-12  Mircea Gherzan  <mircea.gherzan@intel.com>
> 	    Walfred Tedeschi  <walfred.tedeschi@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 "mpx set-bounds" command.
> 	(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): Add new commands
> 	to commands list.
> 	* 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: Add documentation about "mpx-info-bounds"
> 	and "mpx-set-bounds"

Sorry again for the late review. In the future, please give us a couple
of weeks to review, and then feel free to ping us weekly afterwards.

For the new commands, since they are mpx-specific, I would prefer
if they were prefixed, rather than in the global "domain". I don't
know how the other maintainers feel about that, but I'd rather keep
non-prefixed commands to commands that are indeed general.

I would like to propose...
    show mpx bound
    set mpx bound lbound ubound
... but the fact that setting the mpx bounds takes 2 arguments,
this is somewhat of a departure from the typical "set" command.
So I sent an RFC to discuss this separately:
https://www.sourceware.org/ml/gdb-patches/2014-11/msg00550.html
Please do not change your code until we have a resolution of
the above.

Comments on the code below...

> +/* Show the MPX bounds for the given array/pointer.  */
> +
> +#define MPX_BASE_MASK (~(ULONGEST)0xfff)

Space after "(ULONGEST)".

> +
> +static unsigned long
> +i386_mpx_bd_base (void)

All new functions should have an introductory comment. We decided to
make no exceptions, even for obvious functions. However, for functions
that implement a given interface, a simple one-line comment saying so
suffices. Eg:

    /* Implement the "mpx-set-bound" command.  */

Or:

    /* Implement the "frame_align" gdbarch method.  */

Or:

    /* Implement the "to_open" target_ops method.  */

The idea in the last couple of examples is that the expected behavior
of that routine is expected to match what the gdbarch/target_ops vector
documents. So, no need to duplicate it (which makes also changing the
interface less labour intensive, as we only need to adjust the doco
at a single location).

Can you add those to all new functions you're defining here?

> +  struct regcache *rcache;
> +  struct gdbarch_tdep *tdep;
> +  ULONGEST ret;
> +  enum register_status stat;
> +  struct gdb_exception except;
> +
> +  rcache = get_current_regcache ();
> +  tdep = gdbarch_tdep (get_regcache_arch (rcache));
> +
> +  stat = regcache_raw_read_unsigned (rcache, tdep->bndcfgu_regnum, &ret);
> +
> +  if (stat != REG_VALID)
> +    error (_("BNDCFGU register invalid, read status %d."), stat);
> +
> +  return ret & MPX_BASE_MASK;
> +}
> +
> +/* Check if the current target is MPX enabled.  */
> +
> +int
> +i386_mpx_enabled (void)

Can you explain why this function is made global rather than static?

> +/* Pure pointer math for 64 bit address translation.  */
> +static CORE_ADDR
> +i386_mpx_get_bt_entry (CORE_ADDR ptr, CORE_ADDR bd_base)

Another GDB Coding Style rule (sorry...):
Can you always have an empty line between a function's documentation
and the function's start?

Also, can you adjust the comment to actually say what it does,
rather than talk about how it does it?

> +{
> +  ULONGEST offset1;
> +  ULONGEST offset2;
> +  ULONGEST mpx_bd_mask, bd_ptr_r_shift, bd_ptr_l_shift;
> +  ULONGEST 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 ();
> +  int ret = 0;

Any reason for the empty space above? As it turns out, it tricked
me into thinking that the gdbarch assignment wasn't also a variable
declaration, and so I thought that "ret" was declared in the middle
of code, which is currently not allowed.

> +  ret = target_read_memory (bd_entry_addr, (gdb_byte *) &bd_entry,
> +			    sizeof (bd_entry));

That looks suspicious - I think you're making the assumption that
host and target have the same endianness. To read a pointer from
inferior memory, you have read_memory_typed_address which should
be doing what you are looking for.

> +
> +  if (ret)
> +    error (_("Cannot read bounds directory entry at 0x%lx."),
> +	   (long) bd_entry_addr);
> +
> +  if ((bd_entry & 0x1) == 0)
> +    error (_("Invalid bounds directory entry at 0x%lx."),
> +	   (long) bd_entry_addr);

If you use read_memory_typed_address, then the first error message
should disappear. But if not, then use %s combined with paddress
in both error messages to format CORE_ADDR values.

> +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 ret;
> +
> +  if (!i386_mpx_enabled ())
> +    error (_("MPX not supported on this target."));
> +
> +  if (!args)
> +    error (_("Address of pointer variable expected."));

For pointer comparisons, the GDB project uses explicit comparisons.
So can you replace "!args" by "args != NULL"?

> +  addr = parse_and_eval_address (args);
> +
> +  bd_base = i386_mpx_bd_base ();
> +  bt_entry_addr = i386_mpx_get_bt_entry (addr, bd_base);
> +
> +  ret = target_read_memory (bt_entry_addr, (gdb_byte *) bt_entry,
> +			    sizeof (bt_entry));

Same as above - I think there is an endianness issue, here.

> +  if (ret)
> +    error (_("Cannot read bounds table entry at %lx."), bt_entry_addr);

If you decide to implement the read above such that you keep
the error message, then use paddress to print the address.

> +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[4];
> +  int ret;
> +  char *addr_str, *lower_str, *upper_str, *tmp;
> +
> +  if (!i386_mpx_enabled ())
> +    error ("MPX not supported on this target.");

Missing i18n marker.

> +
> +  if (!args)
> +    error ("Address of pointer variable expected.");

args != NULL again.
Missing i18n marker. There are other instances of these below,
but it's unclear if that code is going to stay. So I will allow
myself to skip mentioning those, but could you please make sure
you fix them all?

> +  addr_str = strtok (args, " ");

Please use gdb_buildargv to parse the arguments instead of doing it
yourself via strtok. Besides, those function modify its argument,
which means your function has an unexpected side-effect, and it
would also stop working the day we make "args" parameter a const.
I suspect this is also going to simplify a bit the verification
of the number of arguments.

> +  ret = target_read_memory (bt_entry_addr, (gdb_byte *) bt_entry,
> +			    sizeof (bt_entry));
> +  if (ret)
> +    error ("Cannot read bounds table entry at 0x%lx", (long) bt_entry_addr);

Same as before regarding reading an address from inferior memory.

> +  bt_entry[0] = (uint64_t) lower;
> +  bt_entry[1] = ~(uint64_t) upper;
> +
> +  ret = target_write_memory (bt_entry_addr, (gdb_byte *) bt_entry,
> +			     sizeof (bt_entry));

Same thing here. You'll need to convert the address to target
endianness first, and then write that.

> +  if (ret)
> +    error ("Cannot write bounds table entry at 0x%lx", (long) bt_entry_addr);
> +  else
> +    i386_mpx_print_bounds (bt_entry);

Personally, I would not re-print the bounds. If the user really wants
to confirm his operation, I'd let him call the command to show it
again.

> diff --git a/gdb/i386-tdep.h b/gdb/i386-tdep.h
> index e0950a3..42da850 100644
> --- a/gdb/i386-tdep.h
> +++ b/gdb/i386-tdep.h
> @@ -434,4 +434,7 @@ extern int i386_stap_is_single_operand (struct gdbarch *gdbarch,
>  extern int i386_stap_parse_special_token (struct gdbarch *gdbarch,
>  					  struct stap_parse_info *p);
>  
> +int
> +i386_mpx_enabled (void);

This is related to my question asking whether you really need this
function to be non-static. If the answer is yes, for some reason,
The function's name should not be be on the first column of the next
line (this is reserved for function implementations, in order to
facilitate locating them via grep). So:

int i386_mpx_enabled (void);

> 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..441131a
> --- /dev/null
> +++ b/gdb/testsuite/gdb.arch/i386-mpx-map.c
[...]
> +#include <stdio.h>

Is stdio.h really needed here? Using headers such as this one make
it much harder to run this test on bare-metal targets.

> +#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)

No space between "*" and "p", please.

> +{
> +  T *x;

Empty line after the end of the local variable declarations.

> +#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;

Same here, please (empty line).

> +      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..539e1dd
> --- /dev/null
> +++ b/gdb/testsuite/gdb.arch/i386-mpx-map.exp
> @@ -0,0 +1,80 @@
> +# Copyright 2014 Free Software Foundation, Inc.
> +#
> +# Contributed by Intel Corp. <mircea.gherzan@intel.com>,
> +#                            <walfred.tedeschi@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"
> +send_gdb "print have_mpx ()\r"
> +
> +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
> +    }
> +}

I think there is a problem here, and you're probably sending
the "print have_mpx()" command twice. Please remove the call
to "send_gdb" which we try to avoid as much as possible", and
see if the test still works. The part that really surprises me
is that you'd think that this could cause synchronization between
sent commands and expected output in the rest of the testcase
to be off by one command. How did it still work?

> +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 "mpx-info-bounds 0x0" "Invalid bounds directory entry at 0x\[0-9a-fA-F\]+." "NULL address of the pointer"

You can use $hex for matching addreses. I'll let you adjust the rest
of this file without explicitly pointing out the locations where you
can do so...

> +gdb_continue_to_breakpoint "after-decl" ".*after-decl.*"
> +#gdb_test "mpx-info-bounds a" "Invalid bounds directory entry at 0x\[0-9a-fA-F\]+." "pointer instead of pointer address"
> +# Fix size

Please do not introduce commented-out code, at least not without
explicitly explaining why.

> +gdb_test "mpx-info-bounds &a" "Null bounds on map: pointer value = 0x0+." "address of a NULL pointer"

You might want to escape the last dot, if you want to make sure
that value is 0.

> +gdb_test "mpx-info-bounds &x" "\\\{lbound = 0x\[0-9a-fA-F\]+, ubound = 0x\[0-9a-fA-F\]+\\\}: pointer value = 0x\[0-9a-fA-F\]+, size = -1, metadata = 0x0+." "pointer after assignment"

Same here for the last dot.

-- 
Joel

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

[Walfred Tedeschi]

In order to investigate the contents of the MPX boundary table two new commands
are added to GDB.  "mpx-info-bounds" and "mpx-set-bounds" are used to display
and set values on the MPX bound table.

2014-08-12  Mircea Gherzan  <mircea.gherzan@intel.com>
	    Walfred Tedeschi  <walfred.tedeschi@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 "mpx set-bounds" command.
	(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): Add new commands
	to commands list.
	* 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: Add documentation about "mpx-info-bounds"
	and "mpx-set-bounds"

---
 gdb/doc/gdb.texinfo                     |  10 ++
 gdb/i386-tdep.c                         | 227 ++++++++++++++++++++++++++++++++
 gdb/i386-tdep.h                         |   3 +
 gdb/testsuite/gdb.arch/i386-mpx-map.c   |  86 ++++++++++++
 gdb/testsuite/gdb.arch/i386-mpx-map.exp |  80 +++++++++++
 5 files changed, 406 insertions(+)
 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/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 026706a..62283c8 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -21482,6 +21482,16 @@ be returned in a register.
 @kindex show struct-convention
 Show the current setting of the convention to return @code{struct}s
 from functions.
+
+@item mpx-info-bound @var{pointer storage}
+@kindex mpx-info-bound
+Displays the bounds of a pointer given its storage.
+
+@item mpx-set-bound @var{storage} @var{lbound} @var{ubound}
+@kindex mpx-set-bound
+Set the bounds of a pointer in the map given its storage. This command takes
+three parameters @var{storage} is the pointers storage and @var{lbound} and
+@var{ubound} are lower and upper bounds new values respectivelly.
 @end table
 
 @subsubsection Intel(R) @dfn{Memory Protection Extensions} (MPX).
diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index 1e68505..b92139d 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -8619,6 +8619,225 @@ i386_coff_osabi_sniffer (bfd *abfd)
 }
 \f
 
+/* Show the MPX bounds for the given array/pointer.  */
+
+#define MPX_BASE_MASK (~(ULONGEST)0xfff)
+
+static unsigned long
+i386_mpx_bd_base (void)
+{
+  struct regcache *rcache;
+  struct gdbarch_tdep *tdep;
+  ULONGEST ret;
+  enum register_status stat;
+  struct gdb_exception except;
+
+  rcache = get_current_regcache ();
+  tdep = gdbarch_tdep (get_regcache_arch (rcache));
+
+  stat = regcache_raw_read_unsigned (rcache, tdep->bndcfgu_regnum, &ret);
+
+  if (stat != REG_VALID)
+    error (_("BNDCFGU register invalid, read status %d."), stat);
+
+  return ret & MPX_BASE_MASK;
+}
+
+/* Check if the current target is MPX enabled.  */
+
+int
+i386_mpx_enabled (void)
+{
+  const struct gdbarch_tdep *tdep = gdbarch_tdep (get_current_arch ());
+  const struct target_desc *tdesc = tdep->tdesc;
+
+  /* Try MPX registers.  */
+  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]   */
+
+/* Pure pointer math for 64 bit address translation.  */
+static CORE_ADDR
+i386_mpx_get_bt_entry (CORE_ADDR ptr, CORE_ADDR bd_base)
+{
+  ULONGEST offset1;
+  ULONGEST offset2;
+  ULONGEST mpx_bd_mask, bd_ptr_r_shift, bd_ptr_l_shift;
+  ULONGEST 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 ();
+  int ret = 0;
+  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;
+
+  ret = target_read_memory (bd_entry_addr, (gdb_byte *) &bd_entry,
+			    sizeof (bd_entry));
+
+  if (ret)
+    error (_("Cannot read bounds directory entry at 0x%lx."),
+	   (long) bd_entry_addr);
+
+  if ((bd_entry & 0x1) == 0)
+    error (_("Invalid bounds directory entry at 0x%lx."),
+	   (long) bd_entry_addr);
+
+  bt_addr = bd_entry & ~bt_select_r_shift;
+
+  offset2 = ((ptr & bt_mask) >> bt_select_r_shift) << bt_select_l_shift;
+
+  return bt_addr + offset2;
+}
+
+
+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]);
+      ui_out_text (uiout, ", size = ");
+      /* 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 = ((long long int) ~bt_entry[1] - (long long int) bt_entry[0]);
+      size = (size > -1 ? size + 1 : 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");
+    }
+}
+
+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 ret;
+
+  if (!i386_mpx_enabled ())
+    error (_("MPX not supported on this target."));
+
+  if (!args)
+    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);
+
+  ret = target_read_memory (bt_entry_addr, (gdb_byte *) bt_entry,
+			    sizeof (bt_entry));
+  if (ret)
+    error (_("Cannot read bounds table entry at %lx."), bt_entry_addr);
+
+  i386_mpx_print_bounds (bt_entry);
+}
+
+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[4];
+  int ret;
+  char *addr_str, *lower_str, *upper_str, *tmp;
+
+  if (!i386_mpx_enabled ())
+    error ("MPX not supported on this target.");
+
+  if (!args)
+    error ("Address of pointer variable expected.");
+
+  addr_str = strtok (args, " ");
+  if (!addr_str)
+    error ("Missing address of the pointer in the command.");
+
+  lower_str = strtok (NULL, " ");
+  if (!lower_str)
+    error ("Missing lower bound in the command.");
+
+  upper_str = strtok (NULL, " ");
+  if (!upper_str)
+    error ("Missing upper bound in the command.");
+
+  addr = parse_and_eval_address (addr_str);
+  lower = parse_and_eval_address (lower_str);
+  upper = parse_and_eval_address (upper_str);
+
+  bd_base = i386_mpx_bd_base ();
+  bt_entry_addr = i386_mpx_get_bt_entry (addr, bd_base);
+
+  ret = target_read_memory (bt_entry_addr, (gdb_byte *) bt_entry,
+			    sizeof (bt_entry));
+  if (ret)
+    error ("Cannot read bounds table entry at 0x%lx", (long) bt_entry_addr);
+
+  bt_entry[0] = (uint64_t) lower;
+  bt_entry[1] = ~(uint64_t) upper;
+
+  ret = target_write_memory (bt_entry_addr, (gdb_byte *) bt_entry,
+			     sizeof (bt_entry));
+
+  if (ret)
+    error ("Cannot write bounds table entry at 0x%lx", (long) bt_entry_addr);
+  else
+    i386_mpx_print_bounds (bt_entry);
+}
+
+
 /* Provide a prototype to silence -Wmissing-prototypes.  */
 void _initialize_i386_tdep (void);
 
@@ -8649,6 +8868,14 @@ is \"default\"."),
 			NULL, /* FIXME: i18n: */
 			&setlist, &showlist);
 
+  add_cmd ("mpx-info-bounds", no_class, i386_mpx_info_bounds,
+	   "show the memory bounds for a given array/pointer storage.",
+	   &cmdlist);
+
+  add_cmd ("mpx-set-bounds", no_class, i386_mpx_set_bounds,
+	   "set the memory bounds for a given array/pointer storage",
+	   &cmdlist);
+
   gdbarch_register_osabi_sniffer (bfd_arch_i386, bfd_target_coff_flavour,
 				  i386_coff_osabi_sniffer);
 
diff --git a/gdb/i386-tdep.h b/gdb/i386-tdep.h
index e0950a3..42da850 100644
--- a/gdb/i386-tdep.h
+++ b/gdb/i386-tdep.h
@@ -434,4 +434,7 @@ extern int i386_stap_is_single_operand (struct gdbarch *gdbarch,
 extern int i386_stap_parse_special_token (struct gdbarch *gdbarch,
 					  struct stap_parse_info *p);
 
+int
+i386_mpx_enabled (void);
+
 #endif /* i386-tdep.h */
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..441131a
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/i386-mpx-map.c
@@ -0,0 +1,86 @@
+/* Test program for MPX map allocated bounds.
+
+   Copyright 2014 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 <stdio.h>
+#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..539e1dd
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/i386-mpx-map.exp
@@ -0,0 +1,80 @@
+# Copyright 2014 Free Software Foundation, Inc.
+#
+# Contributed by Intel Corp. <mircea.gherzan@intel.com>,
+#                            <walfred.tedeschi@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"
+send_gdb "print have_mpx ()\r"
+
+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 "mpx-info-bounds 0x0" "Invalid bounds directory entry at 0x\[0-9a-fA-F\]+." "NULL address of the pointer"
+
+gdb_continue_to_breakpoint "after-decl" ".*after-decl.*"
+#gdb_test "mpx-info-bounds a" "Invalid bounds directory entry at 0x\[0-9a-fA-F\]+." "pointer instead of pointer address"
+# Fix size
+
+gdb_test "mpx-info-bounds &a" "Null bounds on map: pointer value = 0x0+." "address of a NULL pointer"
+
+gdb_continue_to_breakpoint "after-alloc" ".*after-alloc.*"
+gdb_test "mpx-info-bounds &a" "Null bounds on map: pointer value = 0x\[0-9a-fA-F\]+." "pointer after allocation"
+
+#
+gdb_continue_to_breakpoint "after-assign" ".*after-assign.*"
+gdb_test "mpx-info-bounds &x" "\\\{lbound = 0x\[0-9a-fA-F\]+, ubound = 0x\[0-9a-fA-F\]+\\\}: pointer value = 0x\[0-9a-fA-F\]+, size = -1, metadata = 0x0+." "pointer after assignment"
+gdb_test "mpx-set-bounds 0x0 0x1 0x2" "Invalid bounds directory entry at 0x\[0-9a-fA-F\]+." "mpx-set-bounds: NULL address of the pointer"
+#
+gdb_test "mpx-set-bounds &x 0xcafebabe 0xdeadbeef" "
+\\\{lbound = 0x0*cafebabe, ubound = 0x0*deadbeef\\\}: pointer value = 0x\[0-9a-fA-F\]+, size = 330236978, metadata = 0x0+." "mpx-set-bounds: set bounds for a valid pointer address"
+#
+gdb_test "mpx-info-bounds &x" "\\\{lbound = 0x0*cafebabe, ubound = 0x0*deadbeef\\\}: pointer value = 0x\[0-9a-fA-F\]+, size = 330236978, metadata = 0x0+." "mpx-set-bounds: bounds map entry after mpx-set-bounds"
-- 
1.9.1