On 5/4/20 10:51 AM, Qing Zhao via Gcc-patches wrote:
Hi,
This is a PING for this patch for gcc11 stage 1.
https://gcc.gnu.org/pipermail/gcc-patches/2020-April/544058.html
<https://gcc.gnu.org/pipermail/gcc-patches/2020-April/544058.html>
Please take a look on it.
Just a couple of minor points:
I noticed a recurring mistake in the spelling of the verb "zeroes"
in the text added to the manual where the added documentation uses
"zeros:" the latter is the plural of the noun zero. "zeroes" is
the third person of the verb to zero.
Another minor (and common) mistake is using the word "which" where
"that" would be appropriate. In the sentence below (and others like
it):
+... @samp{skip}, which is the default, doesn't zero
+caller-saved registers. @samp{used-gpr} zeros caller-saved integer
+registers which are used in function.
the first which is correct (it just adds more detail about skip) but
the second should be "that" because it identifies the specific subset
of registers to which the description applies.
As a matter of style of diagnostics, attribute arguments should be
quoted. In the hunk below:
@@ -40787,6 +41064,29 @@ ix86_handle_fndecl_attribute (tree *node, tree
name, tree args, int,
}
}
+ if (is_attribute_p ("zero_caller_saved_regs", name))
+ {
+ tree cst = TREE_VALUE (args);
+ if (TREE_CODE (cst) != STRING_CST)
+ {
+ warning (OPT_Wattributes,
+ "%qE attribute requires a string constant argument",
+ name);
+ *no_add_attrs = true;
+ }
+ else if (strcmp (TREE_STRING_POINTER (cst), "skip") != 0
+ && strcmp (TREE_STRING_POINTER (cst), "used-gpr") != 0
+ && strcmp (TREE_STRING_POINTER (cst), "all-gpr") != 0
+ && strcmp (TREE_STRING_POINTER (cst), "used") != 0
+ && strcmp (TREE_STRING_POINTER (cst), "all") != 0)
+ {
+ warning (OPT_Wattributes,
+ "argument to %qE attribute is not
(skip|used-gpr|all-gpr|used|all)",
+ name);
I'd suggest to follow the style in gcc/opts.c rather than the existing
examples in the i386 back end. Something like this would be better:
+ warning (OPT_Wattributes,
+ "argument to %qE attribute is not one of %qs",
+ "(skip|used-gpr|all-gpr|used|all)",
+ name);
(There are other examples as well that use different formats still;
it would be nice to pick one style and use it throughout.)
Also, the function issues -Wattributes for invalid forms of
the attribute. While incorrect forms of known attributes aren't
handled completely consistently, -Wattributes is documented to be
issued either for unknown attributes or known attributes used in
unexpected contexts, and as distinct from the errors issued for
incorrect forms of known attributes.
Besides more closely matching the documentation, it seems to me
that either issuing an error or a distinct (yet to be invented)
warning would be preferable to lumping all these sorts of problems
in -Wattributes. As above, choosing the most suitable response
and adopting it consistently throughout would be helpful.
Martin
Thanks.
Qing
Begin forwarded message:
From: Qing Zhao via Gcc-patches <gcc-patches@gcc.gnu.org>
Subject: [PATCH][x86][1/3]: Add
-mzero-caller-saved-regs=[skip|used-gpr|all-gpr|used|all]
Date: April 17, 2020 at 12:31:45 PM CDT
To: richard Biener <richard.guent...@gmail.com>, fwei...@redhat.com,
dal...@libc.org
Cc: gcc-patches@gcc.gnu.org, keesc...@chromium.org
Reply-To: Qing Zhao <qing.z...@oracle.com>
Hi,
This is a PING for an old patch proposed by H. J. Lu on Oct, 2018:
https://gcc.gnu.org/legacy-ml/gcc-patches/2018-10/msg02079.html
This is the first patch of the total 3 patches set, which provides the
following new feature:
-mzero-caller-saved-regs=[skip|used-gpr|all-gpr|used|all] command-line
option and zero_caller_saved_regs("skip|used|all") function attribue:
1. -mzero-caller-saved-regs=skip and zero_caller_saved_regs("skip")
Don't zero caller-saved registers upon function return.
2. -mzero-caller-saved-regs=used-gpr and zero_caller_saved_regs("used-gpr")
Zero used caller-saved integer registers upon function return.
3. -mzero-caller-saved-regs=all-gpr and zero_caller_saved_regs("all-gpr”)
Zero all caller-saved integer registers upon function return.
4. -mzero-caller-saved-regs=used and zero_caller_saved_regs("used")
Zero used caller-saved integer and vector registers upon function return.
5. -mzero-caller-saved-regs=all and zero_caller_saved_regs("all")
Zero all caller-saved integer and vector registers upon function return.
This feature is needed by Linux kernel security improvement. Please refer to
Kees Cook’s talk on Linux Plumber Conference 2019:
https://outflux.net/slides/2019/lpc/gcc-and-clang.pdf
<https://outflux.net/slides/2019/lpc/gcc-and-clang.pdf>
Tested on x86-64 with bootstrapping GCC trunk, regression tests exposed
several new regressions, these new regressions are
fixed by 2 following patches I will send in next two emails.
Any comment?
thanks.
Qing
