Patchwork [asan] Instrument STRING_CSTs

login
register
mail settings
Submitter Jakub Jelinek
Date Dec. 5, 2012, 6:30 p.m.
Message ID <20121205183015.GC2315@tucnak.redhat.com>
Download mbox | patch
Permalink /patch/203920/
State New
Headers show

Comments

Jakub Jelinek - Dec. 5, 2012, 6:30 p.m.
Hi!

Here is instrumentation of STRING_CSTs.  Using
TREE_TYPE (shadow_ptr_types[0]) instead of plain char_type_node is
a hack to avoid instrumenting the asan_pp_string created STRING_CSTs
that describe the protected automatic vars in functions and protected
global vars, no need to waste space for their paddings and waste ctor
cycles protecting the paddings in shadow mem.

With this only
FAIL: g++.dg/asan/asan_test.C  -O2 AddressSanitizer_FileNameInGlobalReportTest Ident(p[15]) output pattern test, should match zoo.*asan_test.cc
fails and that is dealt in upstream.

So, once this series of 8 or how many patches is acked and committed (if),
we can look at bringing the rest of tests/ testcases from llvm, then
there is the optimization thingie (don't instrument what is already
proven instrumented without intervening calls, both for asan/tsan).

But I should take a break from asan/tsan and get back to general bugfixing
and debug info quality issues.

2012-12-05  Jakub Jelinek  <jakub@redhat.com>

	* asan.c (asan_init_shadow_ptr_types): Move earlier in the file.
	Call initialize_sanitizer_builtins at the end.
	(asan_pp_string): Use TREE_TYPE (shadow_ptr_types[0])
	as character type instead of char_type_node.
	(asan_emit_stack_protection): Call asan_init_shadow_ptr_types
	if shadow_ptr_types isn't initialized.
	(asan_protect_global): Return true for STRING_CSTs except those
	created by asan_pp_string.
	(count_string_csts, add_string_csts): New functions.
	(struct asan_add_string_csts_data): New type.
	(asan_finish_file): Clear flag_asan at the beginning, restore at the
	end.  Traverse constant_pool_htab () to look for protected
	STRING_CSTs.  Don't call initialize_sanitizer_builtins,
	instead call asan_init_shadow_ptr_types if shadow_ptr_types isn't
	initialized yet.
	(asan_instrument): Don't call initialize_sanitizer_builtins.
	* varasm.c (output_constant_def_contents): If STRING_CST should be
	asan protected, align it sufficiently and emit padding after it.
	(categorize_decl_for_section): If flag_asan, don't put STRING_CSTs
	that should be asan protected into mergeable sections.  For
	-fmerge-all-constants, ignore it for -fmudflap or if decl is
	asan protected.


	Jakub
Dodji Seketeli - Dec. 10, 2012, noon
Jakub Jelinek <jakub@redhat.com> writes:

> 2012-12-05  Jakub Jelinek  <jakub@redhat.com>
>
> 	* asan.c (asan_init_shadow_ptr_types): Move earlier in the file.
> 	Call initialize_sanitizer_builtins at the end.
> 	(asan_pp_string): Use TREE_TYPE (shadow_ptr_types[0])
> 	as character type instead of char_type_node.
> 	(asan_emit_stack_protection): Call asan_init_shadow_ptr_types
> 	if shadow_ptr_types isn't initialized.
> 	(asan_protect_global): Return true for STRING_CSTs except those
> 	created by asan_pp_string.
> 	(count_string_csts, add_string_csts): New functions.
> 	(struct asan_add_string_csts_data): New type.
> 	(asan_finish_file): Clear flag_asan at the beginning, restore at the
> 	end.  Traverse constant_pool_htab () to look for protected
> 	STRING_CSTs.  Don't call initialize_sanitizer_builtins,
> 	instead call asan_init_shadow_ptr_types if shadow_ptr_types isn't
> 	initialized yet.
> 	(asan_instrument): Don't call initialize_sanitizer_builtins.
> 	* varasm.c (output_constant_def_contents): If STRING_CST should be
> 	asan protected, align it sufficiently and emit padding after it.
> 	(categorize_decl_for_section): If flag_asan, don't put STRING_CSTs
> 	that should be asan protected into mergeable sections.  For
> 	-fmerge-all-constants, ignore it for -fmudflap or if decl is
> 	asan protected.

This seems OK to me, thanks.

Patch

--- gcc/asan.c.jj	2012-12-05 17:00:56.000000000 +0100
+++ gcc/asan.c	2012-12-05 19:12:53.384240629 +0100
@@ -212,6 +212,21 @@  alias_set_type asan_shadow_set = -1;
    alias set is used for all shadow memory accesses.  */
 static GTY(()) tree shadow_ptr_types[2];
 
