[PATCH] hardcfr: make builtin_return tests more portable [PR112334]

[PATCH] hardcfr: make builtin_return tests more portable [PR112334]

[Alexandre Oliva]

Rework __builtin_return tests to explicitly call __builtin_apply and
use its return value rather than anything else.  Also require
untyped_assembly.  Avoid the noise out of exceptions escaping the
builtin-applied function, but add a test to cover their effects as
well.

Regstrapped on x86_64-linux-gnu.  Also tested on arm-eabi, but it's
*not* enough (or needed) to fix the PR, there's another bug lurking
there, with a separate patch coming up.  Regardless, is this ok to
install?


for  gcc/testsuite/ChangeLog

	PR target/112334
	* c-c++-common/torture/harden-cfr-bret.c: Rework for stricter
	untyped_return requirements.  Require untyped_assembly.
	* c-c++-common/torture/harden-cfr-bret-except.c: New.
	* c-c++-common/torture/harden-cfr-bret-always.c: Require
	untyped_assembly.
	* c-c++-common/torture/harden-cfr-bret-never.c: Likewise.
	* c-c++-common/torture/harden-cfr-bret-noopt.c: Likewise.
	* c-c++-common/torture/harden-cfr-bret-noret.c: Likewise.
	* c-c++-common/torture/harden-cfr-bret-no-xthrow.c: Likewise.
	* c-c++-common/torture/harden-cfr-bret-nothrow.c: Likewise.
	* c-c++-common/torture/harden-cfr-bret-retcl.c: Likewise.
---
 .../c-c++-common/torture/harden-cfr-bret-always.c  |    3 ++-
 .../c-c++-common/torture/harden-cfr-bret-except.c  |   17 +++++++++++++++
 .../c-c++-common/torture/harden-cfr-bret-never.c   |    3 ++-
 .../torture/harden-cfr-bret-no-xthrow.c            |    3 ++-
 .../c-c++-common/torture/harden-cfr-bret-noopt.c   |    3 ++-
 .../c-c++-common/torture/harden-cfr-bret-noret.c   |    3 ++-
 .../c-c++-common/torture/harden-cfr-bret-nothrow.c |    3 ++-
 .../c-c++-common/torture/harden-cfr-bret-retcl.c   |    3 ++-
 .../c-c++-common/torture/harden-cfr-bret.c         |   23 ++++++++++++++++----
 .../c-c++-common/torture/harden-cfr-noret.c        |    2 +-
 10 files changed, 50 insertions(+), 13 deletions(-)
 create mode 100644 gcc/testsuite/c-c++-common/torture/harden-cfr-bret-except.c

diff --git a/gcc/testsuite/c-c++-common/torture/harden-cfr-bret-always.c b/gcc/testsuite/c-c++-common/torture/harden-cfr-bret-always.c
index 779896c60e846..3406c4e6ef9dc 100644
--- a/gcc/testsuite/c-c++-common/torture/harden-cfr-bret-always.c
+++ b/gcc/testsuite/c-c++-common/torture/harden-cfr-bret-always.c
@@ -1,5 +1,6 @@
 /* { dg-do compile } */
-/* { dg-options "-fharden-control-flow-redundancy -fhardcfr-check-noreturn-calls=always -fdump-tree-hardcfr -ffat-lto-objects" } */
+/* { dg-options "-fharden-control-flow-redundancy -fhardcfr-check-noreturn-calls=always -fno-exceptions -fdump-tree-hardcfr -ffat-lto-objects" } */
+/* { dg-require-effective-target untyped_assembly } */
 
 /* Check that, even enabling all checks before noreturn calls (leaving
    returning calls enabled), we get checks before __builtin_return without
diff --git a/gcc/testsuite/c-c++-common/torture/harden-cfr-bret-except.c b/gcc/testsuite/c-c++-common/torture/harden-cfr-bret-except.c
new file mode 100644
index 0000000000000..3acb61cb75a77
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/torture/harden-cfr-bret-except.c
@@ -0,0 +1,17 @@
+/* { dg-do compile } */
+/* { dg-options "-fharden-control-flow-redundancy -fexceptions -fdump-tree-hardcfr -ffat-lto-objects" } */
+/* { dg-require-effective-target untyped_assembly } */
+
+/* Check that, with exceptions enabled, even in C, the calls initiated by
+   builtin_apply are enclosed in cleanup handlers that add extra checks.
+   Unfortunately, declaring foobar as nothrow is not enough to avoid the
+   handler around the builtin_apply call, so the other bret tests all use
+   -fno-exceptions.  */
+
+#include "harden-cfr-bret.c"
+
+/* With exceptions, we get an extra check per function, to check before
+   propagating exceptions, so it's 3 in f and 2 in g.  */
+/* { dg-final { scan-tree-dump-times "__hardcfr_check" 5 "hardcfr" } } */
+/* The extra check in g also removes the possibility of inlining the check.  */
+/* { dg-final { scan-tree-dump-times "__builtin_trap" 0 "hardcfr" } } */
diff --git a/gcc/testsuite/c-c++-common/torture/harden-cfr-bret-never.c b/gcc/testsuite/c-c++-common/torture/harden-cfr-bret-never.c
index 49ce17f5b937c..7f8fb64138df1 100644
--- a/gcc/testsuite/c-c++-common/torture/harden-cfr-bret-never.c
+++ b/gcc/testsuite/c-c++-common/torture/harden-cfr-bret-never.c
@@ -1,5 +1,6 @@
 /* { dg-do compile } */
