[PATCH V4 2/2] Add mpx-bnd-init-on-return set/show command for inferior calls.

[PATCH V4 2/2] Add mpx-bnd-init-on-return set/show command for inferior calls.

[Walfred Tedeschi]

When using the return command, execution of a function is aborted
and present values are returned from that point.  That can cause
bound violations in the MPX context.  To avoid such side-effects,
a new setting ("mpx-bnd-init-on-return") was added to control
the initialization of bound register when using the return command.

As default the value of "mpx-bnd-init-on-return" is set to 1.  So
bound register are initilized when using the "return" command.

If set to True (the default), all bound registers are set to zero
when using the "return" command.

2016-03-02  Walfred Tedeschi  <walfred.tedeschi@intel.com>

gdb/ChangeLog:

	* i387-tdep.h (mpx_bnd_init_on_return): Declare new
	external variable.
	* amd64-tdep.c (mpx_bnd_init_on_return): Declare new variable.
	(amd64_return_value) Use mpx-bnd-init-on-return.
	(_initialize_amd64_tdep): Initialize mpx_bnd_init_on_return.
	* i386-tdep.c (mpx_bnd_init_on_return): Declare new	variable.
	(i386_return_value) Use mpx-bnd-init-on-return.
	(_initialize_i386_tdep): Initialize mpx_bnd_init_on_return.
	(show_mpx_init_on_return): New function.
	(_initialize_i386_tdepamd64_init_abi): Add new commands set/show
	mpx-bnd-init-on-return.
	* NEWS: Add entry for set/show mpx-bnd-init-on-return command.

gdb/doc/ChangeLog:

	* gdb.texinfo (Intel Memory Protection Extensions): Add
	documentation on using set/show mpx-bnd-init-on-return.

gdb/testsuite/ChangeLog:

	* i386-mpx-call.c: New file.
	* i386-mpx-call.exp: New file.

---
 gdb/NEWS                                 |   4 +
 gdb/amd64-tdep.c                         |  45 +++++++++++
 gdb/doc/gdb.texinfo                      |  20 +++++
 gdb/i386-tdep.c                          |  57 +++++++++++++-
 gdb/i387-tdep.h                          |   3 +
 gdb/testsuite/gdb.arch/i386-mpx-call.c   | 126 +++++++++++++++++++++++++++++++
 gdb/testsuite/gdb.arch/i386-mpx-call.exp |  92 ++++++++++++++++++++++
 7 files changed, 346 insertions(+), 1 deletion(-)
 create mode 100644 gdb/testsuite/gdb.arch/i386-mpx-call.c
 create mode 100644 gdb/testsuite/gdb.arch/i386-mpx-call.exp

diff --git a/gdb/NEWS b/gdb/NEWS
index be15902..e8bdaa5 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -27,6 +27,10 @@ skip -rfunction regular-expression
   glob-style file names and regular expressions for function names.
   Additionally, a file spec and a function spec may now be combined.
 
+show mpx-bnd-init-on-return
+set mpx-bnd-init-on-return on i386 and amd64
+  Support for set bound registers to INIT state when using the command return.
+
 *** Changes in GDB 7.11
 
 * GDB now supports debugging kernel-based threads on FreeBSD.
diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
index 0e4e89b..9d982d8 100644
--- a/gdb/amd64-tdep.c
+++ b/gdb/amd64-tdep.c
@@ -722,12 +722,15 @@ amd64_classify (struct type *type, enum amd64_reg_class theclass[2])
     amd64_classify_aggregate (type, theclass);
 }
 
