[PATCH] warn on returning alloca and VLA (PR 71924, 90549)

[Martin Sebor <msebor at gmail dot com>]

-Wreturn-local-addr detects a subset of instances of returning
the address of a local object from a function but the warning
doesn't try to handle alloca or VLAs, or some non-trivial cases
of ordinary automatic variables[1].

The attached patch extends the implementation of the warning to
detect those.  It still doesn't detect instances where the address
is the result of a built-in such strcpy[2].

Tested on x86_64-linux.

Martin

[1] For example, this is only diagnosed with the patch:

  void* f (int i)
  {
    struct S { int a[2]; } s[2];
    return &s->a[i];
  }

[2] The following is not diagnosed even with the patch:

  void sink (void*);

  void* f (int i)
  {
    char a[6];
    char *p = __builtin_strcpy (a, "123");
    sink (p);
    return p;
  }

I would expect detecting to be possible and useful.  Maybe as
a follow-up.

PR middle-end/71924 - missing -Wreturn-local-addr returning alloca result
PR middle-end/90549 - missing -Wreturn-local-addr maybe returning an address of a local array plus offset

gcc/ChangeLog:

	PR c/71924
	* gimple-ssa-isolate-paths.c (is_addr_local): New function.
	(warn_return_addr_local_phi_arg, warn_return_addr_local): Same.
	(find_implicit_erroneous_behavior): Call warn_return_addr_local_phi_arg.
	(find_explicit_erroneous_behavior): Call warn_return_addr_local.

gcc/testsuite/ChangeLog:

	PR c/71924
	* gcc.dg/Wreturn-local-addr-2.c: New test.
	* gcc.dg/Walloca-4.c: Prune expected warnings.
	* gcc.dg/pr41551.c: Same.
	* gcc.dg/pr59523.c: Same.
	* gcc.dg/tree-ssa/pr88775-2.c: Same.
	* gcc.dg/winline-7.c: Same.

diff --git a/gcc/gimple-ssa-isolate-paths.c b/gcc/gimple-ssa-isolate-paths.c
index 33fe352bb23..2933ecf502e 100644
--- a/gcc/gimple-ssa-isolate-paths.c
+++ b/gcc/gimple-ssa-isolate-paths.c
@@ -341,6 +341,135 @@ stmt_uses_0_or_null_in_undefined_way (gimple *stmt)
   return false;
 }
 
