This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH v2] Add support for bound table in the Intel MPX context.
- From: Joel Brobecker <brobecker at adacore dot com>
- To: Walfred Tedeschi <walfred dot tedeschi at intel dot com>
- Cc: gdb-patches at sourceware dot org
- Date: Wed, 6 May 2015 11:50:36 -0700
- Subject: Re: [PATCH v2] Add support for bound table in the Intel MPX context.
- Authentication-results: sourceware.org; auth=none
- References: <1429791181-27412-1-git-send-email-walfred dot tedeschi at intel dot com>
> 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