-/* { dg-options "-fharden-control-flow-redundancy -fhardcfr-check-noreturn-calls=never -fdump-tree-hardcfr -ffat-lto-objects" } */
+/* { dg-options "-fharden-control-flow-redundancy -fhardcfr-check-noreturn-calls=never -fno-exceptions -fdump-tree-hardcfr -ffat-lto-objects" } */
+/* { dg-require-effective-target untyped_assembly } */
 
 /* Check that, even enabling checks before never noreturn calls (leaving
    returning calls enabled), we get checks before __builtin_return without
diff --git a/gcc/testsuite/c-c++-common/torture/harden-cfr-bret-no-xthrow.c b/gcc/testsuite/c-c++-common/torture/harden-cfr-bret-no-xthrow.c
index 78e5bf4143927..07588e8d3c341 100644
--- a/gcc/testsuite/c-c++-common/torture/harden-cfr-bret-no-xthrow.c
+++ b/gcc/testsuite/c-c++-common/torture/harden-cfr-bret-no-xthrow.c
@@ -1,5 +1,6 @@
 /* { dg-do compile } */
-/* { dg-options "-fharden-control-flow-redundancy -fhardcfr-check-noreturn-calls=no-xthrow -fdump-tree-hardcfr -ffat-lto-objects" } */
+/* { dg-options "-fharden-control-flow-redundancy -fhardcfr-check-noreturn-calls=no-xthrow -fno-exceptions -fdump-tree-hardcfr -ffat-lto-objects" } */
+/* { dg-require-effective-target untyped_assembly } */
 
 /* Check that, even enabling checks before no-xthrow-throwing noreturn calls
    (leaving returning calls enabled), we get checks before __builtin_return
diff --git a/gcc/testsuite/c-c++-common/torture/harden-cfr-bret-noopt.c b/gcc/testsuite/c-c++-common/torture/harden-cfr-bret-noopt.c
index 1512614791ff2..716d9297c8d94 100644
--- a/gcc/testsuite/c-c++-common/torture/harden-cfr-bret-noopt.c
+++ b/gcc/testsuite/c-c++-common/torture/harden-cfr-bret-noopt.c
@@ -1,5 +1,6 @@
 /* { dg-do compile } */
-/* { dg-options "-fharden-control-flow-redundancy -fhardcfr-check-noreturn-calls=never -fno-hardcfr-check-returning-calls -fdump-tree-hardcfr -ffat-lto-objects" } */
+/* { dg-options "-fharden-control-flow-redundancy -fhardcfr-check-noreturn-calls=never -fno-hardcfr-check-returning-calls -fno-exceptions -fdump-tree-hardcfr -ffat-lto-objects" } */
+/* { dg-require-effective-target untyped_assembly } */
 
 /* Check that, even disabling checks before both noreturn and returning
    calls, we still get checks before __builtin_return.  */
diff --git a/gcc/testsuite/c-c++-common/torture/harden-cfr-bret-noret.c b/gcc/testsuite/c-c++-common/torture/harden-cfr-bret-noret.c
index fd95bb7e3e334..c6d2baa312031 100644
--- a/gcc/testsuite/c-c++-common/torture/harden-cfr-bret-noret.c
+++ b/gcc/testsuite/c-c++-common/torture/harden-cfr-bret-noret.c
@@ -1,5 +1,6 @@
 /* { dg-do compile } */
-/* { dg-options "-fharden-control-flow-redundancy -fno-hardcfr-check-returning-calls -fdump-tree-hardcfr -ffat-lto-objects" } */
+/* { dg-options "-fharden-control-flow-redundancy -fno-hardcfr-check-returning-calls -fno-exceptions -fdump-tree-hardcfr -ffat-lto-objects" } */
+/* { dg-require-effective-target untyped_assembly } */
 
 /* Check that, even disabling checks before returning calls (leaving noreturn
    calls enabled), we still get checks before __builtin_return.  */
diff --git a/gcc/testsuite/c-c++-common/torture/harden-cfr-bret-nothrow.c b/gcc/testsuite/c-c++-common/torture/harden-cfr-bret-nothrow.c
index c5c361234c499..2fd0d82c7ea3e 100644
--- a/gcc/testsuite/c-c++-common/torture/harden-cfr-bret-nothrow.c
+++ b/gcc/testsuite/c-c++-common/torture/harden-cfr-bret-nothrow.c
@@ -1,5 +1,6 @@
 /* { dg-do compile } */
-/* { dg-options "-fharden-control-flow-redundancy -fhardcfr-check-noreturn-calls=nothrow -fdump-tree-hardcfr -ffat-lto-objects" } */
+/* { dg-options "-fharden-control-flow-redundancy -fhardcfr-check-noreturn-calls=nothrow -fno-exceptions -fdump-tree-hardcfr -ffat-lto-objects" } */
+/* { dg-require-effective-target untyped_assembly } */
 
 /* Check that, even enabling checks before nothrow noreturn calls (leaving
    returning calls enabled), we get checks before __builtin_return without
diff --git a/gcc/testsuite/c-c++-common/torture/harden-cfr-bret-retcl.c b/gcc/testsuite/c-c++-common/torture/harden-cfr-bret-retcl.c
index 137dfbb95d6bb..b070294fde8ad 100644
--- a/gcc/testsuite/c-c++-common/torture/harden-cfr-bret-retcl.c
+++ b/gcc/testsuite/c-c++-common/torture/harden-cfr-bret-retcl.c
@@ -1,5 +1,6 @@
 /* { dg-do compile } */
-/* { dg-options "-fharden-control-flow-redundancy -fhardcfr-check-noreturn-calls=never -fdump-tree-hardcfr -ffat-lto-objects" } */
+/* { dg-options "-fharden-control-flow-redundancy -fhardcfr-check-noreturn-calls=never -fno-exceptions -fdump-tree-hardcfr -ffat-lto-objects" } */
+/* { dg-require-effective-target untyped_assembly } */
 
 /* Check that, even disabling checks before noreturn calls (leaving returning
    calls enabled), we still get checks before __builtin_return.  */