+/* Return true if EXPR is a expression of pointer type that refers
+   to the address of a variable with automatic storage duration.
+   If so, set *PLOC to the location of the object or the call that
+   allocated it (for alloca and VLAs).  When PMAYBE is non-null,
+   also consider PHI statements and set *PMAYBE when some but not
+   all arguments of such statements refer to local variables, and
+   to clear it otherwise.  */
+
+static bool
+is_addr_local (tree exp, location_t *ploc, bool *pmaybe = NULL,
+	       hash_set<gphi *> *visited = NULL)
+{
+  if (TREE_CODE (exp) == ADDR_EXPR)
+    {
+      tree baseaddr = get_base_address (TREE_OPERAND (exp, 0));
+      if (TREE_CODE (baseaddr) == MEM_REF)
+	return is_addr_local (TREE_OPERAND (baseaddr, 0), ploc, pmaybe, visited);
+
+      if ((!VAR_P (baseaddr)
+	   || is_global_var (baseaddr))
+	  && TREE_CODE (baseaddr) != PARM_DECL)
+	return false;
+
+      *ploc = DECL_SOURCE_LOCATION (baseaddr);
+      return true;
+    }
+
+  if (TREE_CODE (exp) == SSA_NAME)
+    {
+      gimple *def_stmt = SSA_NAME_DEF_STMT (exp);
+      enum gimple_code code = gimple_code (def_stmt);
+
+      if (is_gimple_assign (def_stmt))
+	{
+	  tree type = TREE_TYPE (gimple_assign_lhs (def_stmt));
+	  if (POINTER_TYPE_P (type))
+	    {
+	      tree ptr = gimple_assign_rhs1 (def_stmt);
+	      return is_addr_local (ptr, ploc, pmaybe, visited);
+	    }
+	  return false;
+	}
+
+      if (code == GIMPLE_CALL
+	  && gimple_call_builtin_p (def_stmt))
+	{
+	  tree fn = gimple_call_fndecl (def_stmt);
+	  int code = DECL_FUNCTION_CODE (fn);
+	  if (code != BUILT_IN_ALLOCA
+	      && code != BUILT_IN_ALLOCA_WITH_ALIGN)
+	    return false;
+
+	  *ploc = gimple_location (def_stmt);
+	  return true;
+	}
+
+      if (code == GIMPLE_PHI && pmaybe)
+	{
+	  unsigned count = 0;
+	  gphi *phi_stmt = as_a <gphi *> (def_stmt);
+
+	  unsigned nargs = gimple_phi_num_args (phi_stmt);
+	  for (unsigned i = 0; i < nargs; ++i)
+	    {
+	      if (!visited->add (phi_stmt))
+		{
+		  tree arg = gimple_phi_arg_def (phi_stmt, i);
+		  if (is_addr_local (arg, ploc, pmaybe, visited))
+		    ++count;
+		}
+	    }
+
+	  *pmaybe = count && count < nargs;
+	  return count != 0;
+	}
+    }
+
+  return false;
+}
+
+/* Detect and diagnose returning the address of a local variable in
+   a PHI result LHS and argument OP and PHI edge E in basic block BB.  */
+
+static basic_block
+warn_return_addr_local_phi_arg (basic_block bb, basic_block duplicate,
+				tree lhs, tree op, edge e)
+{
+  location_t origin;
+  if (!is_addr_local (op, &origin))
+    return NULL;
+
+  gimple *use_stmt;
+  imm_use_iterator iter;
+
+  FOR_EACH_IMM_USE_STMT (use_stmt, iter, lhs)
+    {
+      greturn *return_stmt = dyn_cast <greturn *> (use_stmt);
+      if (!return_stmt)
+	continue;
+
+      if (gimple_return_retval (return_stmt) != lhs)
+	continue;
+
+      {
+	auto_diagnostic_group d;
+	if (warning_at (gimple_location (use_stmt),
+			OPT_Wreturn_local_addr,
+			"function may return address "
+			"of local variable")
+	    && origin != UNKNOWN_LOCATION)
+	  inform (origin, "declared here");
+      }
+
+      if ((flag_isolate_erroneous_paths_dereference
+	   || flag_isolate_erroneous_paths_attribute)
+	  && gimple_bb (use_stmt) == bb)
+	{
+	  duplicate = isolate_path (bb, duplicate, e,
+				    use_stmt, lhs, true);
+
+	  /* When we remove an incoming edge, we need to
+	     reprocess the Ith element.  */
+	  cfg_altered = true;
+	}
+    }
+
+  return duplicate;
+}
+
 /* Look for PHI nodes which feed statements in the same block where
    the value of the PHI node implies the statement is erroneous.
 
@@ -400,58 +529,19 @@ find_implicit_erroneous_behavior (void)
 	    {
 	      tree op = gimple_phi_arg_def (phi, i);
 	      edge e = gimple_phi_arg_edge (phi, i);
-	      imm_use_iterator iter;
-	      gimple *use_stmt;
 
-	      next_i = i + 1;
-
-	      if (TREE_CODE (op) == ADDR_EXPR)
-		{
-		  tree valbase = get_base_address (TREE_OPERAND (op, 0));
-		  if ((VAR_P (valbase) && !is_global_var (valbase))
-		      || TREE_CODE (valbase) == PARM_DECL)
-		    {
-		      FOR_EACH_IMM_USE_STMT (use_stmt, iter, lhs)
-			{
-			  greturn *return_stmt
-			    = dyn_cast <greturn *> (use_stmt);
-			  if (!return_stmt)
-			    continue;
-
-			  if (gimple_return_retval (return_stmt) != lhs)
-			    continue;
-
-			  {
-			    auto_diagnostic_group d;
-			    if (warning_at (gimple_location (use_stmt),
-					      OPT_Wreturn_local_addr,
-					      "function may return address "
-					      "of local variable"))
-			      inform (DECL_SOURCE_LOCATION(valbase),
-					"declared here");
-			  }
-
-			  if ((flag_isolate_erroneous_paths_dereference
-			       || flag_isolate_erroneous_paths_attribute)
-			      && gimple_bb (use_stmt) == bb)
-			    {
-			      duplicate = isolate_path (bb, duplicate, e,
-							use_stmt, lhs, true);
-
-			      /* When we remove an incoming edge, we need to
-				 reprocess the Ith element.  */
-			      next_i = i;
-			      cfg_altered = true;
-			    }
-			}
-		    }
-		}
+	      duplicate = warn_return_addr_local_phi_arg (bb, duplicate,
+							  lhs, op, e);
+	      next_i = duplicate ? i : i + 1;
 
 	      if (!integer_zerop (op))
 		continue;
 
 	      location_t phi_arg_loc = gimple_phi_arg_location (phi, i);
 
