RE: [PATCH v1] Intel(R) MPX - Bound violation handling.

Tedeschi, Walfred
2015-12-14T17:43:56Z

Joel and Pedro,

Thanks a lot for your feedback!

I Could address most of the comments in here.
An important one is still missing, namely this one:

> +{
> + long si_code;
> + struct regcache *regcache = get_current_regcache ();
> + struct gdbarch *gdbarch = get_regcache_arch (regcache);
> +
> + set_running (user_visible_resume_ptid (1), 0);

This is the part that _really_ concerns me, not necessary because I think it's wrong (although, it is a big red flag for me), but because I don't understand why it's needed, and how it will affect things.
(From Joel)
> + si_code = parse_and_eval_long ("$_siginfo.si_code\n");

During the debugging time I understood that inferior was stopped. Gdb is that was in the process to determine in which state the inferior was.
In this sense I set the flag at this point to allow for the evaluation.

I also looked in gdb for error handling while performing evaluations but did not find anything.
I am considering that the way to proceed is to use TRY and CATCH blocks. Would you recommend that?

Thanks and regards,
-Fred

-----Original Message-----
From: Joel Brobecker [mailto:brobecker@adacore.com]
Sent: Thursday, November 19, 2015 1:02 AM
To: Tedeschi, Walfred
Cc: palves@redhat.com; gdb-patches@sourceware.org
Subject: Re: [PATCH v1] Intel(R) MPX - Bound violation handling.

> With Intel(R) Memory Protection Extensions it was introduced the
> concept of boundary violation. A boundary violations is presented to
> the inferior as a segmentation fault having as sigcode the value 3.
> This patch adds a handler for a boundary violation extending the
> information displayed when a bound violation is presented to the inferior (segfault with code 3).
> In this case the debugger will also display the kind of violation
> upper or lower the violated bounds and the address accessed.
>
>
> Thanks a lot for your support!
>
> Changelog:
>
> 2015-07-21 Walfred Tedeschi <walfred.tedeschi@intel.com>
>
> * amd64-linux-tdep.c (amd64_linux_init_abi_common):
> Add handler for bound violation signal.
> * gdbarch.sh (bound_violation_handler): New.
> * 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 (process_segmentation_faults): New.
> (print_signal_received_reason): Use process_segmentation_faults.
>
> testsuite/gdb.arch
> * i386-mpx-sigsegv.c: New.
> * i386-mpx-sigsegv.exp: New.
> * i386-mpx-simple_segv.c: New.
> * i386-mpx-simple_segv.exp: New.

Same here, I would really like to have Pedro's perspective on this patch. Here are my comments.

First, it would be nice to have an example of the patch as work.
What was the output looking like before, and what does it look like with your patch applied (assuming kernel & glibc support).

Answering some of your questions...

> There some open points to be discussed though before asking permission
> to commit.
> They are:
> 1. infrun.c (process_segmentation_faults): The inferior is stopped to
> allow doing some evaluations. I have seen no side effect on doing
> that, on the other hand it does not look so natural to me.

Hmmmm... Isn't the inferior already stopped due to the signal anyway?

> 2. i386-linux-tdep.c (i386_mpx_bound_violation_handler): I was
> wondering if the right place for it wouldn't be the linux-tdep.c as it
> is done for the siginfo things. This is a new structure in the
> kernel. Doing at the linux-tdep.c documentation should be simplified
> and all architectures providing that functionality in a different way
> only have to set the right function pointer.

Possibly. Probably. Do you know if this applies to arm-linux as well, for instance?

> diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh index a13d9b9..b4d231c
> 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
>
> +# Bound violation can be handled differently accross architectures.
> +# UIOUT is the output stream where the handler will place information.
> +# si_code is the value of the field with the same name in the siginfo structure.
> +F:void:bound_violation_handler:struct ui_out *uiout, long
> +si_code:uiout, si_code

For this function's documention, it would be better to describe what this function is supposed to do. If it helps, I would explain when the function is called.

/* Called when a bla bla bla due to a bound violation bla bla.
Print bla bla bla on UIOUT. SI_CODE is ... */

But the way this is currently implemented seems to indicate it is called for any SEGV... Is that what "bound violation" means? I don't think so, right? As far as I can tell, it's only when SI_CODE is SIG_CODE_BONDARY_FAULT...

So the function seems to be misnamed.

> M:char *:make_corefile_notes:bfd *obfd, int *note_size:obfd,
> note_size
>
> +

Seems like an unnecessary change.

> +/* Code for faulty boundary has to be introduced. */ #define
> +SIG_CODE_BONDARY_FAULT 3
> +
> +void
> +i386_mpx_bound_violation_handler (struct ui_out *uiout, long si_code)

Every new function must be documented via a small introductory comment.
In this case, since it implements a gdbarch "method", a trivial comment like below is sufficient:

/* Implement theh "bound_violation_handler" gdbarch method. */

> + lower_bound = parse_and_eval_long (
> + "$_siginfo._sifields._sigfault._addr_bnd._lower\n");
> + upper_bound = parse_and_eval_long (
> + "$_siginfo._sifields._sigfault._addr_bnd._upper\n");
> + access = parse_and_eval_long (
> + "$_siginfo._sifields._sigfault.si_addr\n");

The formatting needs to be adjusted. And I don't think you need the \n at the end. Thus:

lower_bound
= parse_and_eval_long ("$_siginfo._sifields._sigfault._addr_bnd._lower");

But more importantly, I am not sure you really want to use parse_and_eval_long, here. It's convenient, sure, but then avoids questions like error handling. For instance, if an issue happens during evaluation and causes an exception to be raised, are you OK letting it propagate. I personally wouldn't.

> + is_upper = (access > upper_bound ? 1 : 0);
> +
> + if (ui_out_is_mi_like_p (uiout))
> + {
> + 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_field_fmt (uiout,"lower-bound",
> + "0x%lx",
> + lower_bound);
> + ui_out_field_fmt (uiout,"upper-bound",
> + "0x%lx",
> + upper_bound);
> + ui_out_field_fmt (uiout,"bound-access",
> + "0x%lx",
> + access);
> + }
> + else
> + {
> + if (is_upper)
> + ui_out_field_string (uiout, "sigcode-meaning",
> + "\nupper bound violation");
> + else
> + ui_out_field_string (uiout, "sigcode-meaning",
> + "\nlower bound violation");
> +
> + ui_out_field_string (uiout, "bounds",
> + " - bounds");
> + ui_out_field_fmt (uiout,"bound-values",
> + " {lbound = 0x%lx, ubound = 0x%lx}",
> + lower_bound,
> + upper_bound);
> + ui_out_field_fmt (uiout,"bound-access",
> + " accessing 0x%lx",
> + access);
> + }

I think you can do this more simply. Something like this:

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, " - bounds {lbound = ");
ui_out_field_fmt (uiout, "lower-bound", paddress (lower_bound));
ui_out_text (uiout, ", ubound = ");
ui_out_field_fmt (uiout, "upper-bound", paddress (upper_bound));

