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

["Tedeschi, Walfred" <walfred dot tedeschi at intel dot com>]

Joel,

I know that we are under release right now. 
But, in any case, have you had the chance to take a look on this patch?
Should we wait post release for seeing this one?

Also this one:
https://sourceware.org/ml/gdb-patches/2016-01/msg00254.html


Thanks and regards,
-Fred

-----Original Message-----
From: Tedeschi, Walfred 
Sent: Wednesday, January 13, 2016 1:40 PM
To: brobecker@adacore.com; eliz@gnu.org
Cc: gdb-patches@sourceware.org; Tedeschi, Walfred
Subject: [PATCH V3 2/2] Add mpx-bnd-init-on-return set/show command for inferior calls.

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 set variable was added "mpx-bnd-init-on-return" which controls the initialization of bound register when using the return command.

As bound initialization it is understood the set of the BND register to zero allowing the associated pointer to access the whole memory.

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

2016-01-13  Walfred Tedeschi  <walfred.tedeschi@intel.com>

	* i386-tdep.c (mpx_bnd_init_on_return): new external variable.
	(show_mpx_init_on_return): New function.
	(_initialize_i386_tdepamd64_init_abi): Add new commands set/show
	mpx-bnd-init-on-return.
	* amd64-tdep.c (amd64_return_value): Use 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:

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

---
 gdb/NEWS                                  |   4 +
 gdb/amd64-tdep.c                          |  32 ++++++++
 gdb/doc/gdb.texinfo                       |  12 +++
 gdb/i386-tdep.c                           |  27 +++++++
 gdb/testsuite/gdb.arch/amd64-mpx-call.c   | 126 ++++++++++++++++++++++++++++++
 gdb/testsuite/gdb.arch/amd64-mpx-call.exp |  90 +++++++++++++++++++++
 6 files changed, 291 insertions(+)
 create mode 100644 gdb/testsuite/gdb.arch/amd64-mpx-call.c
 create mode 100644 gdb/testsuite/gdb.arch/amd64-mpx-call.exp

diff --git a/gdb/NEWS b/gdb/NEWS
index a222dfb..80022ec 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -3,6 +3,10 @@
 
 *** Changes since GDB 7.10
 
+* 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.
+
 * Record btrace now supports non-stop mode.
 
 * Support for tracepoints on aarch64-linux was added in GDBserver.
diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c index 732e91b..0ce41d1 100644
--- a/gdb/amd64-tdep.c
+++ b/gdb/amd64-tdep.c
@@ -708,12 +708,14 @@ amd64_return_value (struct gdbarch *gdbarch, struct value *function,
 		    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 };
   int integer_reg = 0;
   int sse_reg = 0;
   int i;
+  extern int mpx_bnd_init_on_return;
 
   gdb_assert (!(readbuf && writebuf));
 
@@ -739,6 +741,25 @@ 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, and therefore bnd0 register
+	     unset could then cause the program to receive a spurious
+	     bound violation, but this is only done if
+	     "set mpx-bnd-init-on-return" is true.  */
+
+	  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; @@ -833,6 +854,17 @@ amd64_return_value (struct gdbarch *gdbarch, struct value *function,
 				 writebuf + i * 8);
     }
 
+  if (I387_BND0R_REGNUM (tdep) > 0)
+    {
+      gdb_byte bnd_buf[16];
+      int i, init_bnd;
+
+      memset (bnd_buf, 0, 16);
+      if (writebuf && mpx_bnd_init_on_return)
+	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;  }
 

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index ccde84a..27020ee 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -22072,6 +22072,18 @@ 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 ebb21fc..4c3fa02 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -8864,6 +8864,24 @@ i386_mpx_set_bounds (char *args, int from_tty)
 				   bt_entry[i]);
 }
 
+/* The value of the "set mpx-bnd-init-on-return setting.  */
+
+int mpx_bnd_init_on_return = 1;
+
+/*  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.  */ @@ -8940,6 +8958,15 @@ Show Intel(R) 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/testsuite/gdb.arch/amd64-mpx-call.c b/gdb/testsuite/gdb.arch/amd64-mpx-call.c
new file mode 100644
index 0000000..d05b8a8
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/amd64-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/amd64-mpx-call.exp b/gdb/testsuite/gdb.arch/amd64-mpx-call.exp
new file mode 100644
index 0000000..d9c6eba
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/amd64-mpx-call.exp
@@ -0,0 +1,90 @@
+# 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 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" "" "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" "" "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

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