Re: [PATCH v5] Introduce strub: machine-independent stack scrubbing

[Alexandre Oliva <oliva@adacore.com>]

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

> I intend to install this as part of the monster patch upthread.

I tweaked it a little further, so that exceptions don't mess with the
pattern counts, and extending the same anti-vrp measure to the other
strub-const tests, even though they weren't affected.

I also had to tweak strub-ptrfn2.c, because of the recent
warning-to-error changes.

Finally, the ChangeLog checker noticed that this entry was no longer
applicable:

>	* multiple_target.cc (pass_target_clone::gate): Test seen_error.

I'd duplicated exactly the fix for ipa/107897, and didn't realize it had
been fixed independently.

I'm reposting only the parts of the final patch pertaining to the
modified test files below, to spare you all yet another copy of the
moster patch.  The whole thing is r14-6201.  I've also refreshed my
strub repo with the trunk commit (same commit id), with the extra
patchlets for broader strub testing on top of it.

diff --git a/gcc/testsuite/c-c++-common/torture/strub-const1.c b/gcc/testsuite/c-c++-common/torture/strub-const1.c
new file mode 100644
index 0000000000000..5e956cb1a9b6b
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/torture/strub-const1.c
@@ -0,0 +1,23 @@
+/* { dg-do compile } */
+/* { dg-options "-fstrub=strict -fdump-ipa-strub" } */
+
+/* Check that, along with a strub const function call, we issue an asm
+   statement to make sure the watermark passed to it is held in memory before
+   the call, and another to make sure it is not assumed to be unchanged.  f
+   should not be inlined into g, but if it were too simple it might be folded
+   by interprocedural value-range propagation.  */
+
+extern int __attribute__ ((__strub__ ("callable"),
+			   __const__, __nothrow__)) c ();
+
+int __attribute__ ((__strub__, __const__))
+f () {
+  return c ();
+}
+
+int
+g () {
+  return f ();
+}
+
+/* { dg-final { scan-ipa-dump-times "__asm__" 2 "strub" } } */
diff --git a/gcc/testsuite/c-c++-common/torture/strub-const2.c b/gcc/testsuite/c-c++-common/torture/strub-const2.c
new file mode 100644
index 0000000000000..73d650292dfbf
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/torture/strub-const2.c
@@ -0,0 +1,25 @@
+/* { dg-do compile } */
+/* { dg-options "-fstrub=strict -fdump-ipa-strub" } */
+
+/* Check that, along with a strub implicitly-const function call, we issue an
+   asm statement to make sure the watermark passed to it is held in memory
+   before the call, and another to make sure it is not assumed to be
+   unchanged.  */
+
+extern int __attribute__ ((__strub__ ("callable"),
+			   __const__, __nothrow__)) c ();
+
+int __attribute__ ((__strub__))
+#if ! __OPTIMIZE__
+__attribute__ ((__const__))
+#endif
+f () {
+  return c ();
+}
+
+int
+g () {
+  return f ();
+}
+
+/* { dg-final { scan-ipa-dump-times "__asm__" 2 "strub" } } */
diff --git a/gcc/testsuite/c-c++-common/torture/strub-const3.c b/gcc/testsuite/c-c++-common/torture/strub-const3.c
new file mode 100644
index 0000000000000..2584f1f974a58
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/torture/strub-const3.c
@@ -0,0 +1,16 @@
+/* { dg-do compile } */
+/* { dg-options "-fstrub=strict -fdump-ipa-strub" } */
+
+/* Check that, along with a strub const wrapping call, we issue an asm statement
+   to make sure the watermark passed to it is held in memory before the call,
+   and another to make sure it is not assumed to be unchanged.  */
+
+extern int __attribute__ ((__strub__ ("callable"),
+			   __const__, __nothrow__)) c ();
+
+int __attribute__ ((__strub__ ("internal"), __const__))
+f () {
+  return c ();
+}
+
+/* { dg-final { scan-ipa-dump-times "__asm__" 2 "strub" } } */
diff --git a/gcc/testsuite/c-c++-common/torture/strub-const4.c b/gcc/testsuite/c-c++-common/torture/strub-const4.c
new file mode 100644
index 0000000000000..d819f54ec0230
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/torture/strub-const4.c
@@ -0,0 +1,20 @@
+/* { dg-do compile } */
+/* { dg-options "-fstrub=strict -fdump-ipa-strub" } */
+
+/* Check that, along with a strub implicitly-const wrapping call, we issue an
+   asm statement to make sure the watermark passed to it is held in memory
+   before the call, and another to make sure it is not assumed to be
+   unchanged.  */
+
+extern int __attribute__ ((__strub__ ("callable"),
+			   __const__, __nothrow__)) c ();
+
+int __attribute__ ((__strub__ ("internal")))
+#if ! __OPTIMIZE__
+__attribute__ ((__const__))
+#endif
+f () {
+  return c ();
+}
+
+/* { dg-final { scan-ipa-dump-times "__asm__" 2 "strub" } } */

