Hi,

this patch actually fixes two issues:

* In some cases the dereferences in function argument list are not instrumented and ASan can miss errors, e.g. heap-use-after-free. To resolve this, we can just iterate through gimple_call's arguments and add ASAN_CHECK statements for them if needed.

* As Jakub pointed out, if gimple_call was inlined, ASan catches an error on invalid dereference, but fails to obtain its source location, because it was missed in early inline pass. To resolve this, we can set corresponding source location for inlined parameter in einline pass in initialize_inlined_parameters routine.

Regtested and bootstrapped on x86_64-unknown-linux-gnu, OK to commit?

-Maxim
gcc/ChangeLog:

2016-04-08  Maxim Ostapenko  <m.ostape...@samsung.com>

	PR sanitizer/70541
	* asan.c (instrument_derefs): If we get unknown location, extract it
	with EXPR_LOCATION.
	(maybe_instrument_call): Instrument gimple_call's arguments if needed.
	* tree-inline.c (initialize_inlined_parameters): Set location for
	inlined parameters initialization statements.

gcc/testsuite/ChangeLog:

2016-04-08  Maxim Ostapenko  <m.ostape...@samsung.com>

	PR sanitizer/70541
	* c-c++-common/asan/pr70541-1.c: New test.
	* c-c++-common/pr70541-2.c: Likewise.

diff --git a/gcc/asan.c b/gcc/asan.c
index 47bfdcd..14f97a2e 100644
--- a/gcc/asan.c
+++ b/gcc/asan.c
@@ -1766,6 +1766,8 @@ instrument_derefs (gimple_stmt_iterator *iter, tree t,
 
   tree type, base;
   HOST_WIDE_INT size_in_bytes;
+  if (location == UNKNOWN_LOCATION)
+    location = EXPR_LOCATION (t);
 
   type = TREE_TYPE (t);
   switch (TREE_CODE (t))
@@ -2049,6 +2051,7 @@ maybe_instrument_call (gimple_stmt_iterator *iter)
       gsi_insert_before (iter, g, GSI_SAME_STMT);
     }
 
+  bool instrumented = false;
   if (gimple_store_p (stmt))
     {
       tree ref_expr = gimple_call_lhs (stmt);
@@ -2056,11 +2059,31 @@ maybe_instrument_call (gimple_stmt_iterator *iter)
 			 gimple_location (stmt),
 			 /*is_store=*/true);
 
-      gsi_next (iter);
-      return true;
+      instrumented = true;
     }
 
