On Thu, Sep 1, 2011 at 2:41 PM, Michael Matz <m...@suse.de> wrote:
> Hi,
>
> the last change in ipa-split generated a new use of a previously unused
> PARM_DECL. When one does this one has to call add_referenced_var. Not
> doing so can cause segfault when accessing the (not initialized) var
> annotation. So, fixed with the patch.
>
> I took the opportunity to remove all explicit calls to get_var_ann because
> add_referenced_var is doing so as first thing. Reviewing the code showed
> one potentially problematic case in tree-ssa-pre.c where a new variable
> was created without calling add_referenced_var. It's only for
> place-holder SSA names but those can conceivably leak into the program
> stream by being reused during expression generation.
>
> Currently regstrapping on x86_64-linux (without Ada). Okay for trunk?
Ok. Time to make get_var_ann private?
Richard.
>
> Ciao,
> Michael.
> --
> PR middle-end/50260
> * ipa-split.c (split_function): Call add_referenced_var.
>
> * tree-ssa-phiopt.c (cond_store_replacement): Don't call get_var_ann.
> (cond_if_else_store_replacement_1): Ditto.
> * tree-ssa-pre.c (get_representative_for): Ditto.
> (create_expression_by_pieces): Ditto.
> (insert_into_preds_of_block): Ditto.
> * tree-sra.c (create_access_replacement): Ditto.
> (get_replaced_param_substitute): Ditto.
>
> testsuite/
> * gfortran.fortran-torture/compile/pr50260.f90: New test.
>
> Index: ipa-split.c
> ===================================================================
> --- ipa-split.c (revision 178408)
> +++ ipa-split.c (working copy)
> @@ -988,6 +988,9 @@ split_function (struct split_point *spli
> arg = gimple_default_def (cfun, parm);
> if (!arg)
> {
> + /* This parm wasn't used up to now, but is going to be used,
> + hence register it. */
> + add_referenced_var (parm);
> arg = make_ssa_name (parm, gimple_build_nop ());
> set_default_def (parm, arg);
> }
> Index: tree-ssa-phiopt.c
> ===================================================================
> --- tree-ssa-phiopt.c (revision 178408)
> +++ tree-ssa-phiopt.c (working copy)
> @@ -1269,10 +1269,7 @@ cond_store_replacement (basic_block midd
> /* 2) Create a temporary where we can store the old content
> of the memory touched by the store, if we need to. */
> if (!condstoretemp || TREE_TYPE (lhs) != TREE_TYPE (condstoretemp))
> - {
> - condstoretemp = create_tmp_reg (TREE_TYPE (lhs), "cstore");
> - get_var_ann (condstoretemp);
> - }
> + condstoretemp = create_tmp_reg (TREE_TYPE (lhs), "cstore");
> add_referenced_var (condstoretemp);
>
> /* 3) Insert a load from the memory of the store to the temporary
> @@ -1355,10 +1352,7 @@ cond_if_else_store_replacement_1 (basic_
> /* 2) Create a temporary where we can store the old content
> of the memory touched by the store, if we need to. */
> if (!condstoretemp || TREE_TYPE (lhs) != TREE_TYPE (condstoretemp))
> - {
> - condstoretemp = create_tmp_reg (TREE_TYPE (lhs), "cstore");
> - get_var_ann (condstoretemp);
> - }
> + condstoretemp = create_tmp_reg (TREE_TYPE (lhs), "cstore");
> add_referenced_var (condstoretemp);
>
> /* 3) Create a PHI node at the join block, with one argument
> Index: tree-ssa-pre.c
> ===================================================================
> --- tree-ssa-pre.c (revision 178408)
> +++ tree-ssa-pre.c (working copy)
> @@ -1399,7 +1399,7 @@ get_representative_for (const pre_expr e
> if (!pretemp || exprtype != TREE_TYPE (pretemp))
> {
> pretemp = create_tmp_reg (exprtype, "pretmp");
> - get_var_ann (pretemp);
> + add_referenced_var (pretemp);
> }
>
> name = make_ssa_name (pretemp, gimple_build_nop ());
> @@ -3178,10 +3178,7 @@ create_expression_by_pieces (basic_block
> /* Build and insert the assignment of the end result to the temporary
> that we will return. */
> if (!pretemp || exprtype != TREE_TYPE (pretemp))
> - {
> - pretemp = create_tmp_reg (exprtype, "pretmp");
> - get_var_ann (pretemp);
> - }
> + pretemp = create_tmp_reg (exprtype, "pretmp");
>
> temp = pretemp;
> add_referenced_var (temp);
> @@ -3441,10 +3438,7 @@ insert_into_preds_of_block (basic_block
>
> /* Now build a phi for the new variable. */
> if (!prephitemp || TREE_TYPE (prephitemp) != type)
> - {
> - prephitemp = create_tmp_var (type, "prephitmp");
> - get_var_ann (prephitemp);
> - }
> + prephitemp = create_tmp_var (type, "prephitmp");
>
> temp = prephitemp;
> add_referenced_var (temp);
> Index: tree-sra.c
> ===================================================================
> --- tree-sra.c (revision 178408)
> +++ tree-sra.c (working copy)
> @@ -1825,7 +1825,6 @@ create_access_replacement (struct access
> tree repl;
>
> repl = create_tmp_var (access->type, "SR");
> - get_var_ann (repl);
> add_referenced_var (repl);
> if (rename)
> mark_sym_for_renaming (repl);
> @@ -4106,7 +4105,6 @@ get_replaced_param_substitute (struct ip
> DECL_NAME (repl) = get_identifier (pretty_name);
> obstack_free (&name_obstack, pretty_name);
>
> - get_var_ann (repl);
> add_referenced_var (repl);
> adj->new_ssa_base = repl;
> }
> Index: testsuite/gfortran.fortran-torture/compile/pr50260.f90
> ===================================================================
> --- testsuite/gfortran.fortran-torture/compile/pr50260.f90 (revision 0)
> +++ testsuite/gfortran.fortran-torture/compile/pr50260.f90 (revision 0)
> @@ -0,0 +1,48 @@
> +MODULE cp_parser_methods
> + INTEGER, PARAMETER :: default_string_length=80
> + INTEGER, PARAMETER :: default_path_length=250
> + TYPE ilist_type
> + LOGICAL :: in_use
> + END TYPE ilist_type
> + TYPE cp_parser_type
> + CHARACTER(LEN=default_path_length) :: ifn
> + INTEGER :: icol,icol1,icol2
> + TYPE(ilist_type), POINTER :: ilist
> + END TYPE cp_parser_type
> + TYPE cp_error_type
> + END TYPE cp_error_type
> +CONTAINS
> + FUNCTION cts(i) RESULT(res)
> + CHARACTER(len=6) :: res
> + END FUNCTION cts
> + FUNCTION parser_location(parser,error) RESULT(res)
> + TYPE(cp_parser_type), POINTER :: parser
> + TYPE(cp_error_type), INTENT(inout) :: error
> + CHARACTER(len=default_path_length+default_string_length) :: res
> + LOGICAL :: failure
> + IF (.NOT. failure) THEN
> + res="file:'"//TRIM(parser%ifn)//"' line:"//cts(parser%icol)
> + END IF
> + END FUNCTION parser_location
> + SUBROUTINE parser_get_integer(parser,at_end, error)
> + TYPE(cp_parser_type), POINTER :: parser
> + TYPE(cp_error_type), INTENT(inout) :: error
> + LOGICAL :: failure, my_at_end
> + IF (.NOT.failure) THEN
> + IF (.NOT.parser%ilist%in_use) THEN
> + CALL cp_assert("A"// TRIM(parser_location(parser,error)))
> + END IF
> + END IF
> + END SUBROUTINE parser_get_integer
> + SUBROUTINE parser_get_string(parser,at_end,error)
> + TYPE(cp_parser_type), POINTER :: parser
> + LOGICAL, INTENT(out), OPTIONAL :: at_end
> + TYPE(cp_error_type), INTENT(inout) :: error
> + LOGICAL :: failure, my_at_end
> + IF (.NOT.failure) THEN
> + IF (PRESENT(at_end)) THEN
> + CALL cp_assert("s"//TRIM(parser_location(parser,error)))
> + END IF
> + END IF
> + END SUBROUTINE parser_get_string
> +END MODULE cp_parser_methods
>
