hardened conditionals: drop copied identifiers

hardened conditionals: drop copied identifiers

[Alexandre Oliva]

The copies of identifiers, indended to associate hardening SSA
temporaries to the original variables they refer to, end up causing
-fcompare-debug to fail, because DECL_UIDs are not identical, and the
nouid flag used in compare-debug dumps doesn't affect the uids in
naked identifiers, so the divergence becomes apparent.

This patch drops the naked identifiers.  Though somewhat desirable,
they're not necessary.

Regstrapped on x86_64-linux-gnu.  Ok to install?


for  gcc/ChangeLog

	PR debug/104564
	* gimple-harden-conditionals.cc (detach_value): Keep temps
	anonymous.

for  gcc/testsuite/ChangeLog

	PR debug/104564
	* c-c++-common/torture/harden-comp.c: Adjust.
	* c-c++-common/torture/harden-cond.c: Adjust.
---
 gcc/gimple-harden-conditionals.cc                |   11 ++++-------
 gcc/testsuite/c-c++-common/torture/harden-comp.c |    2 +-
 gcc/testsuite/c-c++-common/torture/harden-cond.c |    2 +-
 3 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/gcc/gimple-harden-conditionals.cc b/gcc/gimple-harden-conditionals.cc
index be01f3ea8c44a..c7e5e077a74f6 100644
--- a/gcc/gimple-harden-conditionals.cc
+++ b/gcc/gimple-harden-conditionals.cc
@@ -126,14 +126,11 @@ detach_value (location_t loc, gimple_stmt_iterator *gsip, tree val)
       return 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.  */
+  /* Create a SSA "copy" of VAL.  It would be nice to have it named
+     after the corresponding variable, but sharing the same decl is
+     problematic when VAL is a DECL_BY_REFERENCE RESULT_DECL, and
+     copying just the identifier hits -fcompare-debug failures.  */
   tree ret = make_ssa_name (TREE_TYPE (val));
-  SET_SSA_NAME_VAR_OR_IDENTIFIER (ret, SSA_NAME_IDENTIFIER (val));
 
   /* Some modes won't fit in general regs, so we fall back to memory
      for them.  ??? It would be ideal to try to identify an alternate,
diff --git a/gcc/testsuite/c-c++-common/torture/harden-comp.c b/gcc/testsuite/c-c++-common/torture/harden-comp.c
index 1ee0b3663443d..502f52e25be24 100644
--- a/gcc/testsuite/c-c++-common/torture/harden-comp.c
+++ b/gcc/testsuite/c-c++-common/torture/harden-comp.c
@@ -11,4 +11,4 @@ f (int i, int j)
 /* { dg-final { scan-tree-dump-times "Adding reversed compare" 1 "hardcmp" } } */
 /* { dg-final { scan-tree-dump-times "__builtin_trap" 1 "hardcmp" } } */
 /* { dg-final { scan-tree-dump-times "_\[0-9\]* = i_\[0-9\]*\[(\]D\[)\] < j_\[0-9\]*\[(\]D\[)\];" 1 "hardcmp" } } */
-/* { dg-final { scan-tree-dump-times "_\[0-9\]* = i_\[0-9\]* >= j_\[0-9\]*;" 1 "hardcmp" } } */
+/* { dg-final { scan-tree-dump-times "_\[0-9\]* = _\[0-9\]* >= _\[0-9\]*;" 1 "hardcmp" } } */
diff --git a/gcc/testsuite/c-c++-common/torture/harden-cond.c b/gcc/testsuite/c-c++-common/torture/harden-cond.c
index 86de8e155ed38..213b048b25af5 100644
--- a/gcc/testsuite/c-c++-common/torture/harden-cond.c
+++ b/gcc/testsuite/c-c++-common/torture/harden-cond.c
@@ -15,4 +15,4 @@ f (int i, int j)
 /* { dg-final { scan-tree-dump-times "Adding reversed compare" 2 "hardcbr" } } */
 /* { dg-final { scan-tree-dump-times "__builtin_trap" 2 "hardcbr" } } */
 /* { dg-final { scan-tree-dump-times "if \[(\]i_\[0-9\]*\[(\]D\[)\] < j_\[0-9\]*\[(\]D\[)\]\[)\]" 1 "hardcbr" } } */
-/* { dg-final { scan-tree-dump-times "if \[(\]i_\[0-9\]* >= j_\[0-9\]*\[)\]" 2 "hardcbr" } } */
+/* { dg-final { scan-tree-dump-times "if \[(\]_\[0-9\]* >= _\[0-9\]*\[)\]" 2 "hardcbr" } } */


-- 
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>

Re: hardened conditionals: drop copied identifiers

[Richard Biener]