etc etc etc. To be verified, though, especially in the GDB/MI output, to make sure the ui_out_text calls do not bleed text into the GDB/MI output.

Note a couple of important little details:

- There are a number of formatting issues (space after comma in
function call, alignment issues on multi-line statements, etc).
Our coding standard is documented at:
https://sourceware.org/gdb/wiki/Internals%20GDB-C-Coding-Standards

- do not print a CORE_ADDR using %0xlx. Use paddress instead.
This requires a gdbarch, but you can get that by changing
the type of your gdbarch to a method (IIRC).

If I may say so, the new output might be a little heavy. Wouldn't there be a way to do this more on demand?

> +/* Handles and displays information related to the MPX bound violation
> + to the user. */
> +void
> +i386_mpx_bound_violation_handler (struct ui_out *uiout, long
> +si_code);

Ah - I missed that one when I said all function should be documented.
There are two school of thoughts within the GDB group; some want the documentation to be next to the implementation (in the .c file, some think it's more natural to be in the .h file, which people can then browse when they are looking for something). The consensus is that, if you are going to document in the .h file, then make sure there is a note of it in the .c file. Something like:

/* See "i386-linux-tdep.h". */

More specifically for this function, though, because it implements a hook that has already been documented, it's better to refer to the gdbarch method's documentation. That way, if we have to modify the gdbarch method, we only have to change the documentation once.

> +/* Verify if target is MPX enabled. */ extern int i386_mpx_enabled
> +(void);

Already documented in the .c file, so unnecesarry here.

> +static void
> +process_segmentation_faults (struct ui_out * uiout)

Requires documentation, as said before.
No space after the "*" -> "struct ui_out *uiout".

This is nitpicking, so feel free to ignore, but I think we've been using the term "handle" rather than "process" for this kinds of things.

> +{
> + long si_code;
> + struct regcache *regcache = get_current_regcache ();
> + struct gdbarch *gdbarch = get_regcache_arch (regcache);
> +
> + set_running (user_visible_resume_ptid (1), 0);

This is the part that _really_ concerns me, not necessary because I think it's wrong (although, it is a big red flag for me), but because I don't understand why it's needed, and how it will affect things.

> + si_code = parse_and_eval_long ("$_siginfo.si_code\n");

Same remarks as above.

> + if (gdbarch_bound_violation_handler_p (gdbarch))
> + gdbarch_bound_violation_handler (gdbarch, uiout, si_code); }
> +
> void
> print_signal_received_reason (struct ui_out *uiout, enum gdb_signal
> siggnal) { @@ -7773,6 +7787,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)
> + process_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..5f20aa2
> --- /dev/null
> +++ b/gdb/testsuite/gdb.arch/i386-mpx-sigsegv.c
> @@ -0,0 +1,128 @@
> +/*
> +* Copyright 2012 Free Software Foundation, Inc.
> +*
> +* Contributed by Intel Corp. <christian.himpel@intel.com>,
> +* <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/>.
> +*
> +* This test was converted from
> +idb/test/td/runreg/Biendian/bi.c_bitfield
> +*

No leading '*' in multi-line comments (Gnu Coding Standards).
Also, I would remove the reference to idb/[...]/bi.c_bitfield unless it is something from the GDB project itself.

One tiny but important nit - the copyright year should at least include 2015. If the file was first create in 2012, then it should be 2012-2015.

> +#include <stdio.h>

Do you need stdio.h, for this test?

> +#include "x86-cpuid.h"
> +
> +
> +#define MYTYPE int

Is that really useful?

> +#define OUR_SIZE 5
> +
> +MYTYPE gx[OUR_SIZE];
> +MYTYPE ga[OUR_SIZE];
> +MYTYPE gb[OUR_SIZE];
> +MYTYPE gc[OUR_SIZE];
> +MYTYPE 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 (MYTYPE value)
> +{
> + return 1;
> +}
> +
> +int
> +bp2 (MYTYPE value)
> +{
> + return 1;
> +}
> +
> +void
> +upper (MYTYPE * p, MYTYPE * a, MYTYPE * b, MYTYPE * c, MYTYPE * d,
> +int len) {
> + MYTYPE value;
> + value = *(p + len);
> + value = *(a + len);
> + value = *(b + len);
> + value = *(c + len);
> + value = *(d + len);
> +}
> +
> +void
> +lower (MYTYPE * p, MYTYPE * a, MYTYPE * b, MYTYPE * c, MYTYPE * d,
> +int len) {
> + MYTYPE value;
> + value = *(p - len);
> + value = *(a - len);
> + value = *(b - len);
> + value = *(c - len);
> + bp2 (value);
> + value = *(d - len);
> +}
> +
> +
> +int
> +main (void)
> +{
> + if (have_mpx ())
> + {
> + MYTYPE sx[OUR_SIZE];
> + MYTYPE sa[OUR_SIZE];
> + MYTYPE sb[OUR_SIZE];
> + MYTYPE sc[OUR_SIZE];
> + MYTYPE sd[OUR_SIZE];
> + MYTYPE *x, *a, *b, *c, *d;
> +
> + x = calloc (OUR_SIZE, sizeof (MYTYPE));
> + a = calloc (OUR_SIZE, sizeof (MYTYPE));
> + b = calloc (OUR_SIZE, sizeof (MYTYPE));
> + c = calloc (OUR_SIZE, sizeof (MYTYPE));
> + d = calloc (OUR_SIZE, sizeof (MYTYPE));
> +
> + 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..f689018
> --- /dev/null
> +++ b/gdb/testsuite/gdb.arch/i386-mpx-sigsegv.exp
> @@ -0,0 +1,95 @@
> +# Copyright 2012 Free Software Foundation, Inc.

Same comment about the copyright years.

> +# 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
> + }
> + -re ".*$gdb_prompt $" {
> + fail "check whether processor supports MPX"
> + }
> + timeout {
> + fail "check whether processor supports MPX (timeout)"
> + }

You don't need the last two branches (already taken care of by gdb_test_multiple).

> +set segv_lower_bound ".*Program received signal SIGSEGV,\
> + Segmentation fault\r\nlower bound violation - bounds \\\{lbound\
> + = 0x\[0-9a-fA-F\]+, ubound = 0x\[0-9a-fA-F\]+\\\} accessing\
> + 0x\[0-9a-fA-F\]+.*$gdb_prompt $"
> +
> +set segv_upper_bound ".*Program received signal SIGSEGV,\
> + Segmentation fault\r\nupper bound violation - bounds \\\{lbound\
> + = 0x\[0-9a-fA-F\]+, ubound = 0x\[0-9a-fA-F\]+\\\} accessing\
> + 0x\[0-9a-fA-F\]+.*$gdb_prompt $"
> +
> +for {set i 0} {$i < 15} {incr i} {
> + set message "MPX signal segv Upper: ${i}"
> + send_gdb "continue\n"
> + gdb_expect {
> + -re $segv_upper_bound
> + { pass "$message" }
> + -re ".*$inferior_exited_re normally.*$gdb_prompt $"
> + {
> + fail "$message"
> + break
> + }

I think you can use gdb_test_multiple, here, no? We avoid using gdb_expect like the pest. For instance, you're missing a lot of the stuff that gdb_test_multiple does for you.

The formatting and indentation are not what we usually do:

gdb_test_multiple [...] {
-re "something" {
pass message
}
-re "something else" {
fail message
}
}

> +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"
> +}
> +
> +send_gdb "quit\n"

Unecessary.

> 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..407086b
> --- /dev/null
> +++ b/gdb/testsuite/gdb.arch/i386-mpx-simple_segv.c
> @@ -0,0 +1,70 @@
> +/*
> +* Copyright 2012 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 <stdio.h>
> +#include "x86-cpuid.h"
> +
> +#define MYTYPE int
> +#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 (MYTYPE * p, int len)
> +{
> + MYTYPE value;
> + len++; /* b0-size-test. */
> + value = *(p + len);
> +}
> +
> +int
> +main (void)
> +{
> + if (have_mpx ())
> + {
> + int a = 0; /* Dummy variable for debugging purposes. */

Can you indent the comment a little less so as to avoid exceeding 80 chars?

> + MYTYPE 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..988a8b4
> --- /dev/null
> +++ b/gdb/testsuite/gdb.arch/i386-mpx-simple_segv.exp
> @@ -0,0 +1,130 @@
> +# Copyright 2013 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 {

Same comment as above about 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
> + }
> + -re ".*$gdb_prompt $" {
> + fail "check whether processor supports MPX"
> + }
> + timeout {
> + fail "check whether processor supports MPX (timeout)"
> + }
> +}
> +
> +set segv_bound_with_prompt ".*Program received signal SIGSEGV,\
> + Segmentation fault\r\nupper bound violation - bounds \\\{lbound\
> + = 0x\[0-9a-fA-F\]+, ubound = 0x\[0-9a-fA-F\]+\\\} accessing\
> + 0x\[0-9a-fA-F\]+.*$gdb_prompt $"
> +
> +set segv_bound_with_exit ".*Program received signal SIGSEGV,\
> + Segmentation fault\r\nupper bound violation - bounds \\\{lbound\
> + = 0x\[0-9a-fA-F\]+, ubound = 0x\[0-9a-fA-F\]+\\\} accessing\
> + 0x\[0-9a-fA-F\]+.*$inferior_exited_re.*"
> +
> +# Using the handler for SIGSEGV as "print pass stop"
> +set parameters "print pass stop"
> +runto main

Can you use runto_main instead?

[I'm running out of time fo now, but I think the rest is going to be more of the same comments]

Bye!

> +send_gdb "handle SIGSEGV $parameters\n"
> +send_gdb "continue\n"
> +
> +gdb_expect {
> + -re $segv_bound_with_prompt
> + { pass $parameters}
> + timeout { fail $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_bound_with_exit
> + { pass $parameters}
> + -re "$gdb_prompt $"
> + { fail $parameters }
> + timeout { 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}
> + timeout
> + { fail $parameters }
> +}
> +
> +gdb_test "where" ".*#0 0x\[0-9a-fA-F\]+ 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}
> + timeout { fail $parameters }
> +}
> +
> +gdb_test "where" "No stack." "no inferior $parameters"
> +
> +send_gdb "quit\n"
> +
> --
> 2.1.4

--
Joel
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