Patchwork Add command line parsing of -fsanitize

login
register
mail settings
Submitter Marek Polacek
Date June 14, 2013, 7:04 p.m.
Message ID <20130614190439.GE20883@redhat.com>
Download mbox | patch
Permalink /patch/251508/
State New
Headers show

Comments

Marek Polacek - June 14, 2013, 7:04 p.m.
This patch is needed mainly for ubsan, but since it is an independent
change, I'd like to get this one into trunk.  It adds proper parsing of the
-fsanitize= option, currently -fsanitize=thread and -fsanitize=address
are "hardcoded" in common.opt.  It allows user to specify what exactly
to instrument, thus for ubsan one can say e.g. sanitize only shifts via
-fsanitize=shift, sanitize shifts and division-by-zero using
-fsanitize=shift,integer-divide-by-zero or sanitize everything undefined
using -fsanitize=undefined.  It's what llvm does; I followed llvm's
user-visible names.

Regtested/bootstrapped on x86_64-linux, ok for trunk?

2013-06-14  Marek Polacek  <polacek@redhat.com>

	* common.opt (flag_sanitize): Add variable.
	(fsanitize=): Add option.
	(fsanitize=thread): Remove option.
	(fsanitize=address): Likewise.
	* flag-types.h (sanitize_code): New enum.
	* opts.c (common_handle_option): Parse command line arguments
	of -fsanitize=.
	* varasm.c (get_variable_section): Adjust.
	(assemble_noswitch_variable): Likewise.
	(assemble_variable): Likewise.
	(output_constant_def_contents): Likewise.
	(categorize_decl_for_section): Likewise.
	(place_block_symbol): Likewise.
	(output_object_block): Likewise.
	* builtins.def: Likewise.
	* toplev.c (compile_file): Likewise.
	(process_options): Likewise.
	* cppbuiltin.c: Likewise.
	* tsan.c (tsan_pass): Likewise.
	(tsan_gate): Likewise.
	(tsan_gate_O0): Likewise.
	* cfgexpand.c (partition_stack_vars): Likewise.
	(expand_stack_vars): Likewise.
	(defer_stack_allocation): Likewise.
	(expand_used_vars): Likewise.
	* cfgcleanup.c (old_insns_match_p): Likewise.
	* asan.c (asan_finish_file): Likewise.
	(asan_instrument): Likewise.
	(gate_asan): Likewise.


	Marek
Jakub Jelinek - June 17, 2013, 2:59 p.m.
Hi!

Looks good, though I'd appreciate Joseph to look at this from the
option handling POV.  Some nits from me below:

On Fri, Jun 14, 2013 at 09:04:40PM +0200, Marek Polacek wrote:
> --- gcc/opts.c.mp	2013-06-14 19:23:02.300554991 +0200
> +++ gcc/opts.c	2013-06-14 20:00:30.377638168 +0200
> @@ -1405,6 +1405,64 @@ common_handle_option (struct gcc_options
>        opts->x_exit_after_options = true;
>        break;
>  
> +    case OPT_fsanitize_:
> +      {
> +	const char *p = arg;
> +	while (*p != 0)
> +	  {
> +	    static const struct
> +	    {
> +	      const char *const name;
> +	      unsigned int flag;
> +	      size_t len;
> +	    } spec[] =
> +	    {
> +	      { "address", SANITIZE_ADDRESS, sizeof "address" - 1 },
> +	      { "thread", SANITIZE_THREAD, sizeof "thread" - 1 },

For the trunk version I'd say the following 4 lines should be commented out,
and only enabled when you actually commit to trunk (or branch) corresponding
support.

> +	      { "shift", SANITIZE_SHIFT, sizeof "shift" - 1 },
> +	      { "integer-divide-by-zero", SANITIZE_DIVIDE,
> +		sizeof "integer-divide-by-zero" - 1 },
> +	      { "undefined", SANITIZE_UNDEFINED, sizeof "undefined" - 1 },
> +	      { NULL, 0, 0 }
> --- gcc/builtins.def.mp	2013-06-14 19:24:52.670944783 +0200
> +++ gcc/builtins.def	2013-06-14 20:00:30.384638196 +0200
> @@ -155,7 +155,8 @@ along with GCC; see the file COPYING3.
>  #define DEF_SANITIZER_BUILTIN(ENUM, NAME, TYPE, ATTRS) \
>    DEF_BUILTIN (ENUM, "__builtin_" NAME, BUILT_IN_NORMAL, TYPE, TYPE,    \
>  	       true, true, true, ATTRS, true, \
> -	       (flag_asan || flag_tsan))
> +	       ((flag_sanitize & SANITIZE_ADDRESS) \
> +	        || (flag_sanitize & SANITIZE_THREAD)))

Use (flag_sanitize & (SANITIZE_ADDRESS | SANITIZE_THREAD))
or just flag_sanitize instead?

> --- gcc/common.opt.mp	2013-06-14 19:24:52.671944787 +0200
> +++ gcc/common.opt	2013-06-14 20:00:30.389638216 +0200
> @@ -207,6 +207,10 @@ unsigned int help_columns
>  Variable
>  bool flag_opts_finished
>  
> +; What the sanitizer should instrument
> +Variable
> +unsigned int flag_sanitize

Can't you just add Var(flag_sanitize) to the line after fsanitize= ?

> +fsanitize=
> +Common Report Joined
> +Select what to sanitize
>  
>  fasynchronous-unwind-tables
>  Common Report Var(flag_asynchronous_unwind_tables) Optimization

	Jakub
Joseph S. Myers - June 17, 2013, 3:52 p.m.
On Mon, 17 Jun 2013, Jakub Jelinek wrote:

> > +; What the sanitizer should instrument
> > +Variable
> > +unsigned int flag_sanitize
> 
> Can't you just add Var(flag_sanitize) to the line after fsanitize= ?

I think that would create a string variable, whereas an integer is what's 
wanted here.
Jakub Jelinek - June 17, 2013, 4:01 p.m.
On Mon, Jun 17, 2013 at 03:52:54PM +0000, Joseph S. Myers wrote:
> On Mon, 17 Jun 2013, Jakub Jelinek wrote:
> 
> > > +; What the sanitizer should instrument
> > > +Variable
> > > +unsigned int flag_sanitize
> > 
> > Can't you just add Var(flag_sanitize) to the line after fsanitize= ?
> 
> I think that would create a string variable, whereas an integer is what's 
> wanted here.

We already have say:

Wstack-usage=
Common Joined RejectNegative UInteger Var(warn_stack_usage) Init(-1) Warning
Warn if stack usage might be larger than specified amount

that creates
#ifdef GENERATOR_FILE
extern int warn_stack_usage;
#else
  int x_warn_stack_usage;
#define warn_stack_usage global_options.x_warn_stack_usage
#endif

so I guess UInteger Var(flag_sanitize) then would do the trick.

Though, now looking at it, -fsanitize={address,thread} aren't
RejectNegative, so they accept also -fno-sanitize=address etc.
Marek, thus your patch should handle that properly too, say if you do:
-fsanitize=undefined -fno-sanitize=shift
it should first or into the flag_sanitize bitmask SANITIZE_UNDEFINED
and then and it with ~SANITIZE_SHIFT.

	Jakub

Patch

--- gcc/opts.c.mp	2013-06-14 19:23:02.300554991 +0200
+++ gcc/opts.c	2013-06-14 20:00:30.377638168 +0200
@@ -1405,6 +1405,64 @@  common_handle_option (struct gcc_options
       opts->x_exit_after_options = true;
       break;
 
+    case OPT_fsanitize_:
+      {
+	const char *p = arg;
+	while (*p != 0)
+	  {
+	    static const struct
+	    {
+	      const char *const name;
+	      unsigned int flag;
+	      size_t len;
+	    } spec[] =
+	    {
+	      { "address", SANITIZE_ADDRESS, sizeof "address" - 1 },
+	      { "thread", SANITIZE_THREAD, sizeof "thread" - 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 },
+	      { 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)
+		{
+		  flag_sanitize |= spec[i].flag;
+		  found = true;
+		  break;
+		}
+
+	    if (! found)
+	      warning_at (loc, 0,
+			  "unrecognized argument to -fsanitize= option: %q.*s",
+			  (int) len, p);
+
+	    if (comma == NULL)
+	      break;
+	    p = comma + 1;
+	  }
+
+	break;
+      }
+
     case OPT_O:
     case OPT_Os:
     case OPT_Ofast:
--- gcc/varasm.c.mp	2013-06-14 19:31:03.725048557 +0200
+++ gcc/varasm.c	2013-06-14 20:00:30.378638172 +0200
@@ -1102,7 +1102,8 @@  get_variable_section (tree decl, bool pr
       && bss_initializer_p (decl))
     {
       if (!TREE_PUBLIC (decl)
-	  && !(flag_asan && asan_protect_global (decl)))
+	  && !((flag_sanitize & SANITIZE_ADDRESS)
+	       && asan_protect_global (decl)))
 	return lcomm_section;
       if (bss_noswitch_section)
 	return bss_noswitch_section;
@@ -1904,7 +1905,7 @@  assemble_noswitch_variable (tree decl, c
   size = tree_low_cst (DECL_SIZE_UNIT (decl), 1);
   rounded = size;
 
-  if (flag_asan && asan_protect_global (decl))
+  if ((flag_sanitize & SANITIZE_ADDRESS) && asan_protect_global (decl))
     size += asan_red_zone_size (size);
 
   /* Don't allocate zero bytes of common,
@@ -2063,7 +2064,7 @@  assemble_variable (tree decl, int top_le
 
   align_variable (decl, dont_output_data);
 
-  if (flag_asan
+  if ((flag_sanitize & SANITIZE_ADDRESS)
       && asan_protect_global (decl))
     {
       asan_protected = true;
@@ -3335,7 +3336,8 @@  output_constant_def_contents (rtx symbol
   /* We are no longer deferring this constant.  */
   TREE_ASM_WRITTEN (decl) = TREE_ASM_WRITTEN (exp) = 1;
 
-  if (flag_asan && TREE_CODE (exp) == STRING_CST
+  if ((flag_sanitize & SANITIZE_ADDRESS)
+      && TREE_CODE (exp) == STRING_CST
       && asan_protect_global (exp))
     {
       asan_protected = true;
@@ -6248,7 +6250,8 @@  categorize_decl_for_section (const_tree
   else if (TREE_CODE (decl) == STRING_CST)
     {
       if (flag_mudflap
-	  || (flag_asan && asan_protect_global (CONST_CAST_TREE (decl))))
+	  || ((flag_sanitize & SANITIZE_ADDRESS)
+	      && asan_protect_global (CONST_CAST_TREE (decl))))
       /* or !flag_merge_constants */
         return SECCAT_RODATA;
       else
@@ -6274,7 +6277,8 @@  categorize_decl_for_section (const_tree
       else if (reloc & targetm.asm_out.reloc_rw_mask ())
 	ret = reloc == 1 ? SECCAT_DATA_REL_RO_LOCAL : SECCAT_DATA_REL_RO;
       else if (reloc || flag_merge_constants < 2 || flag_mudflap
-	       || (flag_asan && asan_protect_global (CONST_CAST_TREE (decl))))
+	       || ((flag_sanitize & SANITIZE_ADDRESS)
+		   && asan_protect_global (CONST_CAST_TREE (decl))))
 	/* C and C++ don't allow different variables to share the same
 	   location.  -fmerge-all-constants allows even that (at the
 	   expense of not conforming).  */
@@ -7032,7 +7036,7 @@  place_block_symbol (rtx symbol)
       decl = SYMBOL_REF_DECL (symbol);
       alignment = DECL_ALIGN (decl);
       size = get_constant_size (DECL_INITIAL (decl));
-      if (flag_asan
+      if ((flag_sanitize & SANITIZE_ADDRESS)
 	  && TREE_CODE (DECL_INITIAL (decl)) == STRING_CST
 	  && asan_protect_global (DECL_INITIAL (decl)))
 	size += asan_red_zone_size (size);
@@ -7042,7 +7046,8 @@  place_block_symbol (rtx symbol)
       decl = SYMBOL_REF_DECL (symbol);
       alignment = get_variable_align (decl);
       size = tree_low_cst (DECL_SIZE_UNIT (decl), 1);
-      if (flag_asan && asan_protect_global (decl))
+      if ((flag_sanitize & SANITIZE_ADDRESS)
+	  && asan_protect_global (decl))
 	{
 	  size += asan_red_zone_size (size);
 	  alignment = MAX (alignment,
@@ -7192,7 +7197,7 @@  output_object_block (struct object_block
 				      DECL_ALIGN (decl));
 	  size = get_constant_size (DECL_INITIAL (decl));
 	  offset += size;
-	  if (flag_asan
+	  if ((flag_sanitize & SANITIZE_ADDRESS)
 	      && TREE_CODE (DECL_INITIAL (decl)) == STRING_CST
 	      && asan_protect_global (DECL_INITIAL (decl)))
 	    {
@@ -7208,7 +7213,8 @@  output_object_block (struct object_block
 	  assemble_variable_contents (decl, XSTR (symbol, 0), false);
 	  size = tree_low_cst (DECL_SIZE_UNIT (decl), 1);
 	  offset += size;
-	  if (flag_asan && asan_protect_global (decl))
+	  if ((flag_sanitize & SANITIZE_ADDRESS)
+	      && asan_protect_global (decl))
 	    {
 	      size = asan_red_zone_size (size);
 	      assemble_zeros (size);
--- gcc/builtins.def.mp	2013-06-14 19:24:52.670944783 +0200
+++ gcc/builtins.def	2013-06-14 20:00:30.384638196 +0200
@@ -155,7 +155,8 @@  along with GCC; see the file COPYING3.
 #define DEF_SANITIZER_BUILTIN(ENUM, NAME, TYPE, ATTRS) \
   DEF_BUILTIN (ENUM, "__builtin_" NAME, BUILT_IN_NORMAL, TYPE, TYPE,    \
 	       true, true, true, ATTRS, true, \
-	       (flag_asan || flag_tsan))
+	       ((flag_sanitize & SANITIZE_ADDRESS) \
+	        || (flag_sanitize & SANITIZE_THREAD)))
 
 #undef DEF_CILKPLUS_BUILTIN
 #define DEF_CILKPLUS_BUILTIN(ENUM, NAME, TYPE, ATTRS) \
--- gcc/toplev.c.mp	2013-06-14 19:58:50.286290309 +0200
+++ gcc/toplev.c	2013-06-14 20:00:06.885554603 +0200
@@ -571,10 +571,10 @@  compile_file (void)
 	mudflap_finish_file ();
 
       /* File-scope initialization for AddressSanitizer.  */
-      if (flag_asan)
+      if (flag_sanitize & SANITIZE_ADDRESS)
         asan_finish_file ();
 
-      if (flag_tsan)
+      if (flag_sanitize & SANITIZE_THREAD)
 	tsan_finish_file ();
 
       output_shared_constant_pool ();
@@ -1536,12 +1536,12 @@  process_options (void)
     warn_stack_protect = 0;
 
   /* Address Sanitizer needs porting to each target architecture.  */
-  if (flag_asan
+  if ((flag_sanitize & SANITIZE_ADDRESS)
       && (targetm.asan_shadow_offset == NULL
 	  || !FRAME_GROWS_DOWNWARD))
     {
       warning (0, "-fsanitize=address not supported for this target");
-      flag_asan = 0;
+      flag_sanitize &= ~SANITIZE_ADDRESS;
     }
 
   /* Enable -Werror=coverage-mismatch when -Werror and -Wno-error
--- gcc/flag-types.h.mp	2013-06-14 19:22:52.609520428 +0200
+++ gcc/flag-types.h	2013-06-14 20:00:30.385638200 +0200
@@ -191,4 +191,16 @@  enum fp_contract_mode {
   FP_CONTRACT_FAST = 2
 };
 
+/* Different instrumentation modes.  */
+enum sanitize_code {
+  /* AddressSanitizer.  */
+  SANITIZE_ADDRESS = 1 << 0,
+  /* ThreadSanitizer.  */
+  SANITIZE_THREAD = 1 << 1,
+  /* UndefinedBehaviorSanitizer.  */
+  SANITIZE_SHIFT = 1 << 2,
+  SANITIZE_DIVIDE = 1 << 3,
+  SANITIZE_UNDEFINED = SANITIZE_SHIFT | SANITIZE_DIVIDE
+};
+
 #endif /* ! GCC_FLAG_TYPES_H */
--- gcc/cppbuiltin.c.mp	2013-06-14 19:31:03.722048545 +0200
+++ gcc/cppbuiltin.c	2013-06-14 20:00:30.385638200 +0200
@@ -90,7 +90,7 @@  define_builtin_macros_for_compilation_fl
       cpp_define_formatted (pfile, "__PIE__=%d", flag_pie);
     }
 
-  if (flag_asan)
+  if (flag_sanitize & SANITIZE_ADDRESS)
     cpp_define (pfile, "__SANITIZE_ADDRESS__");
 
   if (optimize_size)
--- gcc/tsan.c.mp	2013-06-14 19:24:52.669944779 +0200
+++ gcc/tsan.c	2013-06-14 20:23:51.407217842 +0200
@@ -713,7 +713,7 @@  tsan_pass (void)
 static bool
 tsan_gate (void)
 {
-  return flag_tsan != 0;
+  return (flag_sanitize & SANITIZE_THREAD) != 0;
 }
 
 /* Inserts __tsan_init () into the list of CTORs.  */
@@ -756,7 +756,7 @@  struct gimple_opt_pass pass_tsan =
 static bool
 tsan_gate_O0 (void)
 {
-  return flag_tsan != 0 && !optimize;
+  return (flag_sanitize & SANITIZE_THREAD) != 0 && !optimize;
 }
 
 struct gimple_opt_pass pass_tsan_O0 =
--- gcc/common.opt.mp	2013-06-14 19:24:52.671944787 +0200
+++ gcc/common.opt	2013-06-14 20:00:30.389638216 +0200
@@ -207,6 +207,10 @@  unsigned int help_columns
 Variable
 bool flag_opts_finished
 
+; What the sanitizer should instrument
+Variable
+unsigned int flag_sanitize
+
 ###
 Driver
 
@@ -850,13 +854,9 @@  fargument-noalias-anything
 Common Ignore
 Does nothing. Preserved for backward compatibility.
 
-fsanitize=address
-Common Report Var(flag_asan)
-Enable AddressSanitizer, a memory error detector
-
-fsanitize=thread
-Common Report Var(flag_tsan)
-Enable ThreadSanitizer, a data race detector
+fsanitize=
+Common Report Joined
+Select what to sanitize
 
 fasynchronous-unwind-tables
 Common Report Var(flag_asynchronous_unwind_tables) Optimization
--- gcc/cfgexpand.c.mp	2013-06-14 19:31:03.721048541 +0200
+++ gcc/cfgexpand.c	2013-06-14 20:00:30.391638224 +0200
@@ -764,7 +764,7 @@  partition_stack_vars (void)
 	     sizes, as the shorter vars wouldn't be adequately protected.
 	     Don't do that for "large" (unsupported) alignment objects,
 	     those aren't protected anyway.  */
-	  if (flag_asan && isize != jsize
+	  if ((flag_sanitize & SANITIZE_ADDRESS) && isize != jsize
 	      && ialign * BITS_PER_UNIT <= MAX_SUPPORTED_STACK_ALIGNMENT)
 	    break;
 
@@ -940,7 +940,7 @@  expand_stack_vars (bool (*pred) (size_t)
       alignb = stack_vars[i].alignb;
       if (alignb * BITS_PER_UNIT <= MAX_SUPPORTED_STACK_ALIGNMENT)
 	{
-	  if (flag_asan && pred)
+	  if ((flag_sanitize & SANITIZE_ADDRESS) && pred)
 	    {
 	      HOST_WIDE_INT prev_offset = frame_offset;
 	      tree repr_decl = NULL_TREE;
@@ -1110,7 +1110,7 @@  defer_stack_allocation (tree var, bool t
   /* If stack protection is enabled, *all* stack variables must be deferred,
      so that we can re-order the strings to the top of the frame.
      Similarly for Address Sanitizer.  */
-  if (flag_stack_protect || flag_asan)
+  if (flag_stack_protect || (flag_sanitize & SANITIZE_ADDRESS))
     return true;
 
   /* We handle "large" alignment via dynamic allocation.  We want to handle
@@ -1753,7 +1753,7 @@  expand_used_vars (void)
 	    expand_stack_vars (stack_protect_decl_phase_2, &data);
 	}
 
-      if (flag_asan)
+      if (flag_sanitize & SANITIZE_ADDRESS)
 	/* Phase 3, any partitions that need asan protection
 	   in addition to phase 1 and 2.  */
 	expand_stack_vars (asan_decl_phase_3, &data);
--- gcc/cfgcleanup.c.mp	2013-06-14 19:31:03.720048537 +0200
+++ gcc/cfgcleanup.c	2013-06-14 20:00:30.394638236 +0200
@@ -1137,7 +1137,7 @@  old_insns_match_p (int mode ATTRIBUTE_UN
 
       /* For address sanitizer, never crossjump __asan_report_* builtins,
 	 otherwise errors might be reported on incorrect lines.  */
-      if (flag_asan)
+      if (flag_sanitize & SANITIZE_ADDRESS)
 	{
 	  rtx call = get_call_rtx_from (i1);
 	  if (call && GET_CODE (XEXP (XEXP (call, 0), 0)) == SYMBOL_REF)
--- gcc/asan.c.mp	2013-06-14 19:29:27.132692727 +0200
+++ gcc/asan.c	2013-06-14 20:00:30.396638244 +0200
@@ -2175,7 +2175,7 @@  asan_finish_file (void)
   /* Avoid instrumenting code in the asan ctors/dtors.
      We don't need to insert padding after the description strings,
      nor after .LASAN* array.  */
-  flag_asan = 0;
+  flag_sanitize &= ~SANITIZE_ADDRESS;
 
   tree fn = builtin_decl_implicit (BUILT_IN_ASAN_INIT);
   append_to_statement_list (build_call_expr (fn, 0), &asan_ctor_statements);
@@ -2232,7 +2232,7 @@  asan_finish_file (void)
     }
   cgraph_build_static_cdtor ('I', asan_ctor_statements,
 			     MAX_RESERVED_INIT_PRIORITY - 1);
-  flag_asan = 1;
+  flag_sanitize |= SANITIZE_ADDRESS;
 }
 
 /* Instrument the current function.  */
@@ -2249,7 +2249,7 @@  asan_instrument (void)
 static bool
 gate_asan (void)
 {
-  return flag_asan != 0
+  return (flag_sanitize & SANITIZE_ADDRESS) != 0
 	  && !lookup_attribute ("no_sanitize_address",
 				DECL_ATTRIBUTES (current_function_decl));
 }