diff --git a/gcc/testsuite/c-c++-common/torture/harden-cfr-bret.c b/gcc/testsuite/c-c++-common/torture/harden-cfr-bret.c
index b459ff6b86491..b6630a6c86695 100644
--- a/gcc/testsuite/c-c++-common/torture/harden-cfr-bret.c
+++ b/gcc/testsuite/c-c++-common/torture/harden-cfr-bret.c
@@ -1,14 +1,27 @@
 /* { dg-do compile } */
-/* { dg-options "-fharden-control-flow-redundancy -fdump-tree-hardcfr -ffat-lto-objects" } */
+/* { dg-options "-fharden-control-flow-redundancy -fno-exceptions -fdump-tree-hardcfr -ffat-lto-objects" } */
+/* { dg-require-effective-target untyped_assembly } */
 
-int f(int i) {
+extern int foobar (void);
+
+#if __cplusplus
+typedef void (*fnt)(...);
+#else
+typedef void (*fnt)();
+#endif
+
+int i;
+
+int f(void) {
   if (i)
-    __builtin_return (&i);
+    __builtin_return (__builtin_apply ((fnt)foobar,
+				       __builtin_apply_args (), 0));
   return i;
 }
 
-int g(int i) {
-  __builtin_return (&i);
+int g(void) {
+  __builtin_return (__builtin_apply ((fnt)foobar,
+				     __builtin_apply_args (), 0));
 }
 
 /* Out-of-line checking, before both builtin_return and return in f.  */
diff --git a/gcc/testsuite/c-c++-common/torture/harden-cfr-noret.c b/gcc/testsuite/c-c++-common/torture/harden-cfr-noret.c
index fdd803109a4ae..8c7cc01c0ce68 100644
--- a/gcc/testsuite/c-c++-common/torture/harden-cfr-noret.c
+++ b/gcc/testsuite/c-c++-common/torture/harden-cfr-noret.c
@@ -1,5 +1,5 @@
 /* { dg-do compile } */
-/* { dg-options "-fharden-control-flow-redundancy -fhardcfr-check-noreturn-calls=always -fdump-tree-hardcfr -ffat-lto-objects" } */
+/* { dg-options "-fharden-control-flow-redundancy -fhardcfr-check-noreturn-calls=always -fno-exceptions -fdump-tree-hardcfr -ffat-lto-objects" } */
 
 /* Check that we insert checking before all noreturn calls.  */
 

-- 
Alexandre Oliva, happy hacker            https://FSFLA.org/blogs/lxo/
   Free Software Activist                   GNU Toolchain Engineer
More tolerance and less prejudice are key for inclusion and diversity
Excluding neuro-others for not behaving ""normal"" is *not* inclusive

Re: [PATCH] hardcfr: make builtin_return tests more portable [PR112334]

[Richard Biener]

On Fri, Dec 1, 2023 at 1:58 PM Alexandre Oliva <oliva@adacore.com> wrote:
>
>
> Rework __builtin_return tests to explicitly call __builtin_apply and
> use its return value rather than anything else.  Also require
> untyped_assembly.  Avoid the noise out of exceptions escaping the
> builtin-applied function, but add a test to cover their effects as
> well.
>
> Regstrapped on x86_64-linux-gnu.  Also tested on arm-eabi, but it's
> *not* enough (or needed) to fix the PR, there's another bug lurking
> there, with a separate patch coming up.  Regardless, is this ok to
> install?

OK.

>
> for  gcc/testsuite/ChangeLog
>
>         PR target/112334
>         * c-c++-common/torture/harden-cfr-bret.c: Rework for stricter
>         untyped_return requirements.  Require untyped_assembly.
>         * c-c++-common/torture/harden-cfr-bret-except.c: New.
>         * c-c++-common/torture/harden-cfr-bret-always.c: Require
>         untyped_assembly.
>         * c-c++-common/torture/harden-cfr-bret-never.c: Likewise.
>         * c-c++-common/torture/harden-cfr-bret-noopt.c: Likewise.
>         * c-c++-common/torture/harden-cfr-bret-noret.c: Likewise.
>         * c-c++-common/torture/harden-cfr-bret-no-xthrow.c: Likewise.
>         * c-c++-common/torture/harden-cfr-bret-nothrow.c: Likewise.
>         * c-c++-common/torture/harden-cfr-bret-retcl.c: Likewise.
> ---
>  .../c-c++-common/torture/harden-cfr-bret-always.c  |    3 ++-
>  .../c-c++-common/torture/harden-cfr-bret-except.c  |   17 +++++++++++++++
>  .../c-c++-common/torture/harden-cfr-bret-never.c   |    3 ++-
>  .../torture/harden-cfr-bret-no-xthrow.c            |    3 ++-
>  .../c-c++-common/torture/harden-cfr-bret-noopt.c   |    3 ++-
>  .../c-c++-common/torture/harden-cfr-bret-noret.c   |    3 ++-
>  .../c-c++-common/torture/harden-cfr-bret-nothrow.c |    3 ++-
>  .../c-c++-common/torture/harden-cfr-bret-retcl.c   |    3 ++-
>  .../c-c++-common/torture/harden-cfr-bret.c         |   23 ++++++++++++++++----
>  .../c-c++-common/torture/harden-cfr-noret.c        |    2 +-
>  10 files changed, 50 insertions(+), 13 deletions(-)
>  create mode 100644 gcc/testsuite/c-c++-common/torture/harden-cfr-bret-except.c
>
> diff --git a/gcc/testsuite/c-c++-common/torture/harden-cfr-bret-always.c b/gcc/testsuite/c-c++-common/torture/harden-cfr-bret-always.c
> index 779896c60e846..3406c4e6ef9dc 100644
> --- a/gcc/testsuite/c-c++-common/torture/harden-cfr-bret-always.c
> +++ b/gcc/testsuite/c-c++-common/torture/harden-cfr-bret-always.c
> @@ -1,5 +1,6 @@
>  /* { dg-do compile } */
> -/* { dg-options "-fharden-control-flow-redundancy -fhardcfr-check-noreturn-calls=always -fdump-tree-hardcfr -ffat-lto-objects" } */
> +/* { dg-options "-fharden-control-flow-redundancy -fhardcfr-check-noreturn-calls=always -fno-exceptions -fdump-tree-hardcfr -ffat-lto-objects" } */
> +/* { dg-require-effective-target untyped_assembly } */
>
>  /* Check that, even enabling all checks before noreturn calls (leaving
>     returning calls enabled), we get checks before __builtin_return without
> diff --git a/gcc/testsuite/c-c++-common/torture/harden-cfr-bret-except.c b/gcc/testsuite/c-c++-common/torture/harden-cfr-bret-except.c
> new file mode 100644
> index 0000000000000..3acb61cb75a77
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/torture/harden-cfr-bret-except.c
> @@ -0,0 +1,17 @@
> +/* { dg-do compile } */
> +/* { dg-options "-fharden-control-flow-redundancy -fexceptions -fdump-tree-hardcfr -ffat-lto-objects" } */
> +/* { dg-require-effective-target untyped_assembly } */
> +
> +/* Check that, with exceptions enabled, even in C, the calls initiated by
> +   builtin_apply are enclosed in cleanup handlers that add extra checks.
> +   Unfortunately, declaring foobar as nothrow is not enough to avoid the
> +   handler around the builtin_apply call, so the other bret tests all use
> +   -fno-exceptions.  */
> +
> +#include "harden-cfr-bret.c"
> +
> +/* With exceptions, we get an extra check per function, to check before
> +   propagating exceptions, so it's 3 in f and 2 in g.  */
> +/* { dg-final { scan-tree-dump-times "__hardcfr_check" 5 "hardcfr" } } */
> +/* The extra check in g also removes the possibility of inlining the check.  */
> +/* { dg-final { scan-tree-dump-times "__builtin_trap" 0 "hardcfr" } } */
> diff --git a/gcc/testsuite/c-c++-common/torture/harden-cfr-bret-never.c b/gcc/testsuite/c-c++-common/torture/harden-cfr-bret-never.c
> index 49ce17f5b937c..7f8fb64138df1 100644
> --- a/gcc/testsuite/c-c++-common/torture/harden-cfr-bret-never.c
> +++ b/gcc/testsuite/c-c++-common/torture/harden-cfr-bret-never.c
> @@ -1,5 +1,6 @@
>  /* { dg-do compile } */
> -/* { dg-options "-fharden-control-flow-redundancy -fhardcfr-check-noreturn-calls=never -fdump-tree-hardcfr -ffat-lto-objects" } */
> +/* { dg-options "-fharden-control-flow-redundancy -fhardcfr-check-noreturn-calls=never -fno-exceptions -fdump-tree-hardcfr -ffat-lto-objects" } */
> +/* { dg-require-effective-target untyped_assembly } */
>
>  /* Check that, even enabling checks before never noreturn calls (leaving
>     returning calls enabled), we get checks before __builtin_return without
> diff --git a/gcc/testsuite/c-c++-common/torture/harden-cfr-bret-no-xthrow.c b/gcc/testsuite/c-c++-common/torture/harden-cfr-bret-no-xthrow.c
> index 78e5bf4143927..07588e8d3c341 100644
> --- a/gcc/testsuite/c-c++-common/torture/harden-cfr-bret-no-xthrow.c
> +++ b/gcc/testsuite/c-c++-common/torture/harden-cfr-bret-no-xthrow.c
> @@ -1,5 +1,6 @@
>  /* { dg-do compile } */
> -/* { dg-options "-fharden-control-flow-redundancy -fhardcfr-check-noreturn-calls=no-xthrow -fdump-tree-hardcfr -ffat-lto-objects" } */
> +/* { dg-options "-fharden-control-flow-redundancy -fhardcfr-check-noreturn-calls=no-xthrow -fno-exceptions -fdump-tree-hardcfr -ffat-lto-objects" } */
> +/* { dg-require-effective-target untyped_assembly } */
>
>  /* Check that, even enabling checks before no-xthrow-throwing noreturn calls
>     (leaving returning calls enabled), we get checks before __builtin_return
> diff --git a/gcc/testsuite/c-c++-common/torture/harden-cfr-bret-noopt.c b/gcc/testsuite/c-c++-common/torture/harden-cfr-bret-noopt.c
> index 1512614791ff2..716d9297c8d94 100644
> --- a/gcc/testsuite/c-c++-common/torture/harden-cfr-bret-noopt.c
> +++ b/gcc/testsuite/c-c++-common/torture/harden-cfr-bret-noopt.c
> @@ -1,5 +1,6 @@
>  /* { dg-do compile } */
> -/* { dg-options "-fharden-control-flow-redundancy -fhardcfr-check-noreturn-calls=never -fno-hardcfr-check-returning-calls -fdump-tree-hardcfr -ffat-lto-objects" } */
> +/* { dg-options "-fharden-control-flow-redundancy -fhardcfr-check-noreturn-calls=never -fno-hardcfr-check-returning-calls -fno-exceptions -fdump-tree-hardcfr -ffat-lto-objects" } */
> +/* { dg-require-effective-target untyped_assembly } */
>
>  /* Check that, even disabling checks before both noreturn and returning
>     calls, we still get checks before __builtin_return.  */
> diff --git a/gcc/testsuite/c-c++-common/torture/harden-cfr-bret-noret.c b/gcc/testsuite/c-c++-common/torture/harden-cfr-bret-noret.c
> index fd95bb7e3e334..c6d2baa312031 100644
> --- a/gcc/testsuite/c-c++-common/torture/harden-cfr-bret-noret.c
> +++ b/gcc/testsuite/c-c++-common/torture/harden-cfr-bret-noret.c
> @@ -1,5 +1,6 @@
>  /* { dg-do compile } */
> -/* { dg-options "-fharden-control-flow-redundancy -fno-hardcfr-check-returning-calls -fdump-tree-hardcfr -ffat-lto-objects" } */
> +/* { dg-options "-fharden-control-flow-redundancy -fno-hardcfr-check-returning-calls -fno-exceptions -fdump-tree-hardcfr -ffat-lto-objects" } */
> +/* { dg-require-effective-target untyped_assembly } */
>
>  /* Check that, even disabling checks before returning calls (leaving noreturn
>     calls enabled), we still get checks before __builtin_return.  */
> diff --git a/gcc/testsuite/c-c++-common/torture/harden-cfr-bret-nothrow.c b/gcc/testsuite/c-c++-common/torture/harden-cfr-bret-nothrow.c
> index c5c361234c499..2fd0d82c7ea3e 100644
> --- a/gcc/testsuite/c-c++-common/torture/harden-cfr-bret-nothrow.c
> +++ b/gcc/testsuite/c-c++-common/torture/harden-cfr-bret-nothrow.c
> @@ -1,5 +1,6 @@
>  /* { dg-do compile } */
> -/* { dg-options "-fharden-control-flow-redundancy -fhardcfr-check-noreturn-calls=nothrow -fdump-tree-hardcfr -ffat-lto-objects" } */
> +/* { dg-options "-fharden-control-flow-redundancy -fhardcfr-check-noreturn-calls=nothrow -fno-exceptions -fdump-tree-hardcfr -ffat-lto-objects" } */
> +/* { dg-require-effective-target untyped_assembly } */
>
>  /* Check that, even enabling checks before nothrow noreturn calls (leaving
>     returning calls enabled), we get checks before __builtin_return without
> diff --git a/gcc/testsuite/c-c++-common/torture/harden-cfr-bret-retcl.c b/gcc/testsuite/c-c++-common/torture/harden-cfr-bret-retcl.c
> index 137dfbb95d6bb..b070294fde8ad 100644
> --- a/gcc/testsuite/c-c++-common/torture/harden-cfr-bret-retcl.c
> +++ b/gcc/testsuite/c-c++-common/torture/harden-cfr-bret-retcl.c
> @@ -1,5 +1,6 @@
>  /* { dg-do compile } */
> -/* { dg-options "-fharden-control-flow-redundancy -fhardcfr-check-noreturn-calls=never -fdump-tree-hardcfr -ffat-lto-objects" } */
> +/* { dg-options "-fharden-control-flow-redundancy -fhardcfr-check-noreturn-calls=never -fno-exceptions -fdump-tree-hardcfr -ffat-lto-objects" } */
> +/* { dg-require-effective-target untyped_assembly } */
>
>  /* Check that, even disabling checks before noreturn calls (leaving returning
>     calls enabled), we still get checks before __builtin_return.  */
> diff --git a/gcc/testsuite/c-c++-common/torture/harden-cfr-bret.c b/gcc/testsuite/c-c++-common/torture/harden-cfr-bret.c
> index b459ff6b86491..b6630a6c86695 100644
> --- a/gcc/testsuite/c-c++-common/torture/harden-cfr-bret.c
> +++ b/gcc/testsuite/c-c++-common/torture/harden-cfr-bret.c
> @@ -1,14 +1,27 @@
>  /* { dg-do compile } */
> -/* { dg-options "-fharden-control-flow-redundancy -fdump-tree-hardcfr -ffat-lto-objects" } */
> +/* { dg-options "-fharden-control-flow-redundancy -fno-exceptions -fdump-tree-hardcfr -ffat-lto-objects" } */
> +/* { dg-require-effective-target untyped_assembly } */
>
> -int f(int i) {
> +extern int foobar (void);
> +
> +#if __cplusplus
> +typedef void (*fnt)(...);
> +#else
> +typedef void (*fnt)();
> +#endif
> +
> +int i;
> +
> +int f(void) {
>    if (i)
> -    __builtin_return (&i);
> +    __builtin_return (__builtin_apply ((fnt)foobar,
> +                                      __builtin_apply_args (), 0));
>    return i;
>  }
>
> -int g(int i) {
> -  __builtin_return (&i);
> +int g(void) {
> +  __builtin_return (__builtin_apply ((fnt)foobar,
> +                                    __builtin_apply_args (), 0));
>  }
>
>  /* Out-of-line checking, before both builtin_return and return in f.  */
> diff --git a/gcc/testsuite/c-c++-common/torture/harden-cfr-noret.c b/gcc/testsuite/c-c++-common/torture/harden-cfr-noret.c
> index fdd803109a4ae..8c7cc01c0ce68 100644
> --- a/gcc/testsuite/c-c++-common/torture/harden-cfr-noret.c
> +++ b/gcc/testsuite/c-c++-common/torture/harden-cfr-noret.c
> @@ -1,5 +1,5 @@
>  /* { dg-do compile } */
> -/* { dg-options "-fharden-control-flow-redundancy -fhardcfr-check-noreturn-calls=always -fdump-tree-hardcfr -ffat-lto-objects" } */
> +/* { dg-options "-fharden-control-flow-redundancy -fhardcfr-check-noreturn-calls=always -fno-exceptions -fdump-tree-hardcfr -ffat-lto-objects" } */
>
>  /* Check that we insert checking before all noreturn calls.  */
>
>
> --
> Alexandre Oliva, happy hacker            https://FSFLA.org/blogs/lxo/
>    Free Software Activist                   GNU Toolchain Engineer
> More tolerance and less prejudice are key for inclusion and diversity
> Excluding neuro-others for not behaving ""normal"" is *not* inclusive

[PATCH] untyped calls: enable target switching [PR112334]

[Alexandre Oliva]

On Dec  1, 2023, Alexandre Oliva <oliva@adacore.com> wrote:

> Also tested on arm-eabi, but it's *not* enough (or needed) to fix the
> PR, there's another bug lurking there, with a separate patch coming
> up.

Here it is.

----

The computation of apply_args_size and apply_result_size is saved in a
static variable, so that the corresponding _mode arrays are
initialized only once.  That is not compatible with switchable
targets, and ARM's arm_set_current_function, by saving and restoring
target globals, exercises this problem with a testcase such as that in
the PR, in which more than one function in the translation unit calls
__builtin_apply or __builtin_return, respectively.

This patch moves the _size statics into the target_builtins array,
with a bit of ugliness over _plus_one so that zero initialization of
the struct does the right thing.

Regstrapped on x86_64-linux-gnu, tested on arm-eabi with and without the
upthread patch.  It fixes the hardcfr fails either way.  As for the
ugliness, there's a follow up patch below that attempts to alleviate it
a little (also regstrapped and tested), but I'm not sure we want to go
down that path.  WDYT?


for  gcc/ChangeLog

	PR target/112334
	* builtins.h (target_builtins): Add fields for apply_args_size
	and apply_result_size.
	* builtins.cc (apply_args_size, apply_result_size): Cache
	results in fields rather than in static variables.
	(get_apply_args_size, set_apply_args_size): New.
	(get_apply_result_size, set_apply_result_size): New.
---
 gcc/builtins.cc |   16 ++++++++++++++--
 gcc/builtins.h  |    7 +++++++
 2 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/gcc/builtins.cc b/gcc/builtins.cc
index 4fc58a0bda9b8..039bb5e997a2c 100644
--- a/gcc/builtins.cc
+++ b/gcc/builtins.cc
@@ -1398,8 +1398,16 @@ get_memory_rtx (tree exp, tree len)
 \f
 /* Built-in functions to perform an untyped call and return.  */
 
+#define set_apply_args_size(x) \
+  (this_target_builtins->x_apply_args_size_plus_one = 1 + (x))
+#define get_apply_args_size() \
+  (this_target_builtins->x_apply_args_size_plus_one - 1)
 #define apply_args_mode \
   (this_target_builtins->x_apply_args_mode)
+#define set_apply_result_size(x) \
+  (this_target_builtins->x_apply_result_size_plus_one = 1 + (x))
+#define get_apply_result_size() \
+  (this_target_builtins->x_apply_result_size_plus_one - 1)
 #define apply_result_mode \
   (this_target_builtins->x_apply_result_mode)
 
@@ -1409,7 +1417,7 @@ get_memory_rtx (tree exp, tree len)
 static int
 apply_args_size (void)
 {
-  static int size = -1;
+  int size = get_apply_args_size ();
   int align;
   unsigned int regno;
 
@@ -1442,6 +1450,8 @@ apply_args_size (void)
 	  }
 	else
 	  apply_args_mode[regno] = as_a <fixed_size_mode> (VOIDmode);
+
+      set_apply_args_size (size);
     }
   return size;
 }
@@ -1452,7 +1462,7 @@ apply_args_size (void)
 static int
 apply_result_size (void)
 {
-  static int size = -1;
+  int size = get_apply_result_size ();
   int align, regno;
 
   /* The values computed by this function never change.  */
@@ -1484,6 +1494,8 @@ apply_result_size (void)
 #ifdef APPLY_RESULT_SIZE
       size = APPLY_RESULT_SIZE;
 #endif
+
+      set_apply_result_size (size);
     }
   return size;
 }
diff --git a/gcc/builtins.h b/gcc/builtins.h
index 88a26d70cd5a8..1a26fc63a6d10 100644
--- a/gcc/builtins.h
+++ b/gcc/builtins.h
@@ -37,6 +37,13 @@ struct target_builtins {
      register windows, this gives only the outbound registers.
      INCOMING_REGNO gives the corresponding inbound register.  */
   fixed_size_mode_pod x_apply_result_mode[FIRST_PSEUDO_REGISTER];
+
+  /* Nonzero iff the arrays above have been initialized.  The _plus_one suffix
+     is for zero initialization to make it an unreasonable size, used to signal
+     that the size and the corresponding mode array has not been
+     initialized.  */
+  int x_apply_args_size_plus_one;
+  int x_apply_result_size_plus_one;
 };
 
 extern struct target_builtins default_target_builtins;


----

untyped calls: use wrapper class type for implicit plus_one

Instead of get and set macros to apply a delta, use a single macro
that resorts to a temporary wrapper class to apply it.

To be combined (or not) with the previous patch.
---
 gcc/builtins.cc |   32 ++++++++++++++++++++------------
 1 file changed, 20 insertions(+), 12 deletions(-)

diff --git a/gcc/builtins.cc b/gcc/builtins.cc
index 039bb5e997a2c..a60e0a7084513 100644
--- a/gcc/builtins.cc
+++ b/gcc/builtins.cc
@@ -1398,16 +1398,24 @@ get_memory_rtx (tree exp, tree len)
 \f
 /* Built-in functions to perform an untyped call and return.  */
 
-#define set_apply_args_size(x) \
-  (this_target_builtins->x_apply_args_size_plus_one = 1 + (x))
-#define get_apply_args_size() \
-  (this_target_builtins->x_apply_args_size_plus_one - 1)
+/* Wrapper that implicitly applies a delta when getting or setting the
+   enclosed value.  */
+template <typename T>
+class delta_type
+{
+  T &value; T const delta;
+public:
+  delta_type (T &val, T dlt) : value (val), delta (dlt) {}
+  operator T () const { return value + delta; }
+  T operator = (T val) const { value = val - delta; return val; }
+};
+
+#define saved_apply_args_size \
+  (delta_type<int> (this_target_builtins->x_apply_args_size_plus_one, -1))
 #define apply_args_mode \
   (this_target_builtins->x_apply_args_mode)
-#define set_apply_result_size(x) \
-  (this_target_builtins->x_apply_result_size_plus_one = 1 + (x))
-#define get_apply_result_size() \
-  (this_target_builtins->x_apply_result_size_plus_one - 1)
+#define saved_apply_result_size \
+  (delta_type<int> (this_target_builtins->x_apply_result_size_plus_one, -1))
 #define apply_result_mode \
   (this_target_builtins->x_apply_result_mode)
 
@@ -1417,7 +1425,7 @@ get_memory_rtx (tree exp, tree len)
 static int
 apply_args_size (void)
 {
-  int size = get_apply_args_size ();
+  int size = saved_apply_args_size;
   int align;
   unsigned int regno;
 
@@ -1451,7 +1459,7 @@ apply_args_size (void)
 	else
 	  apply_args_mode[regno] = as_a <fixed_size_mode> (VOIDmode);
 
-      set_apply_args_size (size);
+      saved_apply_args_size = size;
     }
   return size;
 }
