Handle -fsanitize=* in lto-wrapper (PR lto/69254)

Hi!

Here is an attempt to handle -f{,no-}sanitize= options in LTO wrapper.
In addition to that I've noticed ICEs e.g. if some OpenMP code is compiled
with -c -flto -fopenmp, but final link is -fno-openmp, similarly for
openacc, -fcilkplus is similar but used to be handled even less.

The intended behavior for -f{,no-}sanitize= is that for the ubsan
sanitizers which are typically lowered before IPA, but are often using
builtins that need initialization even at the LTO level, we collect
from each TU info on whether any ubsan sanitizers have been enabled
(note, this needs parsing of the options, because we can e.g. have 
-fsanitize=shift,return -fno-sanitize=undefined -fsanitize=integer-divide-by-zero
) and turn that into -fsanitize=shift from all the TUs if any of them
needed any (randomly chosen sanitizer that is handled by FEs only).
For address or thread sanitizers, which are handled solely post IPA,
the choice whether to sanitize is left to the linker command line.
And finally we need to ensure that e.g. -fno-sanitize=address,shift
doesn't turn off the ubsan sanitizers.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2016-01-25  Jakub Jelinek  <jakub@redhat.com>

	PR lto/69254
	* opts.h (parse_sanitizer_options): New prototype.
	* opts.c (sanitizer_opts): New array.
	(parse_sanitizer_options): New function.
	(common_handle_option): Use parse_sanitizer_options.
	* lto-opts.c (lto_write_options): Write also -f{,no-}sanitize=
	options.
	* lto-wrapper.c (sanitize_shift_decoded_opt): New function.
	(merge_and_complain): Determine if any -fsanitize= options
	enabled at the end any undefined behavior sanitizers, and
	append -fsanitize=shift if needed.  Handle -fcilkplus.
	(append_compiler_options): Handle -fcilkplus and -fsanitize=.
	(append_linker_options): Ignore -fno-{openmp,openacc,cilkplus}.
	(find_and_merge_options): Canonicalize -fsanitize= options.
	(run_gcc): Append -fsanitize=shift if compiler options set it
	and linker options might override it.


	Jakub

On 01/25/2016 09:30 PM, Jakub Jelinek wrote:

> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

I've been staring at it for a while, and on the whole I think I can make 
sense of this. However - it does not have test coverage. Can this be 
added? Also, is this a regression?

> 	(parse_sanitizer_options): New function.
> 	(common_handle_option): Use parse_sanitizer_options.

I think this can go in, it just moves code around, right? That'll make 
followup patches smaller.

> --- gcc/lto-opts.c.jj	2016-01-23 00:13:00.897015402 +0100
> +++ gcc/lto-opts.c	2016-01-25 14:06:31.834127398 +0100
> @@ -199,9 +199,11 @@ lto_write_options (void)
>         /* Also drop all options that are handled by the driver as well,
>   	 which includes things like -o and -v or -fhelp for example.
>   	 We do not need those.  The only exception is -foffload option, if we
> -	 write it in offload_lto section.  Also drop all diagnostic options.  */
> +	 write it in offload_lto section.  Also drop all diagnostic options,
> +	 and -fsanitize=.  */
>         if ((cl_options[option->opt_index].flags & (CL_DRIVER|CL_WARNING))
> -	  && (!lto_stream_offload_p || option->opt_index != OPT_foffload_))
> +	  && (!lto_stream_offload_p || option->opt_index != OPT_foffload_)
> +	  && option->opt_index != OPT_fsanitize_)
>   	continue;

This one puzzles me, doesn't it mean that no sanitizer options make it 
into the LTO stream, which would mean the new code in lto-wrapper 
doesn't trigger?

> +/* Set *OPT to decoded -f{,no-}sanitize=shift.  */

I think I'd like some sort of comment about how this is an arbitrary 
choice, just designed to enable DEF_SANITIZER_BUILTIN (IIUC). Also, why 
use shift and not just sanitize=undefined?

