Patchwork Add command line parsing of -fsanitize

login
register
mail settings
Submitter Jakub Jelinek
Date July 13, 2013, 9:47 p.m.
Message ID <20130713214729.GI2475@laptop.redhat.com>
Download mbox | patch
Permalink /patch/258843/
State New
Headers show

Comments

Jakub Jelinek - July 13, 2013, 9:47 p.m.
On Wed, Jun 19, 2013 at 10:17:15AM +0200, Jakub Jelinek wrote:
> While it would be possible to define say %:sanitize(thread LIBTSAN_EARLY)
> that would work roughly like %{fsanitize=thread:LIBTSAN_EARLY} worked
> until now (variable number of arguments that would be concatenated together
> if flag_sanitize & ..., otherwise return empty), we use e.g. %e inside
> of the %{fsanitize=thread:...} etc.
> So, I wonder if we couldn't extend the handle_braces, I think right now
> empty atoms are disallowed for the first choice, so perhaps
> %{%:function(args):...}
> where %:function(args) would be expanded to either non-empty or empty string
> and depending on that the condition would be then true resp. false.
> As % is not considered part of the atom name, and we require after atom name
> optional * and then only one of |, }, &, :, I think this wouldn't be
> ambiguous in the grammar.
> We could then have:
> %{!%:function1():-lfoo;%:function2(bar baz):-lbar -lbaz;-lxxx}
> and for the sanitizer purposes:
> %{%:sanitize(address):LIBTSAN_EARLY}
> %{!nostdlib:%{!nodefaultlibs:%{%:sanitize(address):" LIBASAN_SPEC "\
>     %{static:%ecannot specify -static with -fsanitize=address}\
>     %{%:sanitize(thread):%e-fsanitize=address is incompatible with -fsanitize=thread}}\
>     %{%:sanitize(thread):" LIBTSAN_SPEC "\
>     %{!pie:%{!shared:%e-fsanitize=thread linking must be done with -pie or
>     %-shared}}}}}"

Here is a patch that implements that (of course on top of Marek's patch from
June).

2013-07-13  Jakub Jelinek  <jakub@redhat.com>

	* gcc.c: Document %{%:function(args):X}.
	(SANITIZER_EARLY_SPEC, SANITIZER_SPEC): Use %:sanitize(address)
	instead of fsanitize=address and %:sanitize(thread) instead of
	fsanitize=thread.
	(static_spec_functions): Add sanitize.
	(handle_spec_function): Add retval_nonnull argument and if non-NULL,
	store funcval != NULL there.
	(do_spec_1): Adjust handle_spec_function caller.
	(handle_braces): Allow %:function(args) as condition.
	(sanitize_spec_function): New function.
	* common.opt (fsanitize=): Add Driver.
	* config/darwin.h (LINK_COMMAND_SPEC_A): Use %:sanitize(address)
	instead of fsanitize=address.
	* config/arm/linux-eabi.h (ASAN_CC1_SPEC): Use %:sanitize(address)
	instead of fsanitize=address*.



	Jakub
