Re: Add support to trace comparison instructions and switch statements

["吴潍浠(此彼)" <weixi.wwx@antfin.com>]

[-- Attachment #1: Type: text/plain, Size: 10886 bytes --]

Hi
Attachment is my updated path.
The implementation of parse_sanitizer_options is not elegance enough. Mixing handling flags of fsanitize is easy to make mistakes.

ChangeLog:
gcc/ChangeLog:

2017-09-05  Wish Wu  <wishwu007@gmail.com>

	* asan.c (initialize_sanitizer_builtins):
	Build function type list of trace-cmp.
	* builtin-types.def (BT_FN_VOID_UINT8_UINT8):
	Define function type of trace-cmp.
	(BT_FN_VOID_UINT16_UINT16): Likewise.
	(BT_FN_VOID_UINT32_UINT32): Likewise.
	(BT_FN_VOID_FLOAT_FLOAT): Likewise.
	(BT_FN_VOID_DOUBLE_DOUBLE): Likewise.
	(BT_FN_VOID_UINT64_PTR): Likewise.
	* common.opt: Add options of sanitize coverage.
	* flag-types.h (enum sanitize_coverage_code):
	Declare flags of sanitize coverage.
	* fold-const.c (fold_range_test):
	Disable non-short-circuit feature when sanitize coverage is enabled.
	(fold_truth_andor): Likewise.
	* tree-ssa-ifcombine.c (ifcombine_ifandif): Likewise.
	* opts.c (COVERAGE_SANITIZER_OPT):
	Define coverage sanitizer options.
	(get_closest_sanitizer_option): Make OPT_fsanitize_,
	OPT_fsanitize_recover_ and OPT_fsanitize_coverage_ to use same
	function.
	(parse_sanitizer_options): Likewise.
	(common_handle_option): Add OPT_fsanitize_coverage_.
	* sancov.c (instrument_comparison): Instrument comparisons.
	(instrument_switch): Likewise.
	(sancov_pass): Add trace-cmp support.
	* sanitizer.def (BUILT_IN_SANITIZER_COV_TRACE_CMP1):
	Define builtin functions of trace-cmp.
	(BUILT_IN_SANITIZER_COV_TRACE_CMP2): Likewise.
	(BUILT_IN_SANITIZER_COV_TRACE_CMP4): Likewise.
	(BUILT_IN_SANITIZER_COV_TRACE_CMP8): Likewise.
	(BUILT_IN_SANITIZER_COV_TRACE_CMPF): Likewise.
	(BUILT_IN_SANITIZER_COV_TRACE_CMPD): Likewise.
	(BUILT_IN_SANITIZER_COV_TRACE_SWITCH): Likewise.

gcc/testsuite/ChangeLog:

2017-09-05  Wish Wu  <wishwu007@gmail.com>

	* gcc.dg/sancov/basic3.c: New test.

Thank you every much for improving my codes.

Wish Wu

------------------------------------------------------------------
From:Jakub Jelinek <jakub@redhat.com>
Time:2017 Sep 5 (Tue) 01:34
To:Wish Wu <weixi.wwx@antfin.com>
Cc:Dmitry Vyukov <dvyukov@google.com>; gcc-patches <gcc-patches@gcc.gnu.org>; Jeff Law <law@redhat.com>; wishwu007 <wishwu007@gmail.com>
Subject:Re: Add support to trace comparison instructions and switch statements


On Mon, Sep 04, 2017 at 09:36:40PM +0800, 吴潍浠(此彼) wrote:
> gcc/ChangeLog:                                                     
> 
> 2017-09-04  Wish Wu  <wishwu007@gmail.com>                         
> 
>         * asan.c (initialize_sanitizer_builtins):                  
>         * builtin-types.def (BT_FN_VOID_UINT8_UINT8):              
>         (BT_FN_VOID_UINT16_UINT16):                                
>         (BT_FN_VOID_UINT32_UINT32):                                
>         (BT_FN_VOID_FLOAT_FLOAT):                                  
>         (BT_FN_VOID_DOUBLE_DOUBLE):                                
>         (BT_FN_VOID_UINT64_PTR):                                   
>         * common.opt:                                              
>         * flag-types.h (enum sanitize_coverage_code):              
>         * opts.c (COVERAGE_SANITIZER_OPT):                         
>         (get_closest_sanitizer_option):                            
>         (parse_sanitizer_options):                                 
>         (common_handle_option):                                    
>         * sancov.c (instrument_cond):                              
>         (instrument_switch):                                       
>         (sancov_pass):                                             
>         * sanitizer.def (BUILT_IN_SANITIZER_COV_TRACE_CMP1):       
>         (BUILT_IN_SANITIZER_COV_TRACE_CMP2):                       
>         (BUILT_IN_SANITIZER_COV_TRACE_CMP4):                       
>         (BUILT_IN_SANITIZER_COV_TRACE_CMP8):                       
>         (BUILT_IN_SANITIZER_COV_TRACE_CMPF):                       
>         (BUILT_IN_SANITIZER_COV_TRACE_CMPD):                       
>         (BUILT_IN_SANITIZER_COV_TRACE_SWITCH):                     

mklog just generates a template, you need to fill in the details
on what has been changed or added or removed.  See other ChangeLog
entries etc. to see what is expected.

> For code :
> void bar (void);
> void
> foo (int x)
> {
>   if (x == 21 || x == 64 || x == 98 || x == 135)
>     bar ();
> }
> GIMPLE IL on x86_64:
>   1 
>   2 ;; Function foo (foo, funcdef_no=0, decl_uid=2161, cgraph_uid=0, symbol_order=0)
>   3 
>   4 foo (int x)
>   5 {

...

That is with -O0 though?  With -O2 you'll see that it changes.
IMNSHO you really want to also handle the GIMPLE_ASSIGN with tcc_comparison
class rhs_code.  Shouldn't be that hard to handle that within
instrument_cond, just the way how you extract lhs and rhs from the insn will
differ based on if it is a GIMPLE_COND or GIMPLE_ASSIGN (and in that case
also for tcc_comparison rhs_code or for COND_EXPR with tcc_comparison first
operand).
And I really think we should change the 2 LOGICAL_OP_NON_SHORT_CIRCUIT
uses in fold-const.c and one in tree-ssa-ifcombine.c with
  LOGICAL_OP_NON_SHORT_CIRCUIT
  && !flag_sanitize_coverage
with a comment that for sanitize coverage we want to avoid this optimization
because it negatively affects it.

@@ -1611,38 +1631,51 @@ parse_sanitizer_options (const char *p, location_t
  }
 
       /* 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)
+      for (i = 0; opts[i].name != NULL; ++i)
+ if (len == opts[i].len
+     && memcmp (p, opts[i].name, len) == 0)
    {
-     /* Handle both -fsanitize and -fno-sanitize cases.  */
-     if (value && sanitizer_opts[i].flag == ~0U)
+     if (code == OPT_fsanitize_coverage_)
        {
-  if (code == OPT_fsanitize_)
-    {
-      if (complain)
-        error_at (loc, "%<-fsanitize=all%> option is not valid");
-    }
+  if (value)
+    flags |= opts[i].flag;
   else
-    flags |= ~(SANITIZE_THREAD | SANITIZE_LEAK
-        | SANITIZE_UNREACHABLE | SANITIZE_RETURN);
+    flags &= ~opts[i].flag;
+  found = true;
+  break;
        }
-     else if (value)
+     else
        {
-  /* Do not enable -fsanitize-recover=unreachable and
-     -fsanitize-recover=return if -fsanitize-recover=undefined
-     is selected.  */
-  if (code == OPT_fsanitize_recover_
-      && sanitizer_opts[i].flag == SANITIZE_UNDEFINED)
-    flags |= (SANITIZE_UNDEFINED
-       & ~(SANITIZE_UNREACHABLE | SANITIZE_RETURN));
+  /* Handle both -fsanitize and -fno-sanitize cases.  */
+  if (value && opts[i].flag == ~0U)
+    {
+      if (code == OPT_fsanitize_)
+        {
+   if (complain)
+     error_at (loc,
+        "%<-fsanitize=all%> option is not valid");
+        }
+      else
+        flags |= ~(SANITIZE_THREAD | SANITIZE_LEAK
+     | SANITIZE_UNREACHABLE | SANITIZE_RETURN);
+    }
+  else if (value)
+    {
+      /* Do not enable -fsanitize-recover=unreachable and
+         -fsanitize-recover=return if -fsanitize-recover=undefined
+         is selected.  */
+      if (code == OPT_fsanitize_recover_
+   && opts[i].flag == SANITIZE_UNDEFINED)
+        flags |= (SANITIZE_UNDEFINED
+    & ~(SANITIZE_UNREACHABLE | SANITIZE_RETURN));
+      else
+        flags |= opts[i].flag;
+    }
   else
-    flags |= sanitizer_opts[i].flag;
+    flags &= ~opts[i].flag;
+  found = true;
+  break;
        }
-     else
-       flags &= ~sanitizer_opts[i].flag;
-     found = true;
-     break;
    }

I don't see a need for this hunk.  For code == OPT_fsanitize_coverage_
(where you know that there is no entry with ~0U flag value and also
know that code is not OPT_fsanitize_recover_) I think it will
just DTRT without any changes.
 
 namespace {

There should be a function comment before the function here, explaining
what the function is for.
 
+static void
+instrument_cond (gimple_stmt_iterator *gsi, gimple *stmt)
+{
+  tree lhs = gimple_cond_lhs (stmt);
+  tree rhs = gimple_cond_rhs (stmt);
+  tree lhs_type = TREE_TYPE (lhs);
+  tree rhs_type = TREE_TYPE (rhs);
+
+  HOST_WIDE_INT size_in_bytes = MAX (int_size_in_bytes (lhs_type),
+         int_size_in_bytes (rhs_type));
+  if (size_in_bytes == -1)
+    return;

As I said, GIMPLE_COND operands should have the same (or compatible)
type, so there is no need to use rhs_type at all, nor test it.
And there is no need to test size_in_bytes before the if, just do
it right before the switch in there (and no need to special case -1,
because it is like any other default: handled by return;).

+  else if (SCALAR_FLOAT_TYPE_P (lhs_type) && SCALAR_FLOAT_TYPE_P (rhs_type))

Again, no need to test both.

+    {
+      if (TYPE_MODE (lhs_type) == TYPE_MODE (double_type_node)
+   || TYPE_MODE (rhs_type) == TYPE_MODE (double_type_node))

Or here.

+ {
+         fncode = BUILT_IN_SANITIZER_COV_TRACE_CMPD;
+   to_type = double_type_node;
+ }
+      else if (TYPE_MODE (lhs_type) == TYPE_MODE (float_type_node)
+        || TYPE_MODE (rhs_type) == TYPE_MODE (float_type_node))

Or here.  Just use type instead of lhs_type.

+ {
+         fncode = BUILT_IN_SANITIZER_COV_TRACE_CMPF;
+   to_type = float_type_node;
+ }
+
+    }
+  if (to_type != NULL_TREE)
+    {
+      gimple_seq seq = NULL;
+
+      gimple_seq_add_stmt (&seq, build_type_cast (to_type, lhs));
+      tree clhs = gimple_assign_lhs (gimple_seq_last_stmt (seq));
+
+      gimple_seq_add_stmt (&seq, build_type_cast (to_type, rhs));
+      tree crhs = gimple_assign_lhs (gimple_seq_last_stmt (seq));

If the var already has the right type, that will just create waste
that further opts will need to clean up.  Better:
  if (!useless_type_conversion_p (to_type, lhs_type)) // or s/lhs_// if you do the above
    {
      gimple_seq_add_stmt (&seq, build_type_cast (to_type, lhs));
      lhs = gimple_assign_lhs (gimple_seq_last_stmt (seq));

      gimple_seq_add_stmt (&seq, build_type_cast (to_type, rhs));
      rhs = gimple_assign_rhs (gimple_seq_last_stmt (seq));
    }
or perhaps even 
  if (!useless_type_conversion_p (to_type, type))
    {
      if (TREE_CODE (lhs) == INTEGER_CST)
 lhs = fold_convert (to_type, lhs);
      else
 {
   gimple_seq_add_stmt (&seq, build_type_cast (to_type, lhs));
   lhs = gimple_assign_lhs (gimple_seq_last_stmt (seq));
 }
...
    }

+  tree index = gimple_switch_index (switch_stmt);
+
+  HOST_WIDE_INT size_in_bytes = int_size_in_bytes (TREE_TYPE (index));
+  if (size_in_bytes == -1)
+    return;

Well, you want to punt not just when it is -1, but also when it is
> 8.

+
+  unsigned i, n = gimple_switch_num_labels (switch_stmt), num = 0;
+  for (i = 0; i < n; ++i)

I think gimple_switch_label (switch_stmt, 0) must be the
default label and thus have no low/high case, so you should
use for (i = 1; i < n; ++i).

+  for (i = 0; i < n; ++i)

Ditto.

 Jakub

[-- Attachment #2: gcc-svn-201709052016.patch --]
[-- Type: application/octet-stream, Size: 23568 bytes --]

Index: gcc/asan.c
===================================================================
--- gcc/asan.c	(revision 251636)
+++ gcc/asan.c	(working copy)
@@ -2709,6 +2709,29 @@ initialize_sanitizer_builtins (void)
   tree BT_FN_SIZE_CONST_PTR_INT
     = build_function_type_list (size_type_node, const_ptr_type_node,
 				integer_type_node, NULL_TREE);
+
+  tree BT_FN_VOID_UINT8_UINT8
+    = build_function_type_list (void_type_node, unsigned_char_type_node,
+				unsigned_char_type_node, NULL_TREE);
+  tree BT_FN_VOID_UINT16_UINT16
+    = build_function_type_list (void_type_node, uint16_type_node,
+				uint16_type_node, NULL_TREE);
+  tree BT_FN_VOID_UINT32_UINT32
+    = build_function_type_list (void_type_node, uint32_type_node,
+				uint32_type_node, NULL_TREE);
+  tree BT_FN_VOID_UINT64_UINT64
+    = build_function_type_list (void_type_node, uint64_type_node,
+				uint64_type_node, NULL_TREE);
+  tree BT_FN_VOID_FLOAT_FLOAT
+    = build_function_type_list (void_type_node, float_type_node,
+				float_type_node, NULL_TREE);
+  tree BT_FN_VOID_DOUBLE_DOUBLE
+    = build_function_type_list (void_type_node, double_type_node,
+				double_type_node, NULL_TREE);
+  tree BT_FN_VOID_UINT64_PTR
+    = build_function_type_list (void_type_node, uint64_type_node,
+				ptr_type_node, NULL_TREE);
+
   tree BT_FN_BOOL_VPTR_PTR_IX_INT_INT[5];
   tree BT_FN_IX_CONST_VPTR_INT[5];
   tree BT_FN_IX_VPTR_IX_INT[5];
Index: gcc/builtin-types.def
===================================================================
--- gcc/builtin-types.def	(revision 251636)
+++ gcc/builtin-types.def	(working copy)
@@ -338,8 +338,20 @@ DEF_FUNCTION_TYPE_2 (BT_FN_VOID_PTRMODE_PTR,
 		     BT_VOID, BT_PTRMODE, BT_PTR)
 DEF_FUNCTION_TYPE_2 (BT_FN_VOID_PTR_PTRMODE,
 		     BT_VOID, BT_PTR, BT_PTRMODE)
+DEF_FUNCTION_TYPE_2 (BT_FN_VOID_UINT8_UINT8,
+     		     BT_VOID, BT_UINT8, BT_UINT8)
+DEF_FUNCTION_TYPE_2 (BT_FN_VOID_UINT16_UINT16,
+     		     BT_VOID, BT_UINT16, BT_UINT16)
+DEF_FUNCTION_TYPE_2 (BT_FN_VOID_UINT32_UINT32,
+     		     BT_VOID, BT_UINT32, BT_UINT32)
 DEF_FUNCTION_TYPE_2 (BT_FN_VOID_UINT64_UINT64,
      		     BT_VOID, BT_UINT64, BT_UINT64)
+DEF_FUNCTION_TYPE_2 (BT_FN_VOID_FLOAT_FLOAT,
+     		     BT_VOID, BT_FLOAT, BT_FLOAT)
+DEF_FUNCTION_TYPE_2 (BT_FN_VOID_DOUBLE_DOUBLE,
+     		     BT_VOID, BT_DOUBLE, BT_DOUBLE)
+DEF_FUNCTION_TYPE_2 (BT_FN_VOID_UINT64_PTR,
+     		     BT_VOID, BT_UINT64, BT_PTR)
 DEF_FUNCTION_TYPE_2 (BT_FN_VOID_VALIST_REF_VALIST_ARG,
 		     BT_VOID, BT_VALIST_REF, BT_VALIST_ARG)
 DEF_FUNCTION_TYPE_2 (BT_FN_LONG_LONG_LONG,
Index: gcc/common.opt
===================================================================
--- gcc/common.opt	(revision 251636)
+++ gcc/common.opt	(working copy)
@@ -233,10 +233,9 @@ unsigned int flag_sanitize
 Variable
 unsigned int flag_sanitize_recover = (SANITIZE_UNDEFINED | SANITIZE_UNDEFINED_NONDEFAULT | SANITIZE_KERNEL_ADDRESS) & ~(SANITIZE_UNREACHABLE | SANITIZE_RETURN)
 
-fsanitize-coverage=trace-pc
-Common Report Var(flag_sanitize_coverage)
-Enable coverage-guided fuzzing code instrumentation.
-Inserts call to __sanitizer_cov_trace_pc into every basic block.
+; What the coverage sanitizers should instrument
+Variable
+unsigned int flag_sanitize_coverage
 
 ; Flag whether a prefix has been added to dump_base_name
 Variable
@@ -982,6 +981,10 @@ fsanitize=
 Common Driver Report Joined
 Select what to sanitize.
 
+fsanitize-coverage=
+Common Report Joined
+Select what to coverage sanitize.
+
 fasan-shadow-offset=
 Common Joined RejectNegative Var(common_deferred_options) Defer
 -fasan-shadow-offset=<number>	Use custom shadow memory offset.
Index: gcc/flag-types.h
===================================================================
--- gcc/flag-types.h	(revision 251636)
+++ gcc/flag-types.h	(working copy)
@@ -252,6 +252,14 @@ enum sanitize_code {
 				  | SANITIZE_BOUNDS_STRICT
 };
 
+/* Different trace modes.  */
+enum sanitize_coverage_code {
+  /* Trace PC.  */
+  SANITIZE_COV_TRACE_PC = 1 << 0,
+  /* Trace Comparison.  */
+  SANITIZE_COV_TRACE_CMP = 1 << 1
+};
+
 /* flag_vtable_verify initialization levels. */
 enum vtv_priority {
   VTV_NO_PRIORITY       = 0,  /* i.E. Do NOT do vtable verification. */
Index: gcc/fold-const.c
===================================================================
--- gcc/fold-const.c	(revision 251636)
+++ gcc/fold-const.c	(working copy)
@@ -5394,6 +5394,7 @@ fold_range_test (location_t loc, enum tree_code co
      short-circuited branch and the underlying object on both sides
      is the same, make a non-short-circuit operation.  */
   else if (LOGICAL_OP_NON_SHORT_CIRCUIT
+	   && !flag_sanitize_coverage
 	   && lhs != 0 && rhs != 0
 	   && (code == TRUTH_ANDIF_EXPR
 	       || code == TRUTH_ORIF_EXPR)
@@ -8035,6 +8036,7 @@ fold_truth_andor (location_t loc, enum tree_code c
     return tem;
 
   if (LOGICAL_OP_NON_SHORT_CIRCUIT
+      && !flag_sanitize_coverage
       && (code == TRUTH_AND_EXPR
           || code == TRUTH_ANDIF_EXPR
           || code == TRUTH_OR_EXPR
Index: gcc/opts.c
===================================================================
--- gcc/opts.c	(revision 251636)
+++ gcc/opts.c	(working copy)
@@ -1526,6 +1526,17 @@ const struct sanitizer_opts_s sanitizer_opts[] =
   { NULL, 0U, 0UL, false }
 };
 
+/* -f{,no-}sanitize-coverage= suboptions.  */
+const struct sanitizer_opts_s coverage_sanitizer_opts[] =
+{
+#define COVERAGE_SANITIZER_OPT(name, flags) \
+    { #name, flags, sizeof #name - 1, true }
+  COVERAGE_SANITIZER_OPT (trace-pc, SANITIZE_COV_TRACE_PC),
+  COVERAGE_SANITIZER_OPT (trace-cmp, SANITIZE_COV_TRACE_CMP),
+#undef COVERAGE_SANITIZER_OPT
+  { NULL, 0U, 0UL, false }
+};
+
 /* A struct for describing a run of chars within a string.  */
 
 struct string_fragment
@@ -1556,31 +1567,34 @@ struct edit_distance_traits<const string_fragment
 
 /* Given ARG, an unrecognized sanitizer option, return the best
    matching sanitizer option, or NULL if there isn't one.
-   CODE is OPT_fsanitize_ or OPT_fsanitize_recover_.
+   OPTS is array of candidate sanitizer options.
+   CODE is OPT_fsanitize_ , OPT_fsanitize_recover_ or
+   OPT_fsanitize_coverage_ .
    VALUE is non-zero for the regular form of the option, zero
    for the "no-" form (e.g. "-fno-sanitize-recover=").  */
 
 static const char *
 get_closest_sanitizer_option (const string_fragment &arg,
+			      const struct sanitizer_opts_s *opts,
 			      enum opt_code code, int value)
 {
   best_match <const string_fragment &, const char*> bm (arg);
-  for (int i = 0; sanitizer_opts[i].name != NULL; ++i)
+  for (int i = 0; opts[i].name != NULL; ++i)
     {
       /* -fsanitize=all is not valid, so don't offer it.  */
-      if (sanitizer_opts[i].flag == ~0U
-	  && code == OPT_fsanitize_
+      if (code == OPT_fsanitize_
+	  && opts[i].flag == ~0U
 	  && value)
 	continue;
 
       /* For -fsanitize-recover= (and not -fno-sanitize-recover=),
 	 don't offer the non-recoverable options.  */
-      if (!sanitizer_opts[i].can_recover
-	  && code == OPT_fsanitize_recover_
+      if (code == OPT_fsanitize_recover_
+	  && !opts[i].can_recover
 	  && value)
 	continue;
 
-      bm.consider (sanitizer_opts[i].name);
+      bm.consider (opts[i].name);
     }
   return bm.get_best_meaningful_candidate ();
 }
@@ -1594,6 +1608,13 @@ parse_sanitizer_options (const char *p, location_t
 			 unsigned int flags, int value, bool complain)
 {
   enum opt_code code = (enum opt_code) scode;
+
+  const struct sanitizer_opts_s *opts;
+  if (code == OPT_fsanitize_coverage_)
+    opts = coverage_sanitizer_opts;
+  else
+    opts = sanitizer_opts;
+
   while (*p != 0)
     {
       size_t len, i;
@@ -1611,38 +1632,51 @@ parse_sanitizer_options (const char *p, location_t
 	}
 
       /* 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)
+      for (i = 0; opts[i].name != NULL; ++i)
+	if (len == opts[i].len
+	    && memcmp (p, opts[i].name, len) == 0)
 	  {
-	    /* Handle both -fsanitize and -fno-sanitize cases.  */
-	    if (value && sanitizer_opts[i].flag == ~0U)
+	    if (code == OPT_fsanitize_coverage_)
 	      {
-		if (code == OPT_fsanitize_)
-		  {
-		    if (complain)
-		      error_at (loc, "%<-fsanitize=all%> option is not valid");
-		  }
+		if (value)
+		  flags |= opts[i].flag;
 		else
-		  flags |= ~(SANITIZE_THREAD | SANITIZE_LEAK
-			     | SANITIZE_UNREACHABLE | SANITIZE_RETURN);
+		  flags &= ~opts[i].flag;
+		found = true;
+		break;
 	      }
-	    else if (value)
+	    else
 	      {
-		/* Do not enable -fsanitize-recover=unreachable and
-		   -fsanitize-recover=return if -fsanitize-recover=undefined
-		   is selected.  */
-		if (code == OPT_fsanitize_recover_
-		    && sanitizer_opts[i].flag == SANITIZE_UNDEFINED)
-		  flags |= (SANITIZE_UNDEFINED
-			    & ~(SANITIZE_UNREACHABLE | SANITIZE_RETURN));
+		/* Handle both -fsanitize and -fno-sanitize cases.  */
+		if (value && opts[i].flag == ~0U)
+		  {
+		    if (code == OPT_fsanitize_)
+		      {
+			if (complain)
+			  error_at (loc,
+				    "%<-fsanitize=all%> option is not valid");
+		      }
+		    else
+		      flags |= ~(SANITIZE_THREAD | SANITIZE_LEAK
+				 | SANITIZE_UNREACHABLE | SANITIZE_RETURN);
+		  }
+		else if (value)
+		  {
+		    /* Do not enable -fsanitize-recover=unreachable and
+		       -fsanitize-recover=return if -fsanitize-recover=undefined
+		       is selected.  */
+		    if (code == OPT_fsanitize_recover_
+			&& opts[i].flag == SANITIZE_UNDEFINED)
+		      flags |= (SANITIZE_UNDEFINED
+				& ~(SANITIZE_UNREACHABLE | SANITIZE_RETURN));
+		    else
+		      flags |= opts[i].flag;
+		  }
 		else
-		  flags |= sanitizer_opts[i].flag;
+		  flags &= ~opts[i].flag;
+		found = true;
+		break;
 	      }
-	    else
-	      flags &= ~sanitizer_opts[i].flag;
-	    found = true;
-	    break;
 	  }
 
       if (! found && complain)
@@ -1649,21 +1683,27 @@ parse_sanitizer_options (const char *p, location_t
 	{
 	  const char *hint
 	    = get_closest_sanitizer_option (string_fragment (p, len),
-					    code, value);
+					    opts, code, value);
 
+	  const char *suffix;
+	  if (code == OPT_fsanitize_recover_)
+	    suffix = "-recover";
+	  else if (code == OPT_fsanitize_coverage_)
+	    suffix = "-coverage";
+	  else
+	    suffix = "";
+
 	  if (hint)
 	    error_at (loc,
 		      "unrecognized argument to -f%ssanitize%s= option: %q.*s;"
 		      " did you mean %qs?",
 		      value ? "" : "no-",
-		      code == OPT_fsanitize_ ? "" : "-recover",
-		      (int) len, p, hint);
+		      suffix, (int) len, p, hint);
 	  else
 	    error_at (loc,
 		      "unrecognized argument to -f%ssanitize%s= option: %q.*s",
 		      value ? "" : "no-",
-		      code == OPT_fsanitize_ ? "" : "-recover",
-		      (int) len, p);
+		      suffix, (int) len, p);
 	}
 
       if (comma == NULL)
@@ -1956,6 +1996,12 @@ common_handle_option (struct gcc_options *opts,
 	  &= ~(SANITIZE_UNDEFINED | SANITIZE_UNDEFINED_NONDEFAULT);
       break;
 
+    case OPT_fsanitize_coverage_:
+      opts->x_flag_sanitize_coverage
+	= parse_sanitizer_options (arg, loc, code,
+				   opts->x_flag_sanitize_coverage, value, true);
+      break;
+
     case OPT_O:
     case OPT_Os:
     case OPT_Ofast:
Index: gcc/sancov.c
===================================================================
--- gcc/sancov.c	(revision 251636)
+++ gcc/sancov.c	(working copy)
@@ -1,6 +1,7 @@
 /* Code coverage instrumentation for fuzzing.
    Copyright (C) 2015-2017 Free Software Foundation, Inc.
-   Contributed by Dmitry Vyukov <dvyukov@google.com>
+   Contributed by Dmitry Vyukov <dvyukov@google.com> and
+   Wish Wu <wishwu007@gmail.com>
 
 This file is part of GCC.
 
@@ -29,33 +30,255 @@ along with GCC; see the file COPYING3.  If not see
 #include "flags.h"
 #include "stmt.h"
 #include "gimple-iterator.h"
+#include "gimple-builder.h"
 #include "tree-cfg.h"
 #include "tree-pass.h"
 #include "tree-iterator.h"
+#include "fold-const.h"
 #include "stringpool.h"
 #include "attribs.h"
+#include "output.h"
+#include "cgraph.h"
 #include "asan.h"
 
 namespace {
 
+/* Instrument one comparison operation.  Deliver lhs and rhs values.  */
+
+static void
+instrument_comparison (gimple_stmt_iterator *gsi, tree lhs, tree rhs)
+{
+  tree type = TREE_TYPE (lhs);
+
+  HOST_WIDE_INT size_in_bytes = MAX (int_size_in_bytes (type),
+				     int_size_in_bytes (TREE_TYPE (rhs)));
+  enum built_in_function fncode;
+  tree to_type = NULL_TREE;
+
+  if (INTEGRAL_TYPE_P (type))
+    {
+      switch (size_in_bytes)
+	{
+	case 1:
+      	  fncode = BUILT_IN_SANITIZER_COV_TRACE_CMP1;
+	  to_type = unsigned_char_type_node;
+	  break;
+
+	case 2:
+      	  fncode = BUILT_IN_SANITIZER_COV_TRACE_CMP2;
+	  to_type = uint16_type_node;
+	  break;
+
+	case 4:
+      	  fncode = BUILT_IN_SANITIZER_COV_TRACE_CMP4;
+	  to_type = uint32_type_node;
+	  break;
+
+	default:
+      	  fncode = BUILT_IN_SANITIZER_COV_TRACE_CMP8;
+	  to_type = uint64_type_node;
+	  break;
+	}
+    }
+  else if (SCALAR_FLOAT_TYPE_P (type))
+    {
+      if (TYPE_MODE (type) == TYPE_MODE (float_type_node))
+	{
+      	  fncode = BUILT_IN_SANITIZER_COV_TRACE_CMPF;
+	  to_type = float_type_node;
+	}
+      else if (TYPE_MODE (type) == TYPE_MODE (double_type_node))
+	{
+      	  fncode = BUILT_IN_SANITIZER_COV_TRACE_CMPD;
+	  to_type = double_type_node;
+	}
+    }
+
+  if (to_type != NULL_TREE)
+    {
+      gimple_seq seq = NULL;
+
+      if (!useless_type_conversion_p (to_type, type))
+	{
+	  if (TREE_CODE (lhs) == INTEGER_CST)
+	    lhs = fold_convert (to_type, lhs);
+	  else
+	    {
+	      gimple_seq_add_stmt (&seq, build_type_cast (to_type, lhs));
+	      lhs = gimple_assign_lhs (gimple_seq_last_stmt (seq));
+	    }
+
+	  if (TREE_CODE (rhs) == INTEGER_CST)
+	    rhs = fold_convert (to_type, rhs);
+	  else
+	    {
+	      gimple_seq_add_stmt (&seq, build_type_cast (to_type, rhs));
+	      rhs = gimple_assign_lhs (gimple_seq_last_stmt (seq));
+	    }
+	}
+
+      tree fndecl = builtin_decl_implicit (fncode);
+      gimple *gcall = gimple_build_call (fndecl, 2, lhs, rhs);
+      gimple_seq_add_stmt (&seq, gcall);
+
+      gimple_seq_set_location (seq, gimple_location (gsi_stmt (*gsi)));
+      gsi_insert_seq_before (gsi, seq, GSI_SAME_STMT);
+    }
+}
+
+/* Instrument switch statement.  Deliver the value of index and
+   an array having case values.  */
+
+static void
+instrument_switch (gimple_stmt_iterator *gsi, gimple *stmt, function *fun)
+{
+  gswitch *switch_stmt = as_a<gswitch *> (stmt);
+  tree index = gimple_switch_index (switch_stmt);
+
+  HOST_WIDE_INT size_in_bytes = int_size_in_bytes (TREE_TYPE (index));
+  if (size_in_bytes == -1 || size_in_bytes > 8)
+    size_in_bytes = 8;
+
+  unsigned i, n = gimple_switch_num_labels (switch_stmt), num = 0;
+  for (i = 1; i < n; ++i)
+    {
+      tree label = gimple_switch_label (switch_stmt, i);
+
+      tree low_case = CASE_LOW (label);
+      if (low_case != NULL_TREE)
+	num++;
+
+      tree high_case = CASE_HIGH (label);
+      if (high_case != NULL_TREE)
+	num++;
+    }
+
+  tree case_array_type
+   = build_array_type (build_type_variant (uint64_type_node, 1, 0),
+		       build_index_type (size_int (num + 2 - 1)));
+
+  char name[64];
+  static size_t case_array_count = 0;
+  ASM_GENERATE_INTERNAL_LABEL (name, "LCASEARRAY", case_array_count++);
+  tree case_array_var = build_decl (UNKNOWN_LOCATION, VAR_DECL,
+				    get_identifier (name), case_array_type);
+  TREE_STATIC (case_array_var) = 1;
+  TREE_PUBLIC (case_array_var) = 0;
+  TREE_CONSTANT (case_array_var) = 1;
+  TREE_READONLY (case_array_var) = 1;
+  DECL_EXTERNAL (case_array_var) = 0;
+  DECL_ARTIFICIAL (case_array_var) = 1;
+  DECL_IGNORED_P (case_array_var) = 1;
+
+  vec <constructor_elt, va_gc> *v = NULL;
+  vec_alloc (v, num + 2);
+  CONSTRUCTOR_APPEND_ELT (v, NULL_TREE,
+			  build_int_cst (uint64_type_node, num));
+  CONSTRUCTOR_APPEND_ELT (v, NULL_TREE,
+			  build_int_cst (uint64_type_node,
+					 size_in_bytes * 8));
+  for (i = 1; i < n; ++i)
+    {
+      tree label = gimple_switch_label (switch_stmt, i);
+
+      tree low_case = CASE_LOW (label);
+      if (low_case != NULL_TREE)
+	CONSTRUCTOR_APPEND_ELT (v, NULL_TREE,
+				fold_convert (uint64_type_node, low_case));
+
+      tree high_case = CASE_HIGH (label);
+      if (high_case != NULL_TREE)
+	CONSTRUCTOR_APPEND_ELT (v, NULL_TREE,
+				fold_convert (uint64_type_node, high_case));
+    }
+  tree ctor = build_constructor (case_array_type, v);
+  TREE_STATIC (ctor) = 1;
+  TREE_PUBLIC (ctor) = 0;
+  TREE_CONSTANT (ctor) = 1;
+  TREE_READONLY (ctor) = 1;
+  DECL_INITIAL (case_array_var) = ctor;
+  varpool_node::finalize_decl (case_array_var);
+  add_local_decl (fun, case_array_var);
+
+  gimple_seq seq = NULL;
+
+  if (!useless_type_conversion_p (uint64_type_node, TREE_TYPE (index)))
+    {
+      if (TREE_CODE (index) == INTEGER_CST)
+	index = fold_convert (uint64_type_node, index);
+      else
+	{
+	  gimple_seq_add_stmt (&seq, build_type_cast (uint64_type_node, index));
+	  index = gimple_assign_lhs (gimple_seq_last_stmt (seq));
+	}
+    }
+
+  tree fndecl = builtin_decl_implicit (BUILT_IN_SANITIZER_COV_TRACE_SWITCH);
+  gimple *gcall = gimple_build_call (fndecl, 2, index,
+				     build_fold_addr_expr (case_array_var));
+  gimple_seq_add_stmt (&seq, gcall);
+
+  gimple_seq_set_location (seq, gimple_location (stmt));
+  gsi_insert_seq_before (gsi, seq, GSI_SAME_STMT);
+}
+
 unsigned
 sancov_pass (function *fun)
 {
   initialize_sanitizer_builtins ();
 
+  basic_block bb;
+
   /* Insert callback into beginning of every BB. */
-  tree fndecl = builtin_decl_implicit (BUILT_IN_SANITIZER_COV_TRACE_PC);
-  basic_block bb;
-  FOR_EACH_BB_FN (bb, fun)
+  if (flag_sanitize_coverage & SANITIZE_COV_TRACE_PC)
     {
-      gimple_stmt_iterator gsi = gsi_start_nondebug_after_labels_bb (bb);
-      if (gsi_end_p (gsi))
-	continue;
-      gimple *stmt = gsi_stmt (gsi);
-      gimple *gcall = gimple_build_call (fndecl, 0);
-      gimple_set_location (gcall, gimple_location (stmt));
-      gsi_insert_before (&gsi, gcall, GSI_SAME_STMT);
+      tree fndecl = builtin_decl_implicit (BUILT_IN_SANITIZER_COV_TRACE_PC);
+      FOR_EACH_BB_FN (bb, fun)
+	{
+	  gimple_stmt_iterator gsi = gsi_start_nondebug_after_labels_bb (bb);
+	  if (gsi_end_p (gsi))
+	    continue;
+	  gimple *stmt = gsi_stmt (gsi);
+	  gimple *gcall = gimple_build_call (fndecl, 0);
+	  gimple_set_location (gcall, gimple_location (stmt));
+	  gsi_insert_before (&gsi, gcall, GSI_SAME_STMT);
+	}
     }
+
+  /* Insert callback into every comparison related operation.  */
+  if (flag_sanitize_coverage & SANITIZE_COV_TRACE_CMP)
+    {
+      FOR_EACH_BB_FN (bb, fun)
+	{
+	  gimple_stmt_iterator gsi;
+	  for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
+	    {
+	      gimple *stmt = gsi_stmt (gsi);
+	      switch (gimple_code (stmt))
+		{
+		case GIMPLE_ASSIGN:
+		  if (TREE_CODE_CLASS (gimple_assign_rhs_code (stmt))
+		      == tcc_comparison)
+		    instrument_comparison (&gsi,
+					   gimple_assign_rhs1 (stmt),
+					   gimple_assign_rhs2 (stmt));
+		  break;
+		case GIMPLE_COND:
+		  instrument_comparison (&gsi,
+					 gimple_cond_lhs (stmt),
+					 gimple_cond_rhs (stmt));
+		  break;
+
+		case GIMPLE_SWITCH:
+		  instrument_switch (&gsi, stmt, fun);
+		  break;
+
+		default:
+		  break;
+		}
+	    }
+	}
+    }
   return 0;
 }
 
Index: gcc/sanitizer.def
===================================================================
--- gcc/sanitizer.def	(revision 251636)
+++ gcc/sanitizer.def	(working copy)
@@ -537,6 +537,27 @@ DEF_SANITIZER_BUILTIN(BUILT_IN_UBSAN_HANDLE_DYNAMI
 DEF_SANITIZER_BUILTIN(BUILT_IN_SANITIZER_COV_TRACE_PC,
 		      "__sanitizer_cov_trace_pc",
 		      BT_FN_VOID, ATTR_NOTHROW_LEAF_LIST)
+DEF_SANITIZER_BUILTIN(BUILT_IN_SANITIZER_COV_TRACE_CMP1,
+		      "__sanitizer_cov_trace_cmp1",
+		      BT_FN_VOID_UINT8_UINT8, ATTR_NOTHROW_LEAF_LIST)
+DEF_SANITIZER_BUILTIN(BUILT_IN_SANITIZER_COV_TRACE_CMP2,
+		      "__sanitizer_cov_trace_cmp2",
+		      BT_FN_VOID_UINT16_UINT16, ATTR_NOTHROW_LEAF_LIST)
+DEF_SANITIZER_BUILTIN(BUILT_IN_SANITIZER_COV_TRACE_CMP4,
+		      "__sanitizer_cov_trace_cmp4",
+		      BT_FN_VOID_UINT32_UINT32, ATTR_NOTHROW_LEAF_LIST)
+DEF_SANITIZER_BUILTIN(BUILT_IN_SANITIZER_COV_TRACE_CMP8,
+		      "__sanitizer_cov_trace_cmp8",
+		      BT_FN_VOID_UINT64_UINT64, ATTR_NOTHROW_LEAF_LIST)
+DEF_SANITIZER_BUILTIN(BUILT_IN_SANITIZER_COV_TRACE_CMPF,
+		      "__sanitizer_cov_trace_cmpf",
+		      BT_FN_VOID_FLOAT_FLOAT, ATTR_NOTHROW_LEAF_LIST)
+DEF_SANITIZER_BUILTIN(BUILT_IN_SANITIZER_COV_TRACE_CMPD,
+		      "__sanitizer_cov_trace_cmpd",
+		      BT_FN_VOID_DOUBLE_DOUBLE, ATTR_NOTHROW_LEAF_LIST)
+DEF_SANITIZER_BUILTIN(BUILT_IN_SANITIZER_COV_TRACE_SWITCH,
+		      "__sanitizer_cov_trace_switch",
+		      BT_FN_VOID_UINT64_PTR, ATTR_NOTHROW_LEAF_LIST)
 
 /* This has to come after all the sanitizer builtins.  */
 DEF_BUILTIN_STUB(END_SANITIZER_BUILTINS, (const char *)0)
Index: gcc/testsuite/gcc.dg/sancov/basic3.c
===================================================================
--- gcc/testsuite/gcc.dg/sancov/basic3.c	(nonexistent)
+++ gcc/testsuite/gcc.dg/sancov/basic3.c	(working copy)
@@ -0,0 +1,69 @@
+/* Basic test on number of inserted callbacks.  */
+/* { dg-do compile } */
+/* { dg-options "-fsanitize-coverage=trace-cmp -fdump-tree-optimized" } */
+
+void foo(char *a, short *b, int *c, long long *d, float *e, double *f)
+{
+  if (*a)
+    *a += 1;
+  if (*b)
+    *b = *a;
+  if (*c)
+    *c += 1;
+  if(*d)
+    *d = *c;
+  if(*e == *c)
+    *e = *c;
+  if(*f == *e)
+    *f = *e;
+  switch(*a)
+    {
+    case 2:
+      *b += 2;
+      break;
+    case 3:
+      *b += 3;
+      break;
+    case 4:
+      *b += 4;
+      break;
+    case 5:
+      *b += 5;
+      break;
+    case 6:
+      *b += 6;
+      break;
+    case 7:
+      *b += 7;
+      break;
+    default:
+      break;
+    }
+  switch(*d)
+    {
+    case 3:
+      *d += 3;
+    case -4:
+      *d -= 4;
+    case -5:
+      *d -= 5;
+    case -6:
+      *d -= 6;
+    case -7:
+      *d -= 7;
+    case -8:
+      *d -= 8;
+    case -9:
+      *d -= 9;
+    case -10:
+      *d -= 10;
+    }
+}
+
+/* { dg-final { scan-tree-dump-times "__builtin___sanitizer_cov_trace_cmp1 \\(" 1 "optimized" } } */
+/* { dg-final { scan-tree-dump-times "__builtin___sanitizer_cov_trace_cmp2 \\(" 1 "optimized" } } */
+/* { dg-final { scan-tree-dump-times "__builtin___sanitizer_cov_trace_cmp4 \\(" 1 "optimized" } } */
+/* { dg-final { scan-tree-dump-times "__builtin___sanitizer_cov_trace_cmp8 \\(" 1 "optimized" } } */
+/* { dg-final { scan-tree-dump-times "__builtin___sanitizer_cov_trace_cmpf \\(" 1 "optimized" } } */
+/* { dg-final { scan-tree-dump-times "__builtin___sanitizer_cov_trace_cmpd \\(" 1 "optimized" } } */
+/* { dg-final { scan-tree-dump-times "__builtin___sanitizer_cov_trace_switch \\(" 2 "optimized" } } */
Index: gcc/tree-ssa-ifcombine.c
===================================================================
--- gcc/tree-ssa-ifcombine.c	(revision 251636)
+++ gcc/tree-ssa-ifcombine.c	(working copy)
@@ -560,7 +560,7 @@ ifcombine_ifandif (basic_block inner_cond_bb, bool
 	{
 	  tree t1, t2;
 	  gimple_stmt_iterator gsi;
-	  if (!LOGICAL_OP_NON_SHORT_CIRCUIT)
+	  if (!LOGICAL_OP_NON_SHORT_CIRCUIT || flag_sanitize_coverage)
 	    return false;
 	  /* Only do this optimization if the inner bb contains only the conditional. */
 	  if (!gsi_one_before_end_p (gsi_start_nondebug_after_labels_bb (inner_cond_bb)))