+int mpx_bnd_init_on_return;
+
 static enum return_value_convention
 amd64_return_value (struct gdbarch *gdbarch, struct value *function,
 		    struct type *type, struct regcache *regcache,
 		    gdb_byte *readbuf, const gdb_byte *writebuf)
 {
   enum amd64_reg_class theclass[2];
+  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
   int len = TYPE_LENGTH (type);
   static int integer_regnum[] = { AMD64_RAX_REGNUM, AMD64_RDX_REGNUM };
   static int sse_regnum[] = { AMD64_XMM0_REGNUM, AMD64_XMM1_REGNUM };
@@ -759,6 +762,28 @@ amd64_return_value (struct gdbarch *gdbarch, struct value *function,
 
 	  regcache_raw_read_unsigned (regcache, AMD64_RAX_REGNUM, &addr);
 	  read_memory (addr, readbuf, TYPE_LENGTH (type));
+
+	  /* In MPX-enabled programs, if the class is MEMORY, boundary is
+	     expected to be stored in bnd0 register.
+
+	     If the return value we are setting is due to the use of the
+	     "return" command (WRITEBUF is not NULL), it is likely that
+	     the return is made prematurely. In that situation, it is
+	     possible that the bnd0 register could still be unset, thus
+	     causing the program to receive a spurious bound violation.
+
+	     When "set mpx-bnd-init-on-return" is true, prevent this
+	     from happening by setting the bnd0 register to zero.  */
+
+	  if (writebuf != NULL
+	      && mpx_bnd_init_on_return
+	      && I387_BND0R_REGNUM (tdep) > 0)
+	    {
+	      gdb_byte bnd_buf[16];
+
+	      memset (bnd_buf, 0, 16);
+	      regcache_raw_write (regcache, I387_BND0R_REGNUM (tdep), bnd_buf);
+	    }
 	}
 
       return RETURN_VALUE_ABI_RETURNS_ADDRESS;
@@ -852,6 +877,25 @@ amd64_return_value (struct gdbarch *gdbarch, struct value *function,
 				 writebuf + i * 8);
     }
 
+  /* Same as in the class AMD64_MEMORY case above, where
+     we set bnd0 to zero when doing a premature "return"
+     and "set mpx-bnd-init-on-return" is true: Set all bound
+     registers to zero in order to avoid spurious bound
+     violations.  */
+
+  if (writebuf != NULL
+      && mpx_bnd_init_on_return
+      && I387_BND0R_REGNUM (tdep) > 0)
+    {
+      gdb_byte bnd_buf[16];
+      int i, init_bnd;
+
+      memset (bnd_buf, 0, 16);
+      for (i = 0; i < I387_NUM_BND_REGS; i++)
+	regcache_raw_write (regcache,
+			    I387_BND0R_REGNUM (tdep) + i, bnd_buf);
+    }
+
   return RETURN_VALUE_REGISTER_CONVENTION;
 }
 \f