Marek Polacek - July 14, 2013, 5:16 a.m.
On Sat, Jul 13, 2013 at 11:47:29PM +0200, Jakub Jelinek wrote:
> On Wed, Jun 19, 2013 at 10:17:15AM +0200, Jakub Jelinek wrote:
> > While it would be possible to define say %:sanitize(thread LIBTSAN_EARLY)
> > that would work roughly like %{fsanitize=thread:LIBTSAN_EARLY} worked
> > until now (variable number of arguments that would be concatenated together
> > if flag_sanitize & ..., otherwise return empty), we use e.g. %e inside
> > of the %{fsanitize=thread:...} etc.
> > So, I wonder if we couldn't extend the handle_braces, I think right now
> > empty atoms are disallowed for the first choice, so perhaps
> > %{%:function(args):...}
> > where %:function(args) would be expanded to either non-empty or empty string
> > and depending on that the condition would be then true resp. false.
> > As % is not considered part of the atom name, and we require after atom name
> > optional * and then only one of |, }, &, :, I think this wouldn't be
> > ambiguous in the grammar.
> > We could then have:
> > %{!%:function1():-lfoo;%:function2(bar baz):-lbar -lbaz;-lxxx}
> > and for the sanitizer purposes:
> > %{%:sanitize(address):LIBTSAN_EARLY}
> > %{!nostdlib:%{!nodefaultlibs:%{%:sanitize(address):" LIBASAN_SPEC "\
> >     %{static:%ecannot specify -static with -fsanitize=address}\
> >     %{%:sanitize(thread):%e-fsanitize=address is incompatible with -fsanitize=thread}}\
> >     %{%:sanitize(thread):" LIBTSAN_SPEC "\
> >     %{!pie:%{!shared:%e-fsanitize=thread linking must be done with -pie or
> >     %-shared}}}}}"
> 
> Here is a patch that implements that (of course on top of Marek's patch from
> June).
> 
> 2013-07-13  Jakub Jelinek  <jakub@redhat.com>
> 
> 	* gcc.c: Document %{%:function(args):X}.
> 	(SANITIZER_EARLY_SPEC, SANITIZER_SPEC): Use %:sanitize(address)
> 	instead of fsanitize=address and %:sanitize(thread) instead of
> 	fsanitize=thread.
> 	(static_spec_functions): Add sanitize.
> 	(handle_spec_function): Add retval_nonnull argument and if non-NULL,
> 	store funcval != NULL there.
> 	(do_spec_1): Adjust handle_spec_function caller.
> 	(handle_braces): Allow %:function(args) as condition.
> 	(sanitize_spec_function): New function.
> 	* common.opt (fsanitize=): Add Driver.
> 	* config/darwin.h (LINK_COMMAND_SPEC_A): Use %:sanitize(address)
> 	instead of fsanitize=address.
> 	* config/arm/linux-eabi.h (ASAN_CC1_SPEC): Use %:sanitize(address)
> 	instead of fsanitize=address*.

Thanks, commited to the ubsan branch.

	Marek
Joseph S. Myers - July 14, 2013, 5:30 p.m.
On Sat, 13 Jul 2013, Jakub Jelinek wrote:

> Here is a patch that implements that (of course on top of Marek's patch from
> June).
> 
> 2013-07-13  Jakub Jelinek  <jakub@redhat.com>
> 
> 	* gcc.c: Document %{%:function(args):X}.
> 	(SANITIZER_EARLY_SPEC, SANITIZER_SPEC): Use %:sanitize(address)
> 	instead of fsanitize=address and %:sanitize(thread) instead of
> 	fsanitize=thread.
> 	(static_spec_functions): Add sanitize.
> 	(handle_spec_function): Add retval_nonnull argument and if non-NULL,
> 	store funcval != NULL there.
> 	(do_spec_1): Adjust handle_spec_function caller.
> 	(handle_braces): Allow %:function(args) as condition.
> 	(sanitize_spec_function): New function.
> 	* common.opt (fsanitize=): Add Driver.
> 	* config/darwin.h (LINK_COMMAND_SPEC_A): Use %:sanitize(address)
> 	instead of fsanitize=address.
> 	* config/arm/linux-eabi.h (ASAN_CC1_SPEC): Use %:sanitize(address)
> 	instead of fsanitize=address*.

This seems fine to me.

Patch

--- gcc/gcc.c.jj	2013-07-12 21:10:00.000000000 +0200
+++ gcc/gcc.c	2013-07-13 20:52:56.571269353 +0200
@@ -215,7 +215,7 @@  static inline void process_marked_switch
 static const char *process_brace_body (const char *, const char *, const char *, int, int);
 static const struct spec_function *lookup_spec_function (const char *);
 static const char *eval_spec_function (const char *, const char *);
-static const char *handle_spec_function (const char *);
+static const char *handle_spec_function (const char *, bool *);
 static char *save_string (const char *, int);
 static void set_collect_gcc_options (void);
 static int do_spec_1 (const char *, int, const char *);
