Re: Fix PR50260

[Richard Guenther <richard.guenther@gmail.com>]

On Thu, Sep 1, 2011 at 2:41 PM, Michael Matz <matz@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
>