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>