+/* Initialize shadow_ptr_types array.  */
+
+static void
+asan_init_shadow_ptr_types (void)
+{
+  asan_shadow_set = new_alias_set ();
+  shadow_ptr_types[0] = build_distinct_type_copy (signed_char_type_node);
+  TYPE_ALIAS_SET (shadow_ptr_types[0]) = asan_shadow_set;
+  shadow_ptr_types[0] = build_pointer_type (shadow_ptr_types[0]);
+  shadow_ptr_types[1] = build_distinct_type_copy (short_integer_type_node);
+  TYPE_ALIAS_SET (shadow_ptr_types[1]) = asan_shadow_set;
+  shadow_ptr_types[1] = build_pointer_type (shadow_ptr_types[1]);
+  initialize_sanitizer_builtins ();
+}
+
 /* Asan pretty-printer, used for buidling of the description STRING_CSTs.  */
 static pretty_printer asan_pp;
 static bool asan_pp_initialized;
@@ -234,10 +249,11 @@  asan_pp_string (void)
   size_t len = strlen (buf);
   tree ret = build_string (len + 1, buf);
   TREE_TYPE (ret)
-    = build_array_type (char_type_node, build_index_type (size_int (len)));
+    = build_array_type (TREE_TYPE (shadow_ptr_types[0]),
+			build_index_type (size_int (len)));
   TREE_READONLY (ret) = 1;
   TREE_STATIC (ret) = 1;
-  return build1 (ADDR_EXPR, build_pointer_type (char_type_node), ret);
+  return build1 (ADDR_EXPR, shadow_ptr_types[0], ret);
 }
 
 /* Return a CONST_INT representing 4 subsequent shadow memory bytes.  */
