This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH V5 5/5] ntel MPX bound violation handling
- From: Pedro Alves <palves at redhat dot com>
- To: Walfred Tedeschi <walfred dot tedeschi at intel dot com>, eliz at gnu dot org, brobecker at adacore dot com
- Cc: gdb-patches at sourceware dot org
- Date: Thu, 28 Jan 2016 13:42:53 +0000
- Subject: Re: [PATCH V5 5/5] ntel MPX bound violation handling
- Authentication-results: sourceware.org; auth=none
- References: <1453474456-13169-1-git-send-email-walfred dot tedeschi at intel dot com> <1453474456-13169-6-git-send-email-walfred dot tedeschi at intel dot com>
"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);
>
>
> /* 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