@@ -253,6 +253,7 @@  static const char *convert_filename (con
 static const char *getenv_spec_function (int, const char **);
 static const char *if_exists_spec_function (int, const char **);
 static const char *if_exists_else_spec_function (int, const char **);
+static const char *sanitize_spec_function (int, const char **);
 static const char *replace_outfile_spec_function (int, const char **);
 static const char *remove_outfile_spec_function (int, const char **);
 static const char *version_compare_spec_function (int, const char **);
@@ -432,6 +433,10 @@  or with constant text in a single argume
 	  than the OR.
 	  If %* appears in X, all of the alternatives must be starred, and
 	  only the first matching alternative is substituted.
+ %{%:function(args):X}
+	  Call function named FUNCTION with args ARGS.  If the function
+	  returns non-NULL, then X is substituted, if it returns
+	  NULL, it isn't substituted.
  %{S:X;   if S was given to GCC, substitutes X;
    T:Y;   else if T was given to GCC, substitutes Y;
     :D}   else substitutes D.  There can be as many clauses as you need.
@@ -708,17 +713,17 @@  proper position among the other output f
 /* Linker command line options for -fsanitize= early on the command line.  */
 #ifndef SANITIZER_EARLY_SPEC
 #define SANITIZER_EARLY_SPEC "\
-%{!nostdlib:%{!nodefaultlibs:%{fsanitize=address:" LIBASAN_EARLY_SPEC "} \
-    %{fsanitize=thread:" LIBTSAN_EARLY_SPEC "}}}"
+%{!nostdlib:%{!nodefaultlibs:%{%:sanitize(address):" LIBASAN_EARLY_SPEC "} \
+    %{%:sanitize(thread):" LIBTSAN_EARLY_SPEC "}}}"
 #endif
 
 /* Linker command line options for -fsanitize= late on the command line.  */
 #ifndef SANITIZER_SPEC
 #define SANITIZER_SPEC "\
-%{!nostdlib:%{!nodefaultlibs:%{fsanitize=address:" LIBASAN_SPEC "\
+%{!nostdlib:%{!nodefaultlibs:%{%:sanitize(address):" LIBASAN_SPEC "\
     %{static:%ecannot specify -static with -fsanitize=address}\
-    %{fsanitize=thread:%e-fsanitize=address is incompatible with -fsanitize=thread}}\
-    %{fsanitize=thread:" LIBTSAN_SPEC "\
+    %{%:sanitize(thread):%e-fsanitize=address is incompatible with -fsanitize=thread}}\
+    %{%:sanitize(thread):" LIBTSAN_SPEC "\
     %{!pie:%{!shared:%e-fsanitize=thread linking must be done with -pie or -shared}}}}}"
 #endif
 
@@ -1323,6 +1328,7 @@  static const struct spec_function static
   { "getenv",                   getenv_spec_function },
   { "if-exists",		if_exists_spec_function },
   { "if-exists-else",		if_exists_else_spec_function },
+  { "sanitize",			sanitize_spec_function },
   { "replace-outfile",		replace_outfile_spec_function },
   { "remove-outfile",		remove_outfile_spec_function },
   { "version-compare",		version_compare_spec_function },
@@ -5273,7 +5279,7 @@  do_spec_1 (const char *spec, int inswitc
 	    break;
 
 	  case ':':
-	    p = handle_spec_function (p);
+	    p = handle_spec_function (p, NULL);
 	    if (p == 0)
 	      return -1;
 	    break;
@@ -5509,10 +5515,13 @@  eval_spec_function (const char *func, co
    ARGS is processed as a spec in a separate context and split into an
    argument vector in the normal fashion.  The function returns a string
    containing a spec which we then process in the caller's context, or
-   NULL if no processing is required.  */
+   NULL if no processing is required.
+
+   If RETVAL_NONNULL is not NULL, then store a bool whether function
+   returned non-NULL.  */
 
 static const char *
-handle_spec_function (const char *p)
+handle_spec_function (const char *p, bool *retval_nonnull)
 {
   char *func, *args;
   const char *endp, *funcval;
@@ -5558,6 +5567,8 @@  handle_spec_function (const char *p)
   funcval = eval_spec_function (func, args);
   if (funcval != NULL && do_spec_1 (funcval, 0, NULL) < 0)
     p = NULL;
+  if (retval_nonnull)
+    *retval_nonnull = funcval != NULL;
 
   free (func);
   free (args);
@@ -5701,19 +5712,28 @@  handle_braces (const char *p)
 	p++, a_is_negated = true;
 
       SKIP_WHITE();
-      if (*p == '.')
-	p++, a_is_suffix = true;
-      else if (*p == ',')
-	p++, a_is_spectype = true;
-
-      atom = p;
-      while (ISIDNUM(*p) || *p == '-' || *p == '+' || *p == '='
-	     || *p == ',' || *p == '.' || *p == '@')
-	p++;
-      end_atom = p;
+      if (*p == '%' && p[1] == ':')
+	{
+	  atom = NULL;
+	  end_atom = NULL;
+	  p = handle_spec_function (p + 2, &a_matched);
+	}
+      else
+	{
+	  if (*p == '.')
+	    p++, a_is_suffix = true;
+	  else if (*p == ',')
+	    p++, a_is_spectype = true;
+
+	  atom = p;
+	  while (ISIDNUM(*p) || *p == '-' || *p == '+' || *p == '='
+		 || *p == ',' || *p == '.' || *p == '@')
+	    p++;
+	  end_atom = p;
 
-      if (*p == '*')
-	p++, a_is_starred = 1;
+	  if (*p == '*')
+	    p++, a_is_starred = 1;
+	}
 
       SKIP_WHITE();
       switch (*p)
@@ -5738,7 +5758,7 @@  handle_braces (const char *p)
 	  if (ordered_set)
 	    goto invalid;
 
-	  if (atom == end_atom)
+	  if (atom && atom == end_atom)
 	    {
 	      if (!n_way_choice || disj_matched || *p == '|'
 		  || a_is_negated || a_is_suffix || a_is_spectype
@@ -5763,7 +5783,9 @@  handle_braces (const char *p)
 		 match.  */
 	      if (!disj_matched && !n_way_matched)
 		{
-		  if (a_is_suffix)
+		  if (atom == NULL)
+		    /* a_matched is already set by handle_spec_function.  */;
+		  else if (a_is_suffix)
 		    a_matched = input_suffix_matches (atom, end_atom);
 		  else if (a_is_spectype)
 		    a_matched = input_spec_matches (atom, end_atom);
@@ -8060,6 +8082,27 @@  if_exists_else_spec_function (int argc,
   return argv[1];
 }
 
+/* sanitize built-in spec function.
+
+   This returns non-NULL, if sanitizing address, thread or
+   any of the undefined behavior sanitizers.  */
+
+static const char *
+sanitize_spec_function (int argc, const char **argv)
+{
+  if (argc != 1)
+    return NULL;
+
+  if (strcmp (argv[0], "address") == 0)
+    return (flag_sanitize & SANITIZE_ADDRESS) ? "" : NULL;
+  if (strcmp (argv[0], "thread") == 0)
+    return (flag_sanitize & SANITIZE_THREAD) ? "" : NULL;
+  if (strcmp (argv[0], "undefined") == 0)
+    return (flag_sanitize & SANITIZE_UNDEFINED) ? "" : NULL;
+
+  return NULL;
+}
+
 /* replace-outfile built-in spec function.
 
    This looks for the first argument in the outfiles array's name and
--- gcc/common.opt.jj	2013-07-12 21:14:03.000000000 +0200
+++ gcc/common.opt	2013-07-13 23:06:26.907390535 +0200
@@ -855,6 +855,6 @@  Common Ignore
 Does nothing. Preserved for backward compatibility.
 
 fsanitize=
-Common Report Joined
+Common Driver Report Joined
 Select what to sanitize
 
--- gcc/config/darwin.h.jj	2013-07-12 21:09:56.384669485 +0200
+++ gcc/config/darwin.h	2013-07-13 20:08:34.189050120 +0200
@@ -178,7 +178,7 @@  extern GTY(()) int darwin_ms_struct;
     %{L*} %(link_libgcc) %o %{fprofile-arcs|fprofile-generate*|coverage:-lgcov} \
     %{fopenmp|ftree-parallelize-loops=*: \
       %{static|static-libgcc|static-libstdc++|static-libgfortran: libgomp.a%s; : -lgomp } } \
-    %{fsanitize=address: -lasan } \
+    %{%:sanitize(address): -lasan } \
     %{fgnu-tm: \
       %{static|static-libgcc|static-libstdc++|static-libgfortran: libitm.a%s; : -litm } } \
     %{!nostdlib:%{!nodefaultlibs:\
--- gcc/config/arm/linux-eabi.h.jj	2013-07-12 21:09:56.241669386 +0200
+++ gcc/config/arm/linux-eabi.h	2013-07-13 20:09:36.163074198 +0200
@@ -85,7 +85,7 @@ 
 		       LINUX_TARGET_LINK_SPEC " " ANDROID_LINK_SPEC)
 
 #undef  ASAN_CC1_SPEC
-#define ASAN_CC1_SPEC "%{fsanitize=*:-funwind-tables}"
+#define ASAN_CC1_SPEC "%{%:sanitize(address):-funwind-tables}"
 
 #undef  CC1_SPEC
 #define CC1_SPEC							\