@@ -1462,7 +1470,7 @@ apply_args_size (void)
 static int
 apply_result_size (void)
 {
-  int size = get_apply_result_size ();
+  int size = saved_apply_result_size;
   int align, regno;
 
   /* The values computed by this function never change.  */
@@ -1495,7 +1503,7 @@ apply_result_size (void)
       size = APPLY_RESULT_SIZE;
 #endif
 
-      set_apply_result_size (size);
+      saved_apply_result_size = size;
     }
   return size;
 }


-- 
Alexandre Oliva, happy hacker            https://FSFLA.org/blogs/lxo/
   Free Software Activist                   GNU Toolchain Engineer
More tolerance and less prejudice are key for inclusion and diversity
Excluding neuro-others for not behaving ""normal"" is *not* inclusive

Re: [PATCH] untyped calls: enable target switching [PR112334]

[Jeff Law]

On 12/1/23 08:10, Alexandre Oliva wrote:
> On Dec  1, 2023, Alexandre Oliva <oliva@adacore.com> wrote:
> 
>> Also tested on arm-eabi, but it's *not* enough (or needed) to fix the
>> PR, there's another bug lurking there, with a separate patch coming
>> up.
> 
> Here it is.
> 
> ----
> 
> The computation of apply_args_size and apply_result_size is saved in a
> static variable, so that the corresponding _mode arrays are
> initialized only once.  That is not compatible with switchable
> targets, and ARM's arm_set_current_function, by saving and restoring
> target globals, exercises this problem with a testcase such as that in
> the PR, in which more than one function in the translation unit calls
> __builtin_apply or __builtin_return, respectively.
> 
> This patch moves the _size statics into the target_builtins array,
> with a bit of ugliness over _plus_one so that zero initialization of
> the struct does the right thing.
> 
> Regstrapped on x86_64-linux-gnu, tested on arm-eabi with and without the
> upthread patch.  It fixes the hardcfr fails either way.  As for the
> ugliness, there's a follow up patch below that attempts to alleviate it
> a little (also regstrapped and tested), but I'm not sure we want to go
> down that path.  WDYT?
It's a wart, but doesn't seem too bad to me.

