This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] Asan constructor init order checking


Hi!

Here is an implementation of init-order checking (at least how I understand
it from looking at libsanitizer and eyeballing what clang emits) plus some
performance improvements.

Previously instrument_derefs ignored stores and loads from VAR_DECLs, that
is of course now not possible generally, because it could be dynamic init
protected globals.  But, as for the improvements, if get_inner_reference
tells us that the access is known to be within bounds of some VAR_DECL,
we can check if we know it must be fine.  Automatic vars in current function
within that function are (at least for now) all known to be accessible,
similarly non-external vars that don't have dynamic initialization.

I had to tweak no-redundant-instrumentation-1.c test a little bit, because
for the foo var accesses instrument_derefs can now find out the
insturmentation is unecessary, similarly for the static tab var that didn't
have dynamic initialization.  Note, some comments in that file seems to be
completely wrong and also the fact that it doesn't instrument high bound
of the memset 4/5 looks like a severe bug (though, of course if memset
isn't expanded inline, but as a library call, it will still be caught by
the runtime library).

2013-11-15  Jakub Jelinek  <jakub@redhat.com>

	* sanitizer.def (BUILT_IN_ASAN_BEFORE_DYNAMIC_INIT,
	BUILT_IN_ASAN_AFTER_DYNAMIC_INIT): New.
	* asan.c (instrument_derefs): Handle also VAR_DECL loads/stores.
	Don't instrument accesses to VAR_DECLs which are known to fit
	into their bounds and the vars are known to have shadow bytes
	indicating allowed access.
	(asan_dynamic_init_call): New function.
	(asan_add_global): If vnode->asan_dynamically_initialized,
	set __has_dynamic_init to 1 instead of 0.
	(initialize_sanitizer_builtins): Add BT_FN_VOID_CONST_PTR var.
	* asan.h (asan_dynamic_init_call): New prototype.
	* cgraph.h (varpool_node): Add asan_dynamically_initialized bitfield.
cp/
	* decl2.c: Include asan.h.
	(one_static_initialization_or_destruction): If -fsanitize=address,
	init is non-NULL and guard is NULL, set
	vnode->asan_dynamically_initialized.
	(do_static_initialization_or_destruction): Call
	__asan_{before,after}_dynamic_init around the static initialization.
testsuite/
	* c-c++-common/asan/no-redundant-instrumentation-1.c: Tweak to avoid
	optimizing away some __asan_report* calls.

--- gcc/sanitizer.def.jj	2013-11-12 11:31:12.000000000 +0100
+++ gcc/sanitizer.def	2013-11-15 12:25:06.160748091 +0100
@@ -60,6 +60,12 @@ DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_UNRE
 DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_HANDLE_NO_RETURN,
 		      "__asan_handle_no_return",
 		      BT_FN_VOID, ATTR_TMPURE_NOTHROW_LEAF_LIST)
+DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_BEFORE_DYNAMIC_INIT,
+		      "__asan_before_dynamic_init",
+		      BT_FN_VOID_CONST_PTR, ATTR_NOTHROW_LEAF_LIST)
+DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_AFTER_DYNAMIC_INIT,
+		      "__asan_after_dynamic_init",
+		      BT_FN_VOID, ATTR_NOTHROW_LEAF_LIST)
 
 /* Thread Sanitizer */
 DEF_SANITIZER_BUILTIN(BUILT_IN_TSAN_INIT, "__tsan_init", 
--- gcc/cp/decl2.c.jj	2013-11-12 23:10:07.000000000 +0100
+++ gcc/cp/decl2.c	2013-11-15 12:16:24.195466188 +0100
@@ -49,6 +49,7 @@ along with GCC; see the file COPYING3.
 #include "splay-tree.h"
 #include "langhooks.h"
 #include "c-family/c-ada-spec.h"
+#include "asan.h"
 
 extern cpp_reader *parse_in;
 
@@ -3456,7 +3457,15 @@ one_static_initialization_or_destruction
   if (initp)
     {
       if (init)
-	finish_expr_stmt (init);
+	{
+	  finish_expr_stmt (init);
+	  if ((flag_sanitize & SANITIZE_ADDRESS) && guard == NULL_TREE)
+	    {
+	      struct varpool_node *vnode = varpool_get_node (decl);
+	      if (vnode)
+		vnode->asan_dynamically_initialized = 1;
+	    }
+	}
 
       /* If we're using __cxa_atexit, register a function that calls the
 	 destructor for the object.  */
@@ -3498,6 +3507,9 @@ do_static_initialization_or_destruction
 			     tf_warning_or_error);
   finish_if_stmt_cond (cond, init_if_stmt);
 
+  if (flag_sanitize & SANITIZE_ADDRESS)
+    finish_expr_stmt (asan_dynamic_init_call (/*after_p=*/false));
+
   node = vars;
   do {
     tree decl = TREE_VALUE (node);
@@ -3546,6 +3558,9 @@ do_static_initialization_or_destruction
 
   } while (node);
 
+  if (flag_sanitize & SANITIZE_ADDRESS)
+    finish_expr_stmt (asan_dynamic_init_call (/*after_p=*/true));
+
   /* Finish up the init/destruct if-stmt body.  */
   finish_then_clause (init_if_stmt);
   finish_if_stmt (init_if_stmt);
--- gcc/asan.c.jj	2013-11-15 12:37:07.000000000 +0100
+++ gcc/asan.c	2013-11-15 13:26:23.648429812 +0100
@@ -214,7 +214,7 @@ along with GCC; see the file COPYING3.
        // Name of the module where the global variable is declared.
        const void *__module_name;
 
-       // This is always set to NULL for now.
+       // 1 if it has dynamic initialization, 0 otherwise.
        uptr __has_dynamic_init;
      }
 