@@ -3191,6 +3235,7 @@ void _initialize_amd64_tdep (void);
 void
 _initialize_amd64_tdep (void)
 {
+  mpx_bnd_init_on_return = 1;
   initialize_tdesc_amd64 ();
   initialize_tdesc_amd64_avx ();
   initialize_tdesc_amd64_mpx ();
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 4ec0ec1..96194e2 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -22438,6 +22438,26 @@ whose bounds are to be changed, @var{lbound} and @var{ubound} are new values
 for lower and upper bounds respectively.
 @end table
 
+While calling functions from the debugger, of an Intel MPX enabled program,
+boundary registers have to be set to the INIT state before performing the
+call, to avoid boundary violations while performing the call.  A bound is
+defined to be in the INIT state when the pointer associated to that boundary
+can access the whole memory, in this case the register bound register
+associated to it has value 0, e.g. if the register associated is bnd0raw
+its value will be @{0x0, 0x0@}.
+
+While the using the @command{return} bounds can propagate through
+execution causing a boundary violation.
+The behaviour of initializing bounds when using @command{return}
+can be controlled and vizualized via the following commands:
+@table @code
+@kindex set mpx-bnd-init-on-return
+When set to true bound registers will be set to the INIT state when
+using the "return" command.
+@kindex show mpx-bnd-init-on-return
+Show the state of mpx-bnd-init-on-return.
+@end table
+
 @node Alpha
 @subsection Alpha
 
diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index 8c3576c..9218fc4 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -2945,6 +2945,7 @@ i386_return_value (struct gdbarch *gdbarch, struct value *function,
 		   gdb_byte *readbuf, const gdb_byte *writebuf)
 {
   enum type_code code = TYPE_CODE (type);
+  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
 
   if (((code == TYPE_CODE_STRUCT
 	|| code == TYPE_CODE_UNION
@@ -2981,6 +2982,17 @@ i386_return_value (struct gdbarch *gdbarch, struct value *function,
 	  read_memory (addr, readbuf, TYPE_LENGTH (type));
 	}
 
+      /* See amd64_return_value in amd64-tdep.c.  */
+      if (writebuf != NULL
+	  && mpx_bnd_init_on_return
+	  && I387_BND0R_REGNUM (tdep) > 0)
+	  {
+	    gdb_byte bnd_buf[16];
+
+	    memset (bnd_buf, 0, 16);
+	    regcache_raw_write (regcache, I387_BND0R_REGNUM (tdep), bnd_buf);
+	  }
+
       return RETURN_VALUE_ABI_RETURNS_ADDRESS;
     }
 
@@ -3001,7 +3013,22 @@ i386_return_value (struct gdbarch *gdbarch, struct value *function,
   if (readbuf)
     i386_extract_return_value (gdbarch, type, regcache, readbuf);
   if (writebuf)
-    i386_store_return_value (gdbarch, type, regcache, writebuf);
+    {
+
+      /* See amd64_return_value in amd64-tdep.c.  */
+      if (mpx_bnd_init_on_return && I387_BND0R_REGNUM (tdep) > 0)
+	  {
+	    gdb_byte bnd_buf[16];
+	    int i, init_bnd;
+
+	    memset (bnd_buf, 0, 16);
+	    for (i = 0; i < I387_NUM_BND_REGS; i++)
+	      regcache_raw_write (regcache,
+				  I387_BND0R_REGNUM (tdep) + i, bnd_buf);
+	  }
+
+      i386_store_return_value (gdbarch, type, regcache, writebuf);
+    }
 
   return RETURN_VALUE_REGISTER_CONVENTION;
 }
@@ -8956,6 +8983,23 @@ i386_mpx_set_bounds (char *args, int from_tty)
 				   bt_entry[i]);
 }
 
+
+int mpx_bnd_init_on_return;
+
+/*  Show state of the setting variable mpx-bnd-init-on-return.  */
+
+static void
+show_mpx_bnd_init_on_return (struct ui_file *file, int from_tty,
+			     struct cmd_list_element *c, const char *value)
+{
+  if (mpx_bnd_init_on_return)
+    fprintf_filtered (file,
+		      _("BND registers will be initialized on return.\n"));
+  else
+    fprintf_filtered (file,
+		      _("BND registers will not be initialized on return.\n"));
+}
+
 static struct cmd_list_element *mpx_set_cmdlist, *mpx_show_cmdlist;
 
 /* Helper function for the CLI commands.  */
@@ -8982,6 +9026,8 @@ _initialize_i386_tdep (void)
 {
   register_gdbarch_init (bfd_arch_i386, i386_gdbarch_init);
 
+  mpx_bnd_init_on_return = 1;
+
   /* Add the variable that controls the disassembly flavor.  */
   add_setshow_enum_cmd ("disassembly-flavor", no_class, valid_flavors,
 			&disassembly_flavor, _("\
@@ -9032,6 +9078,15 @@ Show Intel Memory Protection Extensions specific variables."),
  in the bound table.",
 	   &mpx_set_cmdlist);
 
+  add_setshow_boolean_cmd ("mpx-bnd-init-on-return", no_class,
+			   &mpx_bnd_init_on_return, _("\
+ Set the bnd registers to INIT state when returning from a call."), _("\
+ Show the state of the mpx-bnd-init-on-return."),
+			   NULL,
+			   NULL,
+			   show_mpx_bnd_init_on_return,
+			   &setlist, &showlist);
+
   gdbarch_register_osabi_sniffer (bfd_arch_i386, bfd_target_coff_flavour,
 				  i386_coff_osabi_sniffer);
 
diff --git a/gdb/i387-tdep.h b/gdb/i387-tdep.h
index b4dc2b7..c08ae23 100644
--- a/gdb/i387-tdep.h
+++ b/gdb/i387-tdep.h
@@ -156,4 +156,7 @@ extern void i387_collect_xsave (const struct regcache *regcache,
 extern void i387_return_value (struct gdbarch *gdbarch,
 			       struct regcache *regcache);
 
+/* Value of the "set mpx-bnd-init-on-return setting.  */
+extern int mpx_bnd_init_on_return;
+
 #endif /* i387-tdep.h */
diff --git a/gdb/testsuite/gdb.arch/i386-mpx-call.c b/gdb/testsuite/gdb.arch/i386-mpx-call.c
new file mode 100644
index 0000000..d05b8a8
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/i386-mpx-call.c
@@ -0,0 +1,126 @@
+/* Test for inferior function calls MPX context.
+
+   Copyright (C) 2016 Free Software Foundation, Inc.
+
+   Contributed by Intel Corp.  <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/>. */
+
+#include <stdio.h>
+#include "x86-cpuid.h"
+
+#define OUR_SIZE    5
+
+int gx[OUR_SIZE];
+int ga[OUR_SIZE];
+int gb[OUR_SIZE];
+int gc[OUR_SIZE];
+int gd[OUR_SIZE];
+
+unsigned int
+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;
+}
+
+int
+bp1 (int value)
+{
+  return 1;
+}
+
+int
+bp2 (int value)
+{
+  return 1;
+}
+
+int
+upper (int *p, int *a, int *b, int *c, int *d, int len)
+{
+  int value;
+
+  value = *(p + len);
+  value = *(a + len);
+  value = *(b + len);
+  value = *(c + len);
+  value = *(d + len);
+
+  value = value - value + 1;
+  return value;
+}
+
+int *
+upper_ptr (int *p, int *a, int *b, int *c, int *d, int len)
+{
+  int value;
+
+  value = *(p + len);
+  value = *(a + len);
+  value = *(b + len);
+  value = *(c + len);  /* bkpt 2.  */
+  value = *(d + len);  /* bkpt 3.  */
+  free (p);
+  p = calloc (OUR_SIZE * 2, sizeof (int));
+
+  return ++p;
+}
+
+
+int
+main (void)
+{
+  if (have_mpx ())
+    {
+      int sx[OUR_SIZE];
+      int sa[OUR_SIZE];
+      int sb[OUR_SIZE];
+      int sc[OUR_SIZE];
+      int sd[OUR_SIZE];
+      int *x, *a, *b, *c, *d;
+
+      x = calloc (OUR_SIZE, sizeof (int));
+      a = calloc (OUR_SIZE, sizeof (int));
+      b = calloc (OUR_SIZE, sizeof (int));
+      c = calloc (OUR_SIZE, sizeof (int));
+      d = calloc (OUR_SIZE, sizeof (int));
+
+      upper (sx, sa, sb, sc, sd, 0);  /* bkpt 1.  */
+      x = upper_ptr (sx, sa, sb, sc, sd, 0);
+
+      free (x);
+      free (a);
+      free (b);
+      free (c);
+      free (d);
+    }
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.arch/i386-mpx-call.exp b/gdb/testsuite/gdb.arch/i386-mpx-call.exp
new file mode 100644
index 0000000..b94aff5
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/i386-mpx-call.exp
@@ -0,0 +1,92 @@
+# Copyright (C) 2016 Free Software Foundation, Inc.
+#
+# Contributed by Intel Corp. <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 "-mmpx -fcheck-pointer-bounds -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
+}
+
+gdb_test_multiple "print have_mpx ()" "have mpx" {
+    -re ".*= 1\r\n$gdb_prompt " {
+        pass "check whether processor supports MPX"
+    }
+    -re ".*= 0\r\n$gdb_prompt " {
+        verbose "processor does not support MPX; skipping MPX tests"
+        return
+    }
+}
+
+# Needed by the return command.
+gdb_test_no_output "set confirm off"
+
+set bound_reg " = \\\{lbound = $hex, ubound = $hex\\\}.*"
+
+set break "bkpt 1."
+gdb_breakpoint [gdb_get_line_number "${break}"]
+gdb_continue_to_breakpoint "${break}" ".*${break}.*"
+gdb_test "p upper (x, a, b, c, d, 0)" " = 1"\
+    "test the call of int function - int"
+gdb_test "p upper_ptr (x, a, b, c, d, 0)"\
+    " = \\\(int \\\*\\\) $hex" "test the call of function - ptr"
+
+set break "bkpt 2."
+gdb_breakpoint [gdb_get_line_number "${break}"]
+gdb_continue_to_breakpoint "${break}" ".*${break}.*"
+gdb_test "p \$bound0 = \$bnd0" "$bound_reg" "Saving the\
+    bnd0 result in a convenience variable 1"
+
+gdb_test "return a" "#0  $hex in main.*" "First return"
+gdb_test "p \(\$bnd0\.ubound == \$bound0\.ubound\)"\
+    " = 0" "return with bnd initialization off - ubound"
+gdb_test "p \(\$bnd0\.lbound ==\$bound0\.lbound\)"\
+    " = 0" "return with bnd initialization off - lbound"
+
+# Retesting with initialization off. 
+# All breakpoints were deleted need to recreate what we need.
+runto_main
+gdb_test_no_output "set mpx-bnd-init-on-return off"\
+    "Turn off initialization on return"
+
+set break "bkpt 3."
+gdb_breakpoint [gdb_get_line_number "${break}"]
+gdb_continue_to_breakpoint "${break}" ".*${break}.*"
+gdb_test "p \$bound0 = \$bnd0" "$bound_reg" "Saving the\
+    bnd0 result in a convenience variable 2"
+gdb_test "return a" "#0  $hex in main.*" "Second return"
+gdb_test "p \(\$bnd0\.ubound == \$bound0\.ubound\)"\
+    " = 1" "return with bnd initialization on - ubound"
+gdb_test "p \(\$bnd0\.lbound ==\$bound0\.lbound\)"\
+    " = 1" "return with bnd initialization on - lbound"
+
+gdb_test "show mpx-bnd-init-on-return"\
+    "BND registers will not be initialized on return\\."\
+    "double check initialization on return"
-- 
2.1.4

[PATCH V4 1/2] Initialize bnd register before performing inferior calls.

[Walfred Tedeschi]

BND registers should be initialized before performing an inferior call
to avoid undesired bound violations.

2016-02-03  Walfred Tedeschi <walfred.tedeschi@intel.com>

gdb/ChangeLog:

	* i386-tdep.c (i386_push_dummy_call): Initialize bnd registers.
	* amd64-tdep (amd64_push_dummy_call): Initialize bnd registers.

---
 gdb/amd64-tdep.c | 15 +++++++++++++++
 gdb/i386-tdep.c  | 15 +++++++++++++++
 2 files changed, 30 insertions(+)

diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
index a62efde..0e4e89b 100644
--- a/gdb/amd64-tdep.c
+++ b/gdb/amd64-tdep.c
@@ -995,8 +995,23 @@ amd64_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
 		       int struct_return, CORE_ADDR struct_addr)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
+  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
   gdb_byte buf[8];
 
+  /* When MPX is enabled, all bnd registers have to be initialized
+     before the call.  This avoids an undesired bound violation
+     during the function's execution.  */
+
+  if (I387_BND0R_REGNUM (tdep) > 0)
+    {
+      gdb_byte bnd_buf[16];
+      int i;
+
+      memset (bnd_buf, 0, 16);
+      for (i = 0; i < I387_BND0R_REGNUM (tdep); i++)
+	regcache_raw_write (regcache, I387_BND0R_REGNUM (tdep) + i, bnd_buf);
+    }
+
   /* Pass arguments.  */
   sp = amd64_push_arguments (regcache, nargs, args, sp, struct_return);
 
diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index 4c66edf..8c3576c 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -2660,11 +2660,26 @@ i386_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
 		      CORE_ADDR struct_addr)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
+  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
   gdb_byte buf[4];
   int i;
   int write_pass;
   int args_space = 0;
 
+  /* When MPX is enabled, all bnd registers have to be initialized
+     before the call.  This avoids an undesired bound violation
+     during the function's execution.  */
+
+  if (I387_BND0R_REGNUM (tdep) > 0)
+    {
+      gdb_byte bnd_buf[16];
+      int i;
+
+      memset (bnd_buf, 0, 16);
+      for (i = 0; i < I387_BND0R_REGNUM (tdep); i++)
+	regcache_raw_write (regcache, I387_BND0R_REGNUM (tdep) + i, bnd_buf);
+    }
+
   /* Determine the total space required for arguments and struct
      return address in a first pass (allowing for 16-byte-aligned
      arguments), then push arguments in a second pass.  */
-- 
2.1.4

[PATCH V4 0/2] BND register initialization

[Walfred Tedeschi]

Former patch series for Intel(R) MPX ABI was converted into
two patches.

1. Initialization of bnd registers previous of performing inferior calls.
2. Addition of mpx-bnd-init-on-return setting.

The initialization of bnd registers was added also for i386, instead of concentrating 
all code for amd64.

On the other hand the patch introducing the POINTER class, submitted
in V3, was removed.

Thanks again for your review,
-Fred


Walfred Tedeschi (2):
  Initialize bnd register before performing inferior calls.
  Add mpx-bnd-init-on-return set/show command for inferior calls.

 gdb/NEWS                                 |   4 +
 gdb/amd64-tdep.c                         |  60 +++++++++++++++
 gdb/doc/gdb.texinfo                      |  20 +++++
 gdb/i386-tdep.c                          |  72 +++++++++++++++++-
 gdb/i387-tdep.h                          |   3 +
 gdb/testsuite/gdb.arch/i386-mpx-call.c   | 126 +++++++++++++++++++++++++++++++
 gdb/testsuite/gdb.arch/i386-mpx-call.exp |  92 ++++++++++++++++++++++
 7 files changed, 376 insertions(+), 1 deletion(-)
 create mode 100644 gdb/testsuite/gdb.arch/i386-mpx-call.c
 create mode 100644 gdb/testsuite/gdb.arch/i386-mpx-call.exp

-- 
2.1.4

Re: [PATCH V4 2/2] Add mpx-bnd-init-on-return set/show command for inferior calls.

[Eli Zaretskii]

> From: Walfred Tedeschi <walfred.tedeschi@intel.com>
> Cc: gdb-patches@sourceware.org, Walfred Tedeschi <walfred.tedeschi@intel.com>
> Date: Wed,  2 Mar 2016 17:21:29 +0100
> 
> +While the using the @command{return} bounds can propagate through
> +execution causing a boundary violation.

This sentence needs to be rephrased.

> +@kindex set mpx-bnd-init-on-return
> +When set to true bound registers will be set to the INIT state when
> +using the "return" command.
> +@kindex show mpx-bnd-init-on-return
> +Show the state of mpx-bnd-init-on-return.

This last sentence should explain what is shown; just referring to the
variable name doesn't do the job.

> +  add_setshow_boolean_cmd ("mpx-bnd-init-on-return", no_class,
> +			   &mpx_bnd_init_on_return, _("\
> + Set the bnd registers to INIT state when returning from a call."), _("\
> + Show the state of the mpx-bnd-init-on-return."),

For the 'show" part, we usually use the following wording:

  Show whether to set the bnd registers to INIT state when returning from a call.


Thanks.

Re: [PATCH V4 2/2] Add mpx-bnd-init-on-return set/show command for inferior calls.

[Walfred Tedeschi]

Am 3/2/2016 um 5:35 PM schrieb Eli Zaretskii:
>> From: Walfred Tedeschi <walfred.tedeschi@intel.com>
>> Cc: gdb-patches@sourceware.org, Walfred Tedeschi <walfred.tedeschi@intel.com>
>> Date: Wed,  2 Mar 2016 17:21:29 +0100
>>
>> +While the using the @command{return} bounds can propagate through
>> +execution causing a boundary violation.
>
> This sentence needs to be rephrased.
>
>> +@kindex set mpx-bnd-init-on-return
>> +When set to true bound registers will be set to the INIT state when
>> +using the "return" command.
>> +@kindex show mpx-bnd-init-on-return
>> +Show the state of mpx-bnd-init-on-return.
>
> This last sentence should explain what is shown; just referring to the
> variable name doesn't do the job.
>
>> +  add_setshow_boolean_cmd ("mpx-bnd-init-on-return", no_class,
>> +			   &mpx_bnd_init_on_return, _("\
>> + Set the bnd registers to INIT state when returning from a call."), _("\
>> + Show the state of the mpx-bnd-init-on-return."),
>
> For the 'show" part, we usually use the following wording:
>
>    Show whether to set the bnd registers to INIT state when returning from a call.
>
>
> Thanks.
>
Eli,

Some of your previous comment's were forgotten sorry for that. They were 
absolutely valid.

Documentation part for the new patch will be:

NEWS:
show mpx-bnd-init-on-return
set mpx-bnd-init-on-return on i386 and amd64
   In case MPX-BND-INIT-ON-RETURN is true, bound registers
   will be initialized when the "return" command is used.


gdb.texinfo:
While calling functions from the debugger, of an Intel MPX enabled program,
boundary registers have to be set to the INIT state before performing the
call, to avoid boundary violations while performing the call.  A bound is
defined to be in the INIT state when the pointer associated to that boundary
can access the whole memory, in this case the register bound register
associated to it has value 0, e.g. if the register associated is bnd0raw
its value will be @{0x0, 0x0@}.

When you use the @code{return} command, the bound registers might
cause boundary violations because they were not updated for the
early return from the function.
To countermand that, @value{GDBN} can force initialization of the
bound registers when it performs the @code{return} command.  This is
controlled by the following option:

@table @code
@kindex set mpx-bnd-init-on-return
When set to true bound registers will be set to the INIT state when
using the "return" command.
@kindex show mpx-bnd-init-on-return
Show the state of mpx-bnd-init-on-return.
@end table

and info for the new command:
   add_setshow_boolean_cmd ("mpx-bnd-init-on-return", no_class,
			   &mpx_bnd_init_on_return, _("\
  Set the bnd registers to INIT state when returning from a call."), _("\
  Show whether to set the bnd registers to INIT state when returning 
from a call."),


Would that be ok with you?


Thanks and regards,
-Fred


Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Christian Lamprechter
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

Re: [PATCH V4 2/2] Add mpx-bnd-init-on-return set/show command for inferior calls.

[Eli Zaretskii]

> Cc: palves@redhat.com, brobecker@adacore.com, gdb-patches@sourceware.org
> From: Walfred Tedeschi <walfred.tedeschi@intel.com>
> Date: Thu, 3 Mar 2016 12:04:09 +0100
> 
> NEWS:
> show mpx-bnd-init-on-return
> set mpx-bnd-init-on-return on i386 and amd64
>    In case MPX-BND-INIT-ON-RETURN is true, bound registers
>    will be initialized when the "return" command is used.
> 
> 
> gdb.texinfo:
> While calling functions from the debugger, of an Intel MPX enabled program,
> boundary registers have to be set to the INIT state before performing the
> call, to avoid boundary violations while performing the call.  A bound is
> defined to be in the INIT state when the pointer associated to that boundary
> can access the whole memory, in this case the register bound register
> associated to it has value 0, e.g. if the register associated is bnd0raw
> its value will be @{0x0, 0x0@}.
> 
> When you use the @code{return} command, the bound registers might
> cause boundary violations because they were not updated for the
> early return from the function.
> To countermand that, @value{GDBN} can force initialization of the
> bound registers when it performs the @code{return} command.  This is
> controlled by the following option:
> 
> @table @code
> @kindex set mpx-bnd-init-on-return
> When set to true bound registers will be set to the INIT state when
> using the "return" command.
> @kindex show mpx-bnd-init-on-return
> Show the state of mpx-bnd-init-on-return.
> @end table
> 
> and info for the new command:
>    add_setshow_boolean_cmd ("mpx-bnd-init-on-return", no_class,
> 			   &mpx_bnd_init_on_return, _("\
>   Set the bnd registers to INIT state when returning from a call."), _("\
>   Show whether to set the bnd registers to INIT state when returning 
> from a call."),
> 
> 
> Would that be ok with you?

Yes, except that please change the documentation of "show
mpx-bnd-init-on-return" in the manual to say the same as in the doc
string of the add_setshow_boolean_cmd call.

Thanks.

Re: [PATCH V4 1/2] Initialize bnd register before performing inferior calls.

[Luis Machado]

On 03/02/2016 01:21 PM, Walfred Tedeschi wrote:
> BND registers should be initialized before performing an inferior call
> to avoid undesired bound violations.
>
> 2016-02-03  Walfred Tedeschi <walfred.tedeschi@intel.com>
>
> gdb/ChangeLog:
>
> 	* i386-tdep.c (i386_push_dummy_call): Initialize bnd registers.
> 	* amd64-tdep (amd64_push_dummy_call): Initialize bnd registers.
>
> ---
>   gdb/amd64-tdep.c | 15 +++++++++++++++
>   gdb/i386-tdep.c  | 15 +++++++++++++++
>   2 files changed, 30 insertions(+)
>
> diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
> index a62efde..0e4e89b 100644
> --- a/gdb/amd64-tdep.c
> +++ b/gdb/amd64-tdep.c
> @@ -995,8 +995,23 @@ amd64_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
>   		       int struct_return, CORE_ADDR struct_addr)
>   {
>     enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
> +  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
>     gdb_byte buf[8];
>
> +  /* When MPX is enabled, all bnd registers have to be initialized
> +     before the call.  This avoids an undesired bound violation
> +     during the function's execution.  */
> +
> +  if (I387_BND0R_REGNUM (tdep) > 0)
> +    {
> +      gdb_byte bnd_buf[16];
> +      int i;
> +
> +      memset (bnd_buf, 0, 16);
> +      for (i = 0; i < I387_BND0R_REGNUM (tdep); i++)
> +	regcache_raw_write (regcache, I387_BND0R_REGNUM (tdep) + i, bnd_buf);
> +    }
> +
>     /* Pass arguments.  */
>     sp = amd64_push_arguments (regcache, nargs, args, sp, struct_return);
>
> diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
> index 4c66edf..8c3576c 100644
> --- a/gdb/i386-tdep.c
> +++ b/gdb/i386-tdep.c
> @@ -2660,11 +2660,26 @@ i386_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
>   		      CORE_ADDR struct_addr)
>   {
>     enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
> +  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
>     gdb_byte buf[4];
>     int i;
>     int write_pass;
>     int args_space = 0;
>
> +  /* When MPX is enabled, all bnd registers have to be initialized
> +     before the call.  This avoids an undesired bound violation
> +     during the function's execution.  */
> +
> +  if (I387_BND0R_REGNUM (tdep) > 0)
> +    {
> +      gdb_byte bnd_buf[16];
> +      int i;
> +
> +      memset (bnd_buf, 0, 16);
> +      for (i = 0; i < I387_BND0R_REGNUM (tdep); i++)
> +	regcache_raw_write (regcache, I387_BND0R_REGNUM (tdep) + i, bnd_buf);
> +    }
> +
>     /* Determine the total space required for arguments and struct
>        return address in a first pass (allowing for 16-byte-aligned
>        arguments), then push arguments in a second pass.  */
>

Not sure if it was suggested previously in earlier versions of this 
series (i could not find it), but wouldn't it make sense to have that 
code moved to a new function in, say, i387-tdep.c? Then we wouldn't need 
to duplicate the code as above.