Qing Zhao <qing.z...@oracle.com> writes:
> @@ -3996,22 +3996,19 @@ with a named @code{target} must be @code{static}.
> @cindex @code{zero_call_used_regs} function attribute
>
> The @code{zero_call_used_regs} attribute causes the compiler to zero
> -a subset of all call-used registers at function return according to
> -@var{choice}.
> -This is used to increase the program security by either mitigating
> -Return-Oriented Programming (ROP) or preventing information leak
> +a subset of all call-used registers@footnote{A ``call-used'' register
> +is a register whose contents can be changed by a function call;
> +therefore, a caller cannot assume that the register has the same contents
> +on return from the function as it had before calling the function. Such
> +registers are also called ``call-clobbered'', ``caller-saved'', or
> +``volatile''.} at function return.
> +This is used to increase program security by either mitigating
> +Return-Oriented Programming (ROP) attacks or preventing information leakage
> through registers.
>
> -A ``call-used'' register is a register whose contents can be changed by
> -a function call; therefore, a caller cannot assume that the register has
> -the same contents on return from the function as it had before calling
> -the function. Such registers are also called ``call-clobbered'',
> -``caller-saved'', or ``volatile''.
> -
> In order to satisfy users with different security needs and control the
> -run-time overhead at the same time, GCC provides a flexible way to choose
> -the subset of the call-used registers to be zeroed.
> -
> +run-time overhead at the same time, @var{choice} parameter provides a
I suggested “the @var{choice} parameter provides” in the review yesterday.
The “the” is needed.
> +flexible way to choose the subset of the call-used registers to be zeroed.
> The three basic values of @var{choice} are:
>
> @itemize @bullet
> @@ -4046,42 +4043,41 @@ together, they must appear in the order above.
>
> The full list of @var{choice}s is therefore:
>
> -@itemize @bullet
> -@item
> -@samp{skip} doesn't zero any call-used register.
> +@table @code
> +@item skip
> +doesn't zero any call-used register.
>
> -@item
> -@samp{used} only zeros call-used registers that are used in the function.
> +@item used
> +only zeros call-used registers that are used in the function.
>
> -@item
> -@samp{all} zeros all call-used registers.
> +@item used-gpr
> +only zeros call-used general purpose registers that are used in the function.
>
> -@item
> -@samp{used-arg} only zeros used call-used registers that pass arguments.
> +@item used-arg
> +only zeros call-used registers that are used in the function and pass
> arguments.
>
> -@item
> -@samp{used-gpr} only zeros used call-used general purpose registers.
> +@item used-gpr-arg
> +only zeros call-used general purpose registers that are used in the function
> +and pass arguments.
>
> -@item
> -@samp{used-gpr-arg} only zeros used call-used general purpose registers that
> -pass arguments.
> +@item all
> +zeros all call-used registers.
>
> -@item
> -@samp{all-gpr-arg} zeros all call-used general purpose registers that pass
> -arguments.
> +@item all-gpr
> +zeros all call-used general purpose registers.
>
> -@item
> -@samp{all-arg} zeros all call-used registers that pass arguments.
> +@item all-arg
> +zeros all call-used registers that pass arguments.
>
> -@item
> -@samp{all-gpr} zeros all call-used general purpose registers.
> -@end itemize
> +@item all-gpr-arg
> +zeros all call-used general purpose registers that pass
> +arguments.
> +@end table
TBH I also think the order I suggested yesterday is more natural
than this one, but either's OK. The above certainly addresses
the original concern I had about the order being inconsistent.
> @@ -288,7 +288,7 @@ enum sanitize_code {
> };
>
> /* Different settings for zeroing subset of registers. */
> -namespace zero_regs_code {
> +namespace zero_regs_flag {
I suggested “zero_regs_flags” rather than “zero_reg_flag” yesterday;
I think “zero_regs_flags” is better because the namespace contains
more than one flag.
> @@ -1776,7 +1776,7 @@ const struct sanitizer_opts_s coverage_sanitizer_opts[]
> =
> { NULL, 0U, 0UL, false }
> };
>
> -using namespace zero_regs_code;
> +using namespace zero_regs_flag;
> /* -fzero-call-used-regs= suboptions. */
> const struct zero_call_used_regs_opts_s zero_call_used_regs_opts[] =
> {
Sorry, I didn't notice this yesterday. The table should use
fully-qualified names: zero_regs_flags::SKIP etc. We shouldn't
do a using namespace for the whole file here.
OK with those changes, and thanks for doing this.
The new tests are likely to fail on some targets with the sorry()
message, but I think target maintainers are best placed to decide
whether (a) that's a fundamental restriction of the target and the
tests should just be skipped or (b) the target needs to implement
the new hook.
Richard
