Patchwork [asan] Fix some asan ICEs

login
register
mail settings
Submitter Jakub Jelinek
Date Nov. 27, 2012, 6:40 p.m.
Message ID <20121127184028.GE2315@tucnak.redhat.com>
Download mbox | patch
Permalink /patch/202280/
State New
Headers show

Comments

Jakub Jelinek - Nov. 27, 2012, 6:40 p.m.
Hi!

This fixes a couple of asan ICEs found while running make check
with RUNTESTFLAGS='unix/-fsanitize=address'.
The last two hunks fix ICEs while instrumenting atomics with non-standard
sizes, those are always turned into library calls, and the first argument
is the length, not a pointer.
The other issues fixed are if one uses non-prototyped builtins with wrong
types of arguments, it can result in verify_gimple ICEs (e.g. if last
argument to non-prototyped memcmp is int), or worse (say if instead of
pointer it is a double).  The patch just punts for bogus builtins,
and when len is integral, but not of the right type, it casts it to the
right type (for constant just builds the right offset as constant,
otherwise adds extra stmts as needed).

Ok for trunk?

2012-11-27  Jakub Jelinek  <jakub@redhat.com>

	* asan.c (instrument_mem_region_access): Don't instrument
	if base doesn't have pointer type or len integral type.
	Add cast if len doesn't have size_t compatible type.
	(instrument_builtin_call): Don't instrument BUILT_IN_ATOMIC_LOAD,
	BUILT_IN_ATOMIC_TEST_AND_SET, BUILT_IN_ATOMIC_CLEAR,
	BUILT_IN_ATOMIC_EXCHANGE, BUILT_IN_ATOMIC_COMPARE_EXCHANGE
	and BUILT_IN_ATOMIC_STORE.


	Jakub
Dodji Seketeli - Dec. 3, 2012, 2:52 p.m.
> Ok for trunk?
>
> 2012-11-27  Jakub Jelinek  <jakub@redhat.com>
>
> 	* asan.c (instrument_mem_region_access): Don't instrument
> 	if base doesn't have pointer type or len integral type.
> 	Add cast if len doesn't have size_t compatible type.
> 	(instrument_builtin_call): Don't instrument BUILT_IN_ATOMIC_LOAD,
> 	BUILT_IN_ATOMIC_TEST_AND_SET, BUILT_IN_ATOMIC_CLEAR,
> 	BUILT_IN_ATOMIC_EXCHANGE, BUILT_IN_ATOMIC_COMPARE_EXCHANGE
> 	and BUILT_IN_ATOMIC_STORE.

This is OK.  Thanks, and sorry for the delay.

Patch

