public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Alexandre Oliva <oliva@adacore.com>
To: Qing Zhao <qing.zhao@oracle.com>
Cc: "gcc-patches@gcc.gnu.org" <gcc-patches@gcc.gnu.org>
Subject: Re: [PATCH zero-call-used-regs] Add leafy mode for zero-call-used-regs
Date: Fri, 16 Jun 2023 04:26:30 -0300	[thread overview]
Message-ID: <orttv7x3yx.fsf@lxoliva.fsfla.org> (raw)
In-Reply-To: <3CF608E7-C293-4627-8FE9-8B580D69D764@oracle.com> (Qing Zhao's message of "Thu, 27 Oct 2022 13:30:10 +0000")

Hello, Qing,

On Oct 27, 2022, Qing Zhao <qing.zhao@oracle.com> wrote:
<https://gcc.gnu.org/pipermail/gcc-patches/2022-October/604480.html>

> On Oct 26, 2022, at 5:29 PM, Alexandre Oliva <oliva@adacore.com> wrote:
>> I'm sure there are other scenarios in which keeping at least the
>> possibility of 'all' is useful.
> Okay.


> i.e, instead introducing a new MODE “LEAFY_MODE” and a new user
> sub-option, for LEAF functions, only
> Clear its’ used registers even for “ALL”.

> However, since there is need to clear the un-used registers for leaf
> functions. It looks like it is needed to provide
> This new sub-option to users.

> Is this clear this time?

Yeah, I guess I understand what you mean.  But since there are cases in
which clearing all (call-clobbered) registers in a leaf function is
useful, I suppose it makes sense to offer both possibilities.

If there was a default operation mode for -fzero-call-used-regs, I guess
it would make sense to consider leafy the default, rather than all, but
since there isn't, and it always has to be specified explicitly, that's
not something to be considered.

So the available choices are:

1. introduce 'leafy' as a separate mode, leaving 'all' alone

2. change the behavior of 'all' to that of the proposed 'leafy', and either

2.a) add another mode that retains the currently-useful behavior of 'all',
   or

2.b) make the current behavior of 'all' no longer available

Personally, I find 1. the least disruptive to existing users of
-fzero-call-used-regs.  If we were introducing the option now, maybe 2.a
would be more sensible, but at this point, changing the meaning of 'all'
seems to be a disservice to security-sensitive users.

Those who would prefer the leaner operation on leaf functions can then
switch to 'leafy' mode, but that's better than finding carefully-crafted
code relying on the current behavior of 'all' for security suddenly
changes from under them, isn't it?


That said, I'm willing to implement the alternate change, if changing
the expected behavior is preferred over offering a different choice, if
needed to get closure on this feature.

For now, I'm just pinging the refreshed and retested patch.
Ok to install?


Add leafy mode for zero-call-used-regs

Introduce 'leafy' to auto-select between 'used' and 'all' for leaf and
nonleaf functions, respectively.


for  gcc/ChangeLog

	* doc/extend.texi (zero-call-used-regs): Document leafy and
	variants thereof.
	* flag-types.h (zero_regs_flags): Add LEAFY_MODE, as well as
	LEAFY and variants.
	* function.cc (gen_call_ued_regs_seq): Set only_used for leaf
	functions in leafy mode.
	* opts.cc (zero_call_used_regs_opts): Add leafy and variants.

for  gcc/testsuite/ChangeLog

	* c-c++-common/zero-scratch-regs-leafy-1.c: New.
	* c-c++-common/zero-scratch-regs-leafy-2.c: New.
	* gcc.target/i386/zero-scratch-regs-leafy-1.c: New.
	* gcc.target/i386/zero-scratch-regs-leafy-2.c: New.
---
 gcc/doc/extend.texi                                |   22 ++++++++++++++++++--
 gcc/flag-types.h                                   |    5 +++++
 gcc/function.cc                                    |    3 +++
 gcc/opts.cc                                        |    4 ++++
 .../c-c++-common/zero-scratch-regs-leafy-1.c       |   15 ++++++++++++++
 .../c-c++-common/zero-scratch-regs-leafy-2.c       |   21 +++++++++++++++++++
 .../gcc.target/i386/zero-scratch-regs-leafy-1.c    |   12 +++++++++++
 .../gcc.target/i386/zero-scratch-regs-leafy-2.c    |   16 +++++++++++++++
 8 files changed, 96 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/c-c++-common/zero-scratch-regs-leafy-1.c
 create mode 100644 gcc/testsuite/c-c++-common/zero-scratch-regs-leafy-2.c
 create mode 100644 gcc/testsuite/gcc.target/i386/zero-scratch-regs-leafy-1.c
 create mode 100644 gcc/testsuite/gcc.target/i386/zero-scratch-regs-leafy-2.c

diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index 7b5592502734e..f8b0bb53ef5d4 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -4412,10 +4412,28 @@ zeros all call-used registers that pass arguments.
 @item all-gpr-arg
 zeros all call-used general purpose registers that pass
 arguments.
+
+@item leafy
+Same as @samp{used} in a leaf function, and same as @samp{all} in a
+nonleaf function.
+
+@item leafy-gpr
+Same as @samp{used-gpr} in a leaf function, and same as @samp{all-gpr}
+in a nonleaf function.
+
+@item leafy-arg
+Same as @samp{used-arg} in a leaf function, and same as @samp{all-arg}
+in a nonleaf function.
+
+@item leafy-gpr-arg
+Same as @samp{used-gpr-arg} in a leaf function, and same as
+@samp{all-gpr-arg} in a nonleaf function.
+
 @end table
 
-Of this list, @samp{used-arg}, @samp{used-gpr-arg}, @samp{all-arg},
-and @samp{all-gpr-arg} are mainly used for ROP mitigation.
+Of this list, @samp{used-arg}, @samp{used-gpr-arg}, @samp{leafy-arg},
+@samp{leafy-gpr-arg}, @samp{all-arg}, and @samp{all-gpr-arg} are mainly
+used for ROP mitigation.
 
 The default for the attribute is controlled by @option{-fzero-call-used-regs}.
 @end table