On Thu, Mar 24, 2022 at 5:06 AM Alexandre Oliva via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
>
> The copies of identifiers, indended to associate hardening SSA
> temporaries to the original variables they refer to, end up causing
> -fcompare-debug to fail, because DECL_UIDs are not identical, and the
> nouid flag used in compare-debug dumps doesn't affect the uids in
> naked identifiers, so the divergence becomes apparent.
>
> This patch drops the naked identifiers.  Though somewhat desirable,
> they're not necessary.
>
> Regstrapped on x86_64-linux-gnu.  Ok to install?

OK.

>
> for  gcc/ChangeLog
>
>         PR debug/104564
>         * gimple-harden-conditionals.cc (detach_value): Keep temps
>         anonymous.
>
> for  gcc/testsuite/ChangeLog
>
>         PR debug/104564
>         * c-c++-common/torture/harden-comp.c: Adjust.
>         * c-c++-common/torture/harden-cond.c: Adjust.
> ---
>  gcc/gimple-harden-conditionals.cc                |   11 ++++-------
>  gcc/testsuite/c-c++-common/torture/harden-comp.c |    2 +-
>  gcc/testsuite/c-c++-common/torture/harden-cond.c |    2 +-
>  3 files changed, 6 insertions(+), 9 deletions(-)
>
> diff --git a/gcc/gimple-harden-conditionals.cc b/gcc/gimple-harden-conditionals.cc
> index be01f3ea8c44a..c7e5e077a74f6 100644
> --- a/gcc/gimple-harden-conditionals.cc
> +++ b/gcc/gimple-harden-conditionals.cc
> @@ -126,14 +126,11 @@ detach_value (location_t loc, gimple_stmt_iterator *gsip, tree val)
>        return 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.  */
> +  /* Create a SSA "copy" of VAL.  It would be nice to have it named
> +     after the corresponding variable, but sharing the same decl is
> +     problematic when VAL is a DECL_BY_REFERENCE RESULT_DECL, and
> +     copying just the identifier hits -fcompare-debug failures.  */
>    tree ret = make_ssa_name (TREE_TYPE (val));
> -  SET_SSA_NAME_VAR_OR_IDENTIFIER (ret, SSA_NAME_IDENTIFIER (val));
>
>    /* Some modes won't fit in general regs, so we fall back to memory
>       for them.  ??? It would be ideal to try to identify an alternate,
> diff --git a/gcc/testsuite/c-c++-common/torture/harden-comp.c b/gcc/testsuite/c-c++-common/torture/harden-comp.c
> index 1ee0b3663443d..502f52e25be24 100644
> --- a/gcc/testsuite/c-c++-common/torture/harden-comp.c
> +++ b/gcc/testsuite/c-c++-common/torture/harden-comp.c
> @@ -11,4 +11,4 @@ f (int i, int j)
>  /* { dg-final { scan-tree-dump-times "Adding reversed compare" 1 "hardcmp" } } */
>  /* { dg-final { scan-tree-dump-times "__builtin_trap" 1 "hardcmp" } } */
>  /* { dg-final { scan-tree-dump-times "_\[0-9\]* = i_\[0-9\]*\[(\]D\[)\] < j_\[0-9\]*\[(\]D\[)\];" 1 "hardcmp" } } */
> -/* { dg-final { scan-tree-dump-times "_\[0-9\]* = i_\[0-9\]* >= j_\[0-9\]*;" 1 "hardcmp" } } */
> +/* { dg-final { scan-tree-dump-times "_\[0-9\]* = _\[0-9\]* >= _\[0-9\]*;" 1 "hardcmp" } } */
> diff --git a/gcc/testsuite/c-c++-common/torture/harden-cond.c b/gcc/testsuite/c-c++-common/torture/harden-cond.c
> index 86de8e155ed38..213b048b25af5 100644
> --- a/gcc/testsuite/c-c++-common/torture/harden-cond.c
> +++ b/gcc/testsuite/c-c++-common/torture/harden-cond.c
> @@ -15,4 +15,4 @@ f (int i, int j)
>  /* { dg-final { scan-tree-dump-times "Adding reversed compare" 2 "hardcbr" } } */
>  /* { dg-final { scan-tree-dump-times "__builtin_trap" 2 "hardcbr" } } */
>  /* { dg-final { scan-tree-dump-times "if \[(\]i_\[0-9\]*\[(\]D\[)\] < j_\[0-9\]*\[(\]D\[)\]\[)\]" 1 "hardcbr" } } */
> -/* { dg-final { scan-tree-dump-times "if \[(\]i_\[0-9\]* >= j_\[0-9\]*\[)\]" 2 "hardcbr" } } */
> +/* { dg-final { scan-tree-dump-times "if \[(\]_\[0-9\]* >= _\[0-9\]*\[)\]" 2 "hardcbr" } } */
>
>
> --
> 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>