+	      imm_use_iterator iter;
+	      gimple *use_stmt;
+
 	      /* We've got a NULL PHI argument.  Now see if the
  	         PHI's result is dereferenced within BB.  */
 	      FOR_EACH_IMM_USE_STMT (use_stmt, iter, lhs)
@@ -482,6 +572,51 @@ find_implicit_erroneous_behavior (void)
     }
 }
 
+/* Detect and diagnose returning the address of a local variable
+   in RETURN_STMT in basic block BB.  This only becomes undefined
+   behavior if the result is used, so we do not insert a trap and
+   only return NULL instead.  */
+
+static void
+warn_return_addr_local (basic_block bb, greturn *return_stmt)
+{
+  tree val = gimple_return_retval (return_stmt);
+  if (!val)
+    return;
+
+  bool maybe = false;
+  location_t origin;
+  hash_set<gphi *> visited_phis;
+  if (!is_addr_local (val, &origin, &maybe, &visited_phis))
+    return;
+
+  /* We only need it for this particular case.  */
+  calculate_dominance_info (CDI_POST_DOMINATORS);
+  const char* msg = N_("function returns address of local variable");
+  if (maybe
+      || !dominated_by_p (CDI_POST_DOMINATORS,
+			  single_succ (ENTRY_BLOCK_PTR_FOR_FN (cfun)), bb))
+      msg = N_("function may return address of local variable");
+
+  {
+    auto_diagnostic_group d;
+    if (warning_at (gimple_location (return_stmt),
+		    OPT_Wreturn_local_addr, msg)
+	&& origin != UNKNOWN_LOCATION)
+      inform (origin, "declared here");
+  }
+
+  /* Do not modify code if the user only asked for
+     warnings.  */
+  if (flag_isolate_erroneous_paths_dereference
+      || flag_isolate_erroneous_paths_attribute)
+    {
+      tree zero = build_zero_cst (TREE_TYPE (val));
+      gimple_return_set_retval (return_stmt, zero);
+      update_stmt (return_stmt);
+    }
+}
+
 /* Look for statements which exhibit erroneous behavior.  For example
    a NULL pointer dereference.
 
@@ -525,49 +660,10 @@ find_explicit_erroneous_behavior (void)
 	      break;
 	    }
 
-	  /* Detect returning the address of a local variable.  This only
-	     becomes undefined behavior if the result is used, so we do not
-	     insert a trap and only return NULL instead.  */
+	  /* Look for a return statement that returns the address
+	     of a local variable or the result of alloca.  */
 	  if (greturn *return_stmt = dyn_cast <greturn *> (stmt))
