[PR102988] harden cond: detach without decls

[Richard Biener]

On Fri, Nov 19, 2021 at 2:38 PM Alexandre Oliva via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
>
> When we create copies of SSA_NAMEs to hold "detached" copies of the
> values for the hardening tests, we end up with assignments to
> SSA_NAMEs that refer to the same decls.  That would be generally
> desirable, since it enables the variable to be recognized in dumps,
> and makes coalescing more likely if the original variable dies at that
> point.  When the decl is a DECL_BY_REFERENCE, the SSA_NAME holds the
> address of a parm or result, and it's read-only, so we shouldn't
> create assignments to it.  Gimple checkers flag at least the case of
> results.
>
> This patch arranges for us to avoid referencing the same decls, which
> cures the problem, but retaining the visible association between the
> SSA_NAMEs, by using the same identifier for the copy.
>
> Regstrapped on x86_64-linux-gnu, also bootstrapped with both
> -fharden-compares and -fharden-conditional-branches enabled by default.
> Ok to install?

OK.

>
> for  gcc/ChangeLog
>
>         PR tree-optimization/102988
>         * gimple-harden-conditionals.cc (detach_value): Copy SSA_NAME
>         without decl sharing.
>
> for  gcc/testsuite/ChangeLog
>
>         PR tree-optimization/102988
>         * g++.dg/pr102988.C: New.
> ---
>  gcc/gimple-harden-conditionals.cc |    9 ++++++++-
>  gcc/testsuite/g++.dg/pr102988.C   |   17 +++++++++++++++++
>  2 files changed, 25 insertions(+), 1 deletion(-)
>  create mode 100644 gcc/testsuite/g++.dg/pr102988.C
>
> diff --git a/gcc/gimple-harden-conditionals.cc b/gcc/gimple-harden-conditionals.cc
> index 8916420d7dfe9..cfa2361d65be0 100644
> --- a/gcc/gimple-harden-conditionals.cc
> +++ b/gcc/gimple-harden-conditionals.cc
> @@ -123,7 +123,14 @@ detach_value (location_t loc, gimple_stmt_iterator *gsip, tree val)
>        return val;
>      }
>
> -  tree ret = copy_ssa_name (val);
> +  /* Create a SSA "copy" of VAL.  This could be an anonymous
> +     temporary, but it's nice to have it named after the corresponding
> +     variable.  Alas, when VAL is a DECL_BY_REFERENCE RESULT_DECL,
> +     setting (a copy of) it would be flagged by checking, so we don't
> +     use copy_ssa_name: we create an anonymous SSA name, and then give
> +     it the same identifier (rather than decl) as VAL.  */
> +  tree ret = make_ssa_name (TREE_TYPE (val));
> +  SET_SSA_NAME_VAR_OR_IDENTIFIER (ret, SSA_NAME_IDENTIFIER (val));
>
>    /* Output asm ("" : "=g" (ret) : "0" (val));  */
>    vec<tree, va_gc> *inputs = NULL;
> diff --git a/gcc/testsuite/g++.dg/pr102988.C b/gcc/testsuite/g++.dg/pr102988.C
> new file mode 100644
> index 0000000000000..05a1a8f67ed9b
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/pr102988.C
> @@ -0,0 +1,17 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O -fharden-conditional-branches -fchecking=1" } */
> +
> +/* DECL_BY_REFERENCE RESULT_DECL is read-only, we can't create a copy
> +   of its (address) default def and set it.  */
> +
> +void ll();
> +struct k {
> +  ~k();
> +};
> +k ice(k *a)
> +{
> +  k v;
> +  if (&v!= a)
> +    ll();
> +  return v;
> +}
>
>
> --
> Alexandre Oliva, happy hacker                https://FSFLA.org/blogs/lxo/
>    Free Software Activist                       GNU Toolchain Engineer
> Disinformation flourishes because many people care deeply about injustice
> but very few check the facts.  Ask me about <https://stallmansupport.org>