On 5/22/19 3:34 PM, Martin Sebor wrote:
> -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.
>
> gcc-71924.diff
>
> 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;
> +}
Is there some reason you didn't query the alias oracle here? It would
seem a fairly natural fit. Ultimately given a pointer (which will be an
SSA_NAME) you want to ask whether or not it conclusively points into the
stack.
That would seem to dramatically simplify is_addr_local.
The rest looks pretty reasonable.
Jeff