> 
> 
> for  gcc/ChangeLog
> 
> 	PR target/112334
> 	* builtins.h (target_builtins): Add fields for apply_args_size
> 	and apply_result_size.
> 	* builtins.cc (apply_args_size, apply_result_size): Cache
> 	results in fields rather than in static variables.
> 	(get_apply_args_size, set_apply_args_size): New.
> 	(get_apply_result_size, set_apply_result_size): New.
OK.


> 
> untyped calls: use wrapper class type for implicit plus_one
> 
> Instead of get and set macros to apply a delta, use a single macro
> that resorts to a temporary wrapper class to apply it.
> 
> To be combined (or not) with the previous patch.
I'd be OK with this as well.

jeff

Re: [PATCH] untyped calls: enable target switching [PR112334]

[Alexandre Oliva]

On Dec 11, 2023, Jeff Law <jeffreyalaw@gmail.com> wrote:

>> 
>> for  gcc/ChangeLog
>> PR target/112334
>> * builtins.h (target_builtins): Add fields for apply_args_size
>> and apply_result_size.
>> * builtins.cc (apply_args_size, apply_result_size): Cache
>> results in fields rather than in static variables.
>> (get_apply_args_size, set_apply_args_size): New.
>> (get_apply_result_size, set_apply_result_size): New.
> OK.

