2014-07-31 Marc Glisse <marc.glisse@inria.fr>
authorglisse <glisse@138bc75d-0d04-0410-961f-82ee72b054a4>
Thu, 31 Jul 2014 09:33:58 +0000 (09:33 +0000)
committerglisse <glisse@138bc75d-0d04-0410-961f-82ee72b054a4>
Thu, 31 Jul 2014 09:33:58 +0000 (09:33 +0000)
PR c++/60517
gcc/c/
* c-typeck.c (c_finish_return): Return 0 instead of the address of
a local variable.
gcc/cp/
* typeck.c (maybe_warn_about_returning_address_of_local): Return
whether it is returning the address of a local variable.
(check_return_expr): Return 0 instead of the address of a local
variable.
gcc/c-family/
* c.opt (-Wreturn-local-addr): Move to common.opt.
gcc/
* common.opt (-Wreturn-local-addr): Moved from c.opt.
* gimple-ssa-isolate-paths.c: Include diagnostic-core.h and intl.h.
(isolate_path): New argument to avoid inserting a trap.
(find_implicit_erroneous_behaviour): Handle returning the address
of a local variable.
(find_explicit_erroneous_behaviour): Likewise.
gcc/testsuite/
* c-c++-common/addrtmp.c: New file.
* c-c++-common/uninit-G.c: Adapt.

git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@213323 138bc75d-0d04-0410-961f-82ee72b054a4

12 files changed:
gcc/ChangeLog
gcc/c-family/ChangeLog
gcc/c-family/c.opt
gcc/c/ChangeLog
gcc/c/c-typeck.c
gcc/common.opt
gcc/cp/ChangeLog
gcc/cp/typeck.c
gcc/gimple-ssa-isolate-paths.c
gcc/testsuite/ChangeLog
gcc/testsuite/c-c++-common/addrtmp.c [new file with mode: 0644]
gcc/testsuite/c-c++-common/uninit-G.c

index 0ee4580..64d2fa3 100644 (file)
@@ -1,3 +1,13 @@
+2014-07-31  Marc Glisse  <marc.glisse@inria.fr>
+
+       PR c++/60517
+       * common.opt (-Wreturn-local-addr): Moved from c.opt.
+       * gimple-ssa-isolate-paths.c: Include diagnostic-core.h and intl.h.
+       (isolate_path): New argument to avoid inserting a trap.
+       (find_implicit_erroneous_behaviour): Handle returning the address
+       of a local variable.
+       (find_explicit_erroneous_behaviour): Likewise.
+
 2014-07-31  Bingfeng Mei <bmei@broadcom.com>
 
        PR lto/61868
index 645b940..bf1ad5b 100644 (file)
@@ -1,3 +1,8 @@
+2014-07-31  Marc Glisse  <marc.glisse@inria.fr>
+
+       PR c++/60517
+       * c.opt (-Wreturn-local-addr): Move to common.opt.
+
 2014-07-30  Jason Merrill  <jason@redhat.com>
 
        PR c++/61659
index f427da1..c3be622 100644 (file)
@@ -709,10 +709,6 @@ Wreorder
 C++ ObjC++ Var(warn_reorder) Warning LangEnabledBy(C++ ObjC++,Wall)
 Warn when the compiler reorders code
 
-Wreturn-local-addr
-C ObjC C++ ObjC++ Var(warn_return_local_addr) Init(1) Warning
-Warn about returning a pointer/reference to a local or temporary variable.
-
 Wreturn-type
 C ObjC C++ ObjC++ Var(warn_return_type) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall)
 Warn whenever a function's return type defaults to \"int\" (C), or about inconsistent return types (C++)
index b5e7a17..aec8cf1 100644 (file)
@@ -1,3 +1,9 @@
+2014-07-31  Marc Glisse  <marc.glisse@inria.fr>
+
+       PR c++/60517
+       * c-typeck.c (c_finish_return): Return 0 instead of the address of
+       a local variable.
+
 2014-07-30  Tom Tromey  <tromey@redhat.com>
 
        * c-typeck.c (struct constructor_stack) <designator_depth>: New
index a938933..fe9440c 100644 (file)
@@ -9350,8 +9350,12 @@ c_finish_return (location_t loc, tree retval, tree origtype)
                    warning_at (loc, OPT_Wreturn_local_addr,
                                "function returns address of label");
                  else
-                   warning_at (loc, OPT_Wreturn_local_addr,
-                               "function returns address of local variable");
+                   {
+                     warning_at (loc, OPT_Wreturn_local_addr,
+                                 "function returns address of local variable");
+                     tree zero = build_zero_cst (TREE_TYPE (res));
+                     t = build2 (COMPOUND_EXPR, TREE_TYPE (res), t, zero);
+                   }
                }
              break;
 
index 927e0ed..40c8b3c 100644 (file)
@@ -607,6 +607,10 @@ Wpedantic
 Common Var(pedantic) Warning
 Issue warnings needed for strict compliance to the standard
 
+Wreturn-local-addr
+Common Var(warn_return_local_addr) Init(1) Warning
+Warn about returning a pointer/reference to a local or temporary variable.
+
 Wshadow
 Common Var(warn_shadow) Warning
 Warn when one local variable shadows another