-	    {
-	      tree val = gimple_return_retval (return_stmt);
-	      if (val && TREE_CODE (val) == ADDR_EXPR)
-		{
-		  tree valbase = get_base_address (TREE_OPERAND (val, 0));
-		  if ((VAR_P (valbase) && !is_global_var (valbase))
-		      || TREE_CODE (valbase) == PARM_DECL)
-		    {
-		      /* We only need it for this particular case.  */
-		      calculate_dominance_info (CDI_POST_DOMINATORS);
-		      const char* msg;
-		      bool always_executed = dominated_by_p
-			(CDI_POST_DOMINATORS,
-			 single_succ (ENTRY_BLOCK_PTR_FOR_FN (cfun)), bb);
-		      if (always_executed)
-			msg = N_("function returns address of local variable");
-		      else
-			msg = N_("function may return address of "
-				 "local variable");
-		      {
-			auto_diagnostic_group d;
-			if (warning_at (gimple_location (stmt),
-					  OPT_Wreturn_local_addr, msg))
-			  inform (DECL_SOURCE_LOCATION(valbase),
-				  "declared here");
-		      }
-
-		      /* Do not modify code if the user only asked for
-			 warnings.  */
-		      if (flag_isolate_erroneous_paths_dereference
-			  || flag_isolate_erroneous_paths_attribute)
-			{
-			  tree zero = build_zero_cst (TREE_TYPE (val));
-			  gimple_return_set_retval (return_stmt, zero);
-			  update_stmt (stmt);
-			}
-		    }
-		}
-	    }
+	    warn_return_addr_local (bb, return_stmt);
 	}
     }
 }
diff --git a/gcc/testsuite/gcc.dg/Walloca-4.c b/gcc/testsuite/gcc.dg/Walloca-4.c
index 85dcb7b9bb9..f888e2db2ed 100644
--- a/gcc/testsuite/gcc.dg/Walloca-4.c
+++ b/gcc/testsuite/gcc.dg/Walloca-4.c
@@ -7,7 +7,7 @@
 {
 
   char *src;
- _Bool 
+ _Bool
       use_alloca = (((rear_ptr - w) * sizeof (char)) < 4096U);
  if (use_alloca)
     src = (char *) __builtin_alloca ((rear_ptr - w) * sizeof (char));
@@ -15,3 +15,5 @@
     src = (char *) __builtin_malloc ((rear_ptr - w) * sizeof (char));
   return src;
 }