Thanks, I put this bit in.

>> untyped calls: use wrapper class type for implicit plus_one
>> Instead of get and set macros to apply a delta, use a single macro
>> that resorts to a temporary wrapper class to apply it.
>> To be combined (or not) with the previous patch.

> I'd be OK with this as well.

That conditional made me doubt that this was meant as approval, so I did
*not* put this one in ;-)

If there's firmer/broader buy-in, I'd be glad to put it in as well.

-- 
Alexandre Oliva, happy hacker            https://FSFLA.org/blogs/lxo/
   Free Software Activist                   GNU Toolchain Engineer
More tolerance and less prejudice are key for inclusion and diversity
Excluding neuro-others for not behaving ""normal"" is *not* inclusive

Re: [PATCH] untyped calls: enable target switching [PR112334]

[Jeff Law]

On 12/11/23 21:44, Alexandre Oliva wrote:

> 
>>> untyped calls: use wrapper class type for implicit plus_one
>>> Instead of get and set macros to apply a delta, use a single macro
>>> that resorts to a temporary wrapper class to apply it.
>>> To be combined (or not) with the previous patch.
> 
>> I'd be OK with this as well.
> 
> That conditional made me doubt that this was meant as approval, so I did
> *not* put this one in ;-)
> 
> If there's firmer/broader buy-in, I'd be glad to put it in as well.
Sorry it was meant to be an ACK for the trunk for both patches.