@@ -276,6 +292,9 @@  asan_emit_stack_protection (rtx base, HO
   unsigned char cur_shadow_byte = ASAN_STACK_MAGIC_LEFT;
   tree str_cst;
 
+  if (shadow_ptr_types[0] == NULL_TREE)
+    asan_init_shadow_ptr_types ();
+
   /* First of all, prepare the description string.  */
   if (!asan_pp_initialized)
     asan_pp_initialize ();
@@ -429,6 +448,16 @@  asan_protect_global (tree decl)
 {
   rtx rtl, symbol;
 
+  if (TREE_CODE (decl) == STRING_CST)
+    {
+      /* Instrument all STRING_CSTs except those created
+	 by asan_pp_string here.  */
+      if (shadow_ptr_types[0] != NULL_TREE
+	  && TREE_CODE (TREE_TYPE (decl)) == ARRAY_TYPE
+	  && TREE_TYPE (TREE_TYPE (decl)) == TREE_TYPE (shadow_ptr_types[0]))
+	return false;
+      return true;
+    }
   if (TREE_CODE (decl) != VAR_DECL
       /* TLS vars aren't statically protectable.  */
       || DECL_THREAD_LOCAL_P (decl)
@@ -1596,6 +1625,50 @@  initialize_sanitizer_builtins (void)
 #undef DEF_SANITIZER_BUILTIN
 }
 
+/* Called via htab_traverse.  Count number of emitted
+   STRING_CSTs in the constant hash table.  */
+
+static int
+count_string_csts (void **slot, void *data)
+{
+  struct constant_descriptor_tree *desc
+    = (struct constant_descriptor_tree *) *slot;
+  if (TREE_CODE (desc->value) == STRING_CST
+      && TREE_ASM_WRITTEN (desc->value)
+      && asan_protect_global (desc->value))
+    ++*((unsigned HOST_WIDE_INT *) data);
+  return 1;
+}
+
+/* Helper structure to pass two parameters to
+   add_string_csts.  */
+
+struct asan_add_string_csts_data
+{
+  tree type;
+  vec<constructor_elt, va_gc> *v;
+};
+
+/* Called via htab_traverse.  Call asan_add_global
+   on emitted STRING_CSTs from the constant hash table.  */
+
+static int
+add_string_csts (void **slot, void *data)
+{
+  struct constant_descriptor_tree *desc
+    = (struct constant_descriptor_tree *) *slot;
+  if (TREE_CODE (desc->value) == STRING_CST
+      && TREE_ASM_WRITTEN (desc->value)
+      && asan_protect_global (desc->value))
+    {
+      struct asan_add_string_csts_data *aascd
+	= (struct asan_add_string_csts_data *) data;
+      asan_add_global (SYMBOL_REF_DECL (XEXP (desc->rtl, 0)),
+		       aascd->type, aascd->v);
+    }
+  return 1;
+}
+
 /* Needs to be GTY(()), because cgraph_build_static_cdtor may
    invoke ggc_collect.  */
 static GTY(()) tree asan_ctor_statements;
@@ -1611,13 +1684,20 @@  asan_finish_file (void)
   struct varpool_node *vnode;
   unsigned HOST_WIDE_INT gcount = 0;
 
-  initialize_sanitizer_builtins ();
+  if (shadow_ptr_types[0] == NULL_TREE)
+    asan_init_shadow_ptr_types ();
+  /* 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;
 
   tree fn = builtin_decl_implicit (BUILT_IN_ASAN_INIT);
   append_to_statement_list (build_call_expr (fn, 0), &asan_ctor_statements);
   FOR_EACH_DEFINED_VARIABLE (vnode)
     if (asan_protect_global (vnode->symbol.decl))
       ++gcount;
+  htab_t const_desc_htab = constant_pool_htab ();
+  htab_traverse (const_desc_htab, count_string_csts, &gcount);
   if (gcount)
     {
       tree type = asan_global_struct (), var, ctor;
@@ -1638,6 +1718,10 @@  asan_finish_file (void)
       FOR_EACH_DEFINED_VARIABLE (vnode)
 	if (asan_protect_global (vnode->symbol.decl))
 	  asan_add_global (vnode->symbol.decl, TREE_TYPE (type), v);
+      struct asan_add_string_csts_data aascd;
+      aascd.type = TREE_TYPE (type);
+      aascd.v = v;
+      htab_traverse (const_desc_htab, add_string_csts, &aascd);
       ctor = build_constructor (type, v);
       TREE_CONSTANT (ctor) = 1;
       TREE_STATIC (ctor) = 1;
@@ -1660,20 +1744,7 @@  asan_finish_file (void)
     }
   cgraph_build_static_cdtor ('I', asan_ctor_statements,
 			     MAX_RESERVED_INIT_PRIORITY - 1);
-}
-
-/* Initialize shadow_ptr_types array.  */
-
-static void
-asan_init_shadow_ptr_types (void)
-{
-  asan_shadow_set = new_alias_set ();
-  shadow_ptr_types[0] = build_distinct_type_copy (signed_char_type_node);
-  TYPE_ALIAS_SET (shadow_ptr_types[0]) = asan_shadow_set;
-  shadow_ptr_types[0] = build_pointer_type (shadow_ptr_types[0]);
-  shadow_ptr_types[1] = build_distinct_type_copy (short_integer_type_node);
-  TYPE_ALIAS_SET (shadow_ptr_types[1]) = asan_shadow_set;
-  shadow_ptr_types[1] = build_pointer_type (shadow_ptr_types[1]);
+  flag_asan = 1;
 }
 
 /* Instrument the current function.  */
@@ -1682,10 +1753,7 @@  static unsigned int
 asan_instrument (void)
 {
   if (shadow_ptr_types[0] == NULL_TREE)
-    {
-      asan_init_shadow_ptr_types ();
-      initialize_sanitizer_builtins ();
-    }
+    asan_init_shadow_ptr_types ();
   transform_statements ();
   return 0;
 }
--- gcc/varasm.c.jj	2012-12-05 08:24:59.000000000 +0100
+++ gcc/varasm.c	2012-12-05 18:37:04.704418938 +0100
@@ -3250,11 +3250,23 @@  output_constant_def_contents (rtx symbol
     place_block_symbol (symbol);
   else
     {
+      bool asan_protected = false;
       align = DECL_ALIGN (decl);
       switch_to_section (get_constant_section (exp, align));
+      if (flag_asan && TREE_CODE (exp) == STRING_CST
+	  && asan_protect_global (exp))
+	{
+	  asan_protected = true;
+	  align = MAX (align, ASAN_RED_ZONE_SIZE * BITS_PER_UNIT);
+	}
       if (align > BITS_PER_UNIT)
 	ASM_OUTPUT_ALIGN (asm_out_file, floor_log2 (align / BITS_PER_UNIT));
       assemble_constant_contents (exp, XSTR (symbol, 0), align);
+      if (asan_protected)
+	{
+	  HOST_WIDE_INT size = get_constant_size (exp);
+	  assemble_zeros (asan_red_zone_size (size));
+	}
     }
   if (flag_mudflap)
     mudflap_enqueue_constant (exp);
@@ -6158,7 +6170,9 @@  categorize_decl_for_section (const_tree
     return SECCAT_TEXT;
   else if (TREE_CODE (decl) == STRING_CST)
     {
-      if (flag_mudflap) /* or !flag_merge_constants */
+      if (flag_mudflap
+	  || (flag_asan && asan_protect_global (CONST_CAST_TREE (decl))))
+      /* or !flag_merge_constants */
         return SECCAT_RODATA;
       else
 	return SECCAT_RODATA_MERGE_STR;
@@ -6182,7 +6196,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)
+      else if (reloc || flag_merge_constants < 2 || flag_mudflap
+	       || (flag_asan && 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).  */