+
+/* { dg-prune-output "-Wreturn-local-addr" } */
diff --git a/gcc/testsuite/gcc.dg/Wreturn-local-addr-2.c b/gcc/testsuite/gcc.dg/Wreturn-local-addr-2.c
new file mode 100644
index 00000000000..0e3435c8256
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Wreturn-local-addr-2.c
@@ -0,0 +1,293 @@
+/* PR c/71924 - missing -Wreturn-local-addr returning alloca result
+   { dg-do compile }
+   { dg-options "-O2 -Wall" } */
+
+#define ATTR(...) __attribute__ ((__VA_ARGS__))
+
+struct A { int a, b, c; };
+struct B { int a, b, c[]; };
+
+void sink (void*, ...);
+
+ATTR (noipa) void*
+return_alloca (int n)
+{
+  void *p = __builtin_alloca (n);
+  sink (p);
+  return p;         /* { dg-warning "function returns address of local" } */
+}
+
+ATTR (noipa) void*
+return_alloca_index_cst (int n)
+{
+  int *p = (int*)__builtin_alloca (n);
+  p = &p[1];
+  sink (p);
+  return p;         /* { dg-warning "function returns address of local" } */
+}
+
+ATTR (noipa) void*
+return_alloca_plus_cst (int n)
+{
+  int *p = (int*)__builtin_alloca (n);
+  p += 1;
+  sink (p);
+  return p;         /* { dg-warning "function returns address of local" } */
+}
+
+ATTR (noipa) void*
+return_alloca_plus_var (int n, int i)
+{
+  char *p = (char*)__builtin_alloca (n);
+  p += i;
+  sink (p);
+  return p;         /* { dg-warning "function returns address of local" } */
+}
+
+ATTR (noipa) void*
+return_alloca_member_1 (int n)
+{
+  struct A *p = (struct A*)__builtin_alloca (n);
+  sink (&p->a);
+  return &p->a;     /* { dg-warning "function returns address of local" } */
+}
+
+ATTR (noipa) void*
+return_alloca_member_2 (int n)
+{
+  struct A *p = (struct A*)__builtin_alloca (n);
+  sink (&p->b);
+  return &p->b;     /* { dg-warning "function returns address of local" } */
+}
+
+ATTR (noipa) void*
+return_alloca_flexarray (int n)
+{
+  struct B *p = (struct B*)__builtin_alloca (n);
+  sink (p->c);
+  return p->c;      /* { dg-warning "function returns address of local" } */
+}
+
+
+ATTR (noipa) void*
+return_array (void)
+{
+  int a[32];
+  void *p = a;
+  sink (p);
+  return p;         /* { dg-warning "function returns address of local" } */
+}
+
+ATTR (noipa) void*
+return_array_index_cst (void)
+{
+  int a[32];
+  void *p = &a[2];
+  sink (p);
+  return p;         /* { dg-warning "function returns address of local" } */
+}
+
+ATTR (noipa) void*
+return_array_plus_cst (void)
+{
+  int a[32];
+  void *p = a + 2;
+  sink (p);
+  return p;         /* { dg-warning "function returns address of local" } */
+}
+
+ATTR (noipa) void*
+return_array_plus_var (int i)
+{
+  int a[32];
+  void *p = a + i;
+  sink (p);
+  return p;         /* { dg-warning "function returns address of local" } */
+}
+
+ATTR (noipa) void*
+return_array_member_1 (void)
+{
+  struct A a[2];
+  int *p = &a[1].a;
+  sink (a, p);
+  return p;         /* { dg-warning "function returns address of local" } */
+}
+
+ATTR (noipa) void*
+return_array_member_2 (void)
+{
+  struct A a[32];
+  int *p = &a[1].b;
+  sink (a, p);
+  return p;         /* { dg-warning "function returns address of local" } */
+}
+
+
+ATTR (noipa) void*
+return_vla (int n)
+{
+  char a[n];
+  void *p = a;
+  sink (p);
+  return p;   /* { dg-warning "function returns address of local" } */
+}
+
+ATTR (noipa) void*
+return_vla_index_cst (int n)
+{
+  char a[n];
+  char *p = &a[3];
+  sink (p);
+  return p;   /* { dg-warning "function returns address of local" } */
+}
+
+ATTR (noipa) void*
+return_vla_plus_cst (int n)
+{
+  char a[n];
+  char *p = a + 3;
+  sink (p);
+  return p;   /* { dg-warning "function returns address of local" } */
+}
+
+ATTR (noipa) void*
+return_vla_index_var (int n, int i)
+{
+  char a[n];
+  char *p = &a[i];
+  sink (p);
+  return p;   /* { dg-warning "function returns address of local" } */
+}
+
+ATTR (noipa) void*
+return_vla_plus_var (int n, int i)
+{
+  char a[n];
+  char *p = a + i;
+  sink (p);
+  return p;   /* { dg-warning "function returns address of local" } */
+}
+
+ATTR (noipa) void*
+return_vla_member_1 (int n, int i)
+{
+  struct A a[n];
+  void *p = &a[i].a;
+  sink (a, p);
+  return p;   /* { dg-warning "function returns address of local" } */
+}
+
+ATTR (noipa) void*
+return_vla_member_2 (int n, int i)
+{
+  struct A a[n];
+  void *p = &a[i].b;
+  sink (a, p);
+  return p;   /* { dg-warning "function returns address of local" } */
+}
+
+
+ATTR (noipa) void*
+return_alloca_or_alloca (int n, int i)
+{
+  void *p = i ? __builtin_alloca (n * i) : __builtin_alloca (n);
+  sink (p);
+  /* The warning here should really be "function returns".  */
+  return p;   /* { dg-warning "function (returns|may return) address of local" } */
+}
+
+ATTR (noipa) void*
+return_alloca_or_alloca_2 (int n, int i)
+{
+  void *p0 = __builtin_alloca (n);
+  void *p1 = __builtin_alloca (n * 2);
+  void *p = i ? p0 : p1;
+  sink (p0, p1, p);
+  /* Same as above.  */
+  return p;   /* { dg-warning "function (returns|may return) address of local" } */
+}
+
+ATTR (noipa) void*
+return_array_or_array (int i)
+{
+  int a[5];
+  int b[7];
+  void *p = i ? a : b;
+  sink (a, b, p);
+  /* The warning here should really be "function returns".  */
+  return p;   /* { dg-warning "function (returns|may return) address of local" } */
+}
+
+ATTR (noipa) void*
+return_array_or_array_plus_var (int i, int j)
+{
+  int a[5];
+  int b[7];
+
+  void *p0 = a + i;
+  void *p1 = b + j;
+
+  void *p = i < j ? p0 : p1;
+  sink (a, b, p0, p1, p);
+  /* The warning here should really be "function returns".  */
+  return p;   /* { dg-warning "function (returns|may return) address of local" } */
+}
+
+extern int global[32];
+
+ATTR (noipa) void*
+may_return_global_or_alloca (int n, int i)
+{
+  void *p = i ? global : __builtin_alloca (n);
+  sink (p);
+  return p;   /* { dg-warning "function may return address of local" } */
+}
+
+
+ATTR (noipa) void*
+may_return_global_or_alloca_plus_cst (int n, int i)
+{
+  int *p = i ? global : (int*)__builtin_alloca (n);
+  p += 7;
+  sink (p);
+  return p;   /* { dg-warning "function may return address of local" } */
+}
+
+ATTR (noipa) void*
+may_return_global_or_array (int n, int i)
+{
+  int a[32];
+  void *p = i ? global : a;
+  sink (p);
+  return p;   /* { dg-warning "function may return address of local" } */
+}
+
+ATTR (noipa) void*
+may_return_global_or_array_plus_cst (int n, int i)
+{
+  int a[32];
+  int *p = i ? global : a;
+  p += 4;
+  sink (p);
+  return p;   /* { dg-warning "function may return address of local" } */
+}
+
+ATTR (noipa) void*
+may_return_global_or_vla (int n, int i)
+{
+  int a[n];
+  void *p = i ? global : a;
+  sink (p);
+  return p;   /* { dg-warning "function may return address of local" } */
+}
+
+ATTR (noipa) void*
+may_return_global_or_vla_plus_cst (int n, int i)
+{
+  int a[n];
+  int *p = i ? global : a;
+  p += 4;
+  sink (p);
+  return p;   /* { dg-warning "function may return address of local" } */
+}
diff --git a/gcc/testsuite/gcc.dg/pr41551.c b/gcc/testsuite/gcc.dg/pr41551.c
index 2f2ad2be97e..e1123206cc6 100644
--- a/gcc/testsuite/gcc.dg/pr41551.c
+++ b/gcc/testsuite/gcc.dg/pr41551.c
@@ -10,3 +10,5 @@ int main(void)
  int var, *p = &var;
  return (double)(uintptr_t)(p);
 }