> @@ -392,6 +415,24 @@ merge_and_complain (struct cl_decoded_op
>   	  break;
>   	}
>       }
> +
> +  /* If FDECODED_OPTIONS requested any ubsan sanitization, pass through

I think we want to add "and DECODED_OPTIONS doesn't", right?


Bernd

On Tue, Jan 26, 2016 at 01:54:52PM +0100, Bernd Schmidt wrote:
> On 01/25/2016 09:30 PM, Jakub Jelinek wrote:
> 
> >Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> 
> I've been staring at it for a while, and on the whole I think I can make
> sense of this. However - it does not have test coverage. Can this be added?

I have trouble creating those.  Because this needs to pass different
compile and different linker options, so I think only lto.exp framework
supports this, but then needs options (-fopenmp, -fopenacc, -fcilkplus,
-fsanitize=) that usually aren't unconditionally available, need some
library and/or target support, and thus such tests generally go into test
subdirectories where all that is tested.

> Also, is this a regression?

Andrew Pinski said so, but because further compiler emitted sanitizers have
been added over time into -fsanitize=undefined, it very likely is a
regression at least in regards to those sanitizers (compile with
-c -flto -fsanitize=undefined something that didn't result in any
sanitization in say 4.9, but now has some of the newer sanitizers, e.g.
__attribute__((nonnull)) char *foo (char *);
char *bar (char *p) { return foo (p); }
and then link without -fsanitize=undefined or with -fno-sanitize=undefined.

> >	(parse_sanitizer_options): New function.
> >	(common_handle_option): Use parse_sanitizer_options.
> 
> I think this can go in, it just moves code around, right? That'll make
> followup patches smaller.

Yeah, committed.
> 
> >--- gcc/lto-opts.c.jj	2016-01-23 00:13:00.897015402 +0100
> >+++ gcc/lto-opts.c	2016-01-25 14:06:31.834127398 +0100
> >@@ -199,9 +199,11 @@ lto_write_options (void)
> >        /* Also drop all options that are handled by the driver as well,
> >  	 which includes things like -o and -v or -fhelp for example.
> >  	 We do not need those.  The only exception is -foffload option, if we
> >-	 write it in offload_lto section.  Also drop all diagnostic options.  */
> >+	 write it in offload_lto section.  Also drop all diagnostic options,
> >+	 and -fsanitize=.  */
> >        if ((cl_options[option->opt_index].flags & (CL_DRIVER|CL_WARNING))
> >-	  && (!lto_stream_offload_p || option->opt_index != OPT_foffload_))
> >+	  && (!lto_stream_offload_p || option->opt_index != OPT_foffload_)
> >+	  && option->opt_index != OPT_fsanitize_)
> >  	continue;
> 
> This one puzzles me, doesn't it mean that no sanitizer options make it into
> the LTO stream, which would mean the new code in lto-wrapper doesn't
> trigger?

Sounds like an error in the comment change only to me, I meant to say that
the only exceptions are now
1) -foffload if we write it in offload_lto section
2) -fsanitize= (regardless where we write it)
The continue causes the given option not to be written into the LTO opts
subsection, which is the case here for CL_DRIVER/CL_WARNING options
(-fsanitize= is CL_DRIVER), with the exception of -foffload= under some
condition and with the exception of -fsanitize= (newly).  I.e. the change
is that -fsanitize= used to be ignored, not writte into LTO opts, while with
the change it is written in there.

> >+/* Set *OPT to decoded -f{,no-}sanitize=shift.  */
> 
> I think I'd like some sort of comment about how this is an arbitrary choice,

Ok, will try to write some comment.

> just designed to enable DEF_SANITIZER_BUILTIN (IIUC). Also, why use shift
> and not just sanitize=undefined?

Because -fsanitize=undefined is a large collection of individual sanitizers,
and at least some of them affect also post-IPA code (e.g.
-fsanitize=unreachable).  The goal is to pick one of the sanitizers that are
handled solely pre-IPA only, after that just are present in form of a
builtin call in the IL (and thus all that lto1 needs to do for that option
is initialize the builtins).

