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]

[RFC][PATCH] Speed-up use-after-scope (re-writing to SSA)


Hello

Following patch is a candidate that re-writes VAR_DECLs that are
is_gimple_reg_type with:
my_char_25 = ASAN_POISON ();

that is eventually transformed to:
__builtin___asan_report_use_after_scope_noabort ("my_char", 1);

at places where my_char_25 is used. That introduces a new entry point
to ASAN runtime, reporting:

==18378==ERROR: AddressSanitizer: stack-use-after-scope at pc 0x0000004007b4 bp 0x000000000001 sp 0x000000400603
ACCESS of size 1 for variable 'my_char' thread T0
    #0 0x400602 in main (/tmp/a.out+0x400602)
    #1 0x7fa6e572d290 in __libc_start_main (/lib64/libc.so.6+0x20290)
    #2 0x400669 in _start (/tmp/a.out+0x400669)

SUMMARY: AddressSanitizer: stack-use-after-scope (/tmp/a.out+0x400602) in main

I'm still not sure where exactly do the expansion of ASAN_POISON as some cleanup
after the transformation would be desired.

Thoughts?
Thanks,
Martin 




>From c115207230a5be979119b6ac6572ae6af2a0ccd7 Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Mon, 14 Nov 2016 16:49:05 +0100
Subject: [PATCH] use-after-scope: introduce ASAN_POISON internal fn

---
 gcc/asan.c                       | 72 +++++++++++++++++++++++++++++++++++++++-
 gcc/asan.h                       |  1 +
 gcc/internal-fn.c                |  7 ++++
 gcc/internal-fn.def              |  1 +
 gcc/sanitizer.def                |  8 +++++
 gcc/sanopt.c                     |  9 +++++
 gcc/tree-ssa.c                   | 65 ++++++++++++++++++++++++++++++------
 libsanitizer/asan/asan_errors.cc | 21 ++++++++++++
 libsanitizer/asan/asan_errors.h  | 19 +++++++++++
 libsanitizer/asan/asan_report.cc | 10 ++++++
 libsanitizer/asan/asan_report.h  |  3 ++
 libsanitizer/asan/asan_rtl.cc    | 16 +++++++++
 12 files changed, 221 insertions(+), 11 deletions(-)

diff --git a/gcc/asan.c b/gcc/asan.c
index 6e93ea3..d7d4267 100644
--- a/gcc/asan.c
+++ b/gcc/asan.c
@@ -32,8 +32,8 @@ along with GCC; see the file COPYING3.  If not see
 #include "tree-pass.h"
 #include "memmodel.h"
 #include "tm_p.h"
+#include "ssa.h"
 #include "stringpool.h"
-#include "tree-vrp.h"
 #include "tree-ssanames.h"
 #include "optabs.h"
 #include "emit-rtl.h"
@@ -2979,6 +2979,76 @@ asan_expand_check_ifn (gimple_stmt_iterator *iter, bool use_calls)
   return true;
 }
 