--- gcc/asan.c.jj	2012-11-23 15:21:49.000000000 +0100
+++ gcc/asan.c	2012-11-27 17:37:10.948281831 +0100
@@ -849,7 +849,9 @@  instrument_mem_region_access (tree base,
 			      gimple_stmt_iterator *iter,
 			      location_t location, bool is_store)
 {
-  if (integer_zerop (len))
+  if (!POINTER_TYPE_P (TREE_TYPE (base))
+      || !INTEGRAL_TYPE_P (TREE_TYPE (len))
+      || integer_zerop (len))
     return;
 
   gimple_stmt_iterator gsi = *iter;
@@ -902,20 +904,41 @@  instrument_mem_region_access (tree base,
 
   /* offset = len - 1;  */
   len = unshare_expr (len);
-  gimple offset =
-    gimple_build_assign_with_ops (TREE_CODE (len),
-				  make_ssa_name (TREE_TYPE (len), NULL),
-				  len, NULL);
-  gimple_set_location (offset, location);
-  gsi_insert_before (&gsi, offset, GSI_NEW_STMT);
-
-  offset =
-    gimple_build_assign_with_ops (MINUS_EXPR,
-				  make_ssa_name (size_type_node, NULL),
-				  gimple_assign_lhs (offset),
-				  build_int_cst (size_type_node, 1));
-  gimple_set_location (offset, location);
-  gsi_insert_after (&gsi, offset, GSI_NEW_STMT);
+  tree offset;
+  gimple_seq seq = NULL;
+  if (TREE_CODE (len) == INTEGER_CST)
+    offset = fold_build2 (MINUS_EXPR, size_type_node,
+			  fold_convert (size_type_node, len),
+			  build_int_cst (size_type_node, 1));
+  else
+    {
+      gimple g;
+      tree t;
+
+      if (TREE_CODE (len) != SSA_NAME)
+	{
+	  t = make_ssa_name (TREE_TYPE (len), NULL);
+	  g = gimple_build_assign_with_ops (TREE_CODE (len), t, len, NULL);
+	  gimple_set_location (g, location);
+	  gimple_seq_add_stmt_without_update (&seq, g);
+	  len = t;
+	}
+      if (!useless_type_conversion_p (size_type_node, TREE_TYPE (len)))
+	{
+	  t = make_ssa_name (size_type_node, NULL);
+	  g = gimple_build_assign_with_ops (NOP_EXPR, t, len, NULL);
+	  gimple_set_location (g, location);
+	  gimple_seq_add_stmt_without_update (&seq, g);
+	  len = t;
+	}
+
+      t = make_ssa_name (size_type_node, NULL);
+      g = gimple_build_assign_with_ops (MINUS_EXPR, t, len,
+					build_int_cst (size_type_node, 1));
+      gimple_set_location (g, location);
+      gimple_seq_add_stmt_without_update (&seq, g);
+      offset = gimple_assign_lhs (g);
+    }
 
   /* _1 = base;  */
   base = unshare_expr (base);
@@ -924,14 +947,16 @@  instrument_mem_region_access (tree base,
 				  make_ssa_name (TREE_TYPE (base), NULL),
 				  base, NULL);
   gimple_set_location (region_end, location);
-  gsi_insert_after (&gsi, region_end, GSI_NEW_STMT);
+  gimple_seq_add_stmt_without_update (&seq, region_end);
+  gsi_insert_seq_before (&gsi, seq, GSI_SAME_STMT);
+  gsi_prev (&gsi);
 
   /* _2 = _1 + offset;  */
   region_end =
     gimple_build_assign_with_ops (POINTER_PLUS_EXPR,
 				  make_ssa_name (TREE_TYPE (base), NULL),
 				  gimple_assign_lhs (region_end),
-				  gimple_assign_lhs (offset));
+				  offset);
   gimple_set_location (region_end, location);
   gsi_insert_after (&gsi, region_end, GSI_NEW_STMT);
 
@@ -1089,7 +1114,6 @@  instrument_builtin_call (gimple_stmt_ite
        These are handled differently from the classical memory memory
        access builtins above.  */
 
-    case BUILT_IN_ATOMIC_LOAD:
     case BUILT_IN_ATOMIC_LOAD_1:
     case BUILT_IN_ATOMIC_LOAD_2:
     case BUILT_IN_ATOMIC_LOAD_4:
@@ -1192,23 +1216,18 @@  instrument_builtin_call (gimple_stmt_ite
     case BUILT_IN_SYNC_LOCK_RELEASE_8:
     case BUILT_IN_SYNC_LOCK_RELEASE_16:
 
-    case BUILT_IN_ATOMIC_TEST_AND_SET:
-    case BUILT_IN_ATOMIC_CLEAR:
-    case BUILT_IN_ATOMIC_EXCHANGE:
     case BUILT_IN_ATOMIC_EXCHANGE_1:
     case BUILT_IN_ATOMIC_EXCHANGE_2:
     case BUILT_IN_ATOMIC_EXCHANGE_4:
     case BUILT_IN_ATOMIC_EXCHANGE_8:
     case BUILT_IN_ATOMIC_EXCHANGE_16:
 
-    case BUILT_IN_ATOMIC_COMPARE_EXCHANGE:
     case BUILT_IN_ATOMIC_COMPARE_EXCHANGE_1:
     case BUILT_IN_ATOMIC_COMPARE_EXCHANGE_2:
     case BUILT_IN_ATOMIC_COMPARE_EXCHANGE_4:
     case BUILT_IN_ATOMIC_COMPARE_EXCHANGE_8:
     case BUILT_IN_ATOMIC_COMPARE_EXCHANGE_16:
 
-    case BUILT_IN_ATOMIC_STORE:
     case BUILT_IN_ATOMIC_STORE_1:
     case BUILT_IN_ATOMIC_STORE_2:
     case BUILT_IN_ATOMIC_STORE_4: