[PATCH V8] amd64-mpx: initialize BND register before performing inferior calls.

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

This patch initializes the BND registers before executing the inferior
call.  BND registers can be in arbitrary values at the moment of the
inferior call.  In case the function being called uses as part of the
parameters BND register, e.g. when passing a pointer as parameter, the
current value of the register will be used.  This can cause boundary
violations that are not due to a real bug or even desired by the user.
In this sense the best to be done is set the BND registers to allow
access to the whole memory, i.e. initialized state, before pushing the
inferior call.

Comments:
>From v7 Pedros comments were addressed and tests were extended.
First version of documentation was added.

Thanks a lot for your review!

2017-02-21  Walfred Tedeschi <walfred.tedeschi@intel.com>

gdb/ChangeLog:

	* i387-tdep.h (i387_reset_bnd_regs): Add function definition.
	* i387-tdep.c (i387_reset_bnd_regs): Add function implementation.
	* i386-tdep.c (i386_push_dummy_call): Call i387_reset_bnd_regs.
	* amd64-tdep (amd64_push_dummy_call): Call i387_reset_bnd_regs.
	* NEWS: Add entry for inferior calls on MPX enabled programs.

gdb/testsuite/ChangeLog:

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

gdb/doc/ChangeLog:

	* Memory Protection Extensions: Add information about inferior
	calls.
---
 gdb/NEWS                                 |   4 +
 gdb/amd64-tdep.c                         |   3 +
 gdb/doc/gdb.texinfo                      |  30 +++
 gdb/i386-tdep.c                          |   3 +
 gdb/i387-tdep.c                          |  18 ++
 gdb/i387-tdep.h                          |   4 +
 gdb/testsuite/gdb.arch/i386-mpx-call.c   | 127 ++++++++++
 gdb/testsuite/gdb.arch/i386-mpx-call.exp | 413 +++++++++++++++++++++++++++++++
 8 files changed, 602 insertions(+)
 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 f412887..59faf8d 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -3,6 +3,10 @@
 
 *** Changes since GDB 7.12
 
+* Intel MPX
+
+  ** Set to INIT state bound register to perform calls on inferiors with MPX.
+
 * Python Scripting
 
   ** New functions to start, stop and access a running btrace recording.
diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
index 233c65a..54cc17d 100644
--- a/gdb/amd64-tdep.c
+++ b/gdb/amd64-tdep.c
@@ -999,6 +999,9 @@ amd64_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
   gdb_byte buf[8];
 
+  /* Reset bound registers.  */
+  i387_reset_bnd_regs (gdbarch, regcache);
+
   /* Pass arguments.  */
   sp = amd64_push_arguments (regcache, nargs, args, sp, struct_return);
 
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index f619470..ba6e988 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -22531,6 +22531,36 @@ whose bounds are to be changed, @var{lbound} and @var{ubound} are new values
 for lower and upper bounds respectively.
 @end table
 
+Calling inferior functions on Intel MPX enabled program can generate bound
+violations.  In a normal execution, bound registers are set by the caller
+before performing the call.  However, when you perform a call to a
+function, you are usually in the middle of an execution of other function.
+Bound register are then on an arbitrary state, leading to random bound
+violations.
+
+In this sense GDB sets the bound registers to the init state before performing
+calls in the inferior.  This allow the associated pointers to access the whole
+memory avoiding this way bound violations.
+
+On the other hand you can also examine the influence of the bound register in
+the execution of the function, i.e. verify if modifying bounds would cause bound
+viaolations and how.  You can achieve this by stoppying the execution of the
+called function at its prologue and setting the bnd registers continuing then
+the execution. See the example below:
+
+@smallexample
+	$ break *upper
+	Breakpoint 2 at 0x4009de: file i386-mpx-call.c, line 47.
+	$ print upper (a, b, c, d, 1)
+	Breakpoint 2, upper (a=0x0, b=0x6e0000005b, c=0x0, d=0x0, len=48)....
+	$ print $bnd0
+	{lbound = 0x0, ubound = ffffffff} : size -1
+@end smallexample
+
+At this last step the value of bnd0 can be changed for investigation of bound
+violations caused along the execution of the call.  In order to know how to
+set the bnd registers or bound table for the call consult the ABI.
+
 @node Alpha
 @subsection Alpha
 
diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index bd87720..0be1e80 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -2663,6 +2663,9 @@ i386_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
   int write_pass;
   int args_space = 0;
 
+  /* Reset bound registers.  */
+  i387_reset_bnd_regs (gdbarch, regcache);
+
   /* 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.  */
diff --git a/gdb/i387-tdep.c b/gdb/i387-tdep.c
index adbe721..7f400d8 100644
--- a/gdb/i387-tdep.c
+++ b/gdb/i387-tdep.c
@@ -1772,3 +1772,21 @@ i387_return_value (struct gdbarch *gdbarch, struct regcache *regcache)
   regcache_raw_write_unsigned (regcache, I387_FTAG_REGNUM (tdep), 0x3fff);
 
 }
+
+  /* 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.  */
+void
+i387_reset_bnd_regs (struct gdbarch *gdbarch, struct regcache *regcache)
+{
+  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+
+  if (I387_BND0R_REGNUM (tdep) > 0)
+    {
+      gdb_byte bnd_buf[16];
+
+      memset (bnd_buf, 0, 16);
+      for (int i = 0; i < I387_NUM_BND_REGS; i++)
+	regcache_raw_write (regcache, I387_BND0R_REGNUM (tdep) + i, bnd_buf);
+    }
+}
diff --git a/gdb/i387-tdep.h b/gdb/i387-tdep.h
index 81d45b7..4e4c47c 100644
--- a/gdb/i387-tdep.h
+++ b/gdb/i387-tdep.h
@@ -156,4 +156,8 @@ extern void i387_collect_xsave (const struct regcache *regcache,
 extern void i387_return_value (struct gdbarch *gdbarch,
 			       struct regcache *regcache);
 
+/* Set all bnd registers to the INIT state.  INIT state means
+   all memory range can be accessed.  */
+extern void i387_reset_bnd_regs (struct gdbarch *gdbarch,
+			         struct regcache *regcache);
 #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..8dae133
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/i386-mpx-call.c
@@ -0,0 +1,127 @@
+/* Test for inferior function calls MPX context.
+
+   Copyright (C) 2017 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>. */
+
+#include <stdlib.h>
+#include <string.h>
+#include "x86-cpuid.h"
+
+#define OUR_SIZE    5
+
+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
+upper (int *a, int *b, int *c, int *d, int len)
+{
+  int value;
+
+  value = *(a + len);
+  value = *(b + len);
+  value = *(c + len);
+  value = *(d + len);
+
+  value = value - *a + 1;
+  return value;
+}
+
+
+int
+lower (int *a, int *b, int *c, int *d, int len)
+{
+  int value;
+
+  value = *(a - len);
+  value = *(b - len);
+  value = *(c - len);
+  value = *(d - len);
+
+
+  value = value - *a + 1;
+  return value;
+}
+
+
+char char_upper (char *str, int lenght)
+{
+  char ch;
+  ch = *(str + lenght);
+
+  return ch;
+}
+
+char char_lower (char *str, int lenght)
+{
+  char ch;
+  ch = *(str - lenght);
+
+  return ch;
+}
+
+int
+main (void)
+{
+  if (have_mpx ())
+    {
+      int sa[OUR_SIZE];
+      int sb[OUR_SIZE];
+      int sc[OUR_SIZE];
+      int sd[OUR_SIZE];
+      int *x, *a, *b, *c, *d;
+      char mchar;
+      char hello[] = "Hello";
+
+      x = malloc (sizeof (int) * OUR_SIZE);
+      a = malloc (sizeof (int) * OUR_SIZE);
+      b = malloc (sizeof (int) * OUR_SIZE);
+      c = malloc (sizeof (int) * OUR_SIZE);
+      d = malloc (sizeof (int) * OUR_SIZE);
+
+      *x = upper (sa, sb, sc, sd, 0);  /* bkpt 1.  */
+      *x = lower (a, b, c, d, 0);  /* bkpt 1.  */
+
+      mchar = char_upper (hello, 10);
+      mchar = char_lower (hello, 10);
+
+      free (x);
+      free (a);
+      free (b);
+      free (c);
+      free (d);
+    }
+  return 0; /* bkpt 3.  */
+}
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..b1d52c4
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/i386-mpx-call.exp
@@ -0,0 +1,413 @@
+# Copyright (C) 2017 Free Software Foundation, Inc.
+#
+# 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-*-* ] } {
+    untested "skipping x86 MPX tests."
+    return
+}
+
+standard_testfile
+
+set comp_flags "-mmpx -fcheck-pointer-bounds -I${srcdir}/../nat"
+
+if {[prepare_for_testing "failed to prepare" ${testfile} ${srcfile} \
+    [list debug 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 " {
+        untested "processor does not support MPX; skipping tests"
+        return
+    }
+}
+
+# Need for returning from an inferior call that causes a BND violation.
+gdb_test_no_output "set confirm off"
+
+# Convenience variable.
+#
+set bound_reg " = \\\{lbound = $hex, ubound = $hex\\\}.*"
+set int_braw_reg " = \\\{lbound = 0x0, ubound_raw = 0x0\\\}.*"
+set bndcfg_reg " = \\\{raw = $hex, config = \\\{base = $hex, reserved = $hex,\
+               preserved = $hex, enabled = $hex\\\}\\\}"
+set bndstatus_reg  " = \\\{raw = $hex, status = \\\{bde = $hex,\
+                    error = $hex\\\}\\\}"
+set u_fault [multi_line "Program received signal SIGSEGV, Segmentation fault" \
+                        "Upper bound violation while accessing address $hex" \
+                        "Bounds: \\\[lower = $hex, upper = $hex\\\]"]
+
+
+
+# Simplify the tests below.
+#
+proc sanity_check_bndregs {arglist} {
+
+    global int_braw_reg
+
+    foreach a $arglist {
+        gdb_test "p /x $a" "$int_braw_reg"\
+            "$a"
+    }
+}
+
+# Set bnd register to have no access to memory.
+#
+proc remove_memory_access {reg} {
+    global hex
+
+    sanity_check_bndregs {"\$bnd0raw" "\$bnd1raw" "\$bnd2raw" "\$bnd3raw"}
+
+    gdb_test "p /x $reg.lbound = $reg.ubound" "= $hex"\
+        "$reg changed"
+    gdb_test "p /x $reg.ubound = 0" " = 0x0"\
+        "$reg changed"
+}
+
+
+# Prepare convenience variables for bndconfig and status
+# for posterior comparison
+#
+proc prepare_bndcfg_bndstatus { } {
+
+    global bndcfg_reg
+    global bndstatus_reg
+
+    gdb_test "p /x \$temp_bndcfgu = \$bndcfgu" "$bndcfg_reg"\
+        "bndcfgu should not change"
+
+    gdb_test "p /x \$temp_bndstatus = \$bndstatus" "$bndstatus_reg"\
+        "bndstatus should not change"
+}
+
+# Compare values set for convenience variables and actual values of bndconfig
+# and bndstatus registers.
+#
+proc compare_bndstatus_with_convenience {} {
+
+    gdb_test "p \$temp_bndcfgu == \$bndcfgu" "= 1"\
+        "bndcfgu compare before and after"
+    gdb_test "p \$temp_bndstatus == \$bndstatus" "= 1"\
+        "bndstatus compare before and after"
+
+}
+
+# Perform an inferior call defined in func.
+#
+proc perform_a_call { func } {
+
+    global inf_call_stopped
+    global gdb_prompt
+
+    gdb_test_multiple "p /x $func" "Inferior call test" {
+        -re $inf_call_stopped {
+            pass "stopped within an inferior call"
+        }
+        -re ".. = 0\r\n$gdb_prompt " {
+            verbose "stopped within an inferior call"
+            return -1
+        }
+    }
+
+}
+
+
+# Perform an inferior call defined in func.
+#
+proc perform_a_call { func } {
+
+    global inf_call_stopped
+    global gdb_prompt
+
+    gdb_test_multiple "p /x $func" "Inferior call test" {
+        -re $inf_call_stopped {
+            pass "stopped within an inferior call"
+        }
+        -re ".. = 0\r\n$gdb_prompt " {
+            verbose "stopped within an inferior call"
+            return -1
+        }
+    }
+
+}
+
+
+# Perform an inferior call defined in func.
+#
+proc check_bound_violation { parm parm_type is_positive} {
+
+    global u_fault
+
+    gdb_test "continue" "$u_fault.*" "Check bnd violation"
+
+    if {$is_positive == 1} {
+        gdb_test "p (((void *)\$_siginfo._sifields._sigfault.si_addr\
+                  - (void*)$parm))/sizeof($parm_type) == 1"\
+                  " = 1" "Accessing only one position"
+    } else {
+        gdb_test "p ((void*)$parm\
+                  - (void *)\$_siginfo._sifields._sigfault.si_addr)\
+                  /sizeof($parm_type) == 1"\
+                  " = 1" "Accessing only one position"
+    }
+    gdb_test "return" "\\\#.*main.*i386-mpx-call\\\.c:.*" "return from the fault"
+}
+
+
+# Start testing!
+#
+
+# Set up just stop in middle of main for call function in the inferior.
+#
+set break "bkpt 1."
+gdb_breakpoint [gdb_get_line_number "${break}"]
+gdb_continue_to_breakpoint "${break}" ".*${break}.*"
+set inf_call_stopped [multi_line "The program being debugged\
+                        stopped while in a function called from GDB." \
+                        "Evaluation of the expression containing the function.*" \
+                     ]
+
+# Consistency:
+#    default run execution of call should succeed without violations.
+#
+with_test_prefix "default_run" {
+
+    gdb_test "p \$keep_bnd0_value=\$bnd0" $bound_reg\
+        "Store bnd0 register in a convenience variable"
+
+    gdb_test "p /x upper (a, b, c, d, 0)" " = $hex"\
+        "Inferior call test"
+
+    gdb_test "p ((\$bnd0.lbound==\$keep_bnd0_value.lbound) &&\
+        (\$bnd0.ubound==\$keep_bnd0_value.ubound))" "= 1" "Keep BND register\
+        value calls"
+}
+
+# Consistency:  Examine bnd registers values before and after the call.
+#
+#
+with_test_prefix "verify_default_values" {
+
+    prepare_bndcfg_bndstatus
+
+    gdb_breakpoint "*upper"
+    perform_a_call "upper (a, b, c, d, 1)"
+
+    sanity_check_bndregs {"\$bnd0raw" "\$bnd1raw" "\$bnd2raw" "\$bnd3raw"}
+
+    compare_bndstatus_with_convenience
+
+    gdb_test_multiple "continue" "Inferior call test" {
+        -re ".*Continuing.\r\n$gdb_prompt " {
+            pass "normal continue"
+        }
+    }
+}
+
+# Examine:  Cause a upper bound violation changing BND0.
+#
+#
+with_test_prefix "upper_bnd0" {
+
+    prepare_bndcfg_bndstatus
+
+    gdb_breakpoint "*upper"
+    perform_a_call "upper (a, b, c, d, 1)"
+
+    remove_memory_access "\$bnd0"
+
+    compare_bndstatus_with_convenience
+
+    check_bound_violation "a" "int" 1
+}
+
+# Examine:  Cause an upper bound violation changing BND1.
+#
+#
+with_test_prefix "upper_bnd1" {
+
+    prepare_bndcfg_bndstatus
+
+    gdb_breakpoint "*upper"
+    perform_a_call "upper (a, b, c, d, 1)"
+
+    remove_memory_access "\$bnd1"
+
+    compare_bndstatus_with_convenience
+
+    check_bound_violation "b" "int" 1
+}
+
+# Examine:  Cause an upper bound violation changing BND1.
+#
+#
+with_test_prefix "upper_bnd2" {
+
+    prepare_bndcfg_bndstatus
+
+    gdb_breakpoint "*upper"
+    perform_a_call "upper (a, b, c, d, 1)"
+
+    remove_memory_access "\$bnd2"
+
+    compare_bndstatus_with_convenience
+
+    check_bound_violation "c" "int" 1
+}
+
+# Examine:  Cause an upper bound violation changing BND1.
+#
+#
+with_test_prefix "upper_bnd3" {
+    prepare_bndcfg_bndstatus
+
+    gdb_breakpoint "*upper"
+    perform_a_call "upper (a, b, c, d, 1)"
+
+    remove_memory_access "\$bnd3"
+
+    compare_bndstatus_with_convenience
+
+    check_bound_violation "d" "int" 1
+}
+
+# Examine:  Cause a lower bound violation changing BND0.
+#
+#
+with_test_prefix "lower_bnd0" {
+
+    prepare_bndcfg_bndstatus
+
+    gdb_breakpoint "*lower"
+    perform_a_call "lower (a, b, c, d, 1)"
+
+    remove_memory_access "\$bnd0"
+
+    compare_bndstatus_with_convenience
+
+    check_bound_violation "a" "int" 0
+}
+
+# Examine:  Cause a lower bound violation changing BND1.
+#
+#
+with_test_prefix "lower_bnd1" {
+
+    prepare_bndcfg_bndstatus
+
+    gdb_breakpoint "*lower"
+    perform_a_call "lower (a, b, c, d, 1)"
+
+    remove_memory_access "\$bnd1"
+
+    compare_bndstatus_with_convenience
+
+    check_bound_violation "b" "int" 0
+}
+
+# Examine:  Cause a lower bound violation changing BND2.
+#
+#
+with_test_prefix "lower_bnd2" {
+
+    prepare_bndcfg_bndstatus
+
+    gdb_breakpoint "*lower"
+    perform_a_call "lower (a, b, c, d, 1)"
+
+    remove_memory_access "\$bnd2"
+
+    compare_bndstatus_with_convenience
+
+    check_bound_violation "c" "int" 0
+}
+
+# Examine:  Cause a lower bound violation changing BND3.
+#
+#
+with_test_prefix "lower_bnd3" {
+
+    prepare_bndcfg_bndstatus
+
+    gdb_breakpoint "*lower"
+    perform_a_call "lower (a, b, c, d, 1)"
+
+    remove_memory_access "\$bnd3"
+
+    compare_bndstatus_with_convenience
+
+    check_bound_violation "d" "int" 0
+}
+
+# Examine:  String causing a upper bound violation changing BND0
+#
+#
+with_test_prefix "chars_up" {
+
+    prepare_bndcfg_bndstatus
+
+    gdb_breakpoint "*char_upper"
+    perform_a_call "char_upper (hello, 1)"
+
+    remove_memory_access "\$bnd0"
+
+    compare_bndstatus_with_convenience
+
+    check_bound_violation "str" "char" 1
+}
+
+
+# Examine:  String causing an lower bound violation changing BND0
+#
+#
+with_test_prefix "chars_low" {
+
+    prepare_bndcfg_bndstatus
+
+    gdb_breakpoint "*char_lower"
+    perform_a_call "char_lower (hello, 1)"
+
+    remove_memory_access "\$bnd0"
+
+    compare_bndstatus_with_convenience
+
+    check_bound_violation "str" "char" 0
+}
+
+# Examine:  String causing an lower bound violation changing BND0
+#
+#
+with_test_prefix "chars_low_adhoc_parm" {
+
+    prepare_bndcfg_bndstatus
+
+    gdb_breakpoint "*char_lower"
+    perform_a_call "char_lower (\"tryme\", 1)"
+
+    remove_memory_access "\$bnd0"
+
+    compare_bndstatus_with_convenience
+
+    check_bound_violation "str" "char" 0
+}
-- 
2.9.3