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

Reply via email to