* [PATCH V5 1/5] Preparation for new siginfo on Linux
2016-01-22 15:36 [PATCH V5 0/5] Intel MPX bound violation support Walfred Tedeschi
@ 2016-01-22 15:17 ` Walfred Tedeschi
2016-01-28 12:20 ` Pedro Alves
2016-01-22 15:17 ` [PATCH V5 3/5] Add bound related fields to the siginfo structure Walfred Tedeschi
` (3 subsequent siblings)
4 siblings, 1 reply; 18+ messages in thread
From: Walfred Tedeschi @ 2016-01-22 15:17 UTC (permalink / raw)
To: palves, eliz, brobecker; +Cc: gdb-patches, Walfred Tedeschi
First add new structure and function to allow architecture customization
for the siginfo structure.
2016-01-15 Walfred Tedeschi <walfred.tedeschi@intel.com>
gdb/ChangeLog:
* linux-tdep.h (linux_siginfo_extra_field_values): New enum values.
(linux_siginfo_extra_fields): New enum type.
* linux-tdep.c (linux_get_siginfo_type_with_fields): New function.
(linux_get_siginfo_type): Use new function.
---
gdb/linux-tdep.c | 16 +++++++++++++---
gdb/linux-tdep.h | 12 ++++++++++++
2 files changed, 25 insertions(+), 3 deletions(-)
diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
index 29f5f3c..3757306 100644
--- a/gdb/linux-tdep.c
+++ b/gdb/linux-tdep.c
@@ -243,11 +243,12 @@ get_linux_inferior_data (void)
return info;
}
-/* This function is suitable for architectures that don't
- extend/override the standard siginfo structure. */
+/* This function is suitable for architectures that
+ extend/override the standard siginfo in a specific way. */
static struct type *
-linux_get_siginfo_type (struct gdbarch *gdbarch)
+linux_get_siginfo_type_with_fields (struct gdbarch *gdbarch,
+ linux_siginfo_extra_fields extra_fields)
{
struct linux_gdbarch_data *linux_gdbarch_data;
struct type *int_type, *uint_type, *long_type, *void_ptr_type;
@@ -364,6 +365,15 @@ linux_get_siginfo_type (struct gdbarch *gdbarch)
return siginfo_type;
}
+/* This function is suitable for architectures that don't
+ extend/override the standard siginfo structure. */
+
+static struct type *
+linux_get_siginfo_type (struct gdbarch *gdbarch)
+{
+ return linux_get_siginfo_type_with_fields (gdbarch, 0);
+}
+
/* Return true if the target is running on uClinux instead of normal
Linux kernel. */
diff --git a/gdb/linux-tdep.h b/gdb/linux-tdep.h
index 328c7f0..b2680f8 100644
--- a/gdb/linux-tdep.h
+++ b/gdb/linux-tdep.h
@@ -24,6 +24,18 @@
struct regcache;
+/* Enum used to define the extra fields of the siginfo type used by an
+ architecture. */
+enum linux_siginfo_extra_field_values
+{
+ /* Add bound fields into the segmentation fault field. */
+ LINUX_SIGINFO_FIELD_ADDR_BND = 1
+};
+
+/* Defines a type for the values defined in linux_siginfo_extra_field_values. */
+DEF_ENUM_FLAGS_TYPE (enum linux_siginfo_extra_field_values,
+ linux_siginfo_extra_fields);
+
typedef char *(*linux_collect_thread_registers_ftype) (const struct regcache *,
ptid_t,
bfd *, char *, int *,
--
2.1.4
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH V5 4/5] Adaptation of siginfo fixup for the new bnd fields
2016-01-22 15:36 [PATCH V5 0/5] Intel MPX bound violation support Walfred Tedeschi
2016-01-22 15:17 ` [PATCH V5 1/5] Preparation for new siginfo on Linux Walfred Tedeschi
2016-01-22 15:17 ` [PATCH V5 3/5] Add bound related fields to the siginfo structure Walfred Tedeschi
@ 2016-01-22 15:17 ` Walfred Tedeschi
2016-01-28 12:37 ` Pedro Alves
2016-01-22 15:18 ` [PATCH V5 2/5] Use linux_get_siginfo_type_with_fields for x86 Walfred Tedeschi
2016-01-22 15:18 ` [PATCH V5 5/5] ntel MPX bound violation handling Walfred Tedeschi
4 siblings, 1 reply; 18+ messages in thread
From: Walfred Tedeschi @ 2016-01-22 15:17 UTC (permalink / raw)
To: palves, eliz, brobecker; +Cc: gdb-patches, Walfred Tedeschi
New bnds fields will be always present for x86 architecture.
Fixup for compatibility layer 32bits has to be fixed.
It was added the nat_siginfo to serving as intermediate step
between kernel provided siginfo and the fix up routine.
When executing compat_siginfo_from_siginfo or
compat_x32_siginfo_from_siginfo first the buffer read from the kernel are
converted into the nat_signfo for homogenization, then the fields of
nat_siginfo are use to set the compat and compat_x32 siginfo fields.
In other to make this conversion independent of the system where gdb
is compiled the most complete version of the siginfo, named as native
siginfo, is used internally as an intermediate step.
Conversion using nat_siginfo is exemplified below:
compat_siginfo_from_siginfo or compat_x32_siginfo_from_siginfo:
buffer (from the kernel) -> nat_siginfo -> 32 / X32 siginfo
(memcpy) (field by field)
siginfo_from_compat_x32_siginfo or siginfo_from_compat_siginfo:
32 / X32 siginfo -> nat_siginfo -> buffer (to the kernel)
(field by field) (memcpy)
Caveat: No support for MPX on x32.
2015-01-15 Walfred Tedeschi <walfred.tedeschi@intel.com>
gdb/ChangeLog:
* amd64-linux-siginfo.c (nat_siginfo_t, nat_sigval_t, nat_timeval):
New types.
(compat_siginfo): New bound fields added.
(compat_x32_siginfo): New field added.
(cpt_si_addr_lsb): New define.
(compat_siginfo_from_siginfo): Use nat_siginfo.
(siginfo_from_compat_siginfo): Use nat_siginfo.
(compat_x32_siginfo_from_siginfo): Likewise.
(siginfo_from_compat_x32_siginfo): Likewise.
---
gdb/nat/amd64-linux-siginfo.c | 312 ++++++++++++++++++++++++++++--------------
gdb/nat/amd64-linux-siginfo.h | 16 ++-
2 files changed, 226 insertions(+), 102 deletions(-)
diff --git a/gdb/nat/amd64-linux-siginfo.c b/gdb/nat/amd64-linux-siginfo.c
index 36b5505..f3e190c 100644
--- a/gdb/nat/amd64-linux-siginfo.c
+++ b/gdb/nat/amd64-linux-siginfo.c
@@ -21,6 +21,93 @@
#include "common-defs.h"
#include "amd64-linux-siginfo.h"
+/* These types below (native_*) define a siginfo type that is layout
+ the most complete siginfo available for the architecture. */
+
+typedef int nat_int_t;
+typedef void* nat_uptr_t;
+
+typedef int nat_time_t;
+typedef int nat_timer_t;
+
+/* For native 64-bit, clock_t in _sigchld is 64bit aligned at 4 bytes. */
+typedef long __attribute__ ((__aligned__ (4))) nat_clock_t;
+
+struct nat_timeval
+{
+ nat_time_t tv_sec;
+ int tv_usec;
+};
+
+typedef union nat_sigval
+{
+ nat_int_t sival_int;
+ nat_uptr_t sival_ptr;
+} nat_sigval_t;
+
+typedef struct nat_siginfo
+{
+ int si_signo;
+ int si_errno;
+ int si_code;
+
+ union
+ {
+ int _pad[((128 / sizeof (int)) - 4)];
+ /* kill() */
+ struct
+ {
+ unsigned int _pid;
+ unsigned int _uid;
+ } _kill;
+
+ /* POSIX.1b timers */
+ struct
+ {
+ nat_timer_t _tid;
+ int _overrun;
+ nat_sigval_t _sigval;
+ } _timer;
+
+ /* POSIX.1b signals */
+ struct
+ {
+ unsigned int _pid;
+ unsigned int _uid;
+ nat_sigval_t _sigval;
+ } _rt;
+
+ /* SIGCHLD */
+ struct
+ {
+ unsigned int _pid;
+ unsigned int _uid;
+ int _status;
+ nat_clock_t _utime;
+ nat_clock_t _stime;
+ } _sigchld;
+
+ /* SIGILL, SIGFPE, SIGSEGV, SIGBUS */
+ struct
+ {
+ nat_uptr_t _addr;
+ short int _addr_lsb;
+ struct
+ {
+ nat_uptr_t _lower;
+ nat_uptr_t _upper;
+ } si_addr_bnd;
+ } _sigfault;
+
+ /* SIGPOLL */
+ struct
+ {
+ int _band;
+ int _fd;
+ } _sigpoll;
+ } _sifields;
+} nat_siginfo_t __attribute__ ((__aligned__ (8)));
+
/* These types below (compat_*) define a siginfo type that is layout
compatible with the siginfo type exported by the 32-bit userspace
support. */
@@ -91,6 +178,12 @@ typedef struct compat_siginfo
struct
{
unsigned int _addr;
+ short int _addr_lsb;
+ struct
+ {
+ unsigned int _lower;
+ unsigned int _upper;
+ } si_addr_bnd;
} _sigfault;
/* SIGPOLL */
@@ -152,6 +245,7 @@ typedef struct compat_x32_siginfo
struct
{
unsigned int _addr;
+ unsigned int _addr_lsb;
} _sigfault;
/* SIGPOLL */
@@ -174,6 +268,7 @@ typedef struct compat_x32_siginfo
#define cpt_si_stime _sifields._sigchld._stime
#define cpt_si_ptr _sifields._rt._sigval.sival_ptr
#define cpt_si_addr _sifields._sigfault._addr
+#define cpt_si_addr_lsb _sifields._sigfault._addr_lsb
#define cpt_si_band _sifields._sigpoll._band
#define cpt_si_fd _sifields._sigpoll._fd
@@ -192,54 +287,58 @@ typedef struct compat_x32_siginfo
static void
compat_siginfo_from_siginfo (compat_siginfo_t *to, siginfo_t *from)
{
+ nat_siginfo_t from_nat;
+
+ gdb_assert (sizeof (nat_siginfo_t) == sizeof (siginfo_t));
+ memcpy (&from_nat, from, sizeof (from_nat));
memset (to, 0, sizeof (*to));
- to->si_signo = from->si_signo;
- to->si_errno = from->si_errno;
- to->si_code = from->si_code;
+ to->si_signo = from_nat.si_signo;
+ to->si_errno = from_nat.si_errno;
+ to->si_code = from_nat.si_code;
if (to->si_code == SI_TIMER)
{
- to->cpt_si_timerid = from->si_timerid;
- to->cpt_si_overrun = from->si_overrun;
- to->cpt_si_ptr = (intptr_t) from->si_ptr;
+ to->cpt_si_timerid = from_nat.cpt_si_timerid;
+ to->cpt_si_overrun = from_nat.cpt_si_overrun;
+ to->cpt_si_ptr = (intptr_t) from_nat.cpt_si_ptr;
}
else if (to->si_code == SI_USER)
{
- to->cpt_si_pid = from->si_pid;
- to->cpt_si_uid = from->si_uid;
+ to->cpt_si_pid = from_nat.cpt_si_pid;
+ to->cpt_si_uid = from_nat.cpt_si_uid;
}
else if (to->si_code < 0)
{
- to->cpt_si_pid = from->si_pid;
- to->cpt_si_uid = from->si_uid;
- to->cpt_si_ptr = (intptr_t) from->si_ptr;
+ to->cpt_si_pid = from_nat.cpt_si_pid;
+ to->cpt_si_uid = from_nat.cpt_si_uid;
+ to->cpt_si_ptr = (intptr_t) from_nat.cpt_si_ptr;
}
else
{
switch (to->si_signo)
{
case SIGCHLD:
- to->cpt_si_pid = from->si_pid;
- to->cpt_si_uid = from->si_uid;
- to->cpt_si_status = from->si_status;
- to->cpt_si_utime = from->si_utime;
- to->cpt_si_stime = from->si_stime;
+ to->cpt_si_pid = from_nat.cpt_si_pid;
+ to->cpt_si_uid = from_nat.cpt_si_uid;
+ to->cpt_si_status = from_nat.cpt_si_status;
+ to->cpt_si_utime = from_nat.cpt_si_utime;
+ to->cpt_si_stime = from_nat.cpt_si_stime;
break;
case SIGILL:
case SIGFPE:
case SIGSEGV:
case SIGBUS:
- to->cpt_si_addr = (intptr_t) from->si_addr;
+ to->cpt_si_addr = (intptr_t) from_nat.cpt_si_addr;
break;
case SIGPOLL:
- to->cpt_si_band = from->si_band;
- to->cpt_si_fd = from->si_fd;
+ to->cpt_si_band = from_nat.cpt_si_band;
+ to->cpt_si_fd = from_nat.cpt_si_fd;
break;
default:
- to->cpt_si_pid = from->si_pid;
- to->cpt_si_uid = from->si_uid;
- to->cpt_si_ptr = (intptr_t) from->si_ptr;
+ to->cpt_si_pid = from_nat.cpt_si_pid;
+ to->cpt_si_uid = from_nat.cpt_si_uid;
+ to->cpt_si_ptr = (intptr_t) from_nat.cpt_si_ptr;
break;
}
}
@@ -250,57 +349,62 @@ compat_siginfo_from_siginfo (compat_siginfo_t *to, siginfo_t *from)
static void
siginfo_from_compat_siginfo (siginfo_t *to, compat_siginfo_t *from)
{
- memset (to, 0, sizeof (*to));
+ nat_siginfo_t to_nat;
- to->si_signo = from->si_signo;
- to->si_errno = from->si_errno;
- to->si_code = from->si_code;
+ gdb_assert (sizeof (nat_siginfo_t) == sizeof (siginfo_t));
+ memset (&to_nat, 0, sizeof (to_nat));
- if (to->si_code == SI_TIMER)
+ to_nat.si_signo = from->si_signo;
+ to_nat.si_errno = from->si_errno;
+ to_nat.si_code = from->si_code;
+
+ if (to_nat.si_code == SI_TIMER)
{
- to->si_timerid = from->cpt_si_timerid;
- to->si_overrun = from->cpt_si_overrun;
- to->si_ptr = (void *) (intptr_t) from->cpt_si_ptr;
+ to_nat.cpt_si_timerid = from->cpt_si_timerid;
+ to_nat.cpt_si_overrun = from->cpt_si_overrun;
+ to_nat.cpt_si_ptr = (void *) (intptr_t) from->cpt_si_ptr;
}
- else if (to->si_code == SI_USER)
+ else if (to_nat.si_code == SI_USER)
{
- to->si_pid = from->cpt_si_pid;
- to->si_uid = from->cpt_si_uid;
+ to_nat.cpt_si_pid = from->cpt_si_pid;
+ to_nat.cpt_si_uid = from->cpt_si_uid;
}
- if (to->si_code < 0)
+ if (to_nat.si_code < 0)
{
- to->si_pid = from->cpt_si_pid;
- to->si_uid = from->cpt_si_uid;
- to->si_ptr = (void *) (intptr_t) from->cpt_si_ptr;
+ to_nat.cpt_si_pid = from->cpt_si_pid;
+ to_nat.cpt_si_uid = from->cpt_si_uid;
+ to_nat.cpt_si_ptr = (void *) (intptr_t) from->cpt_si_ptr;
}
else
{
- switch (to->si_signo)
+ switch (to_nat.si_signo)
{
case SIGCHLD:
- to->si_pid = from->cpt_si_pid;
- to->si_uid = from->cpt_si_uid;
- to->si_status = from->cpt_si_status;
- to->si_utime = from->cpt_si_utime;
- to->si_stime = from->cpt_si_stime;
+ to_nat.cpt_si_pid = from->cpt_si_pid;
+ to_nat.cpt_si_uid = from->cpt_si_uid;
+ to_nat.cpt_si_status = from->cpt_si_status;
+ to_nat.cpt_si_utime = from->cpt_si_utime;
+ to_nat.cpt_si_stime = from->cpt_si_stime;
break;
case SIGILL:
case SIGFPE:
case SIGSEGV:
case SIGBUS:
- to->si_addr = (void *) (intptr_t) from->cpt_si_addr;
+ to_nat.cpt_si_addr = (void *) (intptr_t) from->cpt_si_addr;
+ to_nat.cpt_si_addr_lsb = (short) from->cpt_si_addr_lsb;
break;
case SIGPOLL:
- to->si_band = from->cpt_si_band;
- to->si_fd = from->cpt_si_fd;
+ to_nat.cpt_si_band = from->cpt_si_band;
+ to_nat.cpt_si_fd = from->cpt_si_fd;
break;
default:
- to->si_pid = from->cpt_si_pid;
- to->si_uid = from->cpt_si_uid;
- to->si_ptr = (void* ) (intptr_t) from->cpt_si_ptr;
+ to_nat.cpt_si_pid = from->cpt_si_pid;
+ to_nat.cpt_si_uid = from->cpt_si_uid;
+ to_nat.cpt_si_ptr = (void* ) (intptr_t) from->cpt_si_ptr;
break;
}
}
+ memcpy (to, &to_nat, sizeof (to_nat));
}
/* Convert the system provided siginfo into compatible x32 siginfo. */
@@ -309,56 +413,60 @@ static void
compat_x32_siginfo_from_siginfo (compat_x32_siginfo_t *to,
siginfo_t *from)
{
+ nat_siginfo_t from_nat;
+
+ gdb_assert (sizeof (nat_siginfo_t) == sizeof (siginfo_t));
+ memcpy (&from_nat, from, sizeof (from_nat));
memset (to, 0, sizeof (*to));
- to->si_signo = from->si_signo;
- to->si_errno = from->si_errno;
- to->si_code = from->si_code;
+ to->si_signo = from_nat.si_signo;
+ to->si_errno = from_nat.si_errno;
+ to->si_code = from_nat.si_code;
if (to->si_code == SI_TIMER)
{
- to->cpt_si_timerid = from->si_timerid;
- to->cpt_si_overrun = from->si_overrun;
- to->cpt_si_ptr = (intptr_t) from->si_ptr;
+ to->cpt_si_timerid = from_nat.cpt_si_timerid;
+ to->cpt_si_overrun = from_nat.cpt_si_overrun;
+ to->cpt_si_ptr = (intptr_t) from_nat.cpt_si_ptr;
}
else if (to->si_code == SI_USER)
{
- to->cpt_si_pid = from->si_pid;
- to->cpt_si_uid = from->si_uid;
+ to->cpt_si_pid = from_nat.cpt_si_pid;
+ to->cpt_si_uid = from_nat.cpt_si_uid;
}
else if (to->si_code < 0)
{
- to->cpt_si_pid = from->si_pid;
- to->cpt_si_uid = from->si_uid;
- to->cpt_si_ptr = (intptr_t) from->si_ptr;
+ to->cpt_si_pid = from_nat.cpt_si_pid;
+ to->cpt_si_uid = from_nat.cpt_si_uid;
+ to->cpt_si_ptr = (intptr_t) from_nat.cpt_si_ptr;
}
else
{
switch (to->si_signo)
{
case SIGCHLD:
- to->cpt_si_pid = from->si_pid;
- to->cpt_si_uid = from->si_uid;
- to->cpt_si_status = from->si_status;
- memcpy (&to->cpt_si_utime, &from->si_utime,
+ to->cpt_si_pid = from_nat.cpt_si_pid;
+ to->cpt_si_uid = from_nat.cpt_si_uid;
+ to->cpt_si_status = from_nat.cpt_si_status;
+ memcpy (&to->cpt_si_utime, &from_nat.cpt_si_utime,
sizeof (to->cpt_si_utime));
- memcpy (&to->cpt_si_stime, &from->si_stime,
+ memcpy (&to->cpt_si_stime, &from_nat.cpt_si_stime,
sizeof (to->cpt_si_stime));
break;
case SIGILL:
case SIGFPE:
case SIGSEGV:
case SIGBUS:
- to->cpt_si_addr = (intptr_t) from->si_addr;
+ to->cpt_si_addr = (intptr_t) from_nat.cpt_si_addr;
break;
case SIGPOLL:
- to->cpt_si_band = from->si_band;
- to->cpt_si_fd = from->si_fd;
+ to->cpt_si_band = from_nat.cpt_si_band;
+ to->cpt_si_fd = from_nat.cpt_si_fd;
break;
default:
- to->cpt_si_pid = from->si_pid;
- to->cpt_si_uid = from->si_uid;
- to->cpt_si_ptr = (intptr_t) from->si_ptr;
+ to->cpt_si_pid = from_nat.cpt_si_pid;
+ to->cpt_si_uid = from_nat.cpt_si_uid;
+ to->cpt_si_ptr = (intptr_t) from_nat.cpt_si_ptr;
break;
}
}
@@ -369,59 +477,63 @@ static void
siginfo_from_compat_x32_siginfo (siginfo_t *to,
compat_x32_siginfo_t *from)
{
- memset (to, 0, sizeof (*to));
+ nat_siginfo_t to_nat;
- to->si_signo = from->si_signo;
- to->si_errno = from->si_errno;
- to->si_code = from->si_code;
+ gdb_assert (sizeof (nat_siginfo_t) == sizeof (siginfo_t));
+ memset (&to_nat, 0, sizeof (to_nat));
- if (to->si_code == SI_TIMER)
+ to_nat.si_signo = from->si_signo;
+ to_nat.si_errno = from->si_errno;
+ to_nat.si_code = from->si_code;
+
+ if (to_nat.si_code == SI_TIMER)
{
- to->si_timerid = from->cpt_si_timerid;
- to->si_overrun = from->cpt_si_overrun;
- to->si_ptr = (void *) (intptr_t) from->cpt_si_ptr;
+ to_nat.cpt_si_timerid = from->cpt_si_timerid;
+ to_nat.cpt_si_overrun = from->cpt_si_overrun;
+ to_nat.cpt_si_ptr = (void *) (intptr_t) from->cpt_si_ptr;
}
- else if (to->si_code == SI_USER)
+ else if (to_nat.si_code == SI_USER)
{
- to->si_pid = from->cpt_si_pid;
- to->si_uid = from->cpt_si_uid;
+ to_nat.cpt_si_pid = from->cpt_si_pid;
+ to_nat.cpt_si_uid = from->cpt_si_uid;
}
- if (to->si_code < 0)
+ if (to_nat.si_code < 0)
{
- to->si_pid = from->cpt_si_pid;
- to->si_uid = from->cpt_si_uid;
- to->si_ptr = (void *) (intptr_t) from->cpt_si_ptr;
+ to_nat.cpt_si_pid = from->cpt_si_pid;
+ to_nat.cpt_si_uid = from->cpt_si_uid;
+ to_nat.cpt_si_ptr = (void *) (intptr_t) from->cpt_si_ptr;
}
else
{
- switch (to->si_signo)
+ switch (to_nat.si_signo)
{
case SIGCHLD:
- to->si_pid = from->cpt_si_pid;
- to->si_uid = from->cpt_si_uid;
- to->si_status = from->cpt_si_status;
- memcpy (&to->si_utime, &from->cpt_si_utime,
- sizeof (to->si_utime));
- memcpy (&to->si_stime, &from->cpt_si_stime,
- sizeof (to->si_stime));
+ to_nat.cpt_si_pid = from->cpt_si_pid;
+ to_nat.cpt_si_uid = from->cpt_si_uid;
+ to_nat.cpt_si_status = from->cpt_si_status;
+ memcpy (&to_nat.cpt_si_utime, &from->cpt_si_utime,
+ sizeof (to_nat.cpt_si_utime));
+ memcpy (&to_nat.cpt_si_stime, &from->cpt_si_stime,
+ sizeof (to_nat.cpt_si_stime));
break;
case SIGILL:
case SIGFPE:
case SIGSEGV:
case SIGBUS:
- to->si_addr = (void *) (intptr_t) from->cpt_si_addr;
+ to_nat.cpt_si_addr = (void *) (intptr_t) from->cpt_si_addr;
break;
case SIGPOLL:
- to->si_band = from->cpt_si_band;
- to->si_fd = from->cpt_si_fd;
+ to_nat.cpt_si_band = from->cpt_si_band;
+ to_nat.cpt_si_fd = from->cpt_si_fd;
break;
default:
- to->si_pid = from->cpt_si_pid;
- to->si_uid = from->cpt_si_uid;
- to->si_ptr = (void* ) (intptr_t) from->cpt_si_ptr;
+ to_nat.cpt_si_pid = from->cpt_si_pid;
+ to_nat.cpt_si_uid = from->cpt_si_uid;
+ to_nat.cpt_si_ptr = (void* ) (intptr_t) from->cpt_si_ptr;
break;
}
}
+ memcpy (to, &to_nat, sizeof (to_nat));
}
/* Convert a native/host siginfo object, into/from the siginfo in the
diff --git a/gdb/nat/amd64-linux-siginfo.h b/gdb/nat/amd64-linux-siginfo.h
index a396966..5b29cbe 100644
--- a/gdb/nat/amd64-linux-siginfo.h
+++ b/gdb/nat/amd64-linux-siginfo.h
@@ -20,12 +20,24 @@
#ifndef AMD64_LINUX_SIGINFO_H
#define AMD64_LINUX_SIGINFO_H 1
-
/* When GDB is built as a 64-bit application on Linux, the
PTRACE_GETSIGINFO data is always presented in 64-bit layout. Since
debugging a 32-bit inferior with a 64-bit GDB should look the same
as debugging it with a 32-bit GDB, we do the 32-bit <-> 64-bit
- conversion in-place ourselves. */
+ conversion in-place ourselves.
+ In other to make this conversion independent of the system where gdb
+ is compiled the most complete version of the siginfo, named as native
+ siginfo, is used internally as an intermediate step.
+
+ Conversion using nat_siginfo is exemplified below:
+ compat_siginfo_from_siginfo or compat_x32_siginfo_from_siginfo
+
+ buffer (from the kernel) -> nat_siginfo -> 32 / X32 siginfo
+
+ siginfo_from_compat_x32_siginfo or siginfo_from_compat_siginfo
+
+ 32 / X32 siginfo -> nat_siginfo -> buffer (to the kernel) */
+
/* Kind of siginfo fixup to be performed. */
--
2.1.4
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH V5 3/5] Add bound related fields to the siginfo structure
2016-01-22 15:36 [PATCH V5 0/5] Intel MPX bound violation support Walfred Tedeschi
2016-01-22 15:17 ` [PATCH V5 1/5] Preparation for new siginfo on Linux Walfred Tedeschi
@ 2016-01-22 15:17 ` Walfred Tedeschi
2016-01-28 12:23 ` Pedro Alves
2016-01-22 15:17 ` [PATCH V5 4/5] Adaptation of siginfo fixup for the new bnd fields Walfred Tedeschi
` (2 subsequent siblings)
4 siblings, 1 reply; 18+ messages in thread
From: Walfred Tedeschi @ 2016-01-22 15:17 UTC (permalink / raw)
To: palves, eliz, brobecker; +Cc: gdb-patches, Walfred Tedeschi
Both Linux and glibc have introduced bound related fields in the segmentation fault fields of the
siginfo_t type. Ass the new fields to our x86's siginfo_t type too.
Kernel patch:
http://git.kernel.org/cgit/linux/kernel/git/tip/tip.git/commit/?id=ee1b58d36aa1b5a79eaba11f5c3633c88231da83
Glibc patch:
http://repo.or.cz/w/glibc.git/commit/d4358b51c26a634eb885955aea06cad26af6f696
2016-01-15 Walfred Tedeschi <walfred.tedeschi@intel.com>
gdb/ChangeLog:
* linux-tdep.c (linux_get_siginfo_type): Add the _addr_bnd
structure to the siginfo if extra_fields contains
LINUX_SIGINFO_FIELD_ADDR_BND.
---
gdb/linux-tdep.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
index acc6e16..70196d0 100644
--- a/gdb/linux-tdep.c
+++ b/gdb/linux-tdep.c
@@ -251,7 +251,7 @@ linux_get_siginfo_type_with_fields (struct gdbarch *gdbarch,
linux_siginfo_extra_fields extra_fields)
{
struct linux_gdbarch_data *linux_gdbarch_data;
- struct type *int_type, *uint_type, *long_type, *void_ptr_type;
+ struct type *int_type, *uint_type, *long_type, *void_ptr_type, *short_type;
struct type *uid_type, *pid_type;
struct type *sigval_type, *clock_type;
struct type *siginfo_type, *sifields_type;
@@ -267,6 +267,8 @@ linux_get_siginfo_type_with_fields (struct gdbarch *gdbarch,
1, "unsigned int");
long_type = arch_integer_type (gdbarch, gdbarch_long_bit (gdbarch),
0, "long");
+ short_type = arch_integer_type (gdbarch, gdbarch_long_bit (gdbarch),
+ 0, "short");
void_ptr_type = lookup_pointer_type (builtin_type (gdbarch)->builtin_void);
/* sival_t */
@@ -342,6 +344,18 @@ linux_get_siginfo_type_with_fields (struct gdbarch *gdbarch,
/* _sigfault */
type = arch_composite_type (gdbarch, NULL, TYPE_CODE_STRUCT);
append_composite_type_field (type, "si_addr", void_ptr_type);
+
+ /* additional bound fields for _sigfault in case they are added. */
+ if ((extra_fields & LINUX_SIGINFO_FIELD_ADDR_BND) != 0)
+ {
+ struct type *sigfault_bnd_fields;
+
+ append_composite_type_field (type, "_addr_lsb", short_type);
+ sigfault_bnd_fields = arch_composite_type (gdbarch, NULL, TYPE_CODE_STRUCT);
+ append_composite_type_field (sigfault_bnd_fields, "_lower", void_ptr_type);
+ append_composite_type_field (sigfault_bnd_fields, "_upper", void_ptr_type);
+ append_composite_type_field (type, "_addr_bnd", sigfault_bnd_fields);
+ }
append_composite_type_field (sifields_type, "_sigfault", type);
/* _sigpoll */
--
2.1.4
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH V5 2/5] Use linux_get_siginfo_type_with_fields for x86
2016-01-22 15:36 [PATCH V5 0/5] Intel MPX bound violation support Walfred Tedeschi
` (2 preceding siblings ...)
2016-01-22 15:17 ` [PATCH V5 4/5] Adaptation of siginfo fixup for the new bnd fields Walfred Tedeschi
@ 2016-01-22 15:18 ` Walfred Tedeschi
2016-01-28 12:19 ` Pedro Alves
2016-01-22 15:18 ` [PATCH V5 5/5] ntel MPX bound violation handling Walfred Tedeschi
4 siblings, 1 reply; 18+ messages in thread
From: Walfred Tedeschi @ 2016-01-22 15:18 UTC (permalink / raw)
To: palves, eliz, brobecker; +Cc: gdb-patches, Walfred Tedeschi
Use linux_get_siginfo_type_with_fields for adding bound fields on
segmentation fault for i386/amd64 siginfo.
2016-01-15 Walfred Tedeschi <walfred.tedeschi@intel.com>
gdb/ChangeLog:
* linux-tdep.h (linux_get_siginfo_type_with_fields): Make extern.
* linux-tdep.c (linux_get_siginfo_type_with_fields): Make extern.
* i386-linux-tdep.h (x86_linux_get_siginfo_type): New
function.
* amd64-linux-tdep.c (amd64_linux_init_abi_common): Add
x86_linux_get_siginfo_type for the amd64 abi.
* i386-linux-tdep.c (x86_linux_get_siginfo_type): New
Function.
(i386_linux_init_abi): Add new function at the i386 ABI
initialization.
---
gdb/amd64-linux-tdep.c | 2 ++
gdb/i386-linux-tdep.c | 8 ++++++++
gdb/i386-linux-tdep.h | 3 +++
gdb/linux-tdep.c | 2 +-
gdb/linux-tdep.h | 4 ++++
5 files changed, 18 insertions(+), 1 deletion(-)
diff --git a/gdb/amd64-linux-tdep.c b/gdb/amd64-linux-tdep.c
index b948ea7..21bcd99 100644
--- a/gdb/amd64-linux-tdep.c
+++ b/gdb/amd64-linux-tdep.c
@@ -1838,6 +1838,8 @@ amd64_linux_init_abi_common(struct gdbarch_info info, struct gdbarch *gdbarch)
set_gdbarch_process_record (gdbarch, i386_process_record);
set_gdbarch_process_record_signal (gdbarch, amd64_linux_record_signal);
+
+ set_gdbarch_get_siginfo_type (gdbarch, x86_linux_get_siginfo_type);
}
static void
diff --git a/gdb/i386-linux-tdep.c b/gdb/i386-linux-tdep.c
index 1e491e7..af39e78 100644
--- a/gdb/i386-linux-tdep.c
+++ b/gdb/i386-linux-tdep.c
@@ -656,6 +656,12 @@ i386_linux_supply_xstateregset (const struct regset *regset,
i387_supply_xsave (regcache, regnum, xstateregs);
}
+struct type *
+x86_linux_get_siginfo_type (struct gdbarch *gdbarch)
+{
+ return linux_get_siginfo_type_with_fields (gdbarch, LINUX_SIGINFO_FIELD_ADDR_BND);
+}
+
/* Similar to i386_collect_fpregset, but use XSAVE extended state. */
static void
@@ -994,6 +1000,8 @@ i386_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
set_xml_syscall_file_name (gdbarch, XML_SYSCALL_FILENAME_I386);
set_gdbarch_get_syscall_number (gdbarch,
i386_linux_get_syscall_number);
+
+ set_gdbarch_get_siginfo_type (gdbarch, x86_linux_get_siginfo_type);
}
/* Provide a prototype to silence -Wmissing-prototypes. */
diff --git a/gdb/i386-linux-tdep.h b/gdb/i386-linux-tdep.h
index ee6abff..b32f6d1 100644
--- a/gdb/i386-linux-tdep.h
+++ b/gdb/i386-linux-tdep.h
@@ -72,4 +72,7 @@ extern struct target_desc *tdesc_i386_avx512_linux;
extern int i386_linux_gregset_reg_offset[];
+/* Returns x86 siginfo type with extra fields. */
+extern struct type *x86_linux_get_siginfo_type (struct gdbarch *gdbarch);
+
#endif /* i386-linux-tdep.h */
diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
index 3757306..acc6e16 100644
--- a/gdb/linux-tdep.c
+++ b/gdb/linux-tdep.c
@@ -246,7 +246,7 @@ get_linux_inferior_data (void)
/* This function is suitable for architectures that
extend/override the standard siginfo in a specific way. */
-static struct type *
+struct type *
linux_get_siginfo_type_with_fields (struct gdbarch *gdbarch,
linux_siginfo_extra_fields extra_fields)
{
diff --git a/gdb/linux-tdep.h b/gdb/linux-tdep.h
index b2680f8..ac342a3 100644
--- a/gdb/linux-tdep.h
+++ b/gdb/linux-tdep.h
@@ -36,6 +36,10 @@ enum linux_siginfo_extra_field_values
DEF_ENUM_FLAGS_TYPE (enum linux_siginfo_extra_field_values,
linux_siginfo_extra_fields);
+/* Return the siginfo type with additional fields. */
+struct type *linux_get_siginfo_type_with_fields (struct gdbarch *gdbarch,
+ linux_siginfo_extra_fields);
+
typedef char *(*linux_collect_thread_registers_ftype) (const struct regcache *,
ptid_t,
bfd *, char *, int *,
--
2.1.4
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH V5 5/5] ntel MPX bound violation handling
2016-01-22 15:36 [PATCH V5 0/5] Intel MPX bound violation support Walfred Tedeschi
` (3 preceding siblings ...)
2016-01-22 15:18 ` [PATCH V5 2/5] Use linux_get_siginfo_type_with_fields for x86 Walfred Tedeschi
@ 2016-01-22 15:18 ` Walfred Tedeschi
2016-01-28 13:43 ` Pedro Alves
4 siblings, 1 reply; 18+ messages in thread
From: Walfred Tedeschi @ 2016-01-22 15:18 UTC (permalink / raw)
To: palves, eliz, brobecker; +Cc: gdb-patches, Walfred Tedeschi
With Intel Memory Protection Extensions it was introduced the concept of
boundary violation. A boundary violations is presented to the inferior as
a segmentation fault having SIGCODE 3. This patch adds a
handler for a boundary violation extending the information displayed
when a bound violation is presented to the inferior. In the stop mode
case the debugger will also display the kind of violation: "upper" or
"lower", bounds and the address accessed.
On no stop mode the information will still remain unchanged. Additional
information about bound violations are not meaningful in that case user
does not know the line in which violation occurred as well.
When the segmentation fault handler is stop mode the out puts will be
changed as exemplified below.
The usual output of a segfault is:
Program received signal SIGSEGV, Segmentation fault
0x0000000000400d7c in upper (p=0x603010, a=0x603030, b=0x603050,
c=0x603070, d=0x603090, len=7) at i386-mpx-sigsegv.c:68
68 value = *(p + len);
In case it is a bound violation it will be presented as:
Program received signal SIGSEGV, Segmentation fault
Upper bound violation while accessing address 0x7fffffffc3b3
Bounds: [lower = 0x7fffffffc390, upper = 0x7fffffffc3a3]
0x0000000000400d7c in upper (p=0x603010, a=0x603030, b=0x603050,
c=0x603070, d=0x603090, len=7) at i386-mpx-sigsegv.c:68
68 value = *(p + len);
In mi mode the output of a segfault is:
*stopped,reason="signal-received",signal-name="SIGSEGV",
signal-meaning="Segmentation fault", frame={addr="0x0000000000400d7c",
func="upper",args=[{name="p", value="0x603010"},{name="a",value="0x603030"}
,{name="b",value="0x603050"}, {name="c",value="0x603070"},
{name="d",value="0x603090"},{name="len",value="7"}],
file="i386-mpx-sigsegv.c",fullname="i386-mpx-sigsegv.c",line="68"},
thread-id="1",stopped-threads="all",core="6"
in the case of a bound violation:
*stopped,reason="signal-received",signal-name="SIGSEGV",
signal-meaning="Segmentation fault",
sigcode-meaning="Upper bound violation",
lower-bound="0x603010",upper-bound="0x603023",bound-access="0x60302f",
frame={addr="0x0000000000400d7c",func="upper",args=[{name="p",
value="0x603010"},{name="a",value="0x603030"},{name="b",value="0x603050"},
{name="c",value="0x603070"},{name="d",value="0x603090"},
{name="len",value="7"}],file="i386-mpx-sigsegv.c",
fullname="i386-mpx-sigsegv.c",line="68"},thread-id="1",
stopped-threads="all",core="6"
2016-01-15 Walfred Tedeschi <walfred.tedeschi@intel.com>
gdb/ChangeLog:
* NEWS: Add entry for bound violation.
* amd64-linux-tdep.c (amd64_linux_init_abi_common):
Add handler for bound violation.
* gdbarch.sh (bound_violation_handler): New.
* gdbarch.c: Regenerate.
* gdbarch.h: Regenerate.
* i386-linux-tdep.c (i386_mpx_bound_violation_handler): New.
(i386_linux_init_abi): Use i386_mpx_bound_violation_handler.
* i386-linux-tdep.h (i386_mpx_bound_violation_handler) New.
* i386-tdep.c (i386_mpx_enabled): Add as external.
* i386-tdep.c (i386_mpx_enabled): Add as external.
* infrun.c (handle_segmentation_faults): New function.
(print_signal_received_reason): Use handle_segmentation_faults.
(normal_stop): Change order of observer in order to have the
inferior stopped for evaluation.
gdb/testsuite/ChangeLog:
* gdb.arch/i386-mpx-sigsegv.c: New file.
* gdb.arch/i386-mpx-sigsegv.exp: New file.
* gdb.arch/i386-mpx-simple_segv.c: New file.
* gdb.arch/i386-mpx-simple_segv.exp: New file.
gdb/doc/ChangeLog:
* gdb.texinfo (Signals): Add bound violation display hints for
a SIGSEGV.
---
gdb/NEWS | 15 +++
gdb/amd64-linux-tdep.c | 2 +
gdb/doc/gdb.texinfo | 25 +++++
gdb/gdbarch.c | 32 ++++++
gdb/gdbarch.h | 11 +++
gdb/gdbarch.sh | 6 ++
gdb/i386-linux-tdep.c | 46 +++++++++
gdb/i386-linux-tdep.h | 5 +
gdb/i386-tdep.c | 4 +-
gdb/i386-tdep.h | 2 +
gdb/infrun.c | 34 +++++++
gdb/testsuite/gdb.arch/i386-mpx-sigsegv.c | 120 +++++++++++++++++++++++
gdb/testsuite/gdb.arch/i386-mpx-sigsegv.exp | 88 +++++++++++++++++
gdb/testsuite/gdb.arch/i386-mpx-simple_segv.c | 66 +++++++++++++
gdb/testsuite/gdb.arch/i386-mpx-simple_segv.exp | 125 ++++++++++++++++++++++++
15 files changed, 578 insertions(+), 3 deletions(-)
create mode 100644 gdb/testsuite/gdb.arch/i386-mpx-sigsegv.c
create mode 100644 gdb/testsuite/gdb.arch/i386-mpx-sigsegv.exp
create mode 100644 gdb/testsuite/gdb.arch/i386-mpx-simple_segv.c
create mode 100644 gdb/testsuite/gdb.arch/i386-mpx-simple_segv.exp
diff --git a/gdb/NEWS b/gdb/NEWS
index d9cbb80..b8512d6 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -3,6 +3,21 @@
*** Changes since GDB 7.10
+* Intel MPX boud violation handler.
+
+ A boundary violations is presented to the inferior as
+ a segmentation fault having SIGCODE 3. In this case
+ GDB displays also the kind of violation (upper or lower),
+ bounds, poiter value and the memory accessed, besides displaying
+ the usual signal received and code location report.
+
+ As exemplified below:
+ Program received signal SIGSEGV, Segmentation fault
+ Upper bound violation while accessing address 0x7fffffffc3b3
+ Bounds: [lower = 0x7fffffffc390, upper = 0x7fffffffc3a3].
+ 0x0000000000400d7c in upper (p=0x603010, a=0x603030, b=0x603050,
+ c=0x603070, d=0x603090, len=7) at i386-mpx-sigsegv.c:68
+
* Per-inferior thread numbers
Thread numbers are now per inferior instead of global. If you're
diff --git a/gdb/amd64-linux-tdep.c b/gdb/amd64-linux-tdep.c
index 21bcd99..8af15c4 100644
--- a/gdb/amd64-linux-tdep.c
+++ b/gdb/amd64-linux-tdep.c
@@ -1840,6 +1840,8 @@ amd64_linux_init_abi_common(struct gdbarch_info info, struct gdbarch *gdbarch)
set_gdbarch_process_record_signal (gdbarch, amd64_linux_record_signal);
set_gdbarch_get_siginfo_type (gdbarch, x86_linux_get_siginfo_type);
+ set_gdbarch_bound_violation_handler(gdbarch,
+ i386_mpx_bound_violation_handler);
}
static void
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 7da31c8..f4373ed 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -5819,6 +5819,30 @@ $1 = (void *) 0x7ffff7ff7000
Depending on target support, @code{$_siginfo} may also be writable.
+Also on some targets a @code{SIGSEGV} can be related to
+boundary violations, i.e. accessing an address outside of
+allowed range. In those cases @value{GDBN} might displays additional
+information. In @code{STOP} mode @value{GDBN} displays the violation
+kind: "Upper" or "Lower", address accessed and bounds. In @code{NOSTOP}
+no additional information is displayed.
+
+The usual output of a segfault is:
+@smallexample
+Program received signal SIGSEGV, Segmentation fault
+0x0000000000400d7c in upper (p=0x603010, a=0x603030, b=0x603050,
+c=0x603070, d=0x603090, len=7) at i386-mpx-sigsegv.c:68
+68 value = *(p + len);
+@end smallexample
+
+In case it is a bound violation it will be presented as:
+@smallexample
+Upper bound violation while accessing address 0x7fffffffc3b3
+Bounds: [lower = 0x7fffffffc390, upper = 0x7fffffffc3a3]
+0x0000000000400d7c in upper (p=0x603010, a=0x603030, b=0x603050,
+c=0x603070, d=0x603090, len=7) at i386-mpx-sigsegv.c:68
+68 value = *(p + len);
+@end smallexample
+
@node Thread Stops
@section Stopping and Starting Multi-thread Programs
@@ -22267,6 +22291,7 @@ whose bounds are to be changed, @var{lbound} and @var{ubound} are new values
for lower and upper bounds respectively.
@end table
+
@node Alpha
@subsection Alpha
diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
index d45af1a..f7fef25 100644
--- a/gdb/gdbarch.c
+++ b/gdb/gdbarch.c
@@ -189,6 +189,7 @@ struct gdbarch
int num_pseudo_regs;
gdbarch_ax_pseudo_register_collect_ftype *ax_pseudo_register_collect;
gdbarch_ax_pseudo_register_push_stack_ftype *ax_pseudo_register_push_stack;
+ gdbarch_bound_violation_handler_ftype *bound_violation_handler;
int sp_regnum;
int pc_regnum;
int ps_regnum;
@@ -531,6 +532,7 @@ verify_gdbarch (struct gdbarch *gdbarch)
/* Skip verify of num_pseudo_regs, invalid_p == 0 */
/* Skip verify of ax_pseudo_register_collect, has predicate. */
/* Skip verify of ax_pseudo_register_push_stack, has predicate. */
+ /* Skip verify of bound_violation_handler, has predicate. */
/* Skip verify of sp_regnum, invalid_p == 0 */
/* Skip verify of pc_regnum, invalid_p == 0 */
/* Skip verify of ps_regnum, invalid_p == 0 */
@@ -773,6 +775,12 @@ gdbarch_dump (struct gdbarch *gdbarch, struct ui_file *file)
"gdbarch_dump: bits_big_endian = %s\n",
plongest (gdbarch->bits_big_endian));
fprintf_unfiltered (file,
+ "gdbarch_dump: gdbarch_bound_violation_handler_p() = %d\n",
+ gdbarch_bound_violation_handler_p (gdbarch));
+ fprintf_unfiltered (file,
+ "gdbarch_dump: bound_violation_handler = <%s>\n",
+ host_address_to_string (gdbarch->bound_violation_handler));
+ fprintf_unfiltered (file,
"gdbarch_dump: breakpoint_from_pc = <%s>\n",
host_address_to_string (gdbarch->breakpoint_from_pc));
fprintf_unfiltered (file,
@@ -1986,6 +1994,30 @@ set_gdbarch_ax_pseudo_register_push_stack (struct gdbarch *gdbarch,
}
int
+gdbarch_bound_violation_handler_p (struct gdbarch *gdbarch)
+{
+ gdb_assert (gdbarch != NULL);
+ return gdbarch->bound_violation_handler != NULL;
+}
+
+void
+gdbarch_bound_violation_handler (struct gdbarch *gdbarch, struct ui_out *uiout)
+{
+ gdb_assert (gdbarch != NULL);
+ gdb_assert (gdbarch->bound_violation_handler != NULL);
+ if (gdbarch_debug >= 2)
+ fprintf_unfiltered (gdb_stdlog, "gdbarch_bound_violation_handler called\n");
+ gdbarch->bound_violation_handler (gdbarch, uiout);
+}
+
+void
+set_gdbarch_bound_violation_handler (struct gdbarch *gdbarch,
+ gdbarch_bound_violation_handler_ftype bound_violation_handler)
+{
+ gdbarch->bound_violation_handler = bound_violation_handler;
+}
+
+int
gdbarch_sp_regnum (struct gdbarch *gdbarch)
{
gdb_assert (gdbarch != NULL);
diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
index 3c16af2..eb5de0d 100644
--- a/gdb/gdbarch.h
+++ b/gdb/gdbarch.h
@@ -63,6 +63,7 @@ struct ravenscar_arch_ops;
struct elf_internal_linux_prpsinfo;
struct mem_range;
struct syscalls_info;
+struct ui_out;
#include "regcache.h"
@@ -299,6 +300,16 @@ typedef int (gdbarch_ax_pseudo_register_push_stack_ftype) (struct gdbarch *gdbar
extern int gdbarch_ax_pseudo_register_push_stack (struct gdbarch *gdbarch, struct agent_expr *ax, int reg);
extern void set_gdbarch_ax_pseudo_register_push_stack (struct gdbarch *gdbarch, gdbarch_ax_pseudo_register_push_stack_ftype *ax_pseudo_register_push_stack);
+/* Function called when a segmentation fault signal is received by the inferior,
+ having SIGCODE 3 (SIG_CODE_BOUNDARY_FAULT).
+ UIOUT is the output stream where the handler will place information. */
+
+extern int gdbarch_bound_violation_handler_p (struct gdbarch *gdbarch);
+
+typedef void (gdbarch_bound_violation_handler_ftype) (struct gdbarch *gdbarch, struct ui_out *uiout);
+extern void gdbarch_bound_violation_handler (struct gdbarch *gdbarch, struct ui_out *uiout);
+extern void set_gdbarch_bound_violation_handler (struct gdbarch *gdbarch, gdbarch_bound_violation_handler_ftype *bound_violation_handler);
+
/* GDB's standard (or well known) register numbers. These can map onto
a real register or a pseudo (computed) register or not be defined at
all (-1).
diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
index f80cd51..edd155a 100755
--- a/gdb/gdbarch.sh
+++ b/gdb/gdbarch.sh
@@ -446,6 +446,11 @@ M:int:ax_pseudo_register_collect:struct agent_expr *ax, int reg:ax, reg
# Return -1 if something goes wrong, 0 otherwise.
M:int:ax_pseudo_register_push_stack:struct agent_expr *ax, int reg:ax, reg
+# Function called when a segmentation fault signal is received by the inferior,
+# having SIGCODE 3 (SIG_CODE_BOUNDARY_FAULT).
+# UIOUT is the output stream where the handler will place information.
+M:void:bound_violation_handler:struct ui_out *uiout:uiout
+
# GDB's standard (or well known) register numbers. These can map onto
# a real register or a pseudo (computed) register or not be defined at
# all (-1).
@@ -1247,6 +1252,7 @@ struct ravenscar_arch_ops;
struct elf_internal_linux_prpsinfo;
struct mem_range;
struct syscalls_info;
+struct ui_out;
#include "regcache.h"
diff --git a/gdb/i386-linux-tdep.c b/gdb/i386-linux-tdep.c
index af39e78..45212e0 100644
--- a/gdb/i386-linux-tdep.c
+++ b/gdb/i386-linux-tdep.c
@@ -30,6 +30,7 @@
#include "i386-tdep.h"
#include "i386-linux-tdep.h"
#include "linux-tdep.h"
+#include "utils.h"
#include "glibc-tdep.h"
#include "solib-svr4.h"
#include "symtab.h"
@@ -384,6 +385,49 @@ i386_canonicalize_syscall (int syscall)
return gdb_sys_no_syscall;
}
+void
+i386_mpx_bound_violation_handler (struct gdbarch *gdbarch, struct ui_out *uiout)
+{
+ CORE_ADDR lower_bound, upper_bound, access;
+ int is_upper;
+
+ if (!i386_mpx_enabled ())
+ return;
+ TRY
+ {
+ lower_bound
+ = parse_and_eval_long ("$_siginfo._sifields._sigfault._addr_bnd._lower");
+ upper_bound
+ = parse_and_eval_long ("$_siginfo._sifields._sigfault._addr_bnd._upper");
+ access
+ = parse_and_eval_long ("$_siginfo._sifields._sigfault.si_addr");
+ }
+ CATCH (exception, RETURN_MASK_ALL)
+ {
+ return;
+ }
+ END_CATCH
+
+ is_upper = (access > upper_bound ? 1 : 0);
+
+ ui_out_text (uiout, "\n");
+ if (is_upper)
+ ui_out_field_string (uiout, "sigcode-meaning", "Upper bound violation");
+ else
+ ui_out_field_string (uiout, "sigcode-meaning", "Lower bound violation");
+
+ ui_out_text (uiout, " while accessing address ");
+ ui_out_field_fmt (uiout,"bound-access", "%s", paddress (gdbarch, access));
+ ui_out_text (uiout, "\nBounds: [lower = ");
+ ui_out_field_fmt (uiout,"lower-bound", "%s", paddress (gdbarch, lower_bound));
+ ui_out_text (uiout, ", upper = ");
+ ui_out_field_fmt (uiout,"upper-bound", "%s", paddress (gdbarch, upper_bound));
+ ui_out_text (uiout, "]");
+
+
+ return;
+}
+
/* Parse the arguments of current system call instruction and record
the values of the registers and memory that will be changed into
"record_arch_list". This instruction is "int 0x80" (Linux
@@ -1002,6 +1046,8 @@ i386_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
i386_linux_get_syscall_number);
set_gdbarch_get_siginfo_type (gdbarch, x86_linux_get_siginfo_type);
+ set_gdbarch_bound_violation_handler(gdbarch,
+ i386_mpx_bound_violation_handler);
}
/* Provide a prototype to silence -Wmissing-prototypes. */
diff --git a/gdb/i386-linux-tdep.h b/gdb/i386-linux-tdep.h
index b32f6d1..6ffacd4 100644
--- a/gdb/i386-linux-tdep.h
+++ b/gdb/i386-linux-tdep.h
@@ -37,6 +37,11 @@
/* Get XSAVE extended state xcr0 from core dump. */
extern uint64_t i386_linux_core_read_xcr0 (bfd *abfd);
+/* Handles and displays information related to the MPX bound violation
+ to the user. */
+void
+i386_mpx_bound_violation_handler (struct gdbarch *gdbarch, struct ui_out *uiout);
+
/* Linux target description. */
extern struct target_desc *tdesc_i386_linux;
extern struct target_desc *tdesc_i386_mmx_linux;
diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index b706463..b5d0d14 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -8651,9 +8651,7 @@ i386_mpx_bd_base (void)
return ret & MPX_BASE_MASK;
}
-/* Check if the current target is MPX enabled. */
-
-static int
+int
i386_mpx_enabled (void)
{
const struct gdbarch_tdep *tdep = gdbarch_tdep (get_current_arch ());
diff --git a/gdb/i386-tdep.h b/gdb/i386-tdep.h
index 10d2772..26933f2 100644
--- a/gdb/i386-tdep.h
+++ b/gdb/i386-tdep.h
@@ -420,6 +420,8 @@ extern int i386_process_record (struct gdbarch *gdbarch,
struct regcache *regcache, CORE_ADDR addr);
extern const struct target_desc *i386_target_description (uint64_t xcr0);
+/* Verify if target is MPX enabled. */
+extern int i386_mpx_enabled (void);
\f
/* Functions and variables exported from i386bsd-tdep.c. */
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 64c729e..87b930e 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -7893,6 +7893,36 @@ print_exited_reason (struct ui_out *uiout, int exitstatus)
}
}
+/* Value of the sigcode in case of a boundary fault. */
+
+#define SIG_CODE_BONDARY_FAULT 3
+
+/* Verifies if a received segmentation fault is a boundary fault.
+ In the case it is it calls the architecture dependent function
+ to handle the boundary fault. */
+
+static void
+handle_segmentation_faults (struct ui_out *uiout)
+{
+ long sig_code = 0;
+ struct regcache *regcache = get_current_regcache ();
+ struct gdbarch *gdbarch = get_regcache_arch (regcache);
+
+ TRY
+ {
+ sig_code = parse_and_eval_long ("$_siginfo.si_code\n");
+ }
+ CATCH (exception, RETURN_MASK_ALL)
+ {
+ return;
+ }
+ END_CATCH
+
+ if (sig_code == SIG_CODE_BONDARY_FAULT
+ && gdbarch_bound_violation_handler_p (gdbarch))
+ gdbarch_bound_violation_handler (gdbarch, uiout);
+}
+
void
print_signal_received_reason (struct ui_out *uiout, enum gdb_signal siggnal)
{
@@ -7922,6 +7952,10 @@ print_signal_received_reason (struct ui_out *uiout, enum gdb_signal siggnal)
annotate_signal_string ();
ui_out_field_string (uiout, "signal-meaning",
gdb_signal_to_string (siggnal));
+
+ if (siggnal == GDB_SIGNAL_SEGV)
+ handle_segmentation_faults (uiout);
+
annotate_signal_string_end ();
}
ui_out_text (uiout, ".\n");
diff --git a/gdb/testsuite/gdb.arch/i386-mpx-sigsegv.c b/gdb/testsuite/gdb.arch/i386-mpx-sigsegv.c
new file mode 100644
index 0000000..7500352
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/i386-mpx-sigsegv.c
@@ -0,0 +1,120 @@
+/* Copyright (C) 2015 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 "x86-cpuid.h"
+#include <stdio.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;
+}
+
+void
+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);
+}
+
+void
+lower (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);
+ bp2 (value);
+ value = *(d - len);
+}
+
+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 (x, a, b, c, d, OUR_SIZE + 2);
+ upper (sx, sa, sb, sc, sd, OUR_SIZE + 2);
+ upper (gx, ga, gb, gc, gd, OUR_SIZE + 2);
+ lower (x, a, b, c, d, 1);
+ lower (sx, sa, sb, sc, sd, 1);
+ bp1 (*x);
+ lower (gx, ga, gb, gc, gd, 1);
+
+ free (x);
+ free (a);
+ free (b);
+ free (c);
+ free (d);
+ }
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.arch/i386-mpx-sigsegv.exp b/gdb/testsuite/gdb.arch/i386-mpx-sigsegv.exp
new file mode 100644
index 0000000..af7db29
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/i386-mpx-sigsegv.exp
@@ -0,0 +1,88 @@
+# Copyright (C) 2015 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
+ }
+}
+
+set common_ubound_violation ".*Program received signal SIGSEGV,\
+ Segmentation fault\r\nUpper bound violation while accessing\
+ address $hex\r\nBounds: \\\[lower = $hex, upper = $hex\\\]"
+
+set common_lbound_violation ".*Program received signal SIGSEGV,\
+ Segmentation fault\r\nLower bound violation while accessing\
+ address $hex\r\nBounds: \\\[lower = $hex, upper = $hex\\\]"
+
+set segv_upper_bound "$common_ubound_violation.*$gdb_prompt $"
+
+set segv_lower_bound "$common_lbound_violation.*$gdb_prompt $"
+
+for {set i 0} {$i < 15} {incr i} {
+ set message "MPX signal segv Upper: ${i}"
+ gdb_test_multiple "continue" "$message ${i}" {
+ -re $segv_upper_bound {
+ pass "$message"
+ }
+ -re ".*$inferior_exited_re normally.*$gdb_prompt $" {
+ fail "$message"
+ break
+ }
+ }
+ gdb_test "where" ".*#0 0x\[0-9a-fA-F\]+ in upper.*"\
+ "$message: should be in upper"
+}
+
+for {set i 0} {$i < 15} {incr i} {
+ set message "MPX signal segv Lower: ${i}"
+ gdb_test_multiple "continue" "$message ${i}" {
+ -re $segv_lower_bound {
+ pass "$message ${i}"
+ }
+ -re ".*$inferior_exited_re normally.*$gdb_prompt $" {
+ fail "$message ${i}"
+ break
+ }
+ }
+ gdb_test "where" ".*#0 0x\[0-9a-fA-F\]+ in lower.*"\
+ "$message: should be in lower"
+}
diff --git a/gdb/testsuite/gdb.arch/i386-mpx-simple_segv.c b/gdb/testsuite/gdb.arch/i386-mpx-simple_segv.c
new file mode 100644
index 0000000..5317369
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/i386-mpx-simple_segv.c
@@ -0,0 +1,66 @@
+/* Copyright (C) 2015 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 "x86-cpuid.h"
+#include <stdio.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;
+}
+
+void
+upper (int * p, int len)
+{
+ int value;
+ len++; /* b0-size-test. */
+ value = *(p + len);
+}
+
+int
+main (void)
+{
+ if (have_mpx ())
+ {
+ int a = 0; /* Dummy variable for debugging purposes. */
+ int sx[OUR_SIZE];
+ a++; /* register-eval. */
+ upper (sx, OUR_SIZE + 2);
+ return sx[1];
+ }
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.arch/i386-mpx-simple_segv.exp b/gdb/testsuite/gdb.arch/i386-mpx-simple_segv.exp
new file mode 100644
index 0000000..4df3a44
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/i386-mpx-simple_segv.exp
@@ -0,0 +1,125 @@
+# Copyright (C) 2015 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/>.
+
+# Testing handle setup together with boundary violation signals.
+#
+# Some states are not allowed as reported on the manual, as noprint
+# implies nostop, but nostop might print.
+#
+# Caveat: Setting the handle to nopass, ends up in a endless loop.
+
+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
+}
+
+send_gdb "print have_mpx ()\r"
+gdb_expect {
+ -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
+ }
+}
+set common_ubound_violation ".*Program received signal SIGSEGV,\
+ Segmentation fault\r\nUpper bound violation while accessing\
+ address $hex\r\nBounds: \\\[lower = $hex, upper = $hex\\\]"
+
+set segv_bound_with_prompt "$common_ubound_violation.*$gdb_prompt $"
+set segv_bound_with_exit "$common_ubound_violation.*$inferior_exited_re.*"
+
+set segv_with_exit ".*Program received signal SIGSEGV,\
+ Segmentation fault.*$inferior_exited_re.*"
+
+# Using the handler for SIGSEGV as "print pass stop"
+set parameters "print pass stop"
+runto_main
+send_gdb "handle SIGSEGV $parameters\n"
+send_gdb "continue\n"
+
+gdb_expect {
+ -re $segv_bound_with_prompt {
+ pass $parameters
+ }
+}
+gdb_test "where" ".*#0 0x\[0-9a-fA-F\]+ in upper.*"\
+ "should be in upper; $parameters"
+
+# Using the handler for SIGSEGV as "print pass nostop"
+set parameters "print pass nostop"
+runto_main
+
+gdb_test "handle SIGSEGV $parameters" "" "Setting\
+the handler for segfault 0"
+
+gdb_test_multiple "continue" "test 0" {
+ -re $segv_with_exit {
+ pass $parameters
+ }
+ -re "$gdb_prompt $" {
+ fail $parameters
+ }
+}
+
+gdb_test "where" "No stack." "no inferior $parameters"
+
+# Using the handler for SIGSEGV as "print nopass stop"
+set parameters "print nopass stop"
+
+runto_main
+gdb_test "handle SIGSEGV $parameters" "" "Setting\
+the handler for segfault 1"
+
+gdb_test_multiple "continue" "test 1" {
+ -re $segv_bound_with_prompt {
+ pass $parameters
+ }
+}
+
+gdb_test "where" ".*#0 $hex in upper.*"\
+ "should be in upper $parameters"
+
+# print nopass stop
+set parameters "noprint pass nostop"
+runto_main
+
+gdb_test "handle SIGSEGV $parameters" "" "Setting\
+the handler for segfault 2"
+
+gdb_test_multiple "continue" "test 2" {
+ -re "Continuing\..*$inferior_exited_re.*" {
+ pass $parameters
+ }
+}
+
+gdb_test "where" "No stack." "no inferior $parameters"
--
2.1.4
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH V5 0/5] Intel MPX bound violation support
@ 2016-01-22 15:36 Walfred Tedeschi
2016-01-22 15:17 ` [PATCH V5 1/5] Preparation for new siginfo on Linux Walfred Tedeschi
` (4 more replies)
0 siblings, 5 replies; 18+ messages in thread
From: Walfred Tedeschi @ 2016-01-22 15:36 UTC (permalink / raw)
To: palves, eliz, brobecker; +Cc: gdb-patches, Walfred Tedeschi
Hello all,
Thanks for the review!
First 4 patches are preparation on the siginfo type and
siginfo handling. Including the addition of new fields
related to the bound violation.
Last patch is the handler for the bound violation itself.
From V4 to V5:
* GDB and gdbserver siginfo fixup was aproved (not present here)
* Added Pedro's and Eli's feedback for all patches.
* Changed documentation section for bound violations.
* Changed bound violation output.
Thanks Pedro and Eli for the feedback and
sorry for submitting new revision without
adressing the issues.
Feedback e-mails ended up in not expected folder. :(
Sorry again for any inconvenience.
Thanks for the review and regards,
-Fred
Walfred Tedeschi (5):
Preparation for new siginfo on Linux
Use linux_get_siginfo_type_with_fields for x86
Add bound related fields to the siginfo structure
Adaptation of siginfo fixup for the new bnd fields
ntel MPX bound violation handling
gdb/NEWS | 15 ++
gdb/amd64-linux-tdep.c | 4 +
gdb/doc/gdb.texinfo | 25 ++
gdb/gdbarch.c | 32 +++
gdb/gdbarch.h | 11 +
gdb/gdbarch.sh | 6 +
gdb/i386-linux-tdep.c | 54 ++++
gdb/i386-linux-tdep.h | 8 +
gdb/i386-tdep.c | 4 +-
gdb/i386-tdep.h | 2 +
gdb/infrun.c | 34 +++
gdb/linux-tdep.c | 34 ++-
gdb/linux-tdep.h | 16 ++
gdb/nat/amd64-linux-siginfo.c | 312 ++++++++++++++++--------
gdb/nat/amd64-linux-siginfo.h | 16 +-
gdb/testsuite/gdb.arch/i386-mpx-sigsegv.c | 120 +++++++++
gdb/testsuite/gdb.arch/i386-mpx-sigsegv.exp | 88 +++++++
gdb/testsuite/gdb.arch/i386-mpx-simple_segv.c | 66 +++++
gdb/testsuite/gdb.arch/i386-mpx-simple_segv.exp | 125 ++++++++++
19 files changed, 862 insertions(+), 110 deletions(-)
create mode 100644 gdb/testsuite/gdb.arch/i386-mpx-sigsegv.c
create mode 100644 gdb/testsuite/gdb.arch/i386-mpx-sigsegv.exp
create mode 100644 gdb/testsuite/gdb.arch/i386-mpx-simple_segv.c
create mode 100644 gdb/testsuite/gdb.arch/i386-mpx-simple_segv.exp
--
2.1.4
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH V5 2/5] Use linux_get_siginfo_type_with_fields for x86
2016-01-22 15:18 ` [PATCH V5 2/5] Use linux_get_siginfo_type_with_fields for x86 Walfred Tedeschi
@ 2016-01-28 12:19 ` Pedro Alves
0 siblings, 0 replies; 18+ messages in thread
From: Pedro Alves @ 2016-01-28 12:19 UTC (permalink / raw)
To: Walfred Tedeschi, eliz, brobecker; +Cc: gdb-patches
On 01/22/2016 02:54 PM, Walfred Tedeschi wrote:
> Use linux_get_siginfo_type_with_fields for adding bound fields on
> segmentation fault for i386/amd64 siginfo.
>
> 2016-01-15 Walfred Tedeschi <walfred.tedeschi@intel.com>
>
> gdb/ChangeLog:
>
> * linux-tdep.h (linux_get_siginfo_type_with_fields): Make extern.
> * linux-tdep.c (linux_get_siginfo_type_with_fields): Make extern.
> * i386-linux-tdep.h (x86_linux_get_siginfo_type): New
> function.
> * amd64-linux-tdep.c (amd64_linux_init_abi_common): Add
> x86_linux_get_siginfo_type for the amd64 abi.
> * i386-linux-tdep.c (x86_linux_get_siginfo_type): New
> Function.
"function", lowercase.
> (i386_linux_init_abi): Add new function at the i386 ABI
> initialization.
>
> --- a/gdb/i386-linux-tdep.h
> +++ b/gdb/i386-linux-tdep.h
> @@ -72,4 +72,7 @@ extern struct target_desc *tdesc_i386_avx512_linux;
>
> extern int i386_linux_gregset_reg_offset[];
>
> +/* Returns x86 siginfo type with extra fields. */
/* Return the x86 Linux siginfo type. */
> +extern struct type *x86_linux_get_siginfo_type (struct gdbarch *gdbarch);
> +
> #endif /* i386-linux-tdep.h */
> diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
> index 3757306..acc6e16 100644
> --- a/gdb/linux-tdep.c
> +++ b/gdb/linux-tdep.c
> @@ -246,7 +246,7 @@ get_linux_inferior_data (void)
> /* This function is suitable for architectures that
> extend/override the standard siginfo in a specific way. */
>
> -static struct type *
> +struct type *
> linux_get_siginfo_type_with_fields (struct gdbarch *gdbarch,
> linux_siginfo_extra_fields extra_fields)
> {
> diff --git a/gdb/linux-tdep.h b/gdb/linux-tdep.h
> index b2680f8..ac342a3 100644
> --- a/gdb/linux-tdep.h
> +++ b/gdb/linux-tdep.h
> @@ -36,6 +36,10 @@ enum linux_siginfo_extra_field_values
> DEF_ENUM_FLAGS_TYPE (enum linux_siginfo_extra_field_values,
> linux_siginfo_extra_fields);
>
> +/* Return the siginfo type with additional fields. */
With this, we end up with two places documenting the function, one in the
header, and another in the c file, which is a recipe for getting comments out
of sync, which is exactly what happened. This one predates the revised one
on the implementation side. Instead, please make this patch move the
revised comment here, and replace the one there with "See linux-tdep.h."
> +struct type *linux_get_siginfo_type_with_fields (struct gdbarch *gdbarch,
> + linux_siginfo_extra_fields);
> +
OK with those changes.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH V5 1/5] Preparation for new siginfo on Linux
2016-01-22 15:17 ` [PATCH V5 1/5] Preparation for new siginfo on Linux Walfred Tedeschi
@ 2016-01-28 12:20 ` Pedro Alves
0 siblings, 0 replies; 18+ messages in thread
From: Pedro Alves @ 2016-01-28 12:20 UTC (permalink / raw)
To: Walfred Tedeschi, eliz, brobecker; +Cc: gdb-patches
On 01/22/2016 02:54 PM, Walfred Tedeschi wrote:
> First add new structure and function to allow architecture customization
> for the siginfo structure.
>
> 2016-01-15 Walfred Tedeschi <walfred.tedeschi@intel.com>
>
> gdb/ChangeLog:
>
> * linux-tdep.h (linux_siginfo_extra_field_values): New enum values.
> (linux_siginfo_extra_fields): New enum type.
> * linux-tdep.c (linux_get_siginfo_type_with_fields): New function.
> (linux_get_siginfo_type): Use new function.
OK.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH V5 3/5] Add bound related fields to the siginfo structure
2016-01-22 15:17 ` [PATCH V5 3/5] Add bound related fields to the siginfo structure Walfred Tedeschi
@ 2016-01-28 12:23 ` Pedro Alves
0 siblings, 0 replies; 18+ messages in thread
From: Pedro Alves @ 2016-01-28 12:23 UTC (permalink / raw)
To: Walfred Tedeschi, eliz, brobecker; +Cc: gdb-patches
On 01/22/2016 02:54 PM, Walfred Tedeschi wrote:
> Both Linux and glibc have introduced bound related fields in the segmentation fault fields of the
> siginfo_t type. Ass the new fields to our x86's siginfo_t type too.
"Add the " ...
> @@ -342,6 +344,18 @@ linux_get_siginfo_type_with_fields (struct gdbarch *gdbarch,
> /* _sigfault */
> type = arch_composite_type (gdbarch, NULL, TYPE_CODE_STRUCT);
> append_composite_type_field (type, "si_addr", void_ptr_type);
> +
> + /* additional bound fields for _sigfault in case they are added. */
"Additional", uppercase.
"in case they were requested.".
> + if ((extra_fields & LINUX_SIGINFO_FIELD_ADDR_BND) != 0)
> + {
OK with those changes.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH V5 4/5] Adaptation of siginfo fixup for the new bnd fields
2016-01-22 15:17 ` [PATCH V5 4/5] Adaptation of siginfo fixup for the new bnd fields Walfred Tedeschi
@ 2016-01-28 12:37 ` Pedro Alves
2016-02-02 10:31 ` Pedro Alves
0 siblings, 1 reply; 18+ messages in thread
From: Pedro Alves @ 2016-01-28 12:37 UTC (permalink / raw)
To: Walfred Tedeschi, eliz, brobecker; +Cc: gdb-patches
On 01/22/2016 02:54 PM, Walfred Tedeschi wrote:
> Caveat: No support for MPX on x32.
What would it take to support that?
> +/* These types below (native_*) define a siginfo type that is layout
> + the most complete siginfo available for the architecture. */
nat_, not native_, afaics. I'd copy/edit this to:
/* The nat_* types below define the most complete kernel siginfo type
known for the architecture, independent of system/libc headers. */
> +
> +typedef int nat_int_t;
> +typedef void* nat_uptr_t;
> +
> +typedef int nat_time_t;
> +typedef int nat_timer_t;
> +
>
> @@ -192,54 +287,58 @@ typedef struct compat_x32_siginfo
> static void
> compat_siginfo_from_siginfo (compat_siginfo_t *to, siginfo_t *from)
> {
> + nat_siginfo_t from_nat;
> +
> + gdb_assert (sizeof (nat_siginfo_t) == sizeof (siginfo_t));
Make this a gdb_static_assert, and move it out of the function, below
where nat_siginfo_t is defined. (Likewise the other instances in the
other functions).
OK with those changes.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH V5 5/5] ntel MPX bound violation handling
2016-01-22 15:18 ` [PATCH V5 5/5] ntel MPX bound violation handling Walfred Tedeschi
@ 2016-01-28 13:43 ` Pedro Alves
2016-01-28 13:49 ` Walfred Tedeschi
2016-02-02 10:09 ` Walfred Tedeschi
0 siblings, 2 replies; 18+ messages in thread
From: Pedro Alves @ 2016-01-28 13:43 UTC (permalink / raw)
To: Walfred Tedeschi, eliz, brobecker; +Cc: gdb-patches
"Intel" in $subject.
On 01/22/2016 02:54 PM, Walfred Tedeschi wrote:
> With Intel Memory Protection Extensions it was introduced the concept of
> boundary violation. A boundary violations is presented to the inferior as
> a segmentation fault having SIGCODE 3. This patch adds a
> handler for a boundary violation extending the information displayed
> when a bound violation is presented to the inferior. In the stop mode
> case the debugger will also display the kind of violation: "upper" or
> "lower", bounds and the address accessed.
> On no stop mode the information will still remain unchanged. Additional
> information about bound violations are not meaningful in that case user
> does not know the line in which violation occurred as well.
>
> When the segmentation fault handler is stop mode the out puts will be
> changed as exemplified below.
>
> The usual output of a segfault is:
> Program received signal SIGSEGV, Segmentation fault
> 0x0000000000400d7c in upper (p=0x603010, a=0x603030, b=0x603050,
> c=0x603070, d=0x603090, len=7) at i386-mpx-sigsegv.c:68
> 68 value = *(p + len);
>
> In case it is a bound violation it will be presented as:
> Program received signal SIGSEGV, Segmentation fault
> Upper bound violation while accessing address 0x7fffffffc3b3
> Bounds: [lower = 0x7fffffffc390, upper = 0x7fffffffc3a3]
> 0x0000000000400d7c in upper (p=0x603010, a=0x603030, b=0x603050,
> c=0x603070, d=0x603090, len=7) at i386-mpx-sigsegv.c:68
> 68 value = *(p + len);
>
> In mi mode the output of a segfault is:
> *stopped,reason="signal-received",signal-name="SIGSEGV",
> signal-meaning="Segmentation fault", frame={addr="0x0000000000400d7c",
> func="upper",args=[{name="p", value="0x603010"},{name="a",value="0x603030"}
> ,{name="b",value="0x603050"}, {name="c",value="0x603070"},
> {name="d",value="0x603090"},{name="len",value="7"}],
> file="i386-mpx-sigsegv.c",fullname="i386-mpx-sigsegv.c",line="68"},
> thread-id="1",stopped-threads="all",core="6"
>
> in the case of a bound violation:
> *stopped,reason="signal-received",signal-name="SIGSEGV",
> signal-meaning="Segmentation fault",
> sigcode-meaning="Upper bound violation",
> lower-bound="0x603010",upper-bound="0x603023",bound-access="0x60302f",
> frame={addr="0x0000000000400d7c",func="upper",args=[{name="p",
> value="0x603010"},{name="a",value="0x603030"},{name="b",value="0x603050"},
> {name="c",value="0x603070"},{name="d",value="0x603090"},
> {name="len",value="7"}],file="i386-mpx-sigsegv.c",
> fullname="i386-mpx-sigsegv.c",line="68"},thread-id="1",
> stopped-threads="all",core="6"
>
> 2016-01-15 Walfred Tedeschi <walfred.tedeschi@intel.com>
>
> gdb/ChangeLog:
>
> * NEWS: Add entry for bound violation.
> * amd64-linux-tdep.c (amd64_linux_init_abi_common):
> Add handler for bound violation.
> * gdbarch.sh (bound_violation_handler): New.
> * gdbarch.c: Regenerate.
> * gdbarch.h: Regenerate.
> * i386-linux-tdep.c (i386_mpx_bound_violation_handler): New.
> (i386_linux_init_abi): Use i386_mpx_bound_violation_handler.
> * i386-linux-tdep.h (i386_mpx_bound_violation_handler) New.
> * i386-tdep.c (i386_mpx_enabled): Add as external.
> * i386-tdep.c (i386_mpx_enabled): Add as external.
> * infrun.c (handle_segmentation_faults): New function.
> (print_signal_received_reason): Use handle_segmentation_faults.
> (normal_stop): Change order of observer in order to have the
> inferior stopped for evaluation.
>
> gdb/testsuite/ChangeLog:
>
> * gdb.arch/i386-mpx-sigsegv.c: New file.
> * gdb.arch/i386-mpx-sigsegv.exp: New file.
> * gdb.arch/i386-mpx-simple_segv.c: New file.
> * gdb.arch/i386-mpx-simple_segv.exp: New file.
>
> gdb/doc/ChangeLog:
>
> * gdb.texinfo (Signals): Add bound violation display hints for
> a SIGSEGV.
>
> ---
> gdb/NEWS | 15 +++
> gdb/amd64-linux-tdep.c | 2 +
> gdb/doc/gdb.texinfo | 25 +++++
> gdb/gdbarch.c | 32 ++++++
> gdb/gdbarch.h | 11 +++
> gdb/gdbarch.sh | 6 ++
> gdb/i386-linux-tdep.c | 46 +++++++++
> gdb/i386-linux-tdep.h | 5 +
> gdb/i386-tdep.c | 4 +-
> gdb/i386-tdep.h | 2 +
> gdb/infrun.c | 34 +++++++
> gdb/testsuite/gdb.arch/i386-mpx-sigsegv.c | 120 +++++++++++++++++++++++
> gdb/testsuite/gdb.arch/i386-mpx-sigsegv.exp | 88 +++++++++++++++++
> gdb/testsuite/gdb.arch/i386-mpx-simple_segv.c | 66 +++++++++++++
> gdb/testsuite/gdb.arch/i386-mpx-simple_segv.exp | 125 ++++++++++++++++++++++++
> 15 files changed, 578 insertions(+), 3 deletions(-)
> create mode 100644 gdb/testsuite/gdb.arch/i386-mpx-sigsegv.c
> create mode 100644 gdb/testsuite/gdb.arch/i386-mpx-sigsegv.exp
> create mode 100644 gdb/testsuite/gdb.arch/i386-mpx-simple_segv.c
> create mode 100644 gdb/testsuite/gdb.arch/i386-mpx-simple_segv.exp
>
> diff --git a/gdb/NEWS b/gdb/NEWS
> index d9cbb80..b8512d6 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -3,6 +3,21 @@
>
> *** Changes since GDB 7.10
>
> +* Intel MPX boud violation handler.
Typo, "bound".
I think you may have meant s/handler/handling/.
> +
> + A boundary violations is presented to the inferior as
> + a segmentation fault having SIGCODE 3. In this case
> + GDB displays also the kind of violation (upper or lower),
> + bounds, poiter value and the memory accessed, besides displaying
> + the usual signal received and code location report.
I think that talking about sigcode is a bit of implementer-speak.
Also, a few typos and missing double-spaces.
I don't understand what you mean by "pointer value and the
memory accessed", specifically, what pointer value is that.
> + As exemplified below:
> + Program received signal SIGSEGV, Segmentation fault
> + Upper bound violation while accessing address 0x7fffffffc3b3
> + Bounds: [lower = 0x7fffffffc390, upper = 0x7fffffffc3a3].
> + 0x0000000000400d7c in upper (p=0x603010, a=0x603030, b=0x603050,
> + c=0x603070, d=0x603090, len=7) at i386-mpx-sigsegv.c:68
The arguments in "upper()" don't add any info here, and are
causing a line wrap. so I think you should remove them.
The period at the end of:
Bounds: [lower = 0x7fffffffc390, upper = 0x7fffffffc3a3].
looks odd. That's not a sentence, and the real sentence above
didn't get a period. I think you should remove it, saving
some screen estate, and update all examples that might show it,
here and docs.
Thus, I suggest:
* Intel MPX boundary violations
Segmentation faults caused by Intel MPX boundary violations
now display the kind of violation (upper or lower), the memory
address accessed and the memory bounds, along with the usual
signal received and code location.
For example:
Program received signal SIGSEGV, Segmentation fault
Upper bound violation while accessing address 0x7fffffffc3b3
Bounds: [lower = 0x7fffffffc390, upper = 0x7fffffffc3a3]
0x0000000000400d7c in upper () at i386-mpx-sigsegv.c:68
> +
> * Per-inferior thread numbers
>
> Thread numbers are now per inferior instead of global. If you're
> diff --git a/gdb/amd64-linux-tdep.c b/gdb/amd64-linux-tdep.c
> index 21bcd99..8af15c4 100644
> --- a/gdb/amd64-linux-tdep.c
> +++ b/gdb/amd64-linux-tdep.c
> @@ -1840,6 +1840,8 @@ amd64_linux_init_abi_common(struct gdbarch_info info, struct gdbarch *gdbarch)
> set_gdbarch_process_record_signal (gdbarch, amd64_linux_record_signal);
>
> set_gdbarch_get_siginfo_type (gdbarch, x86_linux_get_siginfo_type);
> + set_gdbarch_bound_violation_handler(gdbarch,
> + i386_mpx_bound_violation_handler);
Space before parens.
> }
>
> static void
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index 7da31c8..f4373ed 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -5819,6 +5819,30 @@ $1 = (void *) 0x7ffff7ff7000
>
> Depending on target support, @code{$_siginfo} may also be writable.
>
> +Also on some targets a @code{SIGSEGV} can be related to
> +boundary violations, i.e. accessing an address outside of
> +allowed range. In those cases @value{GDBN} might displays additional
> +information. In @code{STOP} mode @value{GDBN} displays the violation
> +kind: "Upper" or "Lower", address accessed and bounds. In @code{NOSTOP}
> +no additional information is displayed.
I suggest adding a couple cindex's, and the following edit:
@cindex Intel MPX boundary violations
@cindex boundary violations, Intel MPX
On some targets, a @code{SIGSEGV} can be caused by a boundary
violation, i.e., accessing an address outside of the allowed range.
In those cases @value{GDBN} may displays additional information, depending
on how @value{GDBN} has been told to handle the signal. In @code{handle stop}
mode @value{GDBN} displays the violation kind: "Upper" or "Lower", the
memory address accessed and the bounds. In @code{handle nostop} no
additional information is displayed.
> +
> +The usual output of a segfault is:
> +@smallexample
> +Program received signal SIGSEGV, Segmentation fault
> +0x0000000000400d7c in upper (p=0x603010, a=0x603030, b=0x603050,
> +c=0x603070, d=0x603090, len=7) at i386-mpx-sigsegv.c:68
> +68 value = *(p + len);
> +@end smallexample
Remove arguments here too:
The usual output of a segfault is:
@smallexample
Program received signal SIGSEGV, Segmentation fault
0x0000000000400d7c in upper () at i386-mpx-sigsegv.c:68
68 value = *(p + len);
@end smallexample
> +
> +In case it is a bound violation it will be presented as:
"While a bound violation is presented as:"
Note: s/will/is/
> +@smallexample
> +Upper bound violation while accessing address 0x7fffffffc3b3
> +Bounds: [lower = 0x7fffffffc390, upper = 0x7fffffffc3a3]
> +0x0000000000400d7c in upper (p=0x603010, a=0x603030, b=0x603050,
> +c=0x603070, d=0x603090, len=7) at i386-mpx-sigsegv.c:68
> +68 value = *(p + len);
> +@end smallexample
"Program received ..." is missing. Drop arguments:
@smallexample
Program received signal SIGSEGV, Segmentation fault
Upper bound violation while accessing address 0x7fffffffc3b3
Bounds: [lower = 0x7fffffffc390, upper = 0x7fffffffc3a3]
0x0000000000400d7c in upper () at i386-mpx-sigsegv.c:68
68 value = *(p + len);
@end smallexample
> +
> @node Thread Stops
> @section Stopping and Starting Multi-thread Programs
>
> @@ -22267,6 +22291,7 @@ whose bounds are to be changed, @var{lbound} and @var{ubound} are new values
> for lower and upper bounds respectively.
> @end table
>
> +
> @node Alpha
Spurious empty lines?
> @subsection Alpha
>
> diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
> index d45af1a..f7fef25 100644
> --- a/gdb/gdbarch.c
> +++ b/gdb/gdbarch.c
> @@ -189,6 +189,7 @@ struct gdbarch
> int num_pseudo_regs;
> gdbarch_ax_pseudo_register_collect_ftype *ax_pseudo_register_collect;
> gdbarch_ax_pseudo_register_push_stack_ftype *ax_pseudo_register_push_stack;
> + gdbarch_bound_violation_handler_ftype *bound_violation_handler;
> int sp_regnum;
> int pc_regnum;
> int ps_regnum;
> @@ -531,6 +532,7 @@ verify_gdbarch (struct gdbarch *gdbarch)
> /* Skip verify of num_pseudo_regs, invalid_p == 0 */
> /* Skip verify of ax_pseudo_register_collect, has predicate. */
> /* Skip verify of ax_pseudo_register_push_stack, has predicate. */
> + /* Skip verify of bound_violation_handler, has predicate. */
> /* Skip verify of sp_regnum, invalid_p == 0 */
> /* Skip verify of pc_regnum, invalid_p == 0 */
> /* Skip verify of ps_regnum, invalid_p == 0 */
> @@ -773,6 +775,12 @@ gdbarch_dump (struct gdbarch *gdbarch, struct ui_file *file)
> "gdbarch_dump: bits_big_endian = %s\n",
> plongest (gdbarch->bits_big_endian));
> fprintf_unfiltered (file,
> + "gdbarch_dump: gdbarch_bound_violation_handler_p() = %d\n",
> + gdbarch_bound_violation_handler_p (gdbarch));
> + fprintf_unfiltered (file,
> + "gdbarch_dump: bound_violation_handler = <%s>\n",
> + host_address_to_string (gdbarch->bound_violation_handler));
> + fprintf_unfiltered (file,
> "gdbarch_dump: breakpoint_from_pc = <%s>\n",
> host_address_to_string (gdbarch->breakpoint_from_pc));
> fprintf_unfiltered (file,
> @@ -1986,6 +1994,30 @@ set_gdbarch_ax_pseudo_register_push_stack (struct gdbarch *gdbarch,
> }
>
> int
> +gdbarch_bound_violation_handler_p (struct gdbarch *gdbarch)
> +{
> + gdb_assert (gdbarch != NULL);
> + return gdbarch->bound_violation_handler != NULL;
> +}
> +
> +void
> +gdbarch_bound_violation_handler (struct gdbarch *gdbarch, struct ui_out *uiout)
> +{
> + gdb_assert (gdbarch != NULL);
> + gdb_assert (gdbarch->bound_violation_handler != NULL);
> + if (gdbarch_debug >= 2)
> + fprintf_unfiltered (gdb_stdlog, "gdbarch_bound_violation_handler called\n");
> + gdbarch->bound_violation_handler (gdbarch, uiout);
> +}
> +
> +void
> +set_gdbarch_bound_violation_handler (struct gdbarch *gdbarch,
> + gdbarch_bound_violation_handler_ftype bound_violation_handler)
> +{
> + gdbarch->bound_violation_handler = bound_violation_handler;
> +}
> +
> +int
> gdbarch_sp_regnum (struct gdbarch *gdbarch)
> {
> gdb_assert (gdbarch != NULL);
> diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
> index 3c16af2..eb5de0d 100644
> --- a/gdb/gdbarch.h
> +++ b/gdb/gdbarch.h
> @@ -63,6 +63,7 @@ struct ravenscar_arch_ops;
> struct elf_internal_linux_prpsinfo;
> struct mem_range;
> struct syscalls_info;
> +struct ui_out;
>
> #include "regcache.h"
>
> @@ -299,6 +300,16 @@ typedef int (gdbarch_ax_pseudo_register_push_stack_ftype) (struct gdbarch *gdbar
> extern int gdbarch_ax_pseudo_register_push_stack (struct gdbarch *gdbarch, struct agent_expr *ax, int reg);
> extern void set_gdbarch_ax_pseudo_register_push_stack (struct gdbarch *gdbarch, gdbarch_ax_pseudo_register_push_stack_ftype *ax_pseudo_register_push_stack);
>
> +/* Function called when a segmentation fault signal is received by the inferior,
> + having SIGCODE 3 (SIG_CODE_BOUNDARY_FAULT).
> + UIOUT is the output stream where the handler will place information. */
> +
> +extern int gdbarch_bound_violation_handler_p (struct gdbarch *gdbarch);
> +
> +typedef void (gdbarch_bound_violation_handler_ftype) (struct gdbarch *gdbarch, struct ui_out *uiout);
> +extern void gdbarch_bound_violation_handler (struct gdbarch *gdbarch, struct ui_out *uiout);
> +extern void set_gdbarch_bound_violation_handler (struct gdbarch *gdbarch, gdbarch_bound_violation_handler_ftype *bound_violation_handler);
> +
> /* GDB's standard (or well known) register numbers. These can map onto
> a real register or a pseudo (computed) register or not be defined at
> all (-1).
> diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
> index f80cd51..edd155a 100755
> --- a/gdb/gdbarch.sh
> +++ b/gdb/gdbarch.sh
> @@ -446,6 +446,11 @@ M:int:ax_pseudo_register_collect:struct agent_expr *ax, int reg:ax, reg
> # Return -1 if something goes wrong, 0 otherwise.
> M:int:ax_pseudo_register_push_stack:struct agent_expr *ax, int reg:ax, reg
>
> +# Function called when a segmentation fault signal is received by the inferior,
> +# having SIGCODE 3 (SIG_CODE_BOUNDARY_FAULT).
# Function called when a segmentation fault with
# SIGCODE 3 (SIG_CODE_BOUNDARY_FAULT) is received by the inferior.
But, see below.
> +# UIOUT is the output stream where the handler will place information.
> +M:void:bound_violation_handler:struct ui_out *uiout:uiout
> +
> # GDB's standard (or well known) register numbers. These can map onto
> # a real register or a pseudo (computed) register or not be defined at
> # all (-1).
> @@ -1247,6 +1252,7 @@ struct ravenscar_arch_ops;
> struct elf_internal_linux_prpsinfo;
> struct mem_range;
> struct syscalls_info;
> +struct ui_out;
>
> #include "regcache.h"
>
> diff --git a/gdb/i386-linux-tdep.c b/gdb/i386-linux-tdep.c
> index af39e78..45212e0 100644
> --- a/gdb/i386-linux-tdep.c
> +++ b/gdb/i386-linux-tdep.c
> @@ -30,6 +30,7 @@
> #include "i386-tdep.h"
> #include "i386-linux-tdep.h"
> #include "linux-tdep.h"
> +#include "utils.h"
> #include "glibc-tdep.h"
> #include "solib-svr4.h"
> #include "symtab.h"
> @@ -384,6 +385,49 @@ i386_canonicalize_syscall (int syscall)
> return gdb_sys_no_syscall;
> }
>
> +void
> +i386_mpx_bound_violation_handler (struct gdbarch *gdbarch, struct ui_out *uiout)
Missing intro comment. Something like "Implement ... hook." should be enough.
Look for preexisting examples.
> +{
> + CORE_ADDR lower_bound, upper_bound, access;
> + int is_upper;
> +
> + if (!i386_mpx_enabled ())
> + return;
> + TRY
> + {
> + lower_bound
> + = parse_and_eval_long ("$_siginfo._sifields._sigfault._addr_bnd._lower");
> + upper_bound
> + = parse_and_eval_long ("$_siginfo._sifields._sigfault._addr_bnd._upper");
> + access
> + = parse_and_eval_long ("$_siginfo._sifields._sigfault.si_addr");
> + }
> + CATCH (exception, RETURN_MASK_ALL)
> + {
> + return;
> + }
> + END_CATCH
> +
> + is_upper = (access > upper_bound ? 1 : 0);
> +
> + ui_out_text (uiout, "\n");
> + if (is_upper)
> + ui_out_field_string (uiout, "sigcode-meaning", "Upper bound violation");
> + else
> + ui_out_field_string (uiout, "sigcode-meaning", "Lower bound violation");
> +
> + ui_out_text (uiout, " while accessing address ");
> + ui_out_field_fmt (uiout,"bound-access", "%s", paddress (gdbarch, access));
> + ui_out_text (uiout, "\nBounds: [lower = ");
> + ui_out_field_fmt (uiout,"lower-bound", "%s", paddress (gdbarch, lower_bound));
> + ui_out_text (uiout, ", upper = ");
> + ui_out_field_fmt (uiout,"upper-bound", "%s", paddress (gdbarch, upper_bound));
> + ui_out_text (uiout, "]");
Missing empty space after a few "uiout," above. Watch out for line-too-long after
fixing that.
> +
> +
> + return;
Remove these two empty lines and the unnecessary "return".
> +}
> +
> /* Parse the arguments of current system call instruction and record
> the values of the registers and memory that will be changed into
> "record_arch_list". This instruction is "int 0x80" (Linux
> @@ -1002,6 +1046,8 @@ i386_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
> i386_linux_get_syscall_number);
>
> set_gdbarch_get_siginfo_type (gdbarch, x86_linux_get_siginfo_type);
> + set_gdbarch_bound_violation_handler(gdbarch,
> + i386_mpx_bound_violation_handler);
> }
>
> /* Provide a prototype to silence -Wmissing-prototypes. */
> diff --git a/gdb/i386-linux-tdep.h b/gdb/i386-linux-tdep.h
> index b32f6d1..6ffacd4 100644
> --- a/gdb/i386-linux-tdep.h
> +++ b/gdb/i386-linux-tdep.h
> @@ -37,6 +37,11 @@
> /* Get XSAVE extended state xcr0 from core dump. */
> extern uint64_t i386_linux_core_read_xcr0 (bfd *abfd);
>
> +/* Handles and displays information related to the MPX bound violation
> + to the user. */
> +void
> +i386_mpx_bound_violation_handler (struct gdbarch *gdbarch, struct ui_out *uiout);
Only function definitions should have the function name at collumn 0. All other
declarations have explicit "extern". Thus, write:
extern void i386_mpx_bound_violation_handler (struct gdbarch *gdbarch,
struct ui_out *uiout);
> +
> /* Linux target description. */
> extern struct target_desc *tdesc_i386_linux;
> extern struct target_desc *tdesc_i386_mmx_linux;
> diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
> index b706463..b5d0d14 100644
> --- a/gdb/i386-tdep.c
> +++ b/gdb/i386-tdep.c
> @@ -8651,9 +8651,7 @@ i386_mpx_bd_base (void)
> return ret & MPX_BASE_MASK;
> }
>
> -/* Check if the current target is MPX enabled. */
> -
> -static int
> +int
> i386_mpx_enabled (void)
> {
> const struct gdbarch_tdep *tdep = gdbarch_tdep (get_current_arch ());
> diff --git a/gdb/i386-tdep.h b/gdb/i386-tdep.h
> index 10d2772..26933f2 100644
> --- a/gdb/i386-tdep.h
> +++ b/gdb/i386-tdep.h
> @@ -420,6 +420,8 @@ extern int i386_process_record (struct gdbarch *gdbarch,
> struct regcache *regcache, CORE_ADDR addr);
> extern const struct target_desc *i386_target_description (uint64_t xcr0);
>
> +/* Verify if target is MPX enabled. */
Lost "current"? If changing the comment, I'd suggest:
/* Return true iff the current target is MPX enabled. */
> +extern int i386_mpx_enabled (void);
> \f
>
> /* Functions and variables exported from i386bsd-tdep.c. */
> diff --git a/gdb/infrun.c b/gdb/infrun.c
> index 64c729e..87b930e 100644
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -7893,6 +7893,36 @@ print_exited_reason (struct ui_out *uiout, int exitstatus)
> }
> }
>
> +/* Value of the sigcode in case of a boundary fault. */
Spurious double-space before "Value".
> +
> +#define SIG_CODE_BONDARY_FAULT 3
> +
> +/* Verifies if a received segmentation fault is a boundary fault.
> + In the case it is it calls the architecture dependent function
> + to handle the boundary fault. */
> +
> +static void
> +handle_segmentation_faults (struct ui_out *uiout)
"handle_segmentation_fault", singular.
> +{
> + long sig_code = 0;
> + struct regcache *regcache = get_current_regcache ();
> + struct gdbarch *gdbarch = get_regcache_arch (regcache);
> +
> + TRY
> + {
> + sig_code = parse_and_eval_long ("$_siginfo.si_code\n");
It's useless to do this on architectures that don't install
the gdbarch hook. I think this should be pushed down to the
gdbarch hook. You'll end up with a single try/catch.
And then the gdbarch hook can be renamed to a more generic
gdbarch_handle_segmentation_fault. The comments thoughout should be
updated then, like, for this function:
/* Some targets/architectures can do extra processing/display of
segmentation faults. E.g., Intel MPX boundary faults.
Call the architecture dependent function to handle the fault. */
static void
handle_segmentation_fault (struct ui_out *uiout)
{
> + }
> + CATCH (exception, RETURN_MASK_ALL)
> + {
> + return;
> + }
> + END_CATCH
> +
> + if (sig_code == SIG_CODE_BONDARY_FAULT
> + && gdbarch_bound_violation_handler_p (gdbarch))
> + gdbarch_bound_violation_handler (gdbarch, uiout);
> +}
> +
> void
> print_signal_received_reason (struct ui_out *uiout, enum gdb_signal siggnal)
> {
> @@ -7922,6 +7952,10 @@ print_signal_received_reason (struct ui_out *uiout, enum gdb_signal siggnal)
> annotate_signal_string ();
> ui_out_field_string (uiout, "signal-meaning",
> gdb_signal_to_string (siggnal));
> +
> + if (siggnal == GDB_SIGNAL_SEGV)
> + handle_segmentation_faults (uiout);
> +
> annotate_signal_string_end ();
> }
> ui_out_text (uiout, ".\n");
> diff --git a/gdb/testsuite/gdb.arch/i386-mpx-sigsegv.c b/gdb/testsuite/gdb.arch/i386-mpx-sigsegv.c
> new file mode 100644
> index 0000000..7500352
> --- /dev/null
> +++ b/gdb/testsuite/gdb.arch/i386-mpx-sigsegv.c
> @@ -0,0 +1,120 @@
> +/* Copyright (C) 2015 Free Software Foundation, Inc.
2015-2016
> +++ b/gdb/testsuite/gdb.arch/i386-mpx-sigsegv.exp
> @@ -0,0 +1,88 @@
> +# Copyright (C) 2015 Free Software Foundation, Inc.
Ditto.
> +gdb_test_multiple "print have_mpx ()" "have mpx" {
> + -re ".. = 1\r\n$gdb_prompt " {
> + pass "check whether processor supports MPX"
set message "check whether processor supports MPX"
gdb_test_multiple "print have_mpx ()" $message {
-re ".. = 1\r\n$gdb_prompt " {
pass $message
> + }
Alignment of } looks odd, here and throughout the file.
> +set common_ubound_violation ".*Program received signal SIGSEGV,\
> + Segmentation fault\r\nUpper bound violation while accessing\
> + address $hex\r\nBounds: \\\[lower = $hex, upper = $hex\\\]"
> +
> +set common_lbound_violation ".*Program received signal SIGSEGV,\
> + Segmentation fault\r\nLower bound violation while accessing\
> + address $hex\r\nBounds: \\\[lower = $hex, upper = $hex\\\]"
The initial ".*" is useless.
Use [multi_line ...].
> +
> +set segv_upper_bound "$common_ubound_violation.*$gdb_prompt $"
> +
> +set segv_lower_bound "$common_lbound_violation.*$gdb_prompt $"
> +
> +for {set i 0} {$i < 15} {incr i} {
> + set message "MPX signal segv Upper: ${i}"
> + gdb_test_multiple "continue" "$message ${i}" {
> + -re $segv_upper_bound {
> + pass "$message"
> + }
> + -re ".*$inferior_exited_re normally.*$gdb_prompt $" {
> + fail "$message"
The pass/fail calls are missing ${i}. Please make sure test
messages are unique in gdb.sum:
https://sourceware.org/gdb/wiki/GDBTestcaseCookbook#Make_sure_test_messages_are_unique
> + break
> + }
> + }
> + gdb_test "where" ".*#0 0x\[0-9a-fA-F\]+ in upper.*"\
> + "$message: should be in upper"
Ditto.
Useless ".*".
Use $hex.
> +}
> +
> +for {set i 0} {$i < 15} {incr i} {
> + set message "MPX signal segv Lower: ${i}"
> + gdb_test_multiple "continue" "$message ${i}" {
> + -re $segv_lower_bound {
> + pass "$message ${i}"
> + }
> + -re ".*$inferior_exited_re normally.*$gdb_prompt $" {
> + fail "$message ${i}"
> + break
> + }
> + }
> + gdb_test "where" ".*#0 0x\[0-9a-fA-F\]+ in lower.*"\
> + "$message: should be in lower"
> +}
Likewise.
> diff --git a/gdb/testsuite/gdb.arch/i386-mpx-simple_segv.c b/gdb/testsuite/gdb.arch/i386-mpx-simple_segv.c
> new file mode 100644
> index 0000000..5317369
> --- /dev/null
> +++ b/gdb/testsuite/gdb.arch/i386-mpx-simple_segv.c
> @@ -0,0 +1,66 @@
> +/* Copyright (C) 2015 Free Software Foundation, Inc.
2015-2016
> +
> diff --git a/gdb/testsuite/gdb.arch/i386-mpx-simple_segv.exp b/gdb/testsuite/gdb.arch/i386-mpx-simple_segv.exp
Largely the same comments apply to this file. I'll only mention other
extra things I spotted below.
> +# Using the handler for SIGSEGV as "print pass stop"
> +set parameters "print pass stop"
> +runto_main
Check result. Wrap with_test_prefix instead of appending
$parameters, 0, 1, etc. in some test messages. That way,
any fail within runto_main, etc. is covered as well, as well
as being easier.
> +send_gdb "handle SIGSEGV $parameters\n"
> +send_gdb "continue\n"
> +
> +gdb_expect {
> + -re $segv_bound_with_prompt {
> + pass $parameters
> + }
> +}
Use gdb_test/gdb_test_multiple instead of send_gdb/gdb_expect.
> +gdb_test "handle SIGSEGV $parameters" "" "Setting\
> +the handler for segfault 0"
Odd line break. Break it before the string, not in the middle.
> +
> +gdb_test_multiple "continue" "test 0" {
> + -re $segv_with_exit {
> + pass $parameters
> + }
> + -re "$gdb_prompt $" {
> + fail $parameters
> + }
> +}
> +
> +gdb_test "where" "No stack." "no inferior $parameters"
> +
> +# Using the handler for SIGSEGV as "print nopass stop"
> +set parameters "print nopass stop"
> +
> +runto_main
> +gdb_test "handle SIGSEGV $parameters" "" "Setting\
> +the handler for segfault 1"
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH V5 5/5] ntel MPX bound violation handling
2016-01-28 13:43 ` Pedro Alves
@ 2016-01-28 13:49 ` Walfred Tedeschi
2016-01-28 14:18 ` Pedro Alves
2016-02-02 10:09 ` Walfred Tedeschi
1 sibling, 1 reply; 18+ messages in thread
From: Walfred Tedeschi @ 2016-01-28 13:49 UTC (permalink / raw)
To: Pedro Alves, eliz, brobecker; +Cc: gdb-patches
Am 1/28/2016 um 2:42 PM schrieb Pedro Alves:
> "Intel" in $subject.
>
> On 01/22/2016 02:54 PM, Walfred Tedeschi wrote:
>> With Intel Memory Protection Extensions it was introduced the concept of
>> boundary violation. A boundary violations is presented to the inferior as
>> a segmentation fault having SIGCODE 3. This patch adds a
>> handler for a boundary violation extending the information displayed
>> when a bound violation is presented to the inferior. In the stop mode
>> case the debugger will also display the kind of violation: "upper" or
>> "lower", bounds and the address accessed.
>> On no stop mode the information will still remain unchanged. Additional
>> information about bound violations are not meaningful in that case user
>> does not know the line in which violation occurred as well.
>>
>> When the segmentation fault handler is stop mode the out puts will be
>> changed as exemplified below.
>>
>> The usual output of a segfault is:
>> Program received signal SIGSEGV, Segmentation fault
>> 0x0000000000400d7c in upper (p=0x603010, a=0x603030, b=0x603050,
>> c=0x603070, d=0x603090, len=7) at i386-mpx-sigsegv.c:68
>> 68 value = *(p + len);
>>
>> In case it is a bound violation it will be presented as:
>> Program received signal SIGSEGV, Segmentation fault
>> Upper bound violation while accessing address 0x7fffffffc3b3
>> Bounds: [lower = 0x7fffffffc390, upper = 0x7fffffffc3a3]
>> 0x0000000000400d7c in upper (p=0x603010, a=0x603030, b=0x603050,
>> c=0x603070, d=0x603090, len=7) at i386-mpx-sigsegv.c:68
>> 68 value = *(p + len);
>>
>> In mi mode the output of a segfault is:
>> *stopped,reason="signal-received",signal-name="SIGSEGV",
>> signal-meaning="Segmentation fault", frame={addr="0x0000000000400d7c",
>> func="upper",args=[{name="p", value="0x603010"},{name="a",value="0x603030"}
>> ,{name="b",value="0x603050"}, {name="c",value="0x603070"},
>> {name="d",value="0x603090"},{name="len",value="7"}],
>> file="i386-mpx-sigsegv.c",fullname="i386-mpx-sigsegv.c",line="68"},
>> thread-id="1",stopped-threads="all",core="6"
>>
>> in the case of a bound violation:
>> *stopped,reason="signal-received",signal-name="SIGSEGV",
>> signal-meaning="Segmentation fault",
>> sigcode-meaning="Upper bound violation",
>> lower-bound="0x603010",upper-bound="0x603023",bound-access="0x60302f",
>> frame={addr="0x0000000000400d7c",func="upper",args=[{name="p",
>> value="0x603010"},{name="a",value="0x603030"},{name="b",value="0x603050"},
>> {name="c",value="0x603070"},{name="d",value="0x603090"},
>> {name="len",value="7"}],file="i386-mpx-sigsegv.c",
>> fullname="i386-mpx-sigsegv.c",line="68"},thread-id="1",
>> stopped-threads="all",core="6"
>>
>> 2016-01-15 Walfred Tedeschi <walfred.tedeschi@intel.com>
>>
>> gdb/ChangeLog:
>>
>> * NEWS: Add entry for bound violation.
>> * amd64-linux-tdep.c (amd64_linux_init_abi_common):
>> Add handler for bound violation.
>> * gdbarch.sh (bound_violation_handler): New.
>> * gdbarch.c: Regenerate.
>> * gdbarch.h: Regenerate.
>> * i386-linux-tdep.c (i386_mpx_bound_violation_handler): New.
>> (i386_linux_init_abi): Use i386_mpx_bound_violation_handler.
>> * i386-linux-tdep.h (i386_mpx_bound_violation_handler) New.
>> * i386-tdep.c (i386_mpx_enabled): Add as external.
>> * i386-tdep.c (i386_mpx_enabled): Add as external.
>> * infrun.c (handle_segmentation_faults): New function.
>> (print_signal_received_reason): Use handle_segmentation_faults.
>> (normal_stop): Change order of observer in order to have the
>> inferior stopped for evaluation.
>>
>> gdb/testsuite/ChangeLog:
>>
>> * gdb.arch/i386-mpx-sigsegv.c: New file.
>> * gdb.arch/i386-mpx-sigsegv.exp: New file.
>> * gdb.arch/i386-mpx-simple_segv.c: New file.
>> * gdb.arch/i386-mpx-simple_segv.exp: New file.
>>
>> gdb/doc/ChangeLog:
>>
>> * gdb.texinfo (Signals): Add bound violation display hints for
>> a SIGSEGV.
>>
>> ---
>> gdb/NEWS | 15 +++
>> gdb/amd64-linux-tdep.c | 2 +
>> gdb/doc/gdb.texinfo | 25 +++++
>> gdb/gdbarch.c | 32 ++++++
>> gdb/gdbarch.h | 11 +++
>> gdb/gdbarch.sh | 6 ++
>> gdb/i386-linux-tdep.c | 46 +++++++++
>> gdb/i386-linux-tdep.h | 5 +
>> gdb/i386-tdep.c | 4 +-
>> gdb/i386-tdep.h | 2 +
>> gdb/infrun.c | 34 +++++++
>> gdb/testsuite/gdb.arch/i386-mpx-sigsegv.c | 120 +++++++++++++++++++++++
>> gdb/testsuite/gdb.arch/i386-mpx-sigsegv.exp | 88 +++++++++++++++++
>> gdb/testsuite/gdb.arch/i386-mpx-simple_segv.c | 66 +++++++++++++
>> gdb/testsuite/gdb.arch/i386-mpx-simple_segv.exp | 125 ++++++++++++++++++++++++
>> 15 files changed, 578 insertions(+), 3 deletions(-)
>> create mode 100644 gdb/testsuite/gdb.arch/i386-mpx-sigsegv.c
>> create mode 100644 gdb/testsuite/gdb.arch/i386-mpx-sigsegv.exp
>> create mode 100644 gdb/testsuite/gdb.arch/i386-mpx-simple_segv.c
>> create mode 100644 gdb/testsuite/gdb.arch/i386-mpx-simple_segv.exp
>>
>> diff --git a/gdb/NEWS b/gdb/NEWS
>> index d9cbb80..b8512d6 100644
>> --- a/gdb/NEWS
>> +++ b/gdb/NEWS
>> @@ -3,6 +3,21 @@
>>
>> *** Changes since GDB 7.10
>>
>> +* Intel MPX boud violation handler.
>
> Typo, "bound".
>
> I think you may have meant s/handler/handling/.
>
>> +
>> + A boundary violations is presented to the inferior as
>> + a segmentation fault having SIGCODE 3. In this case
>> + GDB displays also the kind of violation (upper or lower),
>> + bounds, poiter value and the memory accessed, besides displaying
>> + the usual signal received and code location report.
>
> I think that talking about sigcode is a bit of implementer-speak.
> Also, a few typos and missing double-spaces.
>
> I don't understand what you mean by "pointer value and the
> memory accessed", specifically, what pointer value is that.
>
>> + As exemplified below:
>> + Program received signal SIGSEGV, Segmentation fault
>> + Upper bound violation while accessing address 0x7fffffffc3b3
>> + Bounds: [lower = 0x7fffffffc390, upper = 0x7fffffffc3a3].
>> + 0x0000000000400d7c in upper (p=0x603010, a=0x603030, b=0x603050,
>> + c=0x603070, d=0x603090, len=7) at i386-mpx-sigsegv.c:68
>
> The arguments in "upper()" don't add any info here, and are
> causing a line wrap. so I think you should remove them.
>
> The period at the end of:
>
> Bounds: [lower = 0x7fffffffc390, upper = 0x7fffffffc3a3].
>
> looks odd. That's not a sentence, and the real sentence above
> didn't get a period. I think you should remove it, saving
> some screen estate, and update all examples that might show it,
> here and docs.
>
> Thus, I suggest:
>
> * Intel MPX boundary violations
>
> Segmentation faults caused by Intel MPX boundary violations
> now display the kind of violation (upper or lower), the memory
> address accessed and the memory bounds, along with the usual
> signal received and code location.
>
> For example:
>
> Program received signal SIGSEGV, Segmentation fault
> Upper bound violation while accessing address 0x7fffffffc3b3
> Bounds: [lower = 0x7fffffffc390, upper = 0x7fffffffc3a3]
> 0x0000000000400d7c in upper () at i386-mpx-sigsegv.c:68
>
>> +
>> * Per-inferior thread numbers
>>
>> Thread numbers are now per inferior instead of global. If you're
>> diff --git a/gdb/amd64-linux-tdep.c b/gdb/amd64-linux-tdep.c
>> index 21bcd99..8af15c4 100644
>> --- a/gdb/amd64-linux-tdep.c
>> +++ b/gdb/amd64-linux-tdep.c
>> @@ -1840,6 +1840,8 @@ amd64_linux_init_abi_common(struct gdbarch_info info, struct gdbarch *gdbarch)
>> set_gdbarch_process_record_signal (gdbarch, amd64_linux_record_signal);
>>
>> set_gdbarch_get_siginfo_type (gdbarch, x86_linux_get_siginfo_type);
>> + set_gdbarch_bound_violation_handler(gdbarch,
>> + i386_mpx_bound_violation_handler);
>
> Space before parens.
>
>> }
>>
>> static void
>> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
>> index 7da31c8..f4373ed 100644
>> --- a/gdb/doc/gdb.texinfo
>> +++ b/gdb/doc/gdb.texinfo
>> @@ -5819,6 +5819,30 @@ $1 = (void *) 0x7ffff7ff7000
>>
>> Depending on target support, @code{$_siginfo} may also be writable.
>>
>> +Also on some targets a @code{SIGSEGV} can be related to
>> +boundary violations, i.e. accessing an address outside of
>> +allowed range. In those cases @value{GDBN} might displays additional
>> +information. In @code{STOP} mode @value{GDBN} displays the violation
>> +kind: "Upper" or "Lower", address accessed and bounds. In @code{NOSTOP}
>> +no additional information is displayed.
>
> I suggest adding a couple cindex's, and the following edit:
>
> @cindex Intel MPX boundary violations
> @cindex boundary violations, Intel MPX
> On some targets, a @code{SIGSEGV} can be caused by a boundary
> violation, i.e., accessing an address outside of the allowed range.
> In those cases @value{GDBN} may displays additional information, depending
> on how @value{GDBN} has been told to handle the signal. In @code{handle stop}
> mode @value{GDBN} displays the violation kind: "Upper" or "Lower", the
> memory address accessed and the bounds. In @code{handle nostop} no
> additional information is displayed.
>
>> +
>> +The usual output of a segfault is:
>> +@smallexample
>> +Program received signal SIGSEGV, Segmentation fault
>> +0x0000000000400d7c in upper (p=0x603010, a=0x603030, b=0x603050,
>> +c=0x603070, d=0x603090, len=7) at i386-mpx-sigsegv.c:68
>> +68 value = *(p + len);
>> +@end smallexample
>
> Remove arguments here too:
>
> The usual output of a segfault is:
>
> @smallexample
> Program received signal SIGSEGV, Segmentation fault
> 0x0000000000400d7c in upper () at i386-mpx-sigsegv.c:68
> 68 value = *(p + len);
> @end smallexample
>
>
>> +
>> +In case it is a bound violation it will be presented as:
>
> "While a bound violation is presented as:"
>
> Note: s/will/is/
>
>> +@smallexample
>> +Upper bound violation while accessing address 0x7fffffffc3b3
>> +Bounds: [lower = 0x7fffffffc390, upper = 0x7fffffffc3a3]
>> +0x0000000000400d7c in upper (p=0x603010, a=0x603030, b=0x603050,
>> +c=0x603070, d=0x603090, len=7) at i386-mpx-sigsegv.c:68
>> +68 value = *(p + len);
>> +@end smallexample
>
> "Program received ..." is missing. Drop arguments:
>
> @smallexample
> Program received signal SIGSEGV, Segmentation fault
> Upper bound violation while accessing address 0x7fffffffc3b3
> Bounds: [lower = 0x7fffffffc390, upper = 0x7fffffffc3a3]
> 0x0000000000400d7c in upper () at i386-mpx-sigsegv.c:68
> 68 value = *(p + len);
> @end smallexample
>
>> +
>> @node Thread Stops
>> @section Stopping and Starting Multi-thread Programs
>>
>> @@ -22267,6 +22291,7 @@ whose bounds are to be changed, @var{lbound} and @var{ubound} are new values
>> for lower and upper bounds respectively.
>> @end table
>>
>> +
>> @node Alpha
>
> Spurious empty lines?
>
>> @subsection Alpha
>>
>> diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
>> index d45af1a..f7fef25 100644
>> --- a/gdb/gdbarch.c
>> +++ b/gdb/gdbarch.c
>> @@ -189,6 +189,7 @@ struct gdbarch
>> int num_pseudo_regs;
>> gdbarch_ax_pseudo_register_collect_ftype *ax_pseudo_register_collect;
>> gdbarch_ax_pseudo_register_push_stack_ftype *ax_pseudo_register_push_stack;
>> + gdbarch_bound_violation_handler_ftype *bound_violation_handler;
>> int sp_regnum;
>> int pc_regnum;
>> int ps_regnum;
>> @@ -531,6 +532,7 @@ verify_gdbarch (struct gdbarch *gdbarch)
>> /* Skip verify of num_pseudo_regs, invalid_p == 0 */
>> /* Skip verify of ax_pseudo_register_collect, has predicate. */
>> /* Skip verify of ax_pseudo_register_push_stack, has predicate. */
>> + /* Skip verify of bound_violation_handler, has predicate. */
>> /* Skip verify of sp_regnum, invalid_p == 0 */
>> /* Skip verify of pc_regnum, invalid_p == 0 */
>> /* Skip verify of ps_regnum, invalid_p == 0 */
>> @@ -773,6 +775,12 @@ gdbarch_dump (struct gdbarch *gdbarch, struct ui_file *file)
>> "gdbarch_dump: bits_big_endian = %s\n",
>> plongest (gdbarch->bits_big_endian));
>> fprintf_unfiltered (file,
>> + "gdbarch_dump: gdbarch_bound_violation_handler_p() = %d\n",
>> + gdbarch_bound_violation_handler_p (gdbarch));
>> + fprintf_unfiltered (file,
>> + "gdbarch_dump: bound_violation_handler = <%s>\n",
>> + host_address_to_string (gdbarch->bound_violation_handler));
>> + fprintf_unfiltered (file,
>> "gdbarch_dump: breakpoint_from_pc = <%s>\n",
>> host_address_to_string (gdbarch->breakpoint_from_pc));
>> fprintf_unfiltered (file,
>> @@ -1986,6 +1994,30 @@ set_gdbarch_ax_pseudo_register_push_stack (struct gdbarch *gdbarch,
>> }
>>
>> int
>> +gdbarch_bound_violation_handler_p (struct gdbarch *gdbarch)
>> +{
>> + gdb_assert (gdbarch != NULL);
>> + return gdbarch->bound_violation_handler != NULL;
>> +}
>> +
>> +void
>> +gdbarch_bound_violation_handler (struct gdbarch *gdbarch, struct ui_out *uiout)
>> +{
>> + gdb_assert (gdbarch != NULL);
>> + gdb_assert (gdbarch->bound_violation_handler != NULL);
>> + if (gdbarch_debug >= 2)
>> + fprintf_unfiltered (gdb_stdlog, "gdbarch_bound_violation_handler called\n");
>> + gdbarch->bound_violation_handler (gdbarch, uiout);
>> +}
>> +
>> +void
>> +set_gdbarch_bound_violation_handler (struct gdbarch *gdbarch,
>> + gdbarch_bound_violation_handler_ftype bound_violation_handler)
>> +{
>> + gdbarch->bound_violation_handler = bound_violation_handler;
>> +}
>> +
>> +int
>> gdbarch_sp_regnum (struct gdbarch *gdbarch)
>> {
>> gdb_assert (gdbarch != NULL);
>> diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
>> index 3c16af2..eb5de0d 100644
>> --- a/gdb/gdbarch.h
>> +++ b/gdb/gdbarch.h
>> @@ -63,6 +63,7 @@ struct ravenscar_arch_ops;
>> struct elf_internal_linux_prpsinfo;
>> struct mem_range;
>> struct syscalls_info;
>> +struct ui_out;
>>
>> #include "regcache.h"
>>
>> @@ -299,6 +300,16 @@ typedef int (gdbarch_ax_pseudo_register_push_stack_ftype) (struct gdbarch *gdbar
>> extern int gdbarch_ax_pseudo_register_push_stack (struct gdbarch *gdbarch, struct agent_expr *ax, int reg);
>> extern void set_gdbarch_ax_pseudo_register_push_stack (struct gdbarch *gdbarch, gdbarch_ax_pseudo_register_push_stack_ftype *ax_pseudo_register_push_stack);
>>
>> +/* Function called when a segmentation fault signal is received by the inferior,
>> + having SIGCODE 3 (SIG_CODE_BOUNDARY_FAULT).
>> + UIOUT is the output stream where the handler will place information. */
>> +
>> +extern int gdbarch_bound_violation_handler_p (struct gdbarch *gdbarch);
>> +
>> +typedef void (gdbarch_bound_violation_handler_ftype) (struct gdbarch *gdbarch, struct ui_out *uiout);
>> +extern void gdbarch_bound_violation_handler (struct gdbarch *gdbarch, struct ui_out *uiout);
>> +extern void set_gdbarch_bound_violation_handler (struct gdbarch *gdbarch, gdbarch_bound_violation_handler_ftype *bound_violation_handler);
>> +
>> /* GDB's standard (or well known) register numbers. These can map onto
>> a real register or a pseudo (computed) register or not be defined at
>> all (-1).
>> diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
>> index f80cd51..edd155a 100755
>> --- a/gdb/gdbarch.sh
>> +++ b/gdb/gdbarch.sh
>> @@ -446,6 +446,11 @@ M:int:ax_pseudo_register_collect:struct agent_expr *ax, int reg:ax, reg
>> # Return -1 if something goes wrong, 0 otherwise.
>> M:int:ax_pseudo_register_push_stack:struct agent_expr *ax, int reg:ax, reg
>>
>> +# Function called when a segmentation fault signal is received by the inferior,
>> +# having SIGCODE 3 (SIG_CODE_BOUNDARY_FAULT).
>
> # Function called when a segmentation fault with
> # SIGCODE 3 (SIG_CODE_BOUNDARY_FAULT) is received by the inferior.
>
> But, see below.
>
>> +# UIOUT is the output stream where the handler will place information.
>> +M:void:bound_violation_handler:struct ui_out *uiout:uiout
>> +
>> # GDB's standard (or well known) register numbers. These can map onto
>> # a real register or a pseudo (computed) register or not be defined at
>> # all (-1).
>> @@ -1247,6 +1252,7 @@ struct ravenscar_arch_ops;
>> struct elf_internal_linux_prpsinfo;
>> struct mem_range;
>> struct syscalls_info;
>> +struct ui_out;
>>
>> #include "regcache.h"
>>
>> diff --git a/gdb/i386-linux-tdep.c b/gdb/i386-linux-tdep.c
>> index af39e78..45212e0 100644
>> --- a/gdb/i386-linux-tdep.c
>> +++ b/gdb/i386-linux-tdep.c
>> @@ -30,6 +30,7 @@
>> #include "i386-tdep.h"
>> #include "i386-linux-tdep.h"
>> #include "linux-tdep.h"
>> +#include "utils.h"
>> #include "glibc-tdep.h"
>> #include "solib-svr4.h"
>> #include "symtab.h"
>> @@ -384,6 +385,49 @@ i386_canonicalize_syscall (int syscall)
>> return gdb_sys_no_syscall;
>> }
>>
>> +void
>> +i386_mpx_bound_violation_handler (struct gdbarch *gdbarch, struct ui_out *uiout)
>
> Missing intro comment. Something like "Implement ... hook." should be enough.
> Look for preexisting examples.
>
>> +{
>> + CORE_ADDR lower_bound, upper_bound, access;
>> + int is_upper;
>> +
>> + if (!i386_mpx_enabled ())
>> + return;
>> + TRY
>> + {
>> + lower_bound
>> + = parse_and_eval_long ("$_siginfo._sifields._sigfault._addr_bnd._lower");
>> + upper_bound
>> + = parse_and_eval_long ("$_siginfo._sifields._sigfault._addr_bnd._upper");
>> + access
>> + = parse_and_eval_long ("$_siginfo._sifields._sigfault.si_addr");
>> + }
>> + CATCH (exception, RETURN_MASK_ALL)
>> + {
>> + return;
>> + }
>> + END_CATCH
>> +
>> + is_upper = (access > upper_bound ? 1 : 0);
>> +
>> + ui_out_text (uiout, "\n");
>> + if (is_upper)
>> + ui_out_field_string (uiout, "sigcode-meaning", "Upper bound violation");
>> + else
>> + ui_out_field_string (uiout, "sigcode-meaning", "Lower bound violation");
>> +
>> + ui_out_text (uiout, " while accessing address ");
>> + ui_out_field_fmt (uiout,"bound-access", "%s", paddress (gdbarch, access));
>> + ui_out_text (uiout, "\nBounds: [lower = ");
>> + ui_out_field_fmt (uiout,"lower-bound", "%s", paddress (gdbarch, lower_bound));
>> + ui_out_text (uiout, ", upper = ");
>> + ui_out_field_fmt (uiout,"upper-bound", "%s", paddress (gdbarch, upper_bound));
>> + ui_out_text (uiout, "]");
>
> Missing empty space after a few "uiout," above. Watch out for line-too-long after
> fixing that.
>
>> +
>> +
>> + return;
>
> Remove these two empty lines and the unnecessary "return".
>
>> +}
>> +
>> /* Parse the arguments of current system call instruction and record
>> the values of the registers and memory that will be changed into
>> "record_arch_list". This instruction is "int 0x80" (Linux
>> @@ -1002,6 +1046,8 @@ i386_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
>> i386_linux_get_syscall_number);
>>
>> set_gdbarch_get_siginfo_type (gdbarch, x86_linux_get_siginfo_type);
>> + set_gdbarch_bound_violation_handler(gdbarch,
>> + i386_mpx_bound_violation_handler);
>> }
>>
>> /* Provide a prototype to silence -Wmissing-prototypes. */
>> diff --git a/gdb/i386-linux-tdep.h b/gdb/i386-linux-tdep.h
>> index b32f6d1..6ffacd4 100644
>> --- a/gdb/i386-linux-tdep.h
>> +++ b/gdb/i386-linux-tdep.h
>> @@ -37,6 +37,11 @@
>> /* Get XSAVE extended state xcr0 from core dump. */
>> extern uint64_t i386_linux_core_read_xcr0 (bfd *abfd);
>>
>> +/* Handles and displays information related to the MPX bound violation
>> + to the user. */
>> +void
>> +i386_mpx_bound_violation_handler (struct gdbarch *gdbarch, struct ui_out *uiout);
>
> Only function definitions should have the function name at collumn 0. All other
> declarations have explicit "extern". Thus, write:
>
> extern void i386_mpx_bound_violation_handler (struct gdbarch *gdbarch,
> struct ui_out *uiout);
>
>> +
>> /* Linux target description. */
>> extern struct target_desc *tdesc_i386_linux;
>> extern struct target_desc *tdesc_i386_mmx_linux;
>> diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
>> index b706463..b5d0d14 100644
>> --- a/gdb/i386-tdep.c
>> +++ b/gdb/i386-tdep.c
>> @@ -8651,9 +8651,7 @@ i386_mpx_bd_base (void)
>> return ret & MPX_BASE_MASK;
>> }
>>
>> -/* Check if the current target is MPX enabled. */
>> -
>> -static int
>> +int
>> i386_mpx_enabled (void)
>> {
>> const struct gdbarch_tdep *tdep = gdbarch_tdep (get_current_arch ());
>> diff --git a/gdb/i386-tdep.h b/gdb/i386-tdep.h
>> index 10d2772..26933f2 100644
>> --- a/gdb/i386-tdep.h
>> +++ b/gdb/i386-tdep.h
>> @@ -420,6 +420,8 @@ extern int i386_process_record (struct gdbarch *gdbarch,
>> struct regcache *regcache, CORE_ADDR addr);
>> extern const struct target_desc *i386_target_description (uint64_t xcr0);
>>
>> +/* Verify if target is MPX enabled. */
>
> Lost "current"? If changing the comment, I'd suggest:
>
> /* Return true iff the current target is MPX enabled. */
>
>> +extern int i386_mpx_enabled (void);
>> \f
>>
>> /* Functions and variables exported from i386bsd-tdep.c. */
>> diff --git a/gdb/infrun.c b/gdb/infrun.c
>> index 64c729e..87b930e 100644
>> --- a/gdb/infrun.c
>> +++ b/gdb/infrun.c
>> @@ -7893,6 +7893,36 @@ print_exited_reason (struct ui_out *uiout, int exitstatus)
>> }
>> }
>>
>> +/* Value of the sigcode in case of a boundary fault. */
>
> Spurious double-space before "Value".
>
>> +
>> +#define SIG_CODE_BONDARY_FAULT 3
>> +
>> +/* Verifies if a received segmentation fault is a boundary fault.
>> + In the case it is it calls the architecture dependent function
>> + to handle the boundary fault. */
>
>
>> +
>> +static void
>> +handle_segmentation_faults (struct ui_out *uiout)
>
> "handle_segmentation_fault", singular.
>
>> +{
>> + long sig_code = 0;
>> + struct regcache *regcache = get_current_regcache ();
>> + struct gdbarch *gdbarch = get_regcache_arch (regcache);
>> +
>> + TRY
>> + {
>> + sig_code = parse_and_eval_long ("$_siginfo.si_code\n");
>
> It's useless to do this on architectures that don't install
> the gdbarch hook. I think this should be pushed down to the
> gdbarch hook. You'll end up with a single try/catch.
> And then the gdbarch hook can be renamed to a more generic
> gdbarch_handle_segmentation_fault. The comments thoughout should be
> updated then, like, for this function:
>
> /* Some targets/architectures can do extra processing/display of
> segmentation faults. E.g., Intel MPX boundary faults.
> Call the architecture dependent function to handle the fault. */
>
> static void
> handle_segmentation_fault (struct ui_out *uiout)
> {
>
>
>> + }
>> + CATCH (exception, RETURN_MASK_ALL)
>> + {
>> + return;
>> + }
>> + END_CATCH
>> +
>> + if (sig_code == SIG_CODE_BONDARY_FAULT
>> + && gdbarch_bound_violation_handler_p (gdbarch))
>> + gdbarch_bound_violation_handler (gdbarch, uiout);
>> +}
>
>
>> +
>> void
>> print_signal_received_reason (struct ui_out *uiout, enum gdb_signal siggnal)
>> {
>> @@ -7922,6 +7952,10 @@ print_signal_received_reason (struct ui_out *uiout, enum gdb_signal siggnal)
>> annotate_signal_string ();
>> ui_out_field_string (uiout, "signal-meaning",
>> gdb_signal_to_string (siggnal));
>> +
>> + if (siggnal == GDB_SIGNAL_SEGV)
>> + handle_segmentation_faults (uiout);
>> +
>> annotate_signal_string_end ();
>> }
>> ui_out_text (uiout, ".\n");
>> diff --git a/gdb/testsuite/gdb.arch/i386-mpx-sigsegv.c b/gdb/testsuite/gdb.arch/i386-mpx-sigsegv.c
>> new file mode 100644
>> index 0000000..7500352
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.arch/i386-mpx-sigsegv.c
>> @@ -0,0 +1,120 @@
>> +/* Copyright (C) 2015 Free Software Foundation, Inc.
>
> 2015-2016
>
>> +++ b/gdb/testsuite/gdb.arch/i386-mpx-sigsegv.exp
>> @@ -0,0 +1,88 @@
>> +# Copyright (C) 2015 Free Software Foundation, Inc.
>
> Ditto.
>
>
>> +gdb_test_multiple "print have_mpx ()" "have mpx" {
>> + -re ".. = 1\r\n$gdb_prompt " {
>> + pass "check whether processor supports MPX"
>
> set message "check whether processor supports MPX"
> gdb_test_multiple "print have_mpx ()" $message {
> -re ".. = 1\r\n$gdb_prompt " {
> pass $message
>
>> + }
>
> Alignment of } looks odd, here and throughout the file.
>
>> +set common_ubound_violation ".*Program received signal SIGSEGV,\
>> + Segmentation fault\r\nUpper bound violation while accessing\
>> + address $hex\r\nBounds: \\\[lower = $hex, upper = $hex\\\]"
>> +
>> +set common_lbound_violation ".*Program received signal SIGSEGV,\
>> + Segmentation fault\r\nLower bound violation while accessing\
>> + address $hex\r\nBounds: \\\[lower = $hex, upper = $hex\\\]"
>
> The initial ".*" is useless.
>
> Use [multi_line ...].
>
>
>> +
>> +set segv_upper_bound "$common_ubound_violation.*$gdb_prompt $"
>> +
>> +set segv_lower_bound "$common_lbound_violation.*$gdb_prompt $"
>> +
>> +for {set i 0} {$i < 15} {incr i} {
>> + set message "MPX signal segv Upper: ${i}"
>> + gdb_test_multiple "continue" "$message ${i}" {
>> + -re $segv_upper_bound {
>> + pass "$message"
>> + }
>> + -re ".*$inferior_exited_re normally.*$gdb_prompt $" {
>> + fail "$message"
>
> The pass/fail calls are missing ${i}. Please make sure test
> messages are unique in gdb.sum:
>
> https://sourceware.org/gdb/wiki/GDBTestcaseCookbook#Make_sure_test_messages_are_unique
>
>> + break
>> + }
>> + }
>> + gdb_test "where" ".*#0 0x\[0-9a-fA-F\]+ in upper.*"\
>> + "$message: should be in upper"
>
> Ditto.
>
> Useless ".*".
>
> Use $hex.
>
>
>> +}
>> +
>> +for {set i 0} {$i < 15} {incr i} {
>> + set message "MPX signal segv Lower: ${i}"
>> + gdb_test_multiple "continue" "$message ${i}" {
>> + -re $segv_lower_bound {
>> + pass "$message ${i}"
>> + }
>> + -re ".*$inferior_exited_re normally.*$gdb_prompt $" {
>> + fail "$message ${i}"
>> + break
>> + }
>> + }
>> + gdb_test "where" ".*#0 0x\[0-9a-fA-F\]+ in lower.*"\
>> + "$message: should be in lower"
>> +}
>
> Likewise.
>
>> diff --git a/gdb/testsuite/gdb.arch/i386-mpx-simple_segv.c b/gdb/testsuite/gdb.arch/i386-mpx-simple_segv.c
>> new file mode 100644
>> index 0000000..5317369
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.arch/i386-mpx-simple_segv.c
>> @@ -0,0 +1,66 @@
>> +/* Copyright (C) 2015 Free Software Foundation, Inc.
>
> 2015-2016
>
>> +
>
>> diff --git a/gdb/testsuite/gdb.arch/i386-mpx-simple_segv.exp b/gdb/testsuite/gdb.arch/i386-mpx-simple_segv.exp
>
> Largely the same comments apply to this file. I'll only mention other
> extra things I spotted below.
>
>> +# Using the handler for SIGSEGV as "print pass stop"
>> +set parameters "print pass stop"
>> +runto_main
>
> Check result. Wrap with_test_prefix instead of appending
> $parameters, 0, 1, etc. in some test messages. That way,
> any fail within runto_main, etc. is covered as well, as well
> as being easier.
>
>> +send_gdb "handle SIGSEGV $parameters\n"
>> +send_gdb "continue\n"
>> +
>> +gdb_expect {
>> + -re $segv_bound_with_prompt {
>> + pass $parameters
>> + }
>> +}
>
> Use gdb_test/gdb_test_multiple instead of send_gdb/gdb_expect.
>
>> +gdb_test "handle SIGSEGV $parameters" "" "Setting\
>> +the handler for segfault 0"
>
> Odd line break. Break it before the string, not in the middle.
>
>> +
>> +gdb_test_multiple "continue" "test 0" {
>> + -re $segv_with_exit {
>> + pass $parameters
>> + }
>> + -re "$gdb_prompt $" {
>> + fail $parameters
>> + }
>> +}
>> +
>> +gdb_test "where" "No stack." "no inferior $parameters"
>> +
>> +# Using the handler for SIGSEGV as "print nopass stop"
>> +set parameters "print nopass stop"
>> +
>> +runto_main
>> +gdb_test "handle SIGSEGV $parameters" "" "Setting\
>> +the handler for segfault 1"
>
>
> Thanks,
> Pedro Alves
>
Thanks a lot for your review!
Will address all comments and answer your question as well!
For the ones that are ok with the proposed changes should we commit
before the 7.11 branch is taken or rather waiting new branch is taken?
(Respecting the release process i was waiting already with the previous
patch)
Thanks again and best 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
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH V5 5/5] ntel MPX bound violation handling
2016-01-28 13:49 ` Walfred Tedeschi
@ 2016-01-28 14:18 ` Pedro Alves
0 siblings, 0 replies; 18+ messages in thread
From: Pedro Alves @ 2016-01-28 14:18 UTC (permalink / raw)
To: Walfred Tedeschi, eliz, brobecker; +Cc: gdb-patches
On 01/28/2016 01:49 PM, Walfred Tedeschi wrote:
>
> Thanks a lot for your review!
>
> Will address all comments and answer your question as well!
>
> For the ones that are ok with the proposed changes should we commit
> before the 7.11 branch is taken or rather waiting new branch is taken?
I think they're low risk enough, and you could push now.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH V5 5/5] ntel MPX bound violation handling
2016-01-28 13:43 ` Pedro Alves
2016-01-28 13:49 ` Walfred Tedeschi
@ 2016-02-02 10:09 ` Walfred Tedeschi
2016-02-02 10:38 ` Pedro Alves
1 sibling, 1 reply; 18+ messages in thread
From: Walfred Tedeschi @ 2016-02-02 10:09 UTC (permalink / raw)
To: Pedro Alves, eliz, brobecker; +Cc: gdb-patches
Am 1/28/2016 um 2:42 PM schrieb Pedro Alves:
Pedro,
I have tried to address all your comments.
For the tests I also verified that there was only one line in the log as
shown in the wiki.
Have though some comments below:
> "Intel" in $subject.
>
> On 01/22/2016 02:54 PM, Walfred Tedeschi wrote:
>> With Intel Memory Protection Extensions it was introduced the concept of
>> boundary violation. A boundary violations is presented to the inferior as
>> a segmentation fault having SIGCODE 3. This patch adds a
>> handler for a boundary violation extending the information displayed
>> when a bound violation is presented to the inferior. In the stop mode
>> case the debugger will also display the kind of violation: "upper" or
>> "lower", bounds and the address accessed.
>> On no stop mode the information will still remain unchanged. Additional
>> information about bound violations are not meaningful in that case user
>> does not know the line in which violation occurred as well.
>>
>> When the segmentation fault handler is stop mode the out puts will be
>> changed as exemplified below.
>>
>> The usual output of a segfault is:
>> Program received signal SIGSEGV, Segmentation fault
>> 0x0000000000400d7c in upper (p=0x603010, a=0x603030, b=0x603050,
>> c=0x603070, d=0x603090, len=7) at i386-mpx-sigsegv.c:68
>> 68 value = *(p + len);
>>
>> In case it is a bound violation it will be presented as:
>> Program received signal SIGSEGV, Segmentation fault
>> Upper bound violation while accessing address 0x7fffffffc3b3
>> Bounds: [lower = 0x7fffffffc390, upper = 0x7fffffffc3a3]
>> 0x0000000000400d7c in upper (p=0x603010, a=0x603030, b=0x603050,
>> c=0x603070, d=0x603090, len=7) at i386-mpx-sigsegv.c:68
>> 68 value = *(p + len);
>>
>> In mi mode the output of a segfault is:
>> *stopped,reason="signal-received",signal-name="SIGSEGV",
>> signal-meaning="Segmentation fault", frame={addr="0x0000000000400d7c",
>> func="upper",args=[{name="p", value="0x603010"},{name="a",value="0x603030"}
>> ,{name="b",value="0x603050"}, {name="c",value="0x603070"},
>> {name="d",value="0x603090"},{name="len",value="7"}],
>> file="i386-mpx-sigsegv.c",fullname="i386-mpx-sigsegv.c",line="68"},
>> thread-id="1",stopped-threads="all",core="6"
>>
>> in the case of a bound violation:
>> *stopped,reason="signal-received",signal-name="SIGSEGV",
>> signal-meaning="Segmentation fault",
>> sigcode-meaning="Upper bound violation",
>> lower-bound="0x603010",upper-bound="0x603023",bound-access="0x60302f",
>> frame={addr="0x0000000000400d7c",func="upper",args=[{name="p",
>> value="0x603010"},{name="a",value="0x603030"},{name="b",value="0x603050"},
>> {name="c",value="0x603070"},{name="d",value="0x603090"},
>> {name="len",value="7"}],file="i386-mpx-sigsegv.c",
>> fullname="i386-mpx-sigsegv.c",line="68"},thread-id="1",
>> stopped-threads="all",core="6"
>>
>> 2016-01-15 Walfred Tedeschi <walfred.tedeschi@intel.com>
>>
>> gdb/ChangeLog:
>>
>> * NEWS: Add entry for bound violation.
>> * amd64-linux-tdep.c (amd64_linux_init_abi_common):
>> Add handler for bound violation.
>> * gdbarch.sh (bound_violation_handler): New.
>> * gdbarch.c: Regenerate.
>> * gdbarch.h: Regenerate.
>> * i386-linux-tdep.c (i386_mpx_bound_violation_handler): New.
>> (i386_linux_init_abi): Use i386_mpx_bound_violation_handler.
>> * i386-linux-tdep.h (i386_mpx_bound_violation_handler) New.
>> * i386-tdep.c (i386_mpx_enabled): Add as external.
>> * i386-tdep.c (i386_mpx_enabled): Add as external.
>> * infrun.c (handle_segmentation_faults): New function.
>> (print_signal_received_reason): Use handle_segmentation_faults.
>> (normal_stop): Change order of observer in order to have the
>> inferior stopped for evaluation.
>>
>> gdb/testsuite/ChangeLog:
>>
>> * gdb.arch/i386-mpx-sigsegv.c: New file.
>> * gdb.arch/i386-mpx-sigsegv.exp: New file.
>> * gdb.arch/i386-mpx-simple_segv.c: New file.
>> * gdb.arch/i386-mpx-simple_segv.exp: New file.
>>
>> gdb/doc/ChangeLog:
>>
>> * gdb.texinfo (Signals): Add bound violation display hints for
>> a SIGSEGV.
>>
>> ---
>> gdb/NEWS | 15 +++
>> gdb/amd64-linux-tdep.c | 2 +
>> gdb/doc/gdb.texinfo | 25 +++++
>> gdb/gdbarch.c | 32 ++++++
>> gdb/gdbarch.h | 11 +++
>> gdb/gdbarch.sh | 6 ++
>> gdb/i386-linux-tdep.c | 46 +++++++++
>> gdb/i386-linux-tdep.h | 5 +
>> gdb/i386-tdep.c | 4 +-
>> gdb/i386-tdep.h | 2 +
>> gdb/infrun.c | 34 +++++++
>> gdb/testsuite/gdb.arch/i386-mpx-sigsegv.c | 120 +++++++++++++++++++++++
>> gdb/testsuite/gdb.arch/i386-mpx-sigsegv.exp | 88 +++++++++++++++++
>> gdb/testsuite/gdb.arch/i386-mpx-simple_segv.c | 66 +++++++++++++
>> gdb/testsuite/gdb.arch/i386-mpx-simple_segv.exp | 125 ++++++++++++++++++++++++
>> 15 files changed, 578 insertions(+), 3 deletions(-)
>> create mode 100644 gdb/testsuite/gdb.arch/i386-mpx-sigsegv.c
>> create mode 100644 gdb/testsuite/gdb.arch/i386-mpx-sigsegv.exp
>> create mode 100644 gdb/testsuite/gdb.arch/i386-mpx-simple_segv.c
>> create mode 100644 gdb/testsuite/gdb.arch/i386-mpx-simple_segv.exp
>>
>> diff --git a/gdb/NEWS b/gdb/NEWS
>> index d9cbb80..b8512d6 100644
>> --- a/gdb/NEWS
>> +++ b/gdb/NEWS
>> @@ -3,6 +3,21 @@
>>
>> *** Changes since GDB 7.10
>>
>> +* Intel MPX boud violation handler.
>
> Typo, "bound".
>
> I think you may have meant s/handler/handling/.
>
>> +
>> + A boundary violations is presented to the inferior as
>> + a segmentation fault having SIGCODE 3. In this case
>> + GDB displays also the kind of violation (upper or lower),
>> + bounds, poiter value and the memory accessed, besides displaying
>> + the usual signal received and code location report.
>
> I think that talking about sigcode is a bit of implementer-speak.
> Also, a few typos and missing double-spaces.
>
> I don't understand what you mean by "pointer value and the
> memory accessed", specifically, what pointer value is that.
>
>> + As exemplified below:
>> + Program received signal SIGSEGV, Segmentation fault
>> + Upper bound violation while accessing address 0x7fffffffc3b3
>> + Bounds: [lower = 0x7fffffffc390, upper = 0x7fffffffc3a3].
>> + 0x0000000000400d7c in upper (p=0x603010, a=0x603030, b=0x603050,
>> + c=0x603070, d=0x603090, len=7) at i386-mpx-sigsegv.c:68
>
> The arguments in "upper()" don't add any info here, and are
> causing a line wrap. so I think you should remove them.
>
> The period at the end of:
>
> Bounds: [lower = 0x7fffffffc390, upper = 0x7fffffffc3a3].
>
> looks odd. That's not a sentence, and the real sentence above
> didn't get a period. I think you should remove it, saving
> some screen estate, and update all examples that might show it,
> here and docs.
>
> Thus, I suggest:
>
> * Intel MPX boundary violations
>
> Segmentation faults caused by Intel MPX boundary violations
> now display the kind of violation (upper or lower), the memory
> address accessed and the memory bounds, along with the usual
> signal received and code location.
>
> For example:
>
> Program received signal SIGSEGV, Segmentation fault
> Upper bound violation while accessing address 0x7fffffffc3b3
> Bounds: [lower = 0x7fffffffc390, upper = 0x7fffffffc3a3]
> 0x0000000000400d7c in upper () at i386-mpx-sigsegv.c:68
>
>> +
>> * Per-inferior thread numbers
>>
>> Thread numbers are now per inferior instead of global. If you're
>> diff --git a/gdb/amd64-linux-tdep.c b/gdb/amd64-linux-tdep.c
>> index 21bcd99..8af15c4 100644
>> --- a/gdb/amd64-linux-tdep.c
>> +++ b/gdb/amd64-linux-tdep.c
>> @@ -1840,6 +1840,8 @@ amd64_linux_init_abi_common(struct gdbarch_info info, struct gdbarch *gdbarch)
>> set_gdbarch_process_record_signal (gdbarch, amd64_linux_record_signal);
>>
>> set_gdbarch_get_siginfo_type (gdbarch, x86_linux_get_siginfo_type);
>> + set_gdbarch_bound_violation_handler(gdbarch,
>> + i386_mpx_bound_violation_handler);
>
> Space before parens.
>
>> }
>>
>> static void
>> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
>> index 7da31c8..f4373ed 100644
>> --- a/gdb/doc/gdb.texinfo
>> +++ b/gdb/doc/gdb.texinfo
>> @@ -5819,6 +5819,30 @@ $1 = (void *) 0x7ffff7ff7000
>>
>> Depending on target support, @code{$_siginfo} may also be writable.
>>
>> +Also on some targets a @code{SIGSEGV} can be related to
>> +boundary violations, i.e. accessing an address outside of
>> +allowed range. In those cases @value{GDBN} might displays additional
>> +information. In @code{STOP} mode @value{GDBN} displays the violation
>> +kind: "Upper" or "Lower", address accessed and bounds. In @code{NOSTOP}
>> +no additional information is displayed.
>
> I suggest adding a couple cindex's, and the following edit:
>
> @cindex Intel MPX boundary violations
> @cindex boundary violations, Intel MPX
> On some targets, a @code{SIGSEGV} can be caused by a boundary
> violation, i.e., accessing an address outside of the allowed range.
> In those cases @value{GDBN} may displays additional information, depending
> on how @value{GDBN} has been told to handle the signal. In @code{handle stop}
> mode @value{GDBN} displays the violation kind: "Upper" or "Lower", the
> memory address accessed and the bounds. In @code{handle nostop} no
> additional information is displayed.
>
>> +
>> +The usual output of a segfault is:
>> +@smallexample
>> +Program received signal SIGSEGV, Segmentation fault
>> +0x0000000000400d7c in upper (p=0x603010, a=0x603030, b=0x603050,
>> +c=0x603070, d=0x603090, len=7) at i386-mpx-sigsegv.c:68
>> +68 value = *(p + len);
>> +@end smallexample
>
> Remove arguments here too:
>
> The usual output of a segfault is:
>
> @smallexample
> Program received signal SIGSEGV, Segmentation fault
> 0x0000000000400d7c in upper () at i386-mpx-sigsegv.c:68
> 68 value = *(p + len);
> @end smallexample
>
>
>> +
>> +In case it is a bound violation it will be presented as:
>
> "While a bound violation is presented as:"
>
> Note: s/will/is/
>
>> +@smallexample
>> +Upper bound violation while accessing address 0x7fffffffc3b3
>> +Bounds: [lower = 0x7fffffffc390, upper = 0x7fffffffc3a3]
>> +0x0000000000400d7c in upper (p=0x603010, a=0x603030, b=0x603050,
>> +c=0x603070, d=0x603090, len=7) at i386-mpx-sigsegv.c:68
>> +68 value = *(p + len);
>> +@end smallexample
>
> "Program received ..." is missing. Drop arguments:
>
> @smallexample
> Program received signal SIGSEGV, Segmentation fault
> Upper bound violation while accessing address 0x7fffffffc3b3
> Bounds: [lower = 0x7fffffffc390, upper = 0x7fffffffc3a3]
> 0x0000000000400d7c in upper () at i386-mpx-sigsegv.c:68
> 68 value = *(p + len);
> @end smallexample
>
>> +
>> @node Thread Stops
>> @section Stopping and Starting Multi-thread Programs
>>
>> @@ -22267,6 +22291,7 @@ whose bounds are to be changed, @var{lbound} and @var{ubound} are new values
>> for lower and upper bounds respectively.
>> @end table
>>
>> +
>> @node Alpha
>
> Spurious empty lines?
>
>> @subsection Alpha
>>
>> diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
>> index d45af1a..f7fef25 100644
>> --- a/gdb/gdbarch.c
>> +++ b/gdb/gdbarch.c
>> @@ -189,6 +189,7 @@ struct gdbarch
>> int num_pseudo_regs;
>> gdbarch_ax_pseudo_register_collect_ftype *ax_pseudo_register_collect;
>> gdbarch_ax_pseudo_register_push_stack_ftype *ax_pseudo_register_push_stack;
>> + gdbarch_bound_violation_handler_ftype *bound_violation_handler;
>> int sp_regnum;
>> int pc_regnum;
>> int ps_regnum;
>> @@ -531,6 +532,7 @@ verify_gdbarch (struct gdbarch *gdbarch)
>> /* Skip verify of num_pseudo_regs, invalid_p == 0 */
>> /* Skip verify of ax_pseudo_register_collect, has predicate. */
>> /* Skip verify of ax_pseudo_register_push_stack, has predicate. */
>> + /* Skip verify of bound_violation_handler, has predicate. */
>> /* Skip verify of sp_regnum, invalid_p == 0 */
>> /* Skip verify of pc_regnum, invalid_p == 0 */
>> /* Skip verify of ps_regnum, invalid_p == 0 */
>> @@ -773,6 +775,12 @@ gdbarch_dump (struct gdbarch *gdbarch, struct ui_file *file)
>> "gdbarch_dump: bits_big_endian = %s\n",
>> plongest (gdbarch->bits_big_endian));
>> fprintf_unfiltered (file,
>> + "gdbarch_dump: gdbarch_bound_violation_handler_p() = %d\n",
>> + gdbarch_bound_violation_handler_p (gdbarch));
>> + fprintf_unfiltered (file,
>> + "gdbarch_dump: bound_violation_handler = <%s>\n",
>> + host_address_to_string (gdbarch->bound_violation_handler));
>> + fprintf_unfiltered (file,
>> "gdbarch_dump: breakpoint_from_pc = <%s>\n",
>> host_address_to_string (gdbarch->breakpoint_from_pc));
>> fprintf_unfiltered (file,
>> @@ -1986,6 +1994,30 @@ set_gdbarch_ax_pseudo_register_push_stack (struct gdbarch *gdbarch,
>> }
>>
>> int
>> +gdbarch_bound_violation_handler_p (struct gdbarch *gdbarch)
>> +{
>> + gdb_assert (gdbarch != NULL);
>> + return gdbarch->bound_violation_handler != NULL;
>> +}
>> +
>> +void
>> +gdbarch_bound_violation_handler (struct gdbarch *gdbarch, struct ui_out *uiout)
>> +{
>> + gdb_assert (gdbarch != NULL);
>> + gdb_assert (gdbarch->bound_violation_handler != NULL);
>> + if (gdbarch_debug >= 2)
>> + fprintf_unfiltered (gdb_stdlog, "gdbarch_bound_violation_handler called\n");
>> + gdbarch->bound_violation_handler (gdbarch, uiout);
>> +}
>> +
>> +void
>> +set_gdbarch_bound_violation_handler (struct gdbarch *gdbarch,
>> + gdbarch_bound_violation_handler_ftype bound_violation_handler)
>> +{
>> + gdbarch->bound_violation_handler = bound_violation_handler;
>> +}
>> +
>> +int
>> gdbarch_sp_regnum (struct gdbarch *gdbarch)
>> {
>> gdb_assert (gdbarch != NULL);
>> diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
>> index 3c16af2..eb5de0d 100644
>> --- a/gdb/gdbarch.h
>> +++ b/gdb/gdbarch.h
>> @@ -63,6 +63,7 @@ struct ravenscar_arch_ops;
>> struct elf_internal_linux_prpsinfo;
>> struct mem_range;
>> struct syscalls_info;
>> +struct ui_out;
>>
>> #include "regcache.h"
>>
>> @@ -299,6 +300,16 @@ typedef int (gdbarch_ax_pseudo_register_push_stack_ftype) (struct gdbarch *gdbar
>> extern int gdbarch_ax_pseudo_register_push_stack (struct gdbarch *gdbarch, struct agent_expr *ax, int reg);
>> extern void set_gdbarch_ax_pseudo_register_push_stack (struct gdbarch *gdbarch, gdbarch_ax_pseudo_register_push_stack_ftype *ax_pseudo_register_push_stack);
>>
>> +/* Function called when a segmentation fault signal is received by the inferior,
>> + having SIGCODE 3 (SIG_CODE_BOUNDARY_FAULT).
>> + UIOUT is the output stream where the handler will place information. */
>> +
>> +extern int gdbarch_bound_violation_handler_p (struct gdbarch *gdbarch);
>> +
>> +typedef void (gdbarch_bound_violation_handler_ftype) (struct gdbarch *gdbarch, struct ui_out *uiout);
>> +extern void gdbarch_bound_violation_handler (struct gdbarch *gdbarch, struct ui_out *uiout);
>> +extern void set_gdbarch_bound_violation_handler (struct gdbarch *gdbarch, gdbarch_bound_violation_handler_ftype *bound_violation_handler);
>> +
>> /* GDB's standard (or well known) register numbers. These can map onto
>> a real register or a pseudo (computed) register or not be defined at
>> all (-1).
>> diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
>> index f80cd51..edd155a 100755
>> --- a/gdb/gdbarch.sh
>> +++ b/gdb/gdbarch.sh
>> @@ -446,6 +446,11 @@ M:int:ax_pseudo_register_collect:struct agent_expr *ax, int reg:ax, reg
>> # Return -1 if something goes wrong, 0 otherwise.
>> M:int:ax_pseudo_register_push_stack:struct agent_expr *ax, int reg:ax, reg
>>
>> +# Function called when a segmentation fault signal is received by the inferior,
>> +# having SIGCODE 3 (SIG_CODE_BOUNDARY_FAULT).
>
> # Function called when a segmentation fault with
> # SIGCODE 3 (SIG_CODE_BOUNDARY_FAULT) is received by the inferior.
>
> But, see below.
>
>> +# UIOUT is the output stream where the handler will place information.
>> +M:void:bound_violation_handler:struct ui_out *uiout:uiout
>> +
>> # GDB's standard (or well known) register numbers. These can map onto
>> # a real register or a pseudo (computed) register or not be defined at
>> # all (-1).
>> @@ -1247,6 +1252,7 @@ struct ravenscar_arch_ops;
>> struct elf_internal_linux_prpsinfo;
>> struct mem_range;
>> struct syscalls_info;
>> +struct ui_out;
>>
>> #include "regcache.h"
>>
>> diff --git a/gdb/i386-linux-tdep.c b/gdb/i386-linux-tdep.c
>> index af39e78..45212e0 100644
>> --- a/gdb/i386-linux-tdep.c
>> +++ b/gdb/i386-linux-tdep.c
>> @@ -30,6 +30,7 @@
>> #include "i386-tdep.h"
>> #include "i386-linux-tdep.h"
>> #include "linux-tdep.h"
>> +#include "utils.h"
>> #include "glibc-tdep.h"
>> #include "solib-svr4.h"
>> #include "symtab.h"
>> @@ -384,6 +385,49 @@ i386_canonicalize_syscall (int syscall)
>> return gdb_sys_no_syscall;
>> }
>>
>> +void
>> +i386_mpx_bound_violation_handler (struct gdbarch *gdbarch, struct ui_out *uiout)
>
> Missing intro comment. Something like "Implement ... hook." should be enough.
> Look for preexisting examples.
>
>> +{
>> + CORE_ADDR lower_bound, upper_bound, access;
>> + int is_upper;
>> +
>> + if (!i386_mpx_enabled ())
>> + return;
>> + TRY
>> + {
>> + lower_bound
>> + = parse_and_eval_long ("$_siginfo._sifields._sigfault._addr_bnd._lower");
>> + upper_bound
>> + = parse_and_eval_long ("$_siginfo._sifields._sigfault._addr_bnd._upper");
>> + access
>> + = parse_and_eval_long ("$_siginfo._sifields._sigfault.si_addr");
>> + }
>> + CATCH (exception, RETURN_MASK_ALL)
>> + {
>> + return;
>> + }
>> + END_CATCH
>> +
>> + is_upper = (access > upper_bound ? 1 : 0);
>> +
>> + ui_out_text (uiout, "\n");
>> + if (is_upper)
>> + ui_out_field_string (uiout, "sigcode-meaning", "Upper bound violation");
>> + else
>> + ui_out_field_string (uiout, "sigcode-meaning", "Lower bound violation");
>> +
>> + ui_out_text (uiout, " while accessing address ");
>> + ui_out_field_fmt (uiout,"bound-access", "%s", paddress (gdbarch, access));
>> + ui_out_text (uiout, "\nBounds: [lower = ");
>> + ui_out_field_fmt (uiout,"lower-bound", "%s", paddress (gdbarch, lower_bound));
>> + ui_out_text (uiout, ", upper = ");
>> + ui_out_field_fmt (uiout,"upper-bound", "%s", paddress (gdbarch, upper_bound));
>> + ui_out_text (uiout, "]");
>
> Missing empty space after a few "uiout," above. Watch out for line-too-long after
> fixing that.
>
>> +
>> +
>> + return;
>
> Remove these two empty lines and the unnecessary "return".
>
>> +}
>> +
>> /* Parse the arguments of current system call instruction and record
>> the values of the registers and memory that will be changed into
>> "record_arch_list". This instruction is "int 0x80" (Linux
>> @@ -1002,6 +1046,8 @@ i386_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
>> i386_linux_get_syscall_number);
>>
>> set_gdbarch_get_siginfo_type (gdbarch, x86_linux_get_siginfo_type);
>> + set_gdbarch_bound_violation_handler(gdbarch,
>> + i386_mpx_bound_violation_handler);
>> }
>>
>> /* Provide a prototype to silence -Wmissing-prototypes. */
>> diff --git a/gdb/i386-linux-tdep.h b/gdb/i386-linux-tdep.h
>> index b32f6d1..6ffacd4 100644
>> --- a/gdb/i386-linux-tdep.h
>> +++ b/gdb/i386-linux-tdep.h
>> @@ -37,6 +37,11 @@
>> /* Get XSAVE extended state xcr0 from core dump. */
>> extern uint64_t i386_linux_core_read_xcr0 (bfd *abfd);
>>
>> +/* Handles and displays information related to the MPX bound violation
>> + to the user. */
>> +void
>> +i386_mpx_bound_violation_handler (struct gdbarch *gdbarch, struct ui_out *uiout);
>
> Only function definitions should have the function name at collumn 0. All other
> declarations have explicit "extern". Thus, write:
>
> extern void i386_mpx_bound_violation_handler (struct gdbarch *gdbarch,
> struct ui_out *uiout);
>
>> +
>> /* Linux target description. */
>> extern struct target_desc *tdesc_i386_linux;
>> extern struct target_desc *tdesc_i386_mmx_linux;
>> diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
>> index b706463..b5d0d14 100644
>> --- a/gdb/i386-tdep.c
>> +++ b/gdb/i386-tdep.c
>> @@ -8651,9 +8651,7 @@ i386_mpx_bd_base (void)
>> return ret & MPX_BASE_MASK;
>> }
>>
>> -/* Check if the current target is MPX enabled. */
>> -
>> -static int
>> +int
>> i386_mpx_enabled (void)
>> {
>> const struct gdbarch_tdep *tdep = gdbarch_tdep (get_current_arch ());
>> diff --git a/gdb/i386-tdep.h b/gdb/i386-tdep.h
>> index 10d2772..26933f2 100644
>> --- a/gdb/i386-tdep.h
>> +++ b/gdb/i386-tdep.h
>> @@ -420,6 +420,8 @@ extern int i386_process_record (struct gdbarch *gdbarch,
>> struct regcache *regcache, CORE_ADDR addr);
>> extern const struct target_desc *i386_target_description (uint64_t xcr0);
>>
>> +/* Verify if target is MPX enabled. */
>
> Lost "current"? If changing the comment, I'd suggest:
>
> /* Return true iff the current target is MPX enabled. */
>
>> +extern int i386_mpx_enabled (void);
>> \f
>>
>> /* Functions and variables exported from i386bsd-tdep.c. */
>> diff --git a/gdb/infrun.c b/gdb/infrun.c
>> index 64c729e..87b930e 100644
>> --- a/gdb/infrun.c
>> +++ b/gdb/infrun.c
>> @@ -7893,6 +7893,36 @@ print_exited_reason (struct ui_out *uiout, int exitstatus)
>> }
>> }
>>
>> +/* Value of the sigcode in case of a boundary fault. */
>
> Spurious double-space before "Value".
>
>> +
>> +#define SIG_CODE_BONDARY_FAULT 3
>> +
>> +/* Verifies if a received segmentation fault is a boundary fault.
>> + In the case it is it calls the architecture dependent function
>> + to handle the boundary fault. */
>
>
>> +
>> +static void
>> +handle_segmentation_faults (struct ui_out *uiout)
>
> "handle_segmentation_fault", singular.
>
>> +{
>> + long sig_code = 0;
>> + struct regcache *regcache = get_current_regcache ();
>> + struct gdbarch *gdbarch = get_regcache_arch (regcache);
>> +
>> + TRY
>> + {
>> + sig_code = parse_and_eval_long ("$_siginfo.si_code\n");
>
> It's useless to do this on architectures that don't install
> the gdbarch hook. I think this should be pushed down to the
> gdbarch hook. You'll end up with a single try/catch.
> And then the gdbarch hook can be renamed to a more generic
> gdbarch_handle_segmentation_fault. The comments thoughout should be
> updated then, like, for this function:
>
> /* Some targets/architectures can do extra processing/display of
> segmentation faults. E.g., Intel MPX boundary faults.
> Call the architecture dependent function to handle the fault. */
>
> static void
> handle_segmentation_fault (struct ui_out *uiout)
> {
>
>
>> + }
>> + CATCH (exception, RETURN_MASK_ALL)
>> + {
>> + return;
>> + }
>> + END_CATCH
>> +
>> + if (sig_code == SIG_CODE_BONDARY_FAULT
>> + && gdbarch_bound_violation_handler_p (gdbarch))
>> + gdbarch_bound_violation_handler (gdbarch, uiout);
>> +}
>
>
>> +
>> void
>> print_signal_received_reason (struct ui_out *uiout, enum gdb_signal siggnal)
>> {
>> @@ -7922,6 +7952,10 @@ print_signal_received_reason (struct ui_out *uiout, enum gdb_signal siggnal)
>> annotate_signal_string ();
>> ui_out_field_string (uiout, "signal-meaning",
>> gdb_signal_to_string (siggnal));
>> +
>> + if (siggnal == GDB_SIGNAL_SEGV)
>> + handle_segmentation_faults (uiout);
>> +
>> annotate_signal_string_end ();
>> }
>> ui_out_text (uiout, ".\n");
>> diff --git a/gdb/testsuite/gdb.arch/i386-mpx-sigsegv.c b/gdb/testsuite/gdb.arch/i386-mpx-sigsegv.c
>> new file mode 100644
>> index 0000000..7500352
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.arch/i386-mpx-sigsegv.c
>> @@ -0,0 +1,120 @@
>> +/* Copyright (C) 2015 Free Software Foundation, Inc.
>
> 2015-2016
>
>> +++ b/gdb/testsuite/gdb.arch/i386-mpx-sigsegv.exp
>> @@ -0,0 +1,88 @@
>> +# Copyright (C) 2015 Free Software Foundation, Inc.
>
> Ditto.
>
>
>> +gdb_test_multiple "print have_mpx ()" "have mpx" {
>> + -re ".. = 1\r\n$gdb_prompt " {
>> + pass "check whether processor supports MPX"
>
> set message "check whether processor supports MPX"
> gdb_test_multiple "print have_mpx ()" $message {
> -re ".. = 1\r\n$gdb_prompt " {
> pass $message
>
>> + }
>
> Alignment of } looks odd, here and throughout the file.
>
>> +set common_ubound_violation ".*Program received signal SIGSEGV,\
>> + Segmentation fault\r\nUpper bound violation while accessing\
>> + address $hex\r\nBounds: \\\[lower = $hex, upper = $hex\\\]"
>> +
>> +set common_lbound_violation ".*Program received signal SIGSEGV,\
>> + Segmentation fault\r\nLower bound violation while accessing\
>> + address $hex\r\nBounds: \\\[lower = $hex, upper = $hex\\\]"
>
> The initial ".*" is useless.
>
> Use [multi_line ...].
>
>
>> +
>> +set segv_upper_bound "$common_ubound_violation.*$gdb_prompt $"
>> +
>> +set segv_lower_bound "$common_lbound_violation.*$gdb_prompt $"
>> +
>> +for {set i 0} {$i < 15} {incr i} {
>> + set message "MPX signal segv Upper: ${i}"
>> + gdb_test_multiple "continue" "$message ${i}" {
>> + -re $segv_upper_bound {
>> + pass "$message"
>> + }
>> + -re ".*$inferior_exited_re normally.*$gdb_prompt $" {
>> + fail "$message"
>
> The pass/fail calls are missing ${i}. Please make sure test
> messages are unique in gdb.sum:
In fail and pass i think we do not need the ${i} it is added at the
message level already see:
set message "MPX signal segv Upper: ${i}"
>
> https://sourceware.org/gdb/wiki/GDBTestcaseCookbook#Make_sure_test_messages_are_unique
>
>> + break
>> + }
>> + }
>> + gdb_test "where" ".*#0 0x\[0-9a-fA-F\]+ in upper.*"\
>> + "$message: should be in upper"
>
> Ditto.
>
> Useless ".*".
>
> Use $hex.
>
>
>> +}
>> +
>> +for {set i 0} {$i < 15} {incr i} {
>> + set message "MPX signal segv Lower: ${i}"
>> + gdb_test_multiple "continue" "$message ${i}" {
>> + -re $segv_lower_bound {
>> + pass "$message ${i}"
>> + }
>> + -re ".*$inferior_exited_re normally.*$gdb_prompt $" {
>> + fail "$message ${i}"
>> + break
>> + }
>> + }
>> + gdb_test "where" ".*#0 0x\[0-9a-fA-F\]+ in lower.*"\
>> + "$message: should be in lower"
>> +}
>
> Likewise.
>
>> diff --git a/gdb/testsuite/gdb.arch/i386-mpx-simple_segv.c b/gdb/testsuite/gdb.arch/i386-mpx-simple_segv.c
>> new file mode 100644
>> index 0000000..5317369
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.arch/i386-mpx-simple_segv.c
>> @@ -0,0 +1,66 @@
>> +/* Copyright (C) 2015 Free Software Foundation, Inc.
>
> 2015-2016
>
>> +
>
>> diff --git a/gdb/testsuite/gdb.arch/i386-mpx-simple_segv.exp b/gdb/testsuite/gdb.arch/i386-mpx-simple_segv.exp
>
> Largely the same comments apply to this file. I'll only mention other
> extra things I spotted below.
>
>> +# Using the handler for SIGSEGV as "print pass stop"
>> +set parameters "print pass stop"
>> +runto_main
>
> Check result. Wrap with_test_prefix instead of appending
> $parameters, 0, 1, etc. in some test messages. That way,
> any fail within runto_main, etc. is covered as well, as well
> as being easier.
>
>> +send_gdb "handle SIGSEGV $parameters\n"
>> +send_gdb "continue\n"
>> +
>> +gdb_expect {
>> + -re $segv_bound_with_prompt {
>> + pass $parameters
>> + }
>> +}
>
> Use gdb_test/gdb_test_multiple instead of send_gdb/gdb_expect.
>
>> +gdb_test "handle SIGSEGV $parameters" "" "Setting\
>> +the handler for segfault 0"
>
> Odd line break. Break it before the string, not in the middle.
>
>> +
>> +gdb_test_multiple "continue" "test 0" {
>> + -re $segv_with_exit {
>> + pass $parameters
>> + }
>> + -re "$gdb_prompt $" {
>> + fail $parameters
>> + }
>> +}
>> +
>> +gdb_test "where" "No stack." "no inferior $parameters"
>> +
>> +# Using the handler for SIGSEGV as "print nopass stop"
>> +set parameters "print nopass stop"
>> +
>> +runto_main
>> +gdb_test "handle SIGSEGV $parameters" "" "Setting\
>> +the handler for segfault 1"
>
>
> Thanks,
> Pedro Alves
>
About your question on x32 and MPX.
MPX does not support 32bit pointers in 64bit mode.
Changes were just sent.
Thanks a lot for your review!
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
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH V5 4/5] Adaptation of siginfo fixup for the new bnd fields
2016-01-28 12:37 ` Pedro Alves
@ 2016-02-02 10:31 ` Pedro Alves
2016-02-02 13:58 ` Walfred Tedeschi
0 siblings, 1 reply; 18+ messages in thread
From: Pedro Alves @ 2016-02-02 10:31 UTC (permalink / raw)
To: Walfred Tedeschi, eliz, brobecker; +Cc: gdb-patches
(Moving your reply to this here, so we can keep this topic's
conversation properly threaded.)
Walfred Tedeschi wrote:
On 01/28/2016 12:37 PM, Pedro Alves wrote:
> On 01/22/2016 02:54 PM, Walfred Tedeschi wrote:
>
>> Caveat: No support for MPX on x32.
>
> What would it take to support that?
> About your question on x32 and MPX.
> MPX does not support 32bit pointers in 64bit mode.
Ok, so IIUC, it's not a GDB limitation, or a limitation of this patch set.
It's really that x32 can't support MPX at all. Correct?
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH V5 5/5] ntel MPX bound violation handling
2016-02-02 10:09 ` Walfred Tedeschi
@ 2016-02-02 10:38 ` Pedro Alves
2016-02-02 10:41 ` Walfred Tedeschi
0 siblings, 1 reply; 18+ messages in thread
From: Pedro Alves @ 2016-02-02 10:38 UTC (permalink / raw)
To: Walfred Tedeschi, eliz, brobecker; +Cc: gdb-patches
On 02/02/2016 10:09 AM, Walfred Tedeschi wrote:
> Am 1/28/2016 um 2:42 PM schrieb Pedro Alves:
> Pedro,
>
> I have tried to address all your comments.
I quickly skimmed v6 (will have to look in more detail), but looks like
you missed the renaming comments. See them quoted below.
> For the tests I also verified that there was only one line in the log as
> shown in the wiki.
Thanks.
> Have though some comments below:
>
>> # Function called when a segmentation fault with
>> # SIGCODE 3 (SIG_CODE_BOUNDARY_FAULT) is received by the inferior.
>>
>> But, see below.
>>> +
>>> +static void
>>> +handle_segmentation_faults (struct ui_out *uiout)
>>
>> "handle_segmentation_fault", singular.
...
>> And then the gdbarch hook can be renamed to a more generic
>> gdbarch_handle_segmentation_fault.
...
The comments thoughout should be
>> updated then, like, for this function:
>>
>> /* Some targets/architectures can do extra processing/display of
>> segmentation faults. E.g., Intel MPX boundary faults.
>> Call the architecture dependent function to handle the fault. */
>>
>> static void
>> handle_segmentation_fault (struct ui_out *uiout)
>> {
>>
(You did update the comment.)
>>> +for {set i 0} {$i < 15} {incr i} {
>>> + set message "MPX signal segv Upper: ${i}"
>>> + gdb_test_multiple "continue" "$message ${i}" {
>>> + -re $segv_upper_bound {
>>> + pass "$message"
>>> + }
>>> + -re ".*$inferior_exited_re normally.*$gdb_prompt $" {
>>> + fail "$message"
>>
>> The pass/fail calls are missing ${i}. Please make sure test
>> messages are unique in gdb.sum:
>
> In fail and pass i think we do not need the ${i} it is added at the
> message level already see:
>
> set message "MPX signal segv Upper: ${i}"
In that case, you shouldn't have an "${i}" in this line:
+ gdb_test_multiple "continue" "$message ${i}" {
because then that expands to "... ${i} ${i}".
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH V5 5/5] ntel MPX bound violation handling
2016-02-02 10:38 ` Pedro Alves
@ 2016-02-02 10:41 ` Walfred Tedeschi
0 siblings, 0 replies; 18+ messages in thread
From: Walfred Tedeschi @ 2016-02-02 10:41 UTC (permalink / raw)
To: Pedro Alves, eliz, brobecker; +Cc: gdb-patches
Am 2/2/2016 um 11:38 AM schrieb Pedro Alves:
> On 02/02/2016 10:09 AM, Walfred Tedeschi wrote:
>> Am 1/28/2016 um 2:42 PM schrieb Pedro Alves:
>> Pedro,
>>
>> I have tried to address all your comments.
>
> I quickly skimmed v6 (will have to look in more detail), but looks like
> you missed the renaming comments. See them quoted below.
>
>> For the tests I also verified that there was only one line in the log as
>> shown in the wiki.
>
> Thanks.
>
>> Have though some comments below:
>>
>
>>> # Function called when a segmentation fault with
>>> # SIGCODE 3 (SIG_CODE_BOUNDARY_FAULT) is received by the inferior.
>>>
>>> But, see below.
>
>>>> +
>>>> +static void
>>>> +handle_segmentation_faults (struct ui_out *uiout)
>>>
>>> "handle_segmentation_fault", singular.
>
> ...
>
>>> And then the gdbarch hook can be renamed to a more generic
>>> gdbarch_handle_segmentation_fault.
>
> ...
>
> The comments thoughout should be
>>> updated then, like, for this function:
>>>
>>> /* Some targets/architectures can do extra processing/display of
>>> segmentation faults. E.g., Intel MPX boundary faults.
>>> Call the architecture dependent function to handle the fault. */
>>>
>>> static void
>>> handle_segmentation_fault (struct ui_out *uiout)
>>> {
>>>
>
> (You did update the comment.)
>
>>>> +for {set i 0} {$i < 15} {incr i} {
>>>> + set message "MPX signal segv Upper: ${i}"
>>>> + gdb_test_multiple "continue" "$message ${i}" {
>>>> + -re $segv_upper_bound {
>>>> + pass "$message"
>>>> + }
>>>> + -re ".*$inferior_exited_re normally.*$gdb_prompt $" {
>>>> + fail "$message"
>>>
>>> The pass/fail calls are missing ${i}. Please make sure test
>>> messages are unique in gdb.sum:
>>
>> In fail and pass i think we do not need the ${i} it is added at the
>> message level already see:
>>
>> set message "MPX signal segv Upper: ${i}"
>
> In that case, you shouldn't have an "${i}" in this line:
>
> + gdb_test_multiple "continue" "$message ${i}" {
>
> because then that expands to "... ${i} ${i}".
>
> Thanks,
> Pedro Alves
>
Pedro,
Thanks a lot!
I will do it still today.
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
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH V5 4/5] Adaptation of siginfo fixup for the new bnd fields
2016-02-02 10:31 ` Pedro Alves
@ 2016-02-02 13:58 ` Walfred Tedeschi
0 siblings, 0 replies; 18+ messages in thread
From: Walfred Tedeschi @ 2016-02-02 13:58 UTC (permalink / raw)
To: Pedro Alves, eliz, brobecker; +Cc: gdb-patches
Am 2/2/2016 um 11:31 AM schrieb Pedro Alves:
> (Moving your reply to this here, so we can keep this topic's
> conversation properly threaded.)
>
> Walfred Tedeschi wrote:
>
> On 01/28/2016 12:37 PM, Pedro Alves wrote:
>> On 01/22/2016 02:54 PM, Walfred Tedeschi wrote:
>>
>>> Caveat: No support for MPX on x32.
>>
>> What would it take to support that?
>
>> About your question on x32 and MPX.
>> MPX does not support 32bit pointers in 64bit mode.
>
> Ok, so IIUC, it's not a GDB limitation, or a limitation of this patch set.
> It's really that x32 can't support MPX at all. Correct?
>
> Thanks,
> Pedro Alves
>
That is correct.
Thanks,
-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
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2016-02-02 13:58 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-22 15:36 [PATCH V5 0/5] Intel MPX bound violation support Walfred Tedeschi
2016-01-22 15:17 ` [PATCH V5 1/5] Preparation for new siginfo on Linux Walfred Tedeschi
2016-01-28 12:20 ` Pedro Alves
2016-01-22 15:17 ` [PATCH V5 3/5] Add bound related fields to the siginfo structure Walfred Tedeschi
2016-01-28 12:23 ` Pedro Alves
2016-01-22 15:17 ` [PATCH V5 4/5] Adaptation of siginfo fixup for the new bnd fields Walfred Tedeschi
2016-01-28 12:37 ` Pedro Alves
2016-02-02 10:31 ` Pedro Alves
2016-02-02 13:58 ` Walfred Tedeschi
2016-01-22 15:18 ` [PATCH V5 2/5] Use linux_get_siginfo_type_with_fields for x86 Walfred Tedeschi
2016-01-28 12:19 ` Pedro Alves
2016-01-22 15:18 ` [PATCH V5 5/5] ntel MPX bound violation handling Walfred Tedeschi
2016-01-28 13:43 ` Pedro Alves
2016-01-28 13:49 ` Walfred Tedeschi
2016-01-28 14:18 ` Pedro Alves
2016-02-02 10:09 ` Walfred Tedeschi
2016-02-02 10:38 ` Pedro Alves
2016-02-02 10:41 ` Walfred Tedeschi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).