This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: Calling 'abort' on bounds violations in libmpx
- From: Ilya Enkovich <enkovich dot gnu at gmail dot com>
- To: Alexander Ivchenko <aivchenk at gmail dot com>
- Cc: GCC Patches <gcc-patches at gcc dot gnu dot org>
- Date: Tue, 29 Nov 2016 20:22:17 +0300
- Subject: Re: Calling 'abort' on bounds violations in libmpx
- Authentication-results: sourceware.org; auth=none
- References: <CACysShjZ8uMs3XCYA+-buXPS-ovYRs2jKfSeKStMrCD6hKLE+w@mail.gmail.com>
2016-11-29 17:43 GMT+03:00 Alexander Ivchenko <aivchenk@gmail.com>:
> Hi,
>
> Attached patch is addressing PR67520. Would that approach work for the
> problem? Should I also change the version of the library?
Hi!
Overall patch is OK. But you need to change version because you
change default behavior. How did you test it? Did you check default
behavior change doesn't affect existing runtime MPX tests? Can we
add new ones?
Thanks,
Ilya
>
> 2016-11-29 Alexander Ivchenko <alexander.ivchenko@intel.com>
>
> * mpxrt/mpxrt-utils.c (set_mpx_rt_stop_handler): New function.
> (print_help): Add help for CHKP_RT_STOP_HANDLER environment
> variable.
> (__mpxrt_init_env_vars): Add initialization of stop_handler.
> (__mpxrt_stop_handler): New function.
> (__mpxrt_stop): Ditto.
> * mpxrt/mpxrt-utils.h (mpx_rt_stop_mode_handler_t): New enum.
>
>
>
> diff --git a/libmpx/mpxrt/mpxrt-utils.c b/libmpx/mpxrt/mpxrt-utils.c
> index 057a355..63ee7c6 100644
> --- a/libmpx/mpxrt/mpxrt-utils.c
> +++ b/libmpx/mpxrt/mpxrt-utils.c
> @@ -60,6 +60,9 @@
> #define MPX_RT_MODE "CHKP_RT_MODE"
> #define MPX_RT_MODE_DEFAULT MPX_RT_COUNT
> #define MPX_RT_MODE_DEFAULT_STR "count"
> +#define MPX_RT_STOP_HANDLER "CHKP_RT_STOP_HANDLER"
> +#define MPX_RT_STOP_HANDLER_DEFAULT MPX_RT_STOP_HANDLER_ABORT
> +#define MPX_RT_STOP_HANDLER_DEFAULT_STR "abort"
> #define MPX_RT_HELP "CHKP_RT_HELP"
> #define MPX_RT_ADDPID "CHKP_RT_ADDPID"
> #define MPX_RT_BNDPRESERVE "CHKP_RT_BNDPRESERVE"
> @@ -84,6 +87,7 @@ typedef struct {
> static int summary;
> static int add_pid;
> static mpx_rt_mode_t mode;
> +static mpx_rt_stop_mode_handler_t stop_handler;
> static env_var_list_t env_var_list;
> static verbose_type verbose_val;
> static FILE *out;
> @@ -226,6 +230,23 @@ set_mpx_rt_mode (const char *env)
> }
> }
>
> +static mpx_rt_stop_mode_handler_t
> +set_mpx_rt_stop_handler (const char *env)
> +{
> + if (env == 0)
> + return MPX_RT_STOP_HANDLER_DEFAULT;
> + else if (strcmp (env, "abort") == 0)
> + return MPX_RT_STOP_HANDLER_ABORT;
> + else if (strcmp (env, "exit") == 0)
> + return MPX_RT_STOP_HANDLER_EXIT;
> + {
> + __mpxrt_print (VERB_ERROR, "Illegal value '%s' for %s. Legal values are"
> + "[abort | exit]\nUsing default value %s\n",
> + env, MPX_RT_STOP_HANDLER, MPX_RT_STOP_HANDLER_DEFAULT);
> + return MPX_RT_STOP_HANDLER_DEFAULT;
> + }
> +}
> +
> static void
> print_help (void)
> {
> @@ -244,6 +265,11 @@ print_help (void)
> fprintf (out, "%s \t\t set MPX runtime behavior on #BR exception."
> " [stop | count]\n"
> "\t\t\t [default: %s]\n", MPX_RT_MODE, MPX_RT_MODE_DEFAULT_STR);
> + fprintf (out, "%s \t set the handler function MPX runtime will call\n"
> + "\t\t\t on #BR exception when %s is set to \'stop\'."
> + " [abort | exit]\n"
> + "\t\t\t [default: %s]\n", MPX_RT_STOP_HANDLER, MPX_RT_MODE,
> + MPX_RT_STOP_HANDLER_DEFAULT_STR);
> fprintf (out, "%s \t\t generate out,err file for each process.\n"
> "\t\t\t generated file will be MPX_RT_{OUT,ERR}_FILE.pid\n"
> "\t\t\t [default: no]\n", MPX_RT_ADDPID);
> @@ -357,6 +383,10 @@ __mpxrt_init_env_vars (int* bndpreserve)
> env_var_list_add (MPX_RT_MODE, env);
> mode = set_mpx_rt_mode (env);
>
> + env = secure_getenv (MPX_RT_STOP_HANDLER);
> + env_var_list_add (MPX_RT_STOP_HANDLER, env);
> + stop_handler = set_mpx_rt_stop_handler (env);
> +
> env = secure_getenv (MPX_RT_BNDPRESERVE);
> env_var_list_add (MPX_RT_BNDPRESERVE, env);
> validate_bndpreserve (env, bndpreserve);
> @@ -487,6 +517,22 @@ __mpxrt_mode (void)
> return mode;
> }
>
> +mpx_rt_mode_t
> +__mpxrt_stop_handler (void)
> +{
> + return stop_handler;
> +}
> +
> +void __attribute__ ((noreturn))
> +__mpxrt_stop (void)
> +{
> + if (__mpxrt_stop_handler () == MPX_RT_STOP_HANDLER_ABORT)
> + abort ();
> + else if (__mpxrt_stop_handler () == MPX_RT_STOP_HANDLER_EXIT)
> + exit (255);
> + __builtin_unreachable ();
> +}
> +
> void
> __mpxrt_print_summary (uint64_t num_brs, uint64_t l1_size)
> {
> diff --git a/libmpx/mpxrt/mpxrt-utils.h b/libmpx/mpxrt/mpxrt-utils.h
> index d62937d..6da12cc 100644
> --- a/libmpx/mpxrt/mpxrt-utils.h
> +++ b/libmpx/mpxrt/mpxrt-utils.h
> @@ -54,6 +54,11 @@ typedef enum {
> MPX_RT_STOP
> } mpx_rt_mode_t;
>
> +typedef enum {
> + MPX_RT_STOP_HANDLER_ABORT,
> + MPX_RT_STOP_HANDLER_EXIT
> +} mpx_rt_stop_mode_handler_t;
> +
> void __mpxrt_init_env_vars (int* bndpreserve);
> void __mpxrt_write_uint (verbose_type vt, uint64_t val, unsigned base);
> void __mpxrt_write (verbose_type vt, const char* str);
> diff --git a/libmpx/mpxrt/mpxrt.c b/libmpx/mpxrt/mpxrt.c
> index b52906b..0bc069c 100644
> --- a/libmpx/mpxrt/mpxrt.c
> +++ b/libmpx/mpxrt/mpxrt.c
> @@ -252,7 +251,7 @@ handler (int sig __attribute__ ((unused)),
> uctxt->uc_mcontext.gregs[REG_IP_IDX] =
> (greg_t)get_next_inst_ip ((uint8_t *)ip);
> if (__mpxrt_mode () == MPX_RT_STOP)
> - exit (255);
> + __mpxrt_stop ();
> return;
>
> default:
> @@ -269,7 +268,7 @@ handler (int sig __attribute__ ((unused)),
> __mpxrt_write (VERB_ERROR, ", ip = 0x");
> __mpxrt_write_uint (VERB_ERROR, ip, 16);
> __mpxrt_write (VERB_ERROR, "\n");
> - exit (255);
> + __mpxrt_stop ();
> }
> else
> {
> @@ -278,7 +277,7 @@ handler (int sig __attribute__ ((unused)),
> __mpxrt_write (VERB_ERROR, "! at 0x");
> __mpxrt_write_uint (VERB_ERROR, ip, 16);
> __mpxrt_write (VERB_ERROR, "\n");
> - exit (255);
> + __mpxrt_stop ();
> }
> }
>
> thanks,
> Alexander