jeff
> 

Re: [PATCH] untyped calls: enable target switching [PR112334]

[Alexandre Oliva]

On Dec 20, 2023, Jeff Law <jeffreyalaw@gmail.com> wrote:

> Sorry it was meant to be an ACK for the trunk for both patches.

Aah, I see.  So this is what I installed last night, upon seeing your
message.


untyped calls: use wrapper class type for implicit plus_one

Instead of get and set macros to apply a delta, use a single macro
that resorts to a temporary wrapper class to apply it.


for  gcc/ChangeLog

	* builtins.cc (delta_type): New template class.
	(set_apply_args_size, get_apply_args_size): Replace with...
	(saved_apply_args_size): ... this.
	(set_apply_result_size, get_apply_result_size): Replace with...
	(saved_apply_result_size): ... this.
	(apply_args_size, apply_result_size): Adjust.
---
 gcc/builtins.cc |   32 ++++++++++++++++++++------------
 1 file changed, 20 insertions(+), 12 deletions(-)

diff --git a/gcc/builtins.cc b/gcc/builtins.cc
index 7c2732ab79e6f..23cc6b6838a01 100644
--- a/gcc/builtins.cc
+++ b/gcc/builtins.cc
@@ -1403,16 +1403,24 @@ get_memory_rtx (tree exp, tree len)
 \f
 /* Built-in functions to perform an untyped call and return.  */
 
