* [PATCH] hardcfr: make builtin_return tests more portable [PR112334]
@ 2023-12-01 12:57 Alexandre Oliva
2023-12-01 14:27 ` Richard Biener
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Alexandre Oliva @ 2023-12-01 12:57 UTC (permalink / raw)
To: gcc-patches
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] hardcfr: make builtin_return tests more portable [PR112334]
2023-12-01 12:57 [PATCH] hardcfr: make builtin_return tests more portable [PR112334] Alexandre Oliva
@ 2023-12-01 14:27 ` Richard Biener
2023-12-01 15:10 ` [PATCH] untyped calls: enable target switching [PR112334] Alexandre Oliva
2023-12-01 17:39 ` [PATCH] hardcfr: make builtin_return tests more portable [PR112334] Alexandre Oliva
2 siblings, 0 replies; 8+ messages in thread
From: Richard Biener @ 2023-12-01 14:27 UTC (permalink / raw)
To: Alexandre Oliva; +Cc: gcc-patches
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] untyped calls: enable target switching [PR112334]
2023-12-01 12:57 [PATCH] hardcfr: make builtin_return tests more portable [PR112334] Alexandre Oliva
2023-12-01 14:27 ` Richard Biener
@ 2023-12-01 15:10 ` Alexandre Oliva
2023-12-11 16:02 ` Jeff Law
2023-12-01 17:39 ` [PATCH] hardcfr: make builtin_return tests more portable [PR112334] Alexandre Oliva
2 siblings, 1 reply; 8+ messages in thread
From: Alexandre Oliva @ 2023-12-01 15:10 UTC (permalink / raw)
To: gcc-patches
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] hardcfr: make builtin_return tests more portable [PR112334]
2023-12-01 12:57 [PATCH] hardcfr: make builtin_return tests more portable [PR112334] Alexandre Oliva
2023-12-01 14:27 ` Richard Biener
2023-12-01 15:10 ` [PATCH] untyped calls: enable target switching [PR112334] Alexandre Oliva
@ 2023-12-01 17:39 ` Alexandre Oliva
2 siblings, 0 replies; 8+ messages in thread
From: Alexandre Oliva @ 2023-12-01 17:39 UTC (permalink / raw)
To: gcc-patches
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] untyped calls: enable target switching [PR112334]
2023-12-01 15:10 ` [PATCH] untyped calls: enable target switching [PR112334] Alexandre Oliva
@ 2023-12-11 16:02 ` Jeff Law
2023-12-12 4:44 ` Alexandre Oliva
0 siblings, 1 reply; 8+ messages in thread
From: Jeff Law @ 2023-12-11 16:02 UTC (permalink / raw)
To: Alexandre Oliva, gcc-patches
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] untyped calls: enable target switching [PR112334]
2023-12-11 16:02 ` Jeff Law
@ 2023-12-12 4:44 ` Alexandre Oliva
2023-12-20 5:08 ` Jeff Law
0 siblings, 1 reply; 8+ messages in thread
From: Alexandre Oliva @ 2023-12-12 4:44 UTC (permalink / raw)
To: Jeff Law; +Cc: gcc-patches
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] untyped calls: enable target switching [PR112334]
2023-12-12 4:44 ` Alexandre Oliva
@ 2023-12-20 5:08 ` Jeff Law
2023-12-21 1:01 ` Alexandre Oliva
0 siblings, 1 reply; 8+ messages in thread
From: Jeff Law @ 2023-12-20 5:08 UTC (permalink / raw)
To: Alexandre Oliva; +Cc: gcc-patches
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
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] untyped calls: enable target switching [PR112334]
2023-12-20 5:08 ` Jeff Law
@ 2023-12-21 1:01 ` Alexandre Oliva
0 siblings, 0 replies; 8+ messages in thread
From: Alexandre Oliva @ 2023-12-21 1:01 UTC (permalink / raw)
To: Jeff Law; +Cc: gcc-patches
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
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2023-12-21 1:01 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-12-01 12:57 [PATCH] hardcfr: make builtin_return tests more portable [PR112334] Alexandre Oliva
2023-12-01 14:27 ` Richard Biener
2023-12-01 15:10 ` [PATCH] untyped calls: enable target switching [PR112334] Alexandre Oliva
2023-12-11 16:02 ` Jeff Law
2023-12-12 4:44 ` Alexandre Oliva
2023-12-20 5:08 ` Jeff Law
2023-12-21 1:01 ` Alexandre Oliva
2023-12-01 17:39 ` [PATCH] hardcfr: make builtin_return tests more portable [PR112334] Alexandre Oliva
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).