> >@@ -392,6 +415,24 @@ merge_and_complain (struct cl_decoded_op
> >  	  break;
> >  	}
> >      }
> >+
> >+  /* If FDECODED_OPTIONS requested any ubsan sanitization, pass through
> 
> I think we want to add "and DECODED_OPTIONS doesn't", right?

Maybe.  It also has to check the option->value though.  The thing is,
because I didn't want to memmove options around when processing the first
TU, it will canonicalize all the -f{,no-}sanitize= options depending on if
in the end some ubsan sanitizers were enabled or not to
-f{,no-}sanitize=shift.  And merge_and_complain thus should ignore
all the -fno-sanitize=shift options in DECODED_OPTIONS, if it finds any
-fsanitize=shift in DECODED_OPTIONS (optionally preceeded by some
-fno-sanitize=shift), then nothing needs to be added, otherwise
-fsanitize=shift is added.

	Jakub

On Mon, 25 Jan 2016, Jakub Jelinek wrote:

> Hi!
> 
> Here is an attempt to handle -f{,no-}sanitize= options in LTO wrapper.
> In addition to that I've noticed ICEs e.g. if some OpenMP code is compiled
> with -c -flto -fopenmp, but final link is -fno-openmp, similarly for
> openacc, -fcilkplus is similar but used to be handled even less.
> 
> The intended behavior for -f{,no-}sanitize= is that for the ubsan
> sanitizers which are typically lowered before IPA, but are often using
> builtins that need initialization even at the LTO level, we collect
> from each TU info on whether any ubsan sanitizers have been enabled
> (note, this needs parsing of the options, because we can e.g. have 
> -fsanitize=shift,return -fno-sanitize=undefined -fsanitize=integer-divide-by-zero
> ) and turn that into -fsanitize=shift from all the TUs if any of them
> needed any (randomly chosen sanitizer that is handled by FEs only).
> For address or thread sanitizers, which are handled solely post IPA,
> the choice whether to sanitize is left to the linker command line.
> And finally we need to ensure that e.g. -fno-sanitize=address,shift
> doesn't turn off the ubsan sanitizers.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Can you split out the non -fsanitize part?  It is ok.

I'm somewhat confused about that you drop -fsanitize options from
the LTO options section writing in lto-opts.c but then add code to
parse it from there in lto-wrapper.c.  The code there also looks
somewhat duplicated - why not just canonicalize any -fsanitize=
option coming in to the first in merge_and_complain
and special-case it in append_compiler_options again by say

  case OPT_fsanitize_:
     obstack_ptr_grow (argv_obstack, "-fsanitize=shift");

?

Thanks,
Richard.

> 2016-01-25  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR lto/69254
> 	* opts.h (parse_sanitizer_options): New prototype.
> 	* opts.c (sanitizer_opts): New array.
> 	(parse_sanitizer_options): New function.
> 	(common_handle_option): Use parse_sanitizer_options.
> 	* lto-opts.c (lto_write_options): Write also -f{,no-}sanitize=
> 	options.
> 	* lto-wrapper.c (sanitize_shift_decoded_opt): New function.
> 	(merge_and_complain): Determine if any -fsanitize= options
> 	enabled at the end any undefined behavior sanitizers, and
> 	append -fsanitize=shift if needed.  Handle -fcilkplus.
> 	(append_compiler_options): Handle -fcilkplus and -fsanitize=.
> 	(append_linker_options): Ignore -fno-{openmp,openacc,cilkplus}.
> 	(find_and_merge_options): Canonicalize -fsanitize= options.
> 	(run_gcc): Append -fsanitize=shift if compiler options set it
> 	and linker options might override it.
> 
> --- gcc/opts.h.jj	2016-01-23 00:13:00.714017906 +0100
> +++ gcc/opts.h	2016-01-25 14:06:31.833127411 +0100
> @@ -372,6 +372,8 @@ extern void control_warning_option (unsi
>  extern char *write_langs (unsigned int mask);
>  extern void print_ignored_options (void);
>  extern void handle_common_deferred_options (void);
> +unsigned int parse_sanitizer_options (const char *, location_t, int,
> +				      unsigned int, int, bool);
>  extern bool common_handle_option (struct gcc_options *opts,
>  				  struct gcc_options *opts_set,
>  				  const struct cl_decoded_option *decoded,
> --- gcc/opts.c.jj	2016-01-23 00:13:00.662018617 +0100
> +++ gcc/opts.c	2016-01-25 14:06:31.834127398 +0100
> @@ -1433,6 +1433,104 @@ enable_fdo_optimizations (struct gcc_opt
>      opts->x_flag_tree_loop_distribute_patterns = value;
>  }
>  
> +/* -f{,no-}sanitize{,-recover}= suboptions.  */
> +static const struct sanitizer_opts_s
> +{
> +  const char *const name;
> +  unsigned int flag;
> +  size_t len;
> +} sanitizer_opts[] =
> +{
> +#define SANITIZER_OPT(name, flags) { #name, flags, sizeof #name - 1 }
> +  SANITIZER_OPT (address, SANITIZE_ADDRESS | SANITIZE_USER_ADDRESS),
> +  SANITIZER_OPT (kernel-address, SANITIZE_ADDRESS | SANITIZE_KERNEL_ADDRESS),
> +  SANITIZER_OPT (thread, SANITIZE_THREAD),
> +  SANITIZER_OPT (leak, SANITIZE_LEAK),
> +  SANITIZER_OPT (shift, SANITIZE_SHIFT),
> +  SANITIZER_OPT (integer-divide-by-zero, SANITIZE_DIVIDE),
> +  SANITIZER_OPT (undefined, SANITIZE_UNDEFINED),
> +  SANITIZER_OPT (unreachable, SANITIZE_UNREACHABLE),
> +  SANITIZER_OPT (vla-bound, SANITIZE_VLA),
> +  SANITIZER_OPT (return, SANITIZE_RETURN),
> +  SANITIZER_OPT (null, SANITIZE_NULL),
> +  SANITIZER_OPT (signed-integer-overflow, SANITIZE_SI_OVERFLOW),
> +  SANITIZER_OPT (bool, SANITIZE_BOOL),
> +  SANITIZER_OPT (enum, SANITIZE_ENUM),
> +  SANITIZER_OPT (float-divide-by-zero, SANITIZE_FLOAT_DIVIDE),
> +  SANITIZER_OPT (float-cast-overflow, SANITIZE_FLOAT_CAST),
> +  SANITIZER_OPT (bounds, SANITIZE_BOUNDS),
> +  SANITIZER_OPT (bounds-strict, SANITIZE_BOUNDS | SANITIZE_BOUNDS_STRICT),
> +  SANITIZER_OPT (alignment, SANITIZE_ALIGNMENT),
> +  SANITIZER_OPT (nonnull-attribute, SANITIZE_NONNULL_ATTRIBUTE),
> +  SANITIZER_OPT (returns-nonnull-attribute, SANITIZE_RETURNS_NONNULL_ATTRIBUTE),
> +  SANITIZER_OPT (object-size, SANITIZE_OBJECT_SIZE),
> +  SANITIZER_OPT (vptr, SANITIZE_VPTR),
> +  SANITIZER_OPT (all, ~0),
> +#undef SANITIZER_OPT
> +  { NULL, 0, 0 }
> +};
> +
> +/* Parse comma separated sanitizer suboptions from P for option SCODE,
> +   adjust previous FLAGS and return new ones.  If COMPLAIN is false,
> +   don't issue diagnostics.  */
> +
> +unsigned int
> +parse_sanitizer_options (const char *p, location_t loc, int scode,
> +			 unsigned int flags, int value, bool complain)
> +{
> +  enum opt_code code = (enum opt_code) scode;
> +  while (*p != 0)
> +    {
> +      size_t len, i;
> +      bool found = false;
> +      const char *comma = strchr (p, ',');
> +
> +      if (comma == NULL)
> +	len = strlen (p);
> +      else
> +	len = comma - p;
> +      if (len == 0)
> +	{
> +	  p = comma + 1;
> +	  continue;
> +	}
> +
> +      /* Check to see if the string matches an option class name.  */
> +      for (i = 0; sanitizer_opts[i].name != NULL; ++i)
> +	if (len == sanitizer_opts[i].len
> +	    && memcmp (p, sanitizer_opts[i].name, len) == 0)
> +	  {
> +	    /* Handle both -fsanitize and -fno-sanitize cases.  */
> +	    if (value && sanitizer_opts[i].flag == ~0U)
> +	      {
> +		if (code == OPT_fsanitize_)
> +		  {
> +		    if (complain)
> +		      error_at (loc, "-fsanitize=all option is not valid");
> +		  }
> +		else
> +		  flags |= ~(SANITIZE_USER_ADDRESS | SANITIZE_THREAD
> +			     | SANITIZE_LEAK);
> +	      }
> +	    else if (value)
> +	      flags |= sanitizer_opts[i].flag;
> +	    else
> +	      flags &= ~sanitizer_opts[i].flag;
> +	    found = true;
> +	    break;
> +	  }
> +
> +      if (! found && complain)
> +	error_at (loc, "unrecognized argument to -fsanitize%s= option: %q.*s",
> +		  code == OPT_fsanitize_ ? "" : "-recover", (int) len, p);
> +
> +      if (comma == NULL)
> +	break;
> +      p = comma + 1;
> +    }
> +  return flags;
> +}
> +
>  /* Handle target- and language-independent options.  Return zero to
>     generate an "unknown option" message.  Only options that need
>     extra handling need to be listed here; if you simply want
> @@ -1626,129 +1724,32 @@ common_handle_option (struct gcc_options
>        break;
>  
>      case OPT_fsanitize_:
> -    case OPT_fsanitize_recover_:
> -      {
> -	const char *p = arg;
> -	unsigned int *flag
> -	  = code == OPT_fsanitize_ ? &opts->x_flag_sanitize
> -	  : &opts->x_flag_sanitize_recover;
> -	while (*p != 0)
> -	  {
> -	    static const struct
> -	    {
> -	      const char *const name;
> -	      unsigned int flag;
> -	      size_t len;
> -	    } spec[] =
> -	    {
> -	      { "address", SANITIZE_ADDRESS | SANITIZE_USER_ADDRESS,
> -		sizeof "address" - 1 },
> -	      { "kernel-address", SANITIZE_ADDRESS | SANITIZE_KERNEL_ADDRESS,
> -		sizeof "kernel-address" - 1 },
> -	      { "thread", SANITIZE_THREAD, sizeof "thread" - 1 },
> -	      { "leak", SANITIZE_LEAK, sizeof "leak" - 1 },
> -	      { "shift", SANITIZE_SHIFT, sizeof "shift" - 1 },
> -	      { "integer-divide-by-zero", SANITIZE_DIVIDE,
> -		sizeof "integer-divide-by-zero" - 1 },
> -	      { "undefined", SANITIZE_UNDEFINED, sizeof "undefined" - 1 },
> -	      { "unreachable", SANITIZE_UNREACHABLE,
> -		sizeof "unreachable" - 1 },
> -	      { "vla-bound", SANITIZE_VLA, sizeof "vla-bound" - 1 },
> -	      { "return", SANITIZE_RETURN, sizeof "return" - 1 },
> -	      { "null", SANITIZE_NULL, sizeof "null" - 1 },
> -	      { "signed-integer-overflow", SANITIZE_SI_OVERFLOW,
> -		sizeof "signed-integer-overflow" -1 },
> -	      { "bool", SANITIZE_BOOL, sizeof "bool" - 1 },
> -	      { "enum", SANITIZE_ENUM, sizeof "enum" - 1 },
> -	      { "float-divide-by-zero", SANITIZE_FLOAT_DIVIDE,
> -		sizeof "float-divide-by-zero" - 1 },
> -	      { "float-cast-overflow", SANITIZE_FLOAT_CAST,
> -		sizeof "float-cast-overflow" - 1 },
> -	      { "bounds", SANITIZE_BOUNDS, sizeof "bounds" - 1 },
> -	      { "bounds-strict", SANITIZE_BOUNDS | SANITIZE_BOUNDS_STRICT,
> -		sizeof "bounds-strict" - 1 },
> -	      { "alignment", SANITIZE_ALIGNMENT, sizeof "alignment" - 1 },
> -	      { "nonnull-attribute", SANITIZE_NONNULL_ATTRIBUTE,
> -		sizeof "nonnull-attribute" - 1 },
> -	      { "returns-nonnull-attribute",
> -		SANITIZE_RETURNS_NONNULL_ATTRIBUTE,
> -		sizeof "returns-nonnull-attribute" - 1 },
> -	      { "object-size", SANITIZE_OBJECT_SIZE,
> -		sizeof "object-size" - 1 },
> -	      { "vptr", SANITIZE_VPTR, sizeof "vptr" - 1 },
> -	      { "all", ~0, sizeof "all" - 1 },
> -	      { NULL, 0, 0 }
> -	    };
> -	    const char *comma;
> -	    size_t len, i;
> -	    bool found = false;
> -
> -	    comma = strchr (p, ',');
> -	    if (comma == NULL)
> -	      len = strlen (p);
> -	    else
> -	      len = comma - p;
> -	    if (len == 0)
> -	      {
> -		p = comma + 1;
> -		continue;
> -	      }
> -
> -	    /* Check to see if the string matches an option class name.  */
> -	    for (i = 0; spec[i].name != NULL; ++i)
> -	      if (len == spec[i].len
> -		  && memcmp (p, spec[i].name, len) == 0)
> -		{
> -		  /* Handle both -fsanitize and -fno-sanitize cases.  */
> -		  if (value && spec[i].flag == ~0U)
> -		    {
> -		      if (code == OPT_fsanitize_)
> -			error_at (loc, "-fsanitize=all option is not valid");
> -		      else
> -			*flag |= ~(SANITIZE_USER_ADDRESS | SANITIZE_THREAD
> -				   | SANITIZE_LEAK);
> -		    }
> -		  else if (value)
> -		    *flag |= spec[i].flag;
> -		  else
> -		    *flag &= ~spec[i].flag;
> -		  found = true;
> -		  break;
> -		}
> -
> -	    if (! found)
> -	      error_at (loc,
> -			"unrecognized argument to -fsanitize%s= option: %q.*s",
> -			code == OPT_fsanitize_ ? "" : "-recover", (int) len, p);
> -
> -	    if (comma == NULL)
> -	      break;
> -	    p = comma + 1;
> -	  }
> -
> -	if (code != OPT_fsanitize_)
> -	  break;
> -
> -	/* Kernel ASan implies normal ASan but does not yet support
> -	   all features.  */
> -	if (opts->x_flag_sanitize & SANITIZE_KERNEL_ADDRESS)
> -	  {
> -	    maybe_set_param_value (PARAM_ASAN_INSTRUMENTATION_WITH_CALL_THRESHOLD, 0,
> -				   opts->x_param_values,
> -				   opts_set->x_param_values);
> -	    maybe_set_param_value (PARAM_ASAN_GLOBALS, 0,
> -				   opts->x_param_values,
> -				   opts_set->x_param_values);
> -	    maybe_set_param_value (PARAM_ASAN_STACK, 0,
> -				   opts->x_param_values,
> -				   opts_set->x_param_values);
> -	    maybe_set_param_value (PARAM_ASAN_USE_AFTER_RETURN, 0,
> -				   opts->x_param_values,
> -				   opts_set->x_param_values);
> -	  }
> +      opts->x_flag_sanitize
> +	= parse_sanitizer_options (arg, loc, code,
> +				   opts->x_flag_sanitize, value, true);
> +
> +      /* Kernel ASan implies normal ASan but does not yet support
> +	 all features.  */
> +      if (opts->x_flag_sanitize & SANITIZE_KERNEL_ADDRESS)
> +	{
> +	  maybe_set_param_value (PARAM_ASAN_INSTRUMENTATION_WITH_CALL_THRESHOLD,
> +				 0, opts->x_param_values,
> +				 opts_set->x_param_values);
> +	  maybe_set_param_value (PARAM_ASAN_GLOBALS, 0, opts->x_param_values,
> +				 opts_set->x_param_values);
> +	  maybe_set_param_value (PARAM_ASAN_STACK, 0, opts->x_param_values,
> +				 opts_set->x_param_values);
> +	  maybe_set_param_value (PARAM_ASAN_USE_AFTER_RETURN, 0,
> +				 opts->x_param_values,
> +				 opts_set->x_param_values);
> +	}
> +      break;
>  
> -	break;
> -      }
> +    case OPT_fsanitize_recover_:
> +      opts->x_flag_sanitize_recover
> +	= parse_sanitizer_options (arg, loc, code,
> +				   opts->x_flag_sanitize_recover, value, true);
> +      break;
>  
>      case OPT_fasan_shadow_offset_:
>        /* Deferred.  */
> --- gcc/lto-opts.c.jj	2016-01-23 00:13:00.897015402 +0100
> +++ gcc/lto-opts.c	2016-01-25 14:06:31.834127398 +0100
> @@ -199,9 +199,11 @@ lto_write_options (void)
>        /* Also drop all options that are handled by the driver as well,
>  	 which includes things like -o and -v or -fhelp for example.
>  	 We do not need those.  The only exception is -foffload option, if we
> -	 write it in offload_lto section.  Also drop all diagnostic options.  */
> +	 write it in offload_lto section.  Also drop all diagnostic options,
> +	 and -fsanitize=.  */
>        if ((cl_options[option->opt_index].flags & (CL_DRIVER|CL_WARNING))
> -	  && (!lto_stream_offload_p || option->opt_index != OPT_foffload_))
> +	  && (!lto_stream_offload_p || option->opt_index != OPT_foffload_)
> +	  && option->opt_index != OPT_fsanitize_)
>  	continue;
>  
>        for (j = 0; j < option->canonical_option_num_elements; ++j)
> --- gcc/lto-wrapper.c.jj	2016-01-23 00:13:00.632019027 +0100
> +++ gcc/lto-wrapper.c	2016-01-25 15:59:49.778877313 +0100
> @@ -190,6 +190,21 @@ append_option (struct cl_decoded_option
>  	  sizeof (struct cl_decoded_option));
>  }
>  
> +/* Set *OPT to decoded -f{,no-}sanitize=shift.  */
> +
> +static void
> +sanitize_shift_decoded_opt (struct cl_decoded_option *opt, int value)
> +{
> +  memset (opt, 0, sizeof (*opt));
> +  opt->opt_index = OPT_fsanitize_;
> +  opt->arg = "shift";
> +  opt->value = value;
> +  opt->orig_option_with_args_text
> +    = value ? "-fsanitize=shift" : "-fno-sanitize=shift";
> +  opt->canonical_option[0] = opt->orig_option_with_args_text;
> +  opt->canonical_option_num_elements = 1;
> +}
> +
>  /* Try to merge and complain about options FDECODED_OPTIONS when applied
>     ontop of DECODED_OPTIONS.  */
>  
> @@ -200,6 +215,7 @@ merge_and_complain (struct cl_decoded_op
>  		    unsigned int fdecoded_options_count)
>  {
>    unsigned int i, j;
> +  unsigned int sanitize = 0;
>  
>    /* ???  Merge options from files.  Most cases can be
>       handled by either unioning or intersecting
> @@ -277,6 +293,7 @@ merge_and_complain (struct cl_decoded_op
>  	case OPT_fwrapv:
>  	case OPT_fopenmp:
>  	case OPT_fopenacc:
> +	case OPT_fcilkplus:
>  	case OPT_fcheck_pointer_bounds:
>  	  /* For selected options we can merge conservatively.  */
>  	  for (j = 0; j < *decoded_options_count; ++j)
> @@ -304,6 +321,12 @@ merge_and_complain (struct cl_decoded_op
>  			 " files", foption->orig_option_with_args_text);
>  	  break;
>  
> +	case OPT_fsanitize_:
> +	  sanitize = parse_sanitizer_options (foption->arg, input_location,
> +					      OPT_fsanitize_, sanitize,
> +					      foption->value, false);
> +	  break;
> +
>  	case OPT_foffload_abi_:
>  	  for (j = 0; j < *decoded_options_count; ++j)
>  	    if ((*decoded_options)[j].opt_index == foption->opt_index)
> @@ -392,6 +415,24 @@ merge_and_complain (struct cl_decoded_op
>  	  break;
>  	}
>      }
> +
> +  /* If FDECODED_OPTIONS requested any ubsan sanitization, pass through
> +     some sanitization that is handled by the FEs to make sure sanitizer
> +     builtins are initialized and if needed libubsan is linked in.  */
> +  if (sanitize & (SANITIZE_UNDEFINED | SANITIZE_NONDEFAULT))
> +    {
> +      for (j = 0; j < *decoded_options_count; ++j)
> +	if ((*decoded_options)[j].opt_index == OPT_fsanitize_
> +	    && (*decoded_options)[j].value == 1)
> +	  break;
> +      if (j == *decoded_options_count)
> +	{
> +	  struct cl_decoded_option sanitize_shift;
> +	  sanitize_shift_decoded_opt (&sanitize_shift, 1);
> +	  append_option (decoded_options, decoded_options_count,
> +			 &sanitize_shift);
> +	}
> +    }
>  }
>  
>  /* Auxiliary function that frees elements of PTR and PTR itself.
> @@ -505,6 +546,7 @@ append_compiler_options (obstack *argv_o
>  	case OPT_fwrapv:
>  	case OPT_fopenmp:
>  	case OPT_fopenacc:
> +	case OPT_fcilkplus:
>  	case OPT_ftrapv:
>  	case OPT_fstrict_overflow:
>  	case OPT_foffload_abi_:
> @@ -513,6 +555,7 @@ append_compiler_options (obstack *argv_o
>  	case OPT_Og:
>  	case OPT_Os:
>  	case OPT_fcheck_pointer_bounds:
> +	case OPT_fsanitize_:
>  	  break;
>  
>  	default:
> @@ -558,6 +601,15 @@ append_linker_options (obstack *argv_obs
>  	     ???  We fail to diagnose a possible mismatch here.  */
>  	  continue;
>  
> +	case OPT_fopenmp:
> +	case OPT_fopenacc:
> +	case OPT_fcilkplus:
> +	  /* Ignore -fno-XXX form of these options, as otherwise
> +	     corresponding builtins will not be enabled.  */
> +	  if (option->value == 0)
> +	    continue;
> +	  break;
> +
>  	default:
>  	  break;
>  	}
> @@ -873,10 +925,40 @@ find_and_merge_options (int fd, off_t fi
>  					    &f2decoded_options,
>  					    &f2decoded_options_count);
>        if (!fdecoded_options)
> -       {
> -	 fdecoded_options = f2decoded_options;
> -	 fdecoded_options_count = f2decoded_options_count;
> -       }
> +	{
> +	  unsigned int sanitize = 0, i;
> +	  bool sanitize_seen = false;
> +	  fdecoded_options = f2decoded_options;
> +	  fdecoded_options_count = f2decoded_options_count;
> +
> +	  /* See if there are any -fsanitize= options.  If yes, change
> +	     them all to either -f{,no-}sanitize=shift, depending on if
> +	     any ubsan sanitizers were enabled in the end.  */
> +	  for (i = 0; i < fdecoded_options_count; ++i)
> +	    {
> +	      struct cl_decoded_option *foption = &fdecoded_options[i];
> +	      if (foption->opt_index == OPT_fsanitize_)
> +		{
> +		  sanitize
> +		    = parse_sanitizer_options (foption->arg, input_location,
> +					       OPT_fsanitize_, sanitize,
> +					       foption->value, false);
> +		  sanitize_seen = true;
> +		}
> +	    }
> +	  if (sanitize_seen)
> +	    {
> +	      int value = 0;
> +	      if (sanitize & (SANITIZE_UNDEFINED | SANITIZE_NONDEFAULT))
> +		value = 1;
> +	      for (i = 0; i < fdecoded_options_count; ++i)
> +		{
> +		  struct cl_decoded_option *foption = &fdecoded_options[i];
> +		  if (foption->opt_index == OPT_fsanitize_)
> +		    sanitize_shift_decoded_opt (foption, value);
> +		}
> +	    }
> +	}
>        else
>  	merge_and_complain (&fdecoded_options,
>  			    &fdecoded_options_count,
> @@ -919,6 +1001,7 @@ run_gcc (unsigned argc, char *argv[])
>    bool have_offload = false;
>    unsigned lto_argc = 0, offload_argc = 0;
>    char **lto_argv, **offload_argv;
> +  bool sanitize = false;
>  
>    /* Get the driver and options.  */
>    collect_gcc = getenv ("COLLECT_GCC");
> @@ -985,6 +1068,45 @@ run_gcc (unsigned argc, char *argv[])
>        close (fd);
>      }
>  
> +  /* For -fsanitize=, we want the linker options to override
> +     the options gathered from TUs, but we still want to enable
> +     one ubsan sanitizer that is handled by the FEs only,
> +     so append -fsanitize=shift to linker options if there are
> +     any sanitizer options passed to the linker.  */
> +  for (j = 1; j < fdecoded_options_count; ++j)
> +    {
> +      struct cl_decoded_option *option = &fdecoded_options[j];
> +      switch (option->opt_index)
> +	{
> +	case OPT_fsanitize_:
> +	  /* find_and_merge_options should already canonicalize
> +	     these into -f{,no-}sanitize=shift.  */
> +	  if (option->value)
> +	    sanitize = true;
> +	}
> +    }
> +  if (sanitize)
> +    {
> +      sanitize = false;
> +      for (j = 1; j < decoded_options_count; ++j)
> +	{
> +	  struct cl_decoded_option *option = &decoded_options[j];
> +	  switch (option->opt_index)
> +	    {
> +	    case OPT_fsanitize_:
> +	      sanitize = true;
> +	      break;
> +	    }
> +	}
> +      if (sanitize)
> +	{
> +	  struct cl_decoded_option sanitize_shift;
> +	  sanitize_shift_decoded_opt (&sanitize_shift, 1);
> +	  append_option (&decoded_options, &decoded_options_count,
> +			 &sanitize_shift);
> +	}
> +    }
> +
>    /* Initalize the common arguments for the driver.  */
>    obstack_init (&argv_obstack);
>    obstack_ptr_grow (&argv_obstack, collect_gcc);
> 
> 	Jakub
> 
>