+
+/* Expand the ASAN_{LOAD,STORE} builtins.  */
+
+bool
+asan_expand_poison_ifn (gimple_stmt_iterator *iter,
+			bool *need_commit_edge_insert)
+{
+  gimple *g = gsi_stmt (*iter);
+  tree poisoned_var = gimple_call_lhs (g);
+  if (!poisoned_var)
+    {
+      gsi_remove (iter, true);
+      return true;
+    }
+
+  tree var_decl = SSA_NAME_VAR (poisoned_var);
+
+  bool recover_p;
+  if (flag_sanitize & SANITIZE_USER_ADDRESS)
+    recover_p = (flag_sanitize_recover & SANITIZE_USER_ADDRESS) != 0;
+  else
+    recover_p = (flag_sanitize_recover & SANITIZE_KERNEL_ADDRESS) != 0;
+
+  use_operand_p use_p;
+  imm_use_iterator imm_iter;
+  FOR_EACH_IMM_USE_FAST (use_p, imm_iter, poisoned_var)
+    {
+      gimple *use = USE_STMT (use_p);
+
+      built_in_function b = (recover_p
+			     ? BUILT_IN_ASAN_REPORT_USE_AFTER_SCOPE_NOABORT
+			     : BUILT_IN_ASAN_REPORT_USE_AFTER_SCOPE);
+      tree fun = builtin_decl_implicit (b);
+      pretty_printer pp;
+      pp_tree_identifier (&pp, DECL_NAME (var_decl));
+
+      gcall *call = gimple_build_call (fun, 2, asan_pp_string (&pp),
+				       DECL_SIZE_UNIT (var_decl));
+      gimple_set_location (call, gimple_location (g));
+
+      /* If ASAN_POISON is used in a PHI node, let's insert the call on
+	 the leading to the PHI node BB.  */
+      if (is_a <gphi *> (use))
+	{
+	  gphi * phi = dyn_cast<gphi *> (use);
+	  for (unsigned i = 0; i < gimple_phi_num_args (phi); ++i)
+	    if (gimple_phi_arg_def (phi, i) == poisoned_var)
+	      {
+		edge e = gimple_phi_arg_edge (phi, i);
+		gsi_insert_seq_on_edge (e, call);
+		*need_commit_edge_insert = true;
+
+		break;
+	      }
+	}
+      else
+	{
+	  gimple_stmt_iterator gsi = gsi_for_stmt (use);
+	  gsi_insert_before (&gsi, call, GSI_NEW_STMT);
+	}
+    }
+
+  gimple *nop = gimple_build_nop ();
+  SSA_NAME_IS_DEFAULT_DEF (poisoned_var) = true;
+  SSA_NAME_DEF_STMT (poisoned_var) = nop;
+  gsi_replace (iter, nop, GSI_NEW_STMT);
+
+  return false;
+}
+
 /* Instrument the current function.  */
 
 static unsigned int
diff --git a/gcc/asan.h b/gcc/asan.h
index 9cf5904..6c25955 100644
--- a/gcc/asan.h
+++ b/gcc/asan.h
@@ -30,6 +30,7 @@ extern void initialize_sanitizer_builtins (void);
 extern tree asan_dynamic_init_call (bool);
 extern bool asan_expand_check_ifn (gimple_stmt_iterator *, bool);
 extern bool asan_expand_mark_ifn (gimple_stmt_iterator *);
+extern bool asan_expand_poison_ifn (gimple_stmt_iterator *, bool *);
 
 extern gimple_stmt_iterator create_cond_insert_point
      (gimple_stmt_iterator *, bool, bool, bool, basic_block *, basic_block *);
diff --git a/gcc/internal-fn.c b/gcc/internal-fn.c
index ca347c5..17624e8 100644
--- a/gcc/internal-fn.c
+++ b/gcc/internal-fn.c
@@ -246,6 +246,13 @@ expand_ASAN_MARK (internal_fn, gcall *)
   gcc_unreachable ();
 }
 
+/* This should get expanded in the sanopt pass.  */
+
+static void
+expand_ASAN_POISON (internal_fn, gcall *)
+{
+  gcc_unreachable ();
+}
 
 /* This should get expanded in the tsan pass.  */
 
diff --git a/gcc/internal-fn.def b/gcc/internal-fn.def
index d1cd1a5..9454afd 100644
--- a/gcc/internal-fn.def
+++ b/gcc/internal-fn.def
@@ -159,6 +159,7 @@ DEF_INTERNAL_FN (ABNORMAL_DISPATCHER, ECF_NORETURN, NULL)
 DEF_INTERNAL_FN (BUILTIN_EXPECT, ECF_CONST | ECF_LEAF | ECF_NOTHROW, NULL)
 DEF_INTERNAL_FN (ASAN_CHECK, ECF_TM_PURE | ECF_LEAF | ECF_NOTHROW, ".R...")
 DEF_INTERNAL_FN (ASAN_MARK, ECF_LEAF | ECF_NOTHROW, ".R..")
+DEF_INTERNAL_FN (ASAN_POISON, ECF_LEAF | ECF_NOTHROW | ECF_NOVOPS, NULL)
 DEF_INTERNAL_FN (ADD_OVERFLOW, ECF_CONST | ECF_LEAF | ECF_NOTHROW, NULL)
 DEF_INTERNAL_FN (SUB_OVERFLOW, ECF_CONST | ECF_LEAF | ECF_NOTHROW, NULL)
 DEF_INTERNAL_FN (MUL_OVERFLOW, ECF_CONST | ECF_LEAF | ECF_NOTHROW, NULL)
