Hello,
we have front-end warnings about returning the address of a local
variable. However, quite often in C++, people don't directly return the
address of a temporary, it goes through a few functions which hide that
fact. After some inlining, the fact that we are returning the address of a
local variable can become obvious to the compiler, to the point where I
have used, for debugging purposes, grep 'return &' on the optimized dump
produced with -O3 -fkeep-inline-functions (I then had to sort through the
global/local variables).
fold_stmt looks like a good place for this, but it could go almost
anywhere. It has to happen after enough inlining / copy propagation to
make it useful though.
One hard part is avoiding duplicate warnings. Replacing the address with 0
is a convenient way to do that, so I did it both for my new warning and
for the existing C/C++ ones. The patch breaks
gfortran.dg/warn_target_lifetime_2.f90 because it ends up warning twice. I
didn't touch that front-end because I don't know fortran, and the warning
message "Pointer at .1. in pointer assignment might outlive the pointer
target" doesn't seem very confident that the thing really is broken enough
to be replaced by 0. I only tested (bootstrap+regression) the default
languages, so ada/go may have a similar issue, to be handled if the
approach seems ok. (I personally wouldn't care about duplicate warnings,
but I know some people can't help complaining about it)
This doesn't actually fix PR 60517, for that I was thinking of checking
for memory reads if the first stop of walk_aliased_vdefs is a clobber
(could also check __builtin_free), though I don't know in which pass to do
that yet.
2014-04-05 Marc Glisse <marc.gli...@inria.fr>
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 (check_return_expr): Likewise.
(maybe_warn_about_returning_address_of_local): Tell the caller if
we warned.
gcc/
* gimple-fold.c (fold_stmt_1): Warn if returning the address of a
local variable.
gcc/testsuite/
* c-c++-common/addrtmp.c: New testcase.
* c-c++-common/uninit-G.c: Adjust.
--
Marc GlisseIndex: gcc/c/c-typeck.c
===================================================================
--- gcc/c/c-typeck.c (revision 209157)
+++ gcc/c/c-typeck.c (working copy)
@@ -9254,23 +9254,25 @@ c_finish_return (location_t loc, tree re
inner = TREE_OPERAND (inner, 0);
while (REFERENCE_CLASS_P (inner)
&& TREE_CODE (inner) != INDIRECT_REF)
inner = TREE_OPERAND (inner, 0);
if (DECL_P (inner)
&& !DECL_EXTERNAL (inner)
&& !TREE_STATIC (inner)
&& DECL_CONTEXT (inner) == current_function_decl)
- 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");
+ t = build_zero_cst (TREE_TYPE (res));
+ }
break;
default:
break;
}
break;
}
retval = build2 (MODIFY_EXPR, TREE_TYPE (res), res, t);
Index: gcc/cp/typeck.c
===================================================================
--- gcc/cp/typeck.c (revision 209157)
+++ gcc/cp/typeck.c (working copy)
@@ -49,21 +49,21 @@ static tree convert_for_assignment (tree
static tree cp_pointer_int_sum (enum tree_code, tree, tree, tsubst_flags_t);
static tree rationalize_conditional_expr (enum tree_code, tree,
tsubst_flags_t);
static int comp_ptr_ttypes_real (tree, tree, int);
static bool comp_except_types (tree, tree, bool);
static bool comp_array_types (const_tree, const_tree, bool);
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,
tsubst_flags_t);
/* Do `exp = require_complete_type (exp);' to make sure exp
does not have an incomplete type. (That includes void types.)
Returns error_mark_node if the VALUE does not have
complete type when this function returns. */
@@ -8253,79 +8253,81 @@ convert_for_initialization (tree exp, tr
return rhs;
if (MAYBE_CLASS_TYPE_P (type))
return perform_implicit_conversion_flags (type, rhs, complain, flags);
return convert_for_assignment (type, rhs, errtype, fndecl, parmnum,
complain, flags);
}
/* 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));
tree whats_returned = retval;
for (;;)
{
if (TREE_CODE (whats_returned) == COMPOUND_EXPR)
whats_returned = TREE_OPERAND (whats_returned, 1);
else if (CONVERT_EXPR_P (whats_returned)
|| TREE_CODE (whats_returned) == NON_LVALUE_EXPR)
whats_returned = TREE_OPERAND (whats_returned, 0);
else
break;
}
if (TREE_CODE (whats_returned) != ADDR_EXPR)
- return;
+ return false;
whats_returned = TREE_OPERAND (whats_returned, 0);
while (TREE_CODE (whats_returned) == COMPONENT_REF
|| TREE_CODE (whats_returned) == ARRAY_REF)
whats_returned = TREE_OPERAND (whats_returned, 0);
if (TREE_CODE (valtype) == REFERENCE_TYPE)
{
if (TREE_CODE (whats_returned) == AGGR_INIT_EXPR
|| 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;
}
}
if (DECL_P (whats_returned)
&& DECL_NAME (whats_returned)
&& DECL_FUNCTION_SCOPE_P (whats_returned)
&& !is_capture_proxy (whats_returned)
&& !(TREE_STATIC (whats_returned)
|| TREE_PUBLIC (whats_returned)))
{
if (TREE_CODE (valtype) == REFERENCE_TYPE)
warning (OPT_Wreturn_local_addr, "reference to local variable %q+D
returned",
whats_returned);
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.
Return an expression explicitly showing all conversions required to
change RETVAL into the function return type, and to assign it to
the DECL_RESULT for the function. Set *NO_WARNING to true if
code reaches end of non-void function warning shouldn't be issued
on this RETURN_EXPR. */
tree
@@ -8606,22 +8608,22 @@ check_return_expr (tree retval, bool *no
/* If the conversion failed, treat this just like `return;'. */
if (retval == error_mark_node)
return retval;
/* We can't initialize a register from a AGGR_INIT_EXPR. */
else if (! cfun->returns_struct
&& TREE_CODE (retval) == TARGET_EXPR
&& 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 = build_zero_cst (TREE_TYPE (retval));
}
/* Actually copy the value returned into the appropriate location. */
if (retval && retval != result)
retval = build2 (INIT_EXPR, TREE_TYPE (result), result, retval);
return retval;
}
Index: gcc/gimple-fold.c
===================================================================
--- gcc/gimple-fold.c (revision 209157)
+++ gcc/gimple-fold.c (working copy)
@@ -45,20 +45,21 @@ along with GCC; see the file COPYING3.
#include "tree-into-ssa.h"
#include "tree-dfa.h"
#include "tree-ssa.h"
#include "tree-ssa-propagate.h"
#include "target.h"
#include "ipa-utils.h"
#include "gimple-pretty-print.h"
#include "tree-ssa-address.h"
#include "langhooks.h"
#include "gimplify-me.h"
+#include "diagnostic-core.h"
/* Return true when DECL can be referenced from current unit.
FROM_DECL (if non-null) specify constructor of variable DECL was taken from.
We can get declarations that are not possible to reference for various
reasons:
1) When analyzing C++ virtual tables.
C++ virtual tables do have known constructors even
when they are keyed to other compilation unit.
Those tables can contain pointers to methods and vars
@@ -1366,20 +1367,39 @@ fold_stmt_1 (gimple_stmt_iterator *gsi,
if (tem)
{
tem = build_fold_addr_expr_with_type (tem, TREE_TYPE (val));
gimple_debug_bind_set_value (stmt, tem);
changed = true;
}
}
}
break;
+ case 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))
+ {
+ if (warning_at (gimple_location (stmt), OPT_Wreturn_local_addr,
+ "function returns address of local variable"))
+ inform (DECL_SOURCE_LOCATION(valbase), "declared here");
+ tree zero = build_zero_cst (TREE_TYPE (val));
+ gimple_return_set_retval (stmt, zero);
+ changed = true;
+ }
+ }
+ }
+ break;
default:;
}
stmt = gsi_stmt (*gsi);
/* Fold *& on the lhs. */
if (gimple_has_lhs (stmt))
{
tree lhs = gimple_get_lhs (stmt);
if (lhs && REFERENCE_CLASS_P (lhs))
Index: gcc/testsuite/c-c++-common/addrtmp.c
===================================================================
--- gcc/testsuite/c-c++-common/addrtmp.c (revision 0)
+++ gcc/testsuite/c-c++-common/addrtmp.c (working copy)
@@ -0,0 +1,13 @@
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+typedef struct A { int a,b; } A;
+int*g(int*x){return x;}
+int*f(){
+ A x[2]={{1,2},{3,4}};
+ return g(&x[1].a); // { dg-warning "address of local variable" }
+}
+A y[2]={{1,2},{3,4}};
+int*h(){
+ return g(&y[1].a);
+}
Property changes on: gcc/testsuite/c-c++-common/addrtmp.c
___________________________________________________________________
Added: svn:keywords
## -0,0 +1 ##
+Author Date Id Revision URL
\ No newline at end of property
Added: svn:eol-style
## -0,0 +1 ##
+native
\ No newline at end of property
Index: gcc/testsuite/c-c++-common/uninit-G.c
===================================================================
--- gcc/testsuite/c-c++-common/uninit-G.c (revision 209157)
+++ gcc/testsuite/c-c++-common/uninit-G.c (working copy)
@@ -1,9 +1,10 @@
/* Test we do not warn about initializing variable with address of self in the
initialization. */
/* { dg-do compile } */
/* { dg-options "-O -Wuninitialized" } */
-void *f()
+void g(void*);
+void f()
{
void *i = &i;
- return i;
+ g(i);
}