On 01/26/2016 02:24 PM, Jakub Jelinek wrote:
>
>> just designed to enable DEF_SANITIZER_BUILTIN (IIUC). Also, why use shift
>> and not just sanitize=undefined?
>
> Because -fsanitize=undefined is a large collection of individual sanitizers,
> and at least some of them affect also post-IPA code (e.g.
> -fsanitize=unreachable).  The goal is to pick one of the sanitizers that are
> handled solely pre-IPA only, after that just are present in form of a
> builtin call in the IL (and thus all that lto1 needs to do for that option
> is initialize the builtins).

Ok. That should also go into one of the comments describing the choice 
of -fsanitize=shift.

> Maybe.  It also has to check the option->value though.  The thing is,
> because I didn't want to memmove options around when processing the first
> TU, it will canonicalize all the -f{,no-}sanitize= options depending on if
> in the end some ubsan sanitizers were enabled or not to
> -f{,no-}sanitize=shift.  And merge_and_complain thus should ignore
> all the -fno-sanitize=shift options in DECODED_OPTIONS, if it finds any
> -fsanitize=shift in DECODED_OPTIONS (optionally preceeded by some
> -fno-sanitize=shift), then nothing needs to be added, otherwise
> -fsanitize=shift is added.

Again that sort of thing might be worthwhile to have in a comment.


Bernd