diff --git a/gcc/sanitizer.def b/gcc/sanitizer.def
index 3db08a7..068c55b 100644
--- a/gcc/sanitizer.def
+++ b/gcc/sanitizer.def
@@ -102,6 +102,14 @@ DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_REPORT_STORE_N_NOABORT,
 		      "__asan_report_store_n_noabort",
 		      BT_FN_VOID_PTR_PTRMODE,
 		      ATTR_TMPURE_NOTHROW_LEAF_LIST)
+DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_REPORT_USE_AFTER_SCOPE,
+		      "__asan_report_use_after_scope",
+		      BT_FN_VOID_PTR_PTRMODE,
+		      ATTR_TMPURE_NOTHROW_LEAF_LIST)
+DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_REPORT_USE_AFTER_SCOPE_NOABORT,
+		      "__asan_report_use_after_scope_noabort",
+		      BT_FN_VOID_PTR_PTRMODE,
+		      ATTR_TMPURE_NOTHROW_LEAF_LIST)
 DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_LOAD1, "__asan_load1",
 		      BT_FN_VOID_PTR, ATTR_TMPURE_NOTHROW_LEAF_LIST)
 DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_LOAD2, "__asan_load2",
diff --git a/gcc/sanopt.c b/gcc/sanopt.c
index 320e14e..77307d9 100644
--- a/gcc/sanopt.c
+++ b/gcc/sanopt.c
@@ -698,6 +698,7 @@ pass_sanopt::execute (function *fun)
   bool use_calls = ASAN_INSTRUMENTATION_WITH_CALL_THRESHOLD < INT_MAX
     && asan_num_accesses >= ASAN_INSTRUMENTATION_WITH_CALL_THRESHOLD;
 
+  bool need_commit_edge_insert = false;
   FOR_EACH_BB_FN (bb, fun)
     {
       gimple_stmt_iterator gsi;
@@ -735,6 +736,10 @@ pass_sanopt::execute (function *fun)
 		case IFN_ASAN_MARK:
 		  no_next = asan_expand_mark_ifn (&gsi);
 		  break;
+		case IFN_ASAN_POISON:
+		  no_next = asan_expand_poison_ifn (&gsi,
+						    &need_commit_edge_insert);
+		  break;
 		default:
 		  break;
 		}
@@ -766,6 +771,10 @@ pass_sanopt::execute (function *fun)
 	    gsi_next (&gsi);
 	}
     }
+
+  if (need_commit_edge_insert)
+    gsi_commit_edge_inserts ();
+
   return 0;
 }
 
diff --git a/gcc/tree-ssa.c b/gcc/tree-ssa.c
index 135952b..8696c08 100644
--- a/gcc/tree-ssa.c
+++ b/gcc/tree-ssa.c
@@ -41,6 +41,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "cfgexpand.h"
 #include "tree-cfg.h"
 #include "tree-dfa.h"
+#include "asan.h"
 
 /* Pointer map of variable mappings, keyed by edge.  */
 static hash_map<edge, auto_vec<edge_var_map> > *edge_var_maps;
@@ -1550,6 +1551,24 @@ maybe_optimize_var (tree var, bitmap addresses_taken, bitmap not_reg_needs,
     }
 }
 
+/* Return true when STMT is ASAN mark where second argument is an address
+   of a local variable.  */
+
+static bool
+is_asan_mark_p (gimple *stmt)
+{
+  if (!gimple_call_internal_p (stmt, IFN_ASAN_MARK))
+    return false;
+
+  tree addr = get_base_address (gimple_call_arg (stmt, 1));
+  if (TREE_CODE (addr) == ADDR_EXPR
+      && VAR_P (TREE_OPERAND (addr, 0))
+      && is_gimple_reg_type (TREE_TYPE (TREE_OPERAND (addr, 0))))
+    return true;
+
+  return false;
+}
+
 /* Compute TREE_ADDRESSABLE and DECL_GIMPLE_REG_P for local variables.  */
 
 void
@@ -1575,17 +1594,23 @@ execute_update_addresses_taken (void)
 	  enum gimple_code code = gimple_code (stmt);
 	  tree decl;
 