[...]

diff --git a/gcc/testsuite/c-c++-common/torture/strub-ptrfn2.c b/gcc/testsuite/c-c++-common/torture/strub-ptrfn2.c
new file mode 100644
index 0000000000000..ef634d351265f
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/torture/strub-ptrfn2.c
@@ -0,0 +1,55 @@
+/* { dg-do compile } */
+/* { dg-options "-fstrub=relaxed -Wpedantic" } */
+
+/* C++ does not warn about the partial incompatibilities.
+
+   The d_p () calls are actually rejected, even in C++, but they are XFAILed
+   here because we don't get far enough in the compilation as to observe them,
+   because the incompatibilities are errors without -fpermissive.
+   strub-ptrfn3.c uses -fpermissive to check those.
+ */
+
+extern int __attribute__ ((strub ("callable"))) bac (void);
+extern int __attribute__ ((strub ("disabled"))) bad (void);
+extern int __attribute__ ((strub ("internal"))) bar (void);
+extern int __attribute__ ((strub ("at-calls"))) bal (void);
+
+void __attribute__ ((strub))
+bap (void)
+{
+  int __attribute__ ((strub ("disabled"))) (*d_p) (void) = bad;
+  int __attribute__ ((strub ("callable"))) (*c_p) (void) = bac;
+  int __attribute__ ((strub ("at-calls"))) (*a_p) (void) = bal;
+
+  d_p = bac; /* { dg-warning "not quite compatible" "" { xfail c++ } } */
+  c_p = bad; /* { dg-warning "not quite compatible" "" { xfail c++ } } */
+  c_p = bar; /* { dg-warning "not quite compatible" "" { xfail c++ } } */
+  c_p = bal; /* { dg-message "incompatible|invalid conversion" } */
+  a_p = bac; /* { dg-message "incompatible|invalid conversion" } */
+
+  d_p (); /* { dg-error "indirect non-.strub. call in .strub. context" "" { xfail *-*-* } } */
+  c_p ();
+  a_p ();
+}
+
+void __attribute__ ((strub))
+baP (void)
+{
+  typedef int __attribute__ ((strub ("disabled"))) d_fn_t (void);
+  typedef int __attribute__ ((strub ("callable"))) c_fn_t (void);
+  typedef int __attribute__ ((strub ("at-calls"))) a_fn_t (void);
+
+  d_fn_t *d_p = bad;
+  c_fn_t *c_p = bac;
+  a_fn_t *a_p = bal;
+
+  d_p = bac; /* { dg-warning "not quite compatible" "" { xfail c++ } } */
+  c_p = bad; /* { dg-warning "not quite compatible" "" { xfail c++ } } */
+  c_p = bar; /* { dg-warning "not quite compatible" "" { xfail c++ } } */
+  c_p = bal; /* { dg-message "incompatible|invalid conversion" } */
+  a_p = bac; /* { dg-message "incompatible|invalid conversion" } */
+
+  d_p (); /* { dg-error "indirect non-.strub. call in .strub. context" "" { xfail *-*-* } } */
+  c_p ();
+  a_p ();
+}


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