@@ -1571,7 +1571,9 @@ instrument_derefs (gimple_stmt_iterator
     case COMPONENT_REF:
     case INDIRECT_REF:
     case MEM_REF:
+    case VAR_DECL:
       break;
+      /* FALLTHRU */
     default:
       return;
     }
@@ -1585,8 +1587,8 @@ instrument_derefs (gimple_stmt_iterator
   tree offset;
   enum machine_mode mode;
   int volatilep = 0, unsignedp = 0;
-  get_inner_reference (t, &bitsize, &bitpos, &offset,
-		       &mode, &unsignedp, &volatilep, false);
+  tree inner = get_inner_reference (t, &bitsize, &bitpos, &offset,
+				    &mode, &unsignedp, &volatilep, false);
   if (bitpos % (size_in_bytes * BITS_PER_UNIT)
       || bitsize != size_in_bytes * BITS_PER_UNIT)
     {
@@ -1601,6 +1603,34 @@ instrument_derefs (gimple_stmt_iterator
       return;
     }
 
+  if (TREE_CODE (inner) == VAR_DECL
+      && offset == NULL_TREE
+      && bitpos >= 0
+      && DECL_SIZE (inner)
+      && host_integerp (DECL_SIZE (inner), 0)
+      && bitpos + bitsize <= tree_low_cst (DECL_SIZE (inner), 0))
+    {
+      if (DECL_THREAD_LOCAL_P (inner))
+	return;
+      if (!TREE_STATIC (inner))
+	{
+	  /* Automatic vars in the current function will be always
+	     accessible.  */
+	  if (decl_function_context (inner) == current_function_decl)
+	    return;
+	}
+      /* Always instrument external vars, they might be dynamically
+	 initialized.  */
+      else if (!DECL_EXTERNAL (inner))
+	{
+	  /* For static vars if they are known not to be dynamically
+	     initialized, they will be always accessible.  */
+	  struct varpool_node *vnode = varpool_get_node (inner);
+	  if (vnode && !vnode->asan_dynamically_initialized)
+	    return;
+	}
+    }
+
   base = build_fold_addr_expr (t);
   if (!has_mem_ref_been_instrumented (base, size_in_bytes))
     {
@@ -2059,6 +2089,34 @@ transform_statements (void)
 }
 
 /* Build
+   __asan_before_dynamic_init (module_name)
+   or
+   __asan_after_dynamic_init ()
+   call.  */
+
+tree
+asan_dynamic_init_call (bool after_p)
+{
+  tree fn = builtin_decl_implicit (after_p
+				   ? BUILT_IN_ASAN_AFTER_DYNAMIC_INIT
+				   : BUILT_IN_ASAN_BEFORE_DYNAMIC_INIT);
+  tree module_name_cst = NULL_TREE;
+  if (!after_p)
+    {
+      pretty_printer module_name_pp;
+      pp_string (&module_name_pp, main_input_filename);
+
+      if (shadow_ptr_types[0] == NULL_TREE)
+	asan_init_shadow_ptr_types ();
+      module_name_cst = asan_pp_string (&module_name_pp);
+      module_name_cst = fold_convert (const_ptr_type_node,
+				      module_name_cst);
+    }
+
+  return build_call_expr (fn, after_p ? 0 : 1, module_name_cst);
+}
+
+/* Build
    struct __asan_global
    {
      const void *__beg;
@@ -2147,7 +2205,10 @@ asan_add_global (tree decl, tree type, v
 			  fold_convert (const_ptr_type_node, str_cst));
   CONSTRUCTOR_APPEND_ELT (vinner, NULL_TREE,
 			  fold_convert (const_ptr_type_node, module_name_cst));
-  CONSTRUCTOR_APPEND_ELT (vinner, NULL_TREE, build_int_cst (uptr, 0));
+  struct varpool_node *vnode = varpool_get_node (decl);
+  int has_dynamic_init = vnode ? vnode->asan_dynamically_initialized : 0;
+  CONSTRUCTOR_APPEND_ELT (vinner, NULL_TREE,
+			  build_int_cst (uptr, has_dynamic_init));
   init = build_constructor (type, vinner);
   CONSTRUCTOR_APPEND_ELT (v, NULL_TREE, init);
 }
@@ -2164,6 +2225,8 @@ initialize_sanitizer_builtins (void)
   tree BT_FN_VOID = build_function_type_list (void_type_node, NULL_TREE);
   tree BT_FN_VOID_PTR
     = build_function_type_list (void_type_node, ptr_type_node, NULL_TREE);
+  tree BT_FN_VOID_CONST_PTR
+    = build_function_type_list (void_type_node, const_ptr_type_node, NULL_TREE);
   tree BT_FN_VOID_PTR_PTR
     = build_function_type_list (void_type_node, ptr_type_node,
 				ptr_type_node, NULL_TREE);
--- gcc/asan.h.jj	2013-11-15 12:37:07.000000000 +0100
+++ gcc/asan.h	2013-11-15 12:37:18.991783166 +0100
@@ -27,6 +27,7 @@ extern rtx asan_emit_stack_protection (r
 				       tree *, int);
 extern bool asan_protect_global (tree);
 extern void initialize_sanitizer_builtins (void);
+extern tree asan_dynamic_init_call (bool);
 
 /* Alias set for accessing the shadow memory.  */
 extern alias_set_type asan_shadow_set;
--- gcc/cgraph.h.jj	2013-11-13 18:32:52.000000000 +0100
+++ gcc/cgraph.h	2013-11-15 12:05:25.950985500 +0100
@@ -520,6 +520,11 @@ class GTY((tag ("SYMTAB_VARIABLE"))) var
 public:
   /* Set when variable is scheduled to be assembled.  */
   unsigned output : 1;
+  /* Set if the variable is dynamically initialized.  Not set for
+     function local statics or variables that can be initialized in
+     multiple compilation units (such as template static data members
+     that need construction).  */
+  unsigned asan_dynamically_initialized : 1;
 };
 
 /* Every top level asm statement is put into a asm_node.  */
--- gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-1.c.jj	2013-02-18 16:39:54.000000000 +0100
+++ gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-1.c	2013-11-15 14:16:22.727049197 +0100
@@ -6,7 +6,7 @@
 /* { dg-do compile } */
 /* { dg-skip-if "" { *-*-* } { "*" } { "-O0" } } */
 
-static char tab[4] = {0};
+extern char tab[4];
 
 static int
 test0 ()
@@ -27,12 +27,14 @@ test0 ()
   return t0 + t1;
 }
 
-static int
-test1 ()
+__attribute__((noinline, noclone)) static int
+test1 (int i)
 {
+  char foo[4] = {};
+  
   /*__builtin___asan_report_store1 called 1 time here to instrument
     the initialization.  */
-  char foo[4] = {1}; 
+  foo[i] = 1;
 
   /*__builtin___asan_report_store1 called 2 times here to instrument
     the store to the memory region of tab.  */
@@ -45,7 +47,7 @@ test1 ()
   /* There are 2 calls to __builtin___asan_report_store1 and 2 calls
      to __builtin___asan_report_load1 to instrument the store to
      (subset of) the memory region of tab.  */
-  __builtin_memcpy (&tab[1], foo, 3);
+  __builtin_memcpy (&tab[1], foo + i, 3);
 
   /* This should not generate a __builtin___asan_report_load1 because
      the reference to tab[1] has been already instrumented above.  */
@@ -58,7 +60,7 @@ test1 ()
 int
 main ()
 {
-  return test0 () && test1 ();
+  return test0 () && test1 (0);
 }
 
 /* { dg-final { scan-tree-dump-times "__builtin___asan_report_store1" 7 "asan0" } } */

	Jakub


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]