-	  if (code == GIMPLE_CALL
-	      && optimize_atomic_compare_exchange_p (stmt))
+	  if (code == GIMPLE_CALL)
 	    {
-	      /* For __atomic_compare_exchange_N if the second argument
-		 is &var, don't mark var addressable;
-		 if it becomes non-addressable, we'll rewrite it into
-		 ATOMIC_COMPARE_EXCHANGE call.  */
-	      tree arg = gimple_call_arg (stmt, 1);
-	      gimple_call_set_arg (stmt, 1, null_pointer_node);
-	      gimple_ior_addresses_taken (addresses_taken, stmt);
-	      gimple_call_set_arg (stmt, 1, arg);
+	      if (optimize_atomic_compare_exchange_p (stmt))
+		{
+		  /* For __atomic_compare_exchange_N if the second argument
+		     is &var, don't mark var addressable;
+		     if it becomes non-addressable, we'll rewrite it into
+		     ATOMIC_COMPARE_EXCHANGE call.  */
+		  tree arg = gimple_call_arg (stmt, 1);
+		  gimple_call_set_arg (stmt, 1, null_pointer_node);
+		  gimple_ior_addresses_taken (addresses_taken, stmt);
+		  gimple_call_set_arg (stmt, 1, arg);
+		}
+	      else if (is_asan_mark_p (stmt))
+		;
+	      else
+		gimple_ior_addresses_taken (addresses_taken, stmt);
 	    }
 	  else
 	    /* Note all addresses taken by the stmt.  */
@@ -1841,6 +1866,26 @@ execute_update_addresses_taken (void)
 			continue;
 		      }
 		  }
