This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


> Intel(R) Memory protection bound information are located in register to be tested
> using the MPX new instructions. Since the number of bound registers are limited
> a table is used to provide storage for bounds during run-time.
> 
> In order to investigate the contents of the MPX bound table two new commands
> are added to GDB.  "show-mpx-bound" and "set-mpx-bound" are used to display
> and set values on the MPX bound table.

Some of the lines above are a little too long. I actually recommend
limiting the length to 76 characters, so as to avoid lines exceeding
80 characters when displayed by "git show" (which adds a 4-spaces
indentation).

The names of the commands I had in mind were actually

   (gdb) set mpx bound
   (gdb) show mpx bound

(spaces instead of the dashes). For that, you'll need to create
a couple of prefix commands "set mpx" and "show mpx" on top of
the "set" and "show" commands. Search for uses of add_prefix_cmd,
and how the prefix commands created can be used to create the
prefixed commands above. One example is in target-descriptions.c
(_initialize_target_descriptions).

> 2015-04-20  Walfred Tedeschi  <walfred.tedeschi@intel.com>
>             Mircea Gherzan  <mircea.gherzan@intel.com>
> 
> 	* i386-tdep.c (MPX_BASE_MASK, MPX_BD_MASK, MPX_BT_MASK, MPX_BD_MASK_32,
> 	MPX_BT_MASK_32): New macros.
> 	(i386_mpx_set_bounds): New function that implements
> 	the command "set-mpx-bound".
> 	(i386_mpx_enabled) Helper function to test MPX availability.
> 	(i386_mpx_bd_base) Helper function to calculate the base directory
> 	address. (i386_mpx_get_bt_entry) Helper function to access a bound
> 	table entry. (i386_mpx_print_bounds) Effectively display bound
> 	information. (_initialize_i386_tdep): 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 "show-mpx-bound"
> 	and "set-mpx-bound".
> 
> ---
>  gdb/NEWS                                |   4 +
>  gdb/doc/gdb.texinfo                     |  20 ++-
>  gdb/i386-tdep.c                         | 225 ++++++++++++++++++++++++++++++++
>  gdb/testsuite/gdb.arch/i386-mpx-map.c   |  93 +++++++++++++
>  gdb/testsuite/gdb.arch/i386-mpx-map.exp |  77 +++++++++++
>  5 files changed, 418 insertions(+), 1 deletion(-)
>  create mode 100644 gdb/testsuite/gdb.arch/i386-mpx-map.c
>  create mode 100644 gdb/testsuite/gdb.arch/i386-mpx-map.exp
> 
> diff --git a/gdb/NEWS b/gdb/NEWS
> index 11326f1..2e57fcd 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -40,6 +40,10 @@ queue-signal signal-name-or-number
>    even in non-stop mode.  The "auto" mode has been removed, and "off"
>    is now the default mode.
>  
> + * "show-mpx-bound" and "set-mpx-bound" on i386 and amd64
> +   for Intel(R) MPX.  Support for bound table investigation on
> +   Intel(R) MPX enabled applications.
> +
>  *** Changes in GDB 7.8
>  
>  * New command line options
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index 026706a..b453039 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -21482,7 +21482,7 @@ be returned in a register.
>  @kindex show struct-convention
>  Show the current setting of the convention to return @code{struct}s
>  from functions.
> -@end table
> +
>  
>  @subsubsection Intel(R) @dfn{Memory Protection Extensions} (MPX).
>  @cindex Intel(R) Memory Protection Extensions (MPX).
> @@ -21517,6 +21517,24 @@ counterpart.  When the bnd0@dots{}bnd3 registers are displayed via
>  Python, the display includes the memory size, in bits, accessible to
>  the pointer.
>  
> +Bounds can also be stored in bounds tables, which are stored in
> +application memory.  Pointers bounds are stored into the bounds table
> +by means of the pointers location address. Evaluating and changing bounds
> +located in bound tables is therefore interesting while investigating bugs
> +on MPX context. New commands are then added for this purpose:
> +
> +@item show-mpx-bound @var{pointer storage}
> +@kindex mpx-info-bound
> +Displays the bounds of a pointer given the pointers location.
> +
> +@item set-mpx-bound @var{storage}, @var{lbound}, @var{ubound}
> +@kindex mpx-set-bound
> +Set the bounds of a pointer in the map given the pointer location.
> +This command takes three parameters: @var{storage} is the pointers
> +location, @var{lbound} and @var{ubound}, are lower and upper
> +bounds new values respectively.
> +@end table
> +
>  @node Alpha
>  @subsection Alpha
>  
> diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
> index 1e68505..7f833ae 100644
> --- a/gdb/i386-tdep.c
> +++ b/gdb/i386-tdep.c
> @@ -8619,6 +8619,223 @@ i386_coff_osabi_sniffer (bfd *abfd)
>  }
>  
>  
> +#define MPX_BASE_MASK (~(ULONGEST) 0xfff)
> +
> +/* Find the bound directory base address.  */
> +
> +static unsigned long
> +i386_mpx_bd_base (void)
> +{
> +  struct regcache *rcache;
> +  struct gdbarch_tdep *tdep;
> +  ULONGEST ret;
> +  enum register_status 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.  */
> +
> +static int
> +i386_mpx_enabled (void)
> +{
> +  const struct gdbarch_tdep *tdep = gdbarch_tdep (get_current_arch ());
> +  const struct target_desc *tdesc = tdep->tdesc;
> +
> +  return (tdesc_find_feature (tdesc, "org.gnu.gdb.i386.mpx") != NULL);
> +}
> +
> +#define MPX_BD_MASK     0xfffffff00000ULL	/* select bits [47:20]  */
> +#define MPX_BT_MASK     0x0000000ffff8	        /* select bits [19:3]   */
> +#define MPX_BD_MASK_32  0xfffff000	        /* select bits [31:12]  */
> +#define MPX_BT_MASK_32  0x00000ffc	        /* select bits [11:2]   */
> +
> +/* Find the bound table entry given the pointer location and the base
> +   address of the table.  */
> +
> +static CORE_ADDR
> +i386_mpx_get_bt_entry (CORE_ADDR ptr, CORE_ADDR bd_base)
> +{
> +  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 ();
> +  struct type *data_ptr_type = builtin_type (gdbarch)->builtin_data_ptr;
> +
> +
> +  if (gdbarch_ptr_bit (gdbarch) == 64)
> +    {
> +      mpx_bd_mask = MPX_BD_MASK;
> +      bd_ptr_r_shift = 20;
> +      bd_ptr_l_shift = 3;
> +      bt_select_r_shift = 3;
> +      bt_select_l_shift = 5;
> +      bt_mask = MPX_BT_MASK;
> +    }
> +  else
> +    {
> +      mpx_bd_mask = MPX_BD_MASK_32;
> +      bd_ptr_r_shift = 12;
> +      bd_ptr_l_shift = 2;
> +      bt_select_r_shift = 2;
> +      bt_select_l_shift = 4;
> +      bt_mask = MPX_BT_MASK_32;
> +    }
> +
> +  offset1 = ((ptr & mpx_bd_mask) >> bd_ptr_r_shift) << bd_ptr_l_shift;
> +  bd_entry_addr = bd_base + offset1;
> +  bd_entry = read_memory_typed_address (bd_entry_addr, data_ptr_type);
> +
> +  if ((bd_entry & 0x1) == 0)
> +    error (_("Invalid bounds directory entry at %s."),
> +	paddress (get_current_arch (), bd_entry_addr));

The formatting for "paddress" looks incorrect - it should be indented
at the same level as the first parameter.
> +
> +  bt_addr = bd_entry & ~bt_select_r_shift;
> +  offset2 = ((ptr & bt_mask) >> bt_select_r_shift) << bt_select_l_shift;
> +
> +  return bt_addr + offset2;
> +}
> +
> +/* Print routine for the mpx bounds.  */
> +
> +static void
> +i386_mpx_print_bounds (const CORE_ADDR bt_entry[])
> +{
> +  struct ui_out *uiout = current_uiout;
> +  long long int size;
> +  struct gdbarch *gdbarch = get_current_arch ();
> +  CORE_ADDR onecompl = ~((CORE_ADDR) 0);
> +  int bounds_in_map = ((~bt_entry[1] == 0 && bt_entry[0] == onecompl) ? 1 : 0);
> +
> +  if (bounds_in_map == 1)
> +    {
> +      ui_out_text (uiout, "Null bounds on map:");
> +      ui_out_text (uiout, " pointer value = ");
> +      ui_out_field_core_addr (uiout, "pointer-value", gdbarch, bt_entry[2]);
> +      ui_out_text (uiout, ".");
> +      ui_out_text (uiout, "\n");
> +    }
> +  else
> +    {
> +      ui_out_text (uiout, "{lbound = ");
> +      ui_out_field_core_addr (uiout, "lower-bound", gdbarch, bt_entry[0]);
> +      ui_out_text (uiout, ", ubound = ");
> +
> +      /* The upper bound is stored in 1's complement.  */
> +      ui_out_field_core_addr (uiout, "upper-bound", gdbarch, ~bt_entry[1]);
> +      ui_out_text (uiout, "}: pointer value = ");
> +      ui_out_field_core_addr (uiout, "pointer-value", gdbarch, bt_entry[2]);
> +      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");
> +    }
> +}
> +
> +/* Implement the command "show-mpx-bound".  */
> +
> +static void
> +i386_mpx_info_bounds (char *args, int from_tty)
> +{
> +  CORE_ADDR bd_base = 0;
> +  CORE_ADDR addr;
> +  CORE_ADDR bt_entry_addr = 0;
> +  CORE_ADDR bt_entry[4];
> +  int i;
> +  struct gdbarch *gdbarch = get_current_arch ();
> +  struct type *data_ptr_type = builtin_type (gdbarch)->builtin_data_ptr;
> +
> +  if (!i386_mpx_enabled ())
> +    error (_("Intel(R) MPX not supported on this target."));
> +
> +  if (args == NULL)
> +    error (_("Address of pointer variable expected."));
> +
> +  addr = parse_and_eval_address (args);
> +
> +  bd_base = i386_mpx_bd_base ();
> +  bt_entry_addr = i386_mpx_get_bt_entry (addr, bd_base);
> +
> +  for (i = 0; i < 4; i++)
> +    bt_entry[i] = read_memory_typed_address (bt_entry_addr + i
> +					     * data_ptr_type->length,
> +					     data_ptr_type);

That's only a personal preference, but I would move the "+ i" to
the next line, to combine it with the '*' operation, to make it clear
that the multiplication affects i, not the result of the multiplication

    bt_entry[i] = read_memory_typed_address (bt_entry_addr
                                             + i * data_ptr_type->length,
					     data_ptr_type);

Alternatively, you can do:

    bt_entry[i]
      = read_memory_typed_address (bt_entry_addr + i * data_ptr_type->length,
				   data_ptr_type);

> +
> +  i386_mpx_print_bounds (bt_entry);
> +}
> +
> +/* Implement the command "set-mpx-bound".  */
> +
> +static void
> +i386_mpx_set_bounds (char *args, int from_tty)
> +{
> +  CORE_ADDR bd_base = 0;
> +  CORE_ADDR addr, lower, upper;
> +  CORE_ADDR bt_entry_addr = 0;
> +  CORE_ADDR bt_entry[2];
> +  const char *input = args;
> +  int i;
> +  struct gdbarch *gdbarch = get_current_arch ();
> +  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
> +  struct type *data_ptr_type = builtin_type (gdbarch)->builtin_data_ptr;
> +
> +  if (!i386_mpx_enabled ())
> +    error (_("MPX not supported on this target."));
> +
> +  if (args == NULL)
> +    error (_("Address of pointer expected."));
> +
> +  addr = value_as_address (parse_to_comma_and_eval (&input));
> +
> +  if (input[0] == ',')
> +    ++input;
> +  if (input[0] == '\0')
> +    error (_("Wrong number of arguments; Missing lower and upper bound."));
> +  lower = value_as_address (parse_to_comma_and_eval (&input));
> +
> +  if (input[0] == ',')
> +    ++input;
> +  if (input[0] == '\0')
> +    error (_("Wrong number of arguments; Missing upper bound."));
> +  upper = value_as_address (parse_to_comma_and_eval (&input));
> +
> +  bd_base = i386_mpx_bd_base ();
> +  bt_entry_addr = i386_mpx_get_bt_entry (addr, bd_base);
> +  for (i = 0; i < 2; i++)
> +    bt_entry[i] = read_memory_typed_address (bt_entry_addr + i
> +					     * data_ptr_type->length,
> +					     data_ptr_type);
> +  bt_entry[0] = (uint64_t) lower;
> +  bt_entry[1] = ~(uint64_t) upper;
> +
> +  for (i = 0; i < 2; i++)
> +    write_memory_unsigned_integer (bt_entry_addr + i * data_ptr_type->length,
> +				   data_ptr_type->length, byte_order,
> +				   bt_entry[i]);
> +}
> +
> +
>  /* Provide a prototype to silence -Wmissing-prototypes.  */
>  void _initialize_i386_tdep (void);
>  
> @@ -8649,6 +8866,14 @@ is \"default\"."),
>  			NULL, /* FIXME: i18n: */
>  			&setlist, &showlist);
>  
> +  add_cmd ("show-mpx-bound", no_class, i386_mpx_info_bounds,
> +	   "show the memory bounds for a given array/pointer storage.",
> +	   &cmdlist);
> +
> +  add_cmd ("set-mpx-bound", 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/testsuite/gdb.arch/i386-mpx-map.c b/gdb/testsuite/gdb.arch/i386-mpx-map.c
> new file mode 100644
> index 0000000..8a9094c
> --- /dev/null
> +++ b/gdb/testsuite/gdb.arch/i386-mpx-map.c
> @@ -0,0 +1,93 @@
> +/* Test program for MPX map allocated bounds.
> +
> +   Copyright 2015 Free Software Foundation, Inc.
> +
> +   Contributed by Intel Corp. <walfred.tedeschi@intel.com>
> +			      <mircea.gherzan@intel.com>
> +
> +   This file is part of GDB.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +#include <stdlib.h>
> +#include "x86-cpuid.h"
> +
> +#ifndef NOINLINE
> +#define NOINLINE __attribute__ ((noinline))
> +#endif
> +
> +#define SIZE  5
> +
> +typedef int T;
> +
> +unsigned int have_mpx (void) NOINLINE;
> +
> +unsigned int NOINLINE
> +have_mpx (void)
> +{
> +  unsigned int eax, ebx, ecx, edx;
> +
> +  if (!__get_cpuid (1, &eax, &ebx, &ecx, &edx))
> +    return 0;
> +
> +  if ((ecx & bit_OSXSAVE) == bit_OSXSAVE)
> +    {
> +      if (__get_cpuid_max (0, NULL) < 7)
> +	return 0;
> +
> +      __cpuid_count (7, 0, eax, ebx, ecx, edx);
> +
> +      if ((ebx & bit_MPX) == bit_MPX)
> +	return 1;
> +      else
> +	return 0;
> +    }
> +  return 0;
> +}
> +
> +void
> +foo (T *p)
> +{
> +  T *x;
> +
> +#if defined  __GNUC__ && !defined __INTEL_COMPILER
> +  __bnd_store_ptr_bounds (p, &p);
> +#endif
> +
> +  x = p + SIZE - 1;
> +
> +#if defined  __GNUC__ && !defined __INTEL_COMPILER
> +  __bnd_store_ptr_bounds (x, &x);
> +#endif
> +
> +  return;			/* after-assign */
> +}
> +
> +int
> +main (void)
> +{
> +  if (have_mpx ())
> +    {
> +      T *a = NULL;
> +
> +      a = calloc (SIZE, sizeof (T));	/* after-decl */
> +#if defined  __GNUC__ && !defined __INTEL_COMPILER
> +      __bnd_store_ptr_bounds (a, &a);
> +#endif
> +
> +      foo (a);				/* after-alloc */
> +      free (a);
> +    }
> +  return 0;
> +}
> diff --git a/gdb/testsuite/gdb.arch/i386-mpx-map.exp b/gdb/testsuite/gdb.arch/i386-mpx-map.exp
> new file mode 100644
> index 0000000..e7c5747
> --- /dev/null
> +++ b/gdb/testsuite/gdb.arch/i386-mpx-map.exp
> @@ -0,0 +1,77 @@
> +# Copyright 2014 Free Software Foundation, Inc.