+
+/* { dg-prune-output "-Wreturn-local-addr" } */
diff --git a/gcc/testsuite/gcc.dg/pr59523.c b/gcc/testsuite/gcc.dg/pr59523.c
index a6c3302a683..49cbe5dd27a 100644
--- a/gcc/testsuite/gcc.dg/pr59523.c
+++ b/gcc/testsuite/gcc.dg/pr59523.c
@@ -16,3 +16,5 @@ foo (int a, int *b, int *c, int *d)
       r[i] = 1;
   return r;
 }
+
+/* { dg-prune-output "-Wreturn-local-addr" } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr88775-2.c b/gcc/testsuite/gcc.dg/tree-ssa/pr88775-2.c
index 292ce6edefc..ed5df826432 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/pr88775-2.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr88775-2.c
@@ -41,3 +41,5 @@ f5 (void)
   int c[64] = {}, d[64] = {};
   return (__UINTPTR_TYPE__) &c[64] != (__UINTPTR_TYPE__) &d[0];
 }
+
+/* { dg-prune-output "-Wreturn-local-addr" } */
diff --git a/gcc/testsuite/gcc.dg/winline-7.c b/gcc/testsuite/gcc.dg/winline-7.c
index 34deca42592..239d748926d 100644
--- a/gcc/testsuite/gcc.dg/winline-7.c
+++ b/gcc/testsuite/gcc.dg/winline-7.c
@@ -13,3 +13,5 @@ inline void *t (void)
 {
 	return q ();		 /* { dg-message "called from here" } */
 }
+
+/* { dg-prune-output "-Wreturn-local-addr" } */