index 3d85843..36d8b39 100644 (file)
@@ -1,3 +1,11 @@
+2014-07-31  Marc Glisse  <marc.glisse@inria.fr>
+
+       PR c++/60517
+       * typeck.c (maybe_warn_about_returning_address_of_local): Return
+       whether it is returning the address of a local variable.
+       (check_return_expr): Return 0 instead of the address of a local
+       variable.
+
 2014-07-30  Jason Merrill  <jason@redhat.com>
 
        PR lto/53808
index a1ca937..735cd58 100644 (file)
@@ -56,7 +56,7 @@ static tree pointer_diff (tree, tree, tree, tsubst_flags_t);
 static tree get_delta_difference (tree, tree, bool, bool, tsubst_flags_t);
 static void casts_away_constness_r (tree *, tree *, tsubst_flags_t);
 static bool casts_away_constness (tree, tree, tsubst_flags_t);
-static void maybe_warn_about_returning_address_of_local (tree);
+static bool maybe_warn_about_returning_address_of_local (tree);
 static tree lookup_destructor (tree, tree, tree, tsubst_flags_t);
 static void warn_args_num (location_t, tree, bool);
 static int convert_arguments (tree, vec<tree, va_gc> **, tree, int,
@@ -8296,9 +8296,9 @@ convert_for_initialization (tree exp, tree type, tree rhs, int flags,
 }
 \f
 /* If RETVAL is the address of, or a reference to, a local variable or
-   temporary give an appropriate warning.  */
+   temporary give an appropriate warning and return true.  */
 
-static void
+static bool
 maybe_warn_about_returning_address_of_local (tree retval)
 {
   tree valtype = TREE_TYPE (DECL_RESULT (current_function_decl));
@@ -8316,7 +8316,7 @@ maybe_warn_about_returning_address_of_local (tree retval)
     }
 
   if (TREE_CODE (whats_returned) != ADDR_EXPR)
-    return;
+    return false;
   whats_returned = TREE_OPERAND (whats_returned, 0);
 
   while (TREE_CODE (whats_returned) == COMPONENT_REF
@@ -8329,14 +8329,14 @@ maybe_warn_about_returning_address_of_local (tree retval)
          || TREE_CODE (whats_returned) == TARGET_EXPR)
        {
          warning (OPT_Wreturn_local_addr, "returning reference to temporary");
-         return;
+         return true;
        }
       if (VAR_P (whats_returned)
          && DECL_NAME (whats_returned)
          && TEMP_NAME_P (DECL_NAME (whats_returned)))
        {
          warning (OPT_Wreturn_local_addr, "reference to non-lvalue returned");
-         return;
+         return true;
        }
     }
 
@@ -8356,8 +8356,10 @@ maybe_warn_about_returning_address_of_local (tree retval)
       else
        warning (OPT_Wreturn_local_addr, "address of local variable %q+D "
                 "returned", whats_returned);
-      return;
+      return true;
     }
+
+  return false;
 }
 
 /* Check that returning RETVAL from the current function is valid.
@@ -8650,8 +8652,9 @@ check_return_expr (tree retval, bool *no_warning)
               && TREE_CODE (TREE_OPERAND (retval, 1)) == AGGR_INIT_EXPR)
        retval = build2 (COMPOUND_EXPR, TREE_TYPE (retval), retval,
                         TREE_OPERAND (retval, 0));
-      else
-       maybe_warn_about_returning_address_of_local (retval);
+      else if (maybe_warn_about_returning_address_of_local (retval))
+       retval = build2 (COMPOUND_EXPR, TREE_TYPE (retval), retval,
+                        build_zero_cst (TREE_TYPE (retval)));
     }
 
   /* Actually copy the value returned into the appropriate location.  */
index 897b180..0b62915 100644 (file)
@@ -42,6 +42,8 @@ along with GCC; see the file COPYING3.  If not see
 #include "cfgloop.h"
 #include "tree-pass.h"
 #include "tree-cfg.h"
+#include "diagnostic-core.h"
+#include "intl.h"
 
 
 static bool cfg_altered;
@@ -132,13 +134,15 @@ insert_trap_and_remove_trailing_statements (gimple_stmt_iterator *si_p, tree op)
    Optimization is simple as well.  Replace STMT in BB' with an
    unconditional trap and remove all outgoing edges from BB'.
 
+   If RET_ZERO, do not trap, only return NULL.
+
    DUPLICATE is a pre-existing duplicate, use it as BB' if it exists.
 
    Return BB'.  */
 
 basic_block
 isolate_path (basic_block bb, basic_block duplicate,
-             edge e, gimple stmt, tree op)
+             edge e, gimple stmt, tree op, bool ret_zero)
 {
   gimple_stmt_iterator si, si2;
   edge_iterator ei;
@@ -151,8 +155,9 @@ isolate_path (basic_block bb, basic_block duplicate,
   if (!duplicate)
     {
       duplicate = duplicate_block (bb, NULL, NULL);
-      for (ei = ei_start (duplicate->succs); (e2 = ei_safe_edge (ei)); )
-       remove_edge (e2);
+      if (!ret_zero)
+       for (ei = ei_start (duplicate->succs); (e2 = ei_safe_edge (ei)); )
+         remove_edge (e2);
     }
 
   /* Complete the isolation step by redirecting E to reach DUPLICATE.  */
@@ -197,7 +202,17 @@ isolate_path (basic_block bb, basic_block duplicate,
      SI2 points to the duplicate of STMT in DUPLICATE.  Insert a trap
      before SI2 and remove SI2 and all trailing statements.  */
   if (!gsi_end_p (si2))
-    insert_trap_and_remove_trailing_statements (&si2, op);
+    {
+      if (ret_zero)
+       {
+         gimple ret = gsi_stmt (si2);
+         tree zero = build_zero_cst (TREE_TYPE (gimple_return_retval (ret)));
+         gimple_return_set_retval (ret, zero);
+         update_stmt (ret);
+       }
+      else
+       insert_trap_and_remove_trailing_statements (&si2, op);
+    }
 
   return duplicate;
 }
@@ -255,16 +270,49 @@ find_implicit_erroneous_behaviour (void)
               i = next_i)
            {
              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 ((TREE_CODE (valbase) == VAR_DECL
+                      && !is_global_var (valbase))
+                     || TREE_CODE (valbase) == PARM_DECL)
+                   {
+                     FOR_EACH_IMM_USE_STMT (use_stmt, iter, lhs)
+                       {
+                         if (gimple_code (use_stmt) != GIMPLE_RETURN
+                             || gimple_return_retval (use_stmt) != lhs)
+                           continue;
+
+                         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 (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;
+                           }
+                       }
+                   }
+               }
+
              if (!integer_zerop (op))
                continue;
 
-             edge e = gimple_phi_arg_edge (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)
@@ -280,8 +328,8 @@ find_implicit_erroneous_behaviour (void)
                                           flag_isolate_erroneous_paths_attribute))
 
                    {
-                     duplicate = isolate_path (bb, duplicate,
-                                               e, use_stmt, lhs);
+                     duplicate = isolate_path (bb, duplicate, e,
+                                               use_stmt, lhs, false);
 
                      /* When we remove an incoming edge, we need to
                         reprocess the Ith element.  */
@@ -347,9 +395,45 @@ find_explicit_erroneous_behaviour (void)
              cfg_altered = true;
              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.  */
+         if (gimple_code (stmt) == GIMPLE_RETURN)
+           {
+             tree val = gimple_return_retval (stmt);
+             if (val && TREE_CODE (val) == ADDR_EXPR)
+               {
+                 tree valbase = get_base_address (TREE_OPERAND (val, 0));
+                 if ((TREE_CODE (valbase) == VAR_DECL
+                      && !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");
+
+                     if (warning_at (gimple_location (stmt),
+                                     OPT_Wreturn_local_addr, msg))
+                       inform (DECL_SOURCE_LOCATION(valbase), "declared here");
+                     tree zero = build_zero_cst (TREE_TYPE (val));
+                     gimple_return_set_retval (stmt, zero);
+                     update_stmt (stmt);
+                   }
+               }
+           }
        }
     }
 }
+
 /* Search the function for statements which, if executed, would cause
    the program to fault such as a dereference of a NULL pointer.
 
index 82ec9ef..c0e5585 100644 (file)
@@ -1,3 +1,9 @@
+2014-07-31  Marc Glisse  <marc.glisse@inria.fr>
+
+       PR c++/60517
+       * c-c++-common/addrtmp.c: New file.
+       * c-c++-common/uninit-G.c: Adapt.
+
 2014-07-31  Bingfeng Mei  <bmei@broadcom.com>
 
        PR lto/61868
diff --git a/gcc/testsuite/c-c++-common/addrtmp.c b/gcc/testsuite/c-c++-common/addrtmp.c
new file mode 100644 (file)
index 0000000..3b8140c
--- /dev/null
@@ -0,0 +1,29 @@
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+typedef struct A { int a,b; } A;
+int*g(int*x){return x;}
+int*f1(){
+  A x[2]={{1,2},{3,4}};
+  return g(&x[1].a); // { dg-warning "returns address of local variable" }
+}
+int*f2(int n){
+  A x[2]={{1,2},{3,4}};
+  return n?0:g(&x[1].a); // { dg-warning "may return address of local variable" }
+}
+A y[2]={{1,2},{3,4}};
+int*h(){
+  return g(&y[1].a);
+}
+int*j(int n){
+  A x[2]={{1,2},{3,4}};
+  int*p=g(&y[1].a);
+  if(n==1)p=g(&x[1].a);
+  if(n==2)p=g(&x[0].b);
+  return p; // { dg-warning "may return address of local variable" }
+}
+int*s()
+{
+  static int i;
+  return &i;
+}
index 08f5f53..1a06f06 100644 (file)
@@ -2,8 +2,9 @@
 /* { dg-do compile } */
 /* { dg-options "-O -Wuninitialized" } */
 
-void *f()
+void g(void*);
+void f()
 {
   void *i = &i;
-  return i;
+  g(i);
 }