The copyright year should include 2015, so -> 2014-2015.

> +#
> +# Contributed by Intel Corp. <walfred.tedeschi@intel.com>,
> +#                            <mircea.gherzan@intel.com>
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +if { ![istarget i?86-*-*] && ![istarget x86_64-*-* ] } {
> +    verbose "Skipping x86 MPX tests."
> +    return
> +}
> +
> +standard_testfile
> +
> +set comp_flags "-fmpx -I${srcdir}/../nat/"
> +
> +if { [prepare_for_testing ${testfile}.exp ${testfile} ${srcfile} \
> + [list debug nowarnings additional_flags=${comp_flags}]] } {
> +    return -1
> +}
> +
> +if ![runto_main] {
> +	untested "could not run to main"
> +	return -1
> +}
> +
> +set supports_mpx 0
> +set test "probe MPX support"
> +
> +gdb_test_multiple "print have_mpx()" $test {
> +    -re ".. = 1\r\n$gdb_prompt $" {
> +        pass $test
> +        set supports_mpx 1
> +    }
> +    -re ".. = 0\r\n$gdb_prompt $" {
> +        pass $test
> +    }
> +}
> +
> +if { !$supports_mpx } {
> +    unsupported "processor does not support MPX"
> +    return
> +}
> +
> +gdb_breakpoint [ gdb_get_line_number "after-decl" ]
> +gdb_breakpoint [ gdb_get_line_number "after-alloc" ]
> +gdb_breakpoint [ gdb_get_line_number "after-assign" ]
> +
> +gdb_test "show-mpx-bound 0x0" "Invalid bounds directory entry at $hex." "NULL address of the pointer"
> +
> +gdb_continue_to_breakpoint "after-decl" ".*after-decl.*"
> +gdb_test "show-mpx-bound a" "Invalid bounds directory entry at $hex." "pointer instead of pointer address"
> +gdb_test "show-mpx-bound &a" "Null bounds on map: pointer value = 0x0+." "address of a NULL pointer"
> +
> +gdb_continue_to_breakpoint "after-alloc" ".*after-alloc.*"
> +gdb_test "show-mpx-bound &a" "Null bounds on map: pointer value = $hex." "pointer after allocation"
> +
> +gdb_continue_to_breakpoint "after-assign" ".*after-assign.*"
> +gdb_test "show-mpx-bound &x" "\\\{lbound = $hex, ubound = $hex\\\}: pointer value = $hex, size = -1, metadata = 0x0+" "pointer after assignment"
> +gdb_test "set-mpx-bound 0x0, 0x1, 0x2" "Invalid bounds directory entry at $hex." "set-mpx-bound: NULL address of the pointer"
> +gdb_test_no_output "set-mpx-bound &x, 0xcafebabe, 0xdeadbeef" "set-mpx-bound: set bounds for a valid pointer address"
> +gdb_test "show-mpx-bound &x" "\\\{lbound = 0x0*cafebabe, ubound = 0x0*deadbeef\\\}: pointer value = $hex, size = 330236978, metadata = 0x0+" "set-mpx-bound: bounds map entry after set-mpx-bound"
> +
> +
> +gdb_test "set-mpx-bound 0x0, 0x1 0x2" "A syntax error in expression.*" "set-mpx-bound: Controlling syntax error, missing comma "
> +gdb_test "set-mpx-bound 0x0, 0x1" "Wrong number of arguments.*" "set-mpx-bound: Controlling syntax error, missing argument "
> -- 
> 2.1.0

-- 
Joel


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]