-#define set_apply_args_size(x) \
-  (this_target_builtins->x_apply_args_size_plus_one = 1 + (x))
-#define get_apply_args_size() \
-  (this_target_builtins->x_apply_args_size_plus_one - 1)
+/* Wrapper that implicitly applies a delta when getting or setting the
+   enclosed value.  */
+template <typename T>
+class delta_type
+{
+  T &value; T const delta;
+public:
+  delta_type (T &val, T dlt) : value (val), delta (dlt) {}
+  operator T () const { return value + delta; }
+  T operator = (T val) const { value = val - delta; return val; }
+};
+
+#define saved_apply_args_size \
+  (delta_type<int> (this_target_builtins->x_apply_args_size_plus_one, -1))
 #define apply_args_mode \
   (this_target_builtins->x_apply_args_mode)
-#define set_apply_result_size(x) \
-  (this_target_builtins->x_apply_result_size_plus_one = 1 + (x))
-#define get_apply_result_size() \
-  (this_target_builtins->x_apply_result_size_plus_one - 1)
+#define saved_apply_result_size \
+  (delta_type<int> (this_target_builtins->x_apply_result_size_plus_one, -1))
 #define apply_result_mode \
   (this_target_builtins->x_apply_result_mode)
 
@@ -1422,7 +1430,7 @@ get_memory_rtx (tree exp, tree len)
 static int
 apply_args_size (void)
 {
-  int size = get_apply_args_size ();
+  int size = saved_apply_args_size;
   int align;
   unsigned int regno;
 
@@ -1456,7 +1464,7 @@ apply_args_size (void)
 	else
 	  apply_args_mode[regno] = as_a <fixed_size_mode> (VOIDmode);
 
-      set_apply_args_size (size);
+      saved_apply_args_size = size;
     }
   return size;
 }
@@ -1467,7 +1475,7 @@ apply_args_size (void)
 static int
 apply_result_size (void)
 {
-  int size = get_apply_result_size ();
+  int size = saved_apply_result_size;
   int align, regno;
 
   /* The values computed by this function never change.  */
@@ -1500,7 +1508,7 @@ apply_result_size (void)
       size = APPLY_RESULT_SIZE;
 #endif
 
-      set_apply_result_size (size);
+      saved_apply_result_size = size;
     }
   return size;
 }


-- 
Alexandre Oliva, happy hacker            https://FSFLA.org/blogs/lxo/
   Free Software Activist                   GNU Toolchain Engineer
More tolerance and less prejudice are key for inclusion and diversity
Excluding neuro-others for not behaving ""normal"" is *not* inclusive

Re: [PATCH] hardcfr: make builtin_return tests more portable [PR112334]

[Alexandre Oliva]

On Dec  1, 2023, Alexandre Oliva <oliva@adacore.com> wrote:

> diff --git a/gcc/testsuite/c-c++-common/torture/harden-cfr-noret.c b/gcc/testsuite/c-c++-common/torture/harden-cfr-noret.c
> index fdd803109a4ae..8c7cc01c0ce68 100644
> --- a/gcc/testsuite/c-c++-common/torture/harden-cfr-noret.c
> +++ b/gcc/testsuite/c-c++-common/torture/harden-cfr-noret.c
> @@ -1,5 +1,5 @@
>  /* { dg-do compile } */
> -/* { dg-options "-fharden-control-flow-redundancy -fhardcfr-check-noreturn-calls=always -fdump-tree-hardcfr -ffat-lto-objects" } */
> +/* { dg-options "-fharden-control-flow-redundancy -fhardcfr-check-noreturn-calls=always -fno-exceptions -fdump-tree-hardcfr -ffat-lto-objects" } */

This one was *not* meant to be modified, it was changed by accident in
my tree and integrated by mistake with the posted patch.  Fortunately
the ChangeLog verifier caught the mistake, and I dropped it from the
patch I've just pushed.

-- 
Alexandre Oliva, happy hacker            https://FSFLA.org/blogs/lxo/
   Free Software Activist                   GNU Toolchain Engineer
More tolerance and less prejudice are key for inclusion and diversity
Excluding neuro-others for not behaving ""normal"" is *not* inclusive