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 <ja...@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 > > -- Richard Biener <rguent...@suse.de> SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)