+		else if (is_asan_mark_p (stmt))
+		  {
+		    tree var = TREE_OPERAND (gimple_call_arg (stmt, 1), 0);
+		    if (bitmap_bit_p (suitable_for_renaming, DECL_UID (var)))
+		      {
+			HOST_WIDE_INT flags
+			  = tree_to_shwi (gimple_call_arg (stmt, 0));
+			unlink_stmt_vdef (stmt);
+			if (flags & ASAN_MARK_CLOBBER)
+			  {
+			    gcall *call
+			      = gimple_build_call_internal (IFN_ASAN_POISON, 0);
+			    gimple_call_set_lhs (call, var);
+			    gsi_replace (&gsi, call, GSI_SAME_STMT);
+			  }
+			else
+			  gsi_remove (&gsi, true);
+			continue;
+		      }
+		  }
 		for (i = 0; i < gimple_call_num_args (stmt); ++i)
 		  {
 		    tree *argp = gimple_call_arg_ptr (stmt, i);
diff --git a/libsanitizer/asan/asan_errors.cc b/libsanitizer/asan/asan_errors.cc
index 73c4cca..f17c9b7 100644
--- a/libsanitizer/asan/asan_errors.cc
+++ b/libsanitizer/asan/asan_errors.cc
@@ -279,6 +279,27 @@ void ErrorInvalidPointerPair::Print() {
   ReportErrorSummary(bug_type, &stack);
 }
 
+void ErrorUseAfterScope::Print() {
+  const char *bug_type = "stack-use-after-scope";
+  Decorator d;
+  Printf("%s", d.Warning());
+
+  Report("ERROR: AddressSanitizer: stack-use-after-scope at pc %p bp %p sp %p\n",
+         variable_name, variable_size, pc, bp, sp);
+  Printf("%s", d.EndWarning());
+  scariness.Print();
+
+  char tname[128];
+  Printf("ACCESS of size %zu for variable '%s' thread T%d%s%s\n",
+         variable_size, variable_name, tid,
+         ThreadNameWithParenthesis(tid, tname, sizeof(tname)), d.EndAccess());
+
+  GET_STACK_TRACE_FATAL(pc, bp);
+  stack.Print();
+  ReportErrorSummary(bug_type, &stack);
+}
+
+
 static bool AdjacentShadowValuesAreFullyPoisoned(u8 *s) {
   return s[-1] > 127 && s[1] > 127;
 }
diff --git a/libsanitizer/asan/asan_errors.h b/libsanitizer/asan/asan_errors.h
index 6262dcf..4d0698f 100644
--- a/libsanitizer/asan/asan_errors.h
+++ b/libsanitizer/asan/asan_errors.h
@@ -294,6 +294,24 @@ struct ErrorInvalidPointerPair : ErrorBase {
   void Print();
 };
 
+struct ErrorUseAfterScope : ErrorBase {
+  uptr pc, bp, sp;
+  const char *variable_name;
+  u32 variable_size;
+  // VS2013 doesn't implement unrestricted unions, so we need a trivial default
+  // constructor
+  ErrorUseAfterScope() = default;
+  ErrorUseAfterScope(u32 tid, uptr pc_, uptr bp_, uptr sp_,
+                     const char *variable_name_, u32 variable_size_)
+      : ErrorBase(tid),
+        pc(pc_),
+        bp(bp_),
+        sp(sp_),
+	variable_name(variable_name_),
+	variable_size(variable_size_) {}
+  void Print();
+};
+
 struct ErrorGeneric : ErrorBase {
   AddressDescription addr_description;
   uptr pc, bp, sp;
@@ -324,6 +342,7 @@ struct ErrorGeneric : ErrorBase {
   macro(BadParamsToAnnotateContiguousContainer) \
   macro(ODRViolation)                           \
   macro(InvalidPointerPair)                     \
+  macro(UseAfterScope)                          \
   macro(Generic)
 // clang-format on
 
diff --git a/libsanitizer/asan/asan_report.cc b/libsanitizer/asan/asan_report.cc
index 84d6764..c03edb9 100644
--- a/libsanitizer/asan/asan_report.cc
+++ b/libsanitizer/asan/asan_report.cc
@@ -353,6 +353,16 @@ static INLINE void CheckForInvalidPointerPair(void *p1, void *p2) {
     return ReportInvalidPointerPair(pc, bp, sp, a1, a2);
   }
 }
+// ----------------------- ReportUseAfterScope ----------- {{{1
+void ReportUseAfterScope(const char *variable_name, u32 variable_size,
+                         bool fatal) {
+  ScopedInErrorReport in_report (fatal);
+  GET_CALLER_PC_BP_SP;
+  ErrorUseAfterScope error(GetCurrentTidOrInvalid(), pc, bp, sp, variable_name,
+                           variable_size);
+  in_report.ReportError(error);
+}
+
 // ----------------------- Mac-specific reports ----------------- {{{1
 
 void ReportMacMzReallocUnknown(uptr addr, uptr zone_ptr, const char *zone_name,
diff --git a/libsanitizer/asan/asan_report.h b/libsanitizer/asan/asan_report.h
index 111b840..dca6a5e 100644
--- a/libsanitizer/asan/asan_report.h
+++ b/libsanitizer/asan/asan_report.h
@@ -68,6 +68,9 @@ void ReportBadParamsToAnnotateContiguousContainer(uptr beg, uptr end,
 void ReportODRViolation(const __asan_global *g1, u32 stack_id1,
                         const __asan_global *g2, u32 stack_id2);
 
+void ReportUseAfterScope(const char *variable_name, u32 variable_size,
+                         bool fatal);
+
 // Mac-specific errors and warnings.
 void ReportMacMzReallocUnknown(uptr addr, uptr zone_ptr,
                                const char *zone_name,
diff --git a/libsanitizer/asan/asan_rtl.cc b/libsanitizer/asan/asan_rtl.cc
index 38009d2..f637d71 100644
--- a/libsanitizer/asan/asan_rtl.cc
+++ b/libsanitizer/asan/asan_rtl.cc
@@ -253,6 +253,22 @@ void __asan_storeN_noabort(uptr addr, uptr size) {
   }
 }
 
+#include <stdio.h>
+
+extern "C"
+NOINLINE INTERFACE_ATTRIBUTE
+void __asan_report_use_after_scope(const char *variable_name,
+                                   uptr variable_size) {
+  ReportUseAfterScope(variable_name, variable_size, true);
+}
+
+extern "C"
+NOINLINE INTERFACE_ATTRIBUTE
+void __asan_report_use_after_scope_noabort(const char *variable_name,
+                                           uptr variable_size) {
+  ReportUseAfterScope(variable_name, variable_size, false);
+}
+
 // Force the linker to keep the symbols for various ASan interface functions.
 // We want to keep those in the executable in order to let the instrumented
 // dynamic libraries access the symbol even if it is not used by the executable
-- 
2.10.1


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