diff --git a/gcc/flag-types.h b/gcc/flag-types.h
index f83d165fbfef1..6a2e1beb997ef 100644
--- a/gcc/flag-types.h
+++ b/gcc/flag-types.h
@@ -348,6 +348,7 @@ namespace zero_regs_flags {
   const unsigned int ONLY_GPR = 1UL << 2;
   const unsigned int ONLY_ARG = 1UL << 3;
   const unsigned int ENABLED = 1UL << 4;
+  const unsigned int LEAFY_MODE = 1UL << 5;
   const unsigned int USED_GPR_ARG = ENABLED | ONLY_USED | ONLY_GPR | ONLY_ARG;
   const unsigned int USED_GPR = ENABLED | ONLY_USED | ONLY_GPR;
   const unsigned int USED_ARG = ENABLED | ONLY_USED | ONLY_ARG;
@@ -356,6 +357,10 @@ namespace zero_regs_flags {
   const unsigned int ALL_GPR = ENABLED | ONLY_GPR;
   const unsigned int ALL_ARG = ENABLED | ONLY_ARG;
   const unsigned int ALL = ENABLED;
+  const unsigned int LEAFY_GPR_ARG = ENABLED | LEAFY_MODE | ONLY_GPR | ONLY_ARG;
+  const unsigned int LEAFY_GPR = ENABLED | LEAFY_MODE | ONLY_GPR;
+  const unsigned int LEAFY_ARG = ENABLED | LEAFY_MODE | ONLY_ARG;
+  const unsigned int LEAFY = ENABLED | LEAFY_MODE;
 }
 
 /* Settings of flag_incremental_link.  */
diff --git a/gcc/function.cc b/gcc/function.cc
index 82102ed78d7e6..7b03f9e744199 100644
--- a/gcc/function.cc
+++ b/gcc/function.cc
@@ -5868,6 +5868,9 @@ gen_call_used_regs_seq (rtx_insn *ret, unsigned int zero_regs_type)
   only_used = zero_regs_type & ONLY_USED;
   only_arg = zero_regs_type & ONLY_ARG;
 
+  if ((zero_regs_type & LEAFY_MODE) && leaf_function_p ())
+    only_used = true;
+
   /* For each of the hard registers, we should zero it if:
 	    1. it is a call-used register;
 	and 2. it is not a fixed register;
diff --git a/gcc/opts.cc b/gcc/opts.cc
index 86b94d62b588c..93c78be8b0d9a 100644
--- a/gcc/opts.cc
+++ b/gcc/opts.cc
@@ -2114,6 +2114,10 @@ const struct zero_call_used_regs_opts_s zero_call_used_regs_opts[] =
   ZERO_CALL_USED_REGS_OPT (all-gpr, zero_regs_flags::ALL_GPR),
   ZERO_CALL_USED_REGS_OPT (all-arg, zero_regs_flags::ALL_ARG),
   ZERO_CALL_USED_REGS_OPT (all, zero_regs_flags::ALL),
+  ZERO_CALL_USED_REGS_OPT (leafy-gpr-arg, zero_regs_flags::LEAFY_GPR_ARG),
+  ZERO_CALL_USED_REGS_OPT (leafy-gpr, zero_regs_flags::LEAFY_GPR),
+  ZERO_CALL_USED_REGS_OPT (leafy-arg, zero_regs_flags::LEAFY_ARG),
+  ZERO_CALL_USED_REGS_OPT (leafy, zero_regs_flags::LEAFY),
 #undef ZERO_CALL_USED_REGS_OPT
   {NULL, 0U}
 };
diff --git a/gcc/testsuite/c-c++-common/zero-scratch-regs-leafy-1.c b/gcc/testsuite/c-c++-common/zero-scratch-regs-leafy-1.c
new file mode 100644
index 0000000000000..c1a0c31ba1c37
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/zero-scratch-regs-leafy-1.c
@@ -0,0 +1,15 @@
+/* { dg-do run } */
+/* { dg-options "-O2 -fzero-call-used-regs=leafy" } */
+
+volatile int result = 0;
+int 
+__attribute__((noipa))
+foo (int x)
+{
+  return x;
+}
+int main()
+{
+  result = foo (2);
+  return 0;
+}
diff --git a/gcc/testsuite/c-c++-common/zero-scratch-regs-leafy-2.c b/gcc/testsuite/c-c++-common/zero-scratch-regs-leafy-2.c
new file mode 100644
index 0000000000000..d450620c1fcfe
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/zero-scratch-regs-leafy-2.c
@@ -0,0 +1,21 @@
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+#include <assert.h>
+int result = 0;
+
+int 
+__attribute__((noipa))
+__attribute__ ((zero_call_used_regs("leafy")))
+foo1 (int x)
+{
+  return (x + 1);
+}
+
+int 
+__attribute__((noipa))
+__attribute__ ((zero_call_used_regs("leafy")))
+foo2 (int x)
+{
+  return foo1 (x + 2);
+}
diff --git a/gcc/testsuite/gcc.target/i386/zero-scratch-regs-leafy-1.c b/gcc/testsuite/gcc.target/i386/zero-scratch-regs-leafy-1.c
new file mode 100644
index 0000000000000..2277710c771b7
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/zero-scratch-regs-leafy-1.c
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fzero-call-used-regs=leafy -fno-stack-protector -fno-PIC" } */
+
+void
+foo (void)
+{
+}
+
+/* { dg-final { scan-assembler-not "vzeroall" } } */
+/* { dg-final { scan-assembler-not "%xmm" } } */
+/* { dg-final { scan-assembler-not "xorl\[ \t\]+%" } } */
+/* { dg-final { scan-assembler-not "movl\[ \t\]+%" } } */
diff --git a/gcc/testsuite/gcc.target/i386/zero-scratch-regs-leafy-2.c b/gcc/testsuite/gcc.target/i386/zero-scratch-regs-leafy-2.c
new file mode 100644
index 0000000000000..24b85c3dbb766
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/zero-scratch-regs-leafy-2.c
@@ -0,0 +1,16 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fzero-call-used-regs=leafy-gpr -fno-stack-protector -fno-PIC" } */
+
+extern int bar (int);
+
+void
+foo (void)
+{
+  int x = bar (0);
+  if (x)
+    bar (1);
+}
+
+/* { dg-final { scan-assembler "xorl\[ \t\]+%eax, %eax" } } */
+/* { dg-final { scan-assembler "xorl\[ \t\]+%edx, %edx" } } */
+/* { dg-final { scan-assembler "xorl\[ \t\]+%ecx, %ecx" } } */


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

  reply	other threads:[~2023-06-16  7:26 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-21  7:31 Alexandre Oliva
2022-10-21 14:26 ` Qing Zhao
2022-10-25  2:48   ` Alexandre Oliva
2022-10-25 15:22     ` Qing Zhao
2022-10-26 21:29       ` Alexandre Oliva
2022-10-27 13:30         ` Qing Zhao
2023-06-16  7:26           ` Alexandre Oliva [this message]
2023-06-16 19:34             ` Qing Zhao
2023-06-22  1:16               ` Alexandre Oliva
2023-06-23 14:47                 ` Qing Zhao
2023-06-23 23:27                   ` [PATCH v3] " Alexandre Oliva
2023-06-26 13:58                     ` Qing Zhao
2023-06-27  6:27                       ` Richard Biener

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=orttv7x3yx.fsf@lxoliva.fsfla.org \
    --to=oliva@adacore.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=qing.zhao@oracle.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).