-  return false;
+  /* Walk through gimple_call arguments and check them id needed.  */
+  unsigned args_num = gimple_call_num_args (stmt);
+  for (unsigned i = 0; i < args_num; ++i)
+    {
+      tree arg = gimple_call_arg (stmt, i);
+      gcc_assert (TREE_CODE (arg) != CALL_EXPR);
+      /* If ARG is not a non-aggregate register variable, compiler in general
+	 creates temporary for it and pass it as argument to gimple call.
+	 But in some cases, e.g. when we pass by value a small structure that
+	 fits to register, compiler can avoid extra overhead by pulling out
+	 these temporaries.  In this case, we should check the argument.  */
+      if (!is_gimple_reg (arg))
+	{
+	  instrument_derefs (iter, arg,
+			     gimple_location (stmt),
+			     /*is_store=*/false);
+	  instrumented = true;
+	}
+    }
+  if (instrumented)
+    gsi_next (iter);
+  return instrumented;
 }
 
 /* Walk each instruction of all basic block and instrument those that
diff --git a/gcc/testsuite/c-c++-common/asan/pr70541-1.c b/gcc/testsuite/c-c++-common/asan/pr70541-1.c
new file mode 100644
index 0000000..af853e7
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/asan/pr70541-1.c
@@ -0,0 +1,33 @@
+/* { dg-do run } */
+/* { dg-options "-fno-builtin-malloc -fno-builtin-free" } */
+/* { dg-shouldfail "asan" } */
+
+#include <stdio.h>
+#include <stdlib.h>
+
+struct Simple {
+  int value;
+};
+
+int f(struct Simple simple) {
+  return simple.value;
+}
+
+int main() {
+  struct Simple *psimple = (struct Simple *) malloc(sizeof(struct Simple));
+  psimple->value = 42;
+  free(psimple);
+  printf("%d\n", f(*psimple));
+  return 0;
+}
+
+/* { dg-output "ERROR: AddressSanitizer:? heap-use-after-free on address\[^\n\r]*" } */
+/* { dg-output "0x\[0-9a-f\]+ at pc 0x\[0-9a-f\]+ bp 0x\[0-9a-f\]+ sp 0x\[0-9a-f\]+\[^\n\r]*(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*READ of size 4 at 0x\[0-9a-f\]+ thread T0\[^\n\r]*(\n|\r\n|\r)" } */
+/* { dg-output "    #0 0x\[0-9a-f\]+ +(in _*main (\[^\n\r]*pr70541-1.c:20|\[^\n\r]*:0)|\[(\]).*(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*freed by thread T0 here:\[^\n\r]*(\n|\r\n|\r)" } */
+/* { dg-output "    #0 0x\[0-9a-f\]+ +(in _*(interceptor_|wrap_|)free|\[(\])\[^\n\r]*(\n|\r\n|\r)" } */
+/* { dg-output "    #1 0x\[0-9a-f\]+ +(in _*main (\[^\n\r]*pr70541-1.c:19|\[^\n\r]*:0)|\[(\]).*(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*previously allocated by thread T0 here:\[^\n\r]*(\n|\r\n|\r)" } */
+/* { dg-output "    #0 0x\[0-9a-f\]+ +(in _*(interceptor_|wrap_|)malloc|\[(\])\[^\n\r]*(\n|\r\n|\r)" } */
+/* { dg-output "    #1 0x\[0-9a-f\]+ +(in _*main (\[^\n\r]*pr70541-1.c:17|\[^\n\r]*:0)|\[(\])\[^\n\r]*(\n|\r\n|\r)" } */
diff --git a/gcc/testsuite/c-c++-common/pr70541-2.c b/gcc/testsuite/c-c++-common/pr70541-2.c
new file mode 100644
index 0000000..1fd6517
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/pr70541-2.c
@@ -0,0 +1,24 @@
+/* { dg-do compile } */
+/* { dg-options " -O2 -fdump-tree-einline-lineno" } */
+
+extern int printf (const char *format, ...);
+
+struct Simple
+{
+  int value;
+};
+
+int f (struct Simple simple)
+{
+  return simple.value;
+}
+
+int main ()
+{
+  struct Simple psimple;
+  struct Simple *psimple_ptr = &psimple;
+  printf ("%d\n", f (*psimple_ptr));
+  return 0;
+}
+
+/* { dg-final { scan-tree-dump "c:17:\[^\n\r\]*simple =" "einline" } } */
diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c
index 9d4f8f7..24bbd0a 100644
--- a/gcc/tree-inline.c
+++ b/gcc/tree-inline.c
@@ -3247,7 +3247,8 @@ initialize_inlined_parameters (copy_body_data *id, gimple *stmt,
     {
       tree val;
       val = i < gimple_call_num_args (stmt) ? gimple_call_arg (stmt, i) : NULL;
-      setup_one_parameter (id, p, val, fn, bb, &vars);
+      if (gimple *new_stmt = setup_one_parameter (id, p, val, fn, bb, &vars))
+	gimple_set_location (new_stmt, gimple_location (stmt));
     }
   /* After remapping parameters remap their types.  This has to be done
      in a second loop over all parameters to appropriately remap
@@ -3285,7 +3286,9 @@ initialize_inlined_parameters (copy_body_data *id, gimple *stmt,
       /* No static chain?  Seems like a bug in tree-nested.c.  */
       gcc_assert (static_chain);
 
-      setup_one_parameter (id, p, static_chain, fn, bb, &vars);
+      if (gimple *new_stmt = setup_one_parameter (id, p, static_chain, fn, bb,
+						  &vars))
+	gimple_set_location (new_stmt, gimple_location (stmt));
     }
 
   declare_inline_vars (id->block, vars);

Reply via email to