Re: 0002-Part-2.-Document-finstrument-control-flow-and-notrack attribute

[Sandra Loosemore <sandra at codesourcery dot com>]

On 09/19/2017 07:45 AM, Tsimbalist, Igor V wrote:
> Here is an updated patch (version #2). Mainly attribute and option  names were changed.
>
> gcc/doc/
> 	* extend.texi: Add 'nocf_check' documentation.
> 	* gimple.texi: Add second parameter to gimple_build_call_from_tree.
> 	* invoke.texi: Add -fcf-protection documentation.
> 	* rtl.texi: Add REG_CALL_NOTRACK documenation.
>
> Is it ok for trunk?
> diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
> index cd5733e..6bdb183 100644
> --- a/gcc/doc/extend.texi
> +++ b/gcc/doc/extend.texi
> @@ -5646,6 +5646,56 @@ Specify which floating-point unit to use.  You must specify the
>  @code{target("fpmath=sse,387")} option as
>  @code{target("fpmath=sse+387")} because the comma would separate
>  different options.
> +
> +@item nocf_check
> +@cindex @code{nocf_check} function attribute
> +The @code{nocf_check} attribute on a function is used to inform the
> +compiler that the function's prolog should not be instrumented when

s/prolog/prologue/

> +compiled with the @option{-fcf-protection=branch} option.  The
> +compiler assumes that the function's address is a valid target for a
> +control-flow transfer.
> +
> +The @code{nocf_check} attribute on a type of pointer to function is
> +used to inform the compiler that a call through the pointer should
> +not be instrumented when compiled with the
> +@option{-fcf-protection=branch} option.  The compiler assumes
> +that the function's address from the pointer is a valid target for
> +a control-flow transfer.  A direct function call through a function
> +name is assumed as a safe call thus direct calls will not be

...is assumed to be a safe call, thus direct calls are not...

> +instrumented by the compiler.
> +
> +The @code{nocf_check} attribute is applied to an object's type.  A
> +The @code{nocf_check} attribute is transfered to a call instruction at
> +the GIMPLE and RTL translation phases.  The attribute is not propagated
> +through assignment, store and load.

extend.texi is user-facing documentation, but the second sentence here 
is implementor-speak and not meaningful to users of GCC.  I don't 
understand what the third sentence is trying to say.

> +
> +@smallexample
> +@{
> +int foo (void) __attribute__(nocf_check);
> +void (*foo1)(void) __attribute__(nocf_check);
> +void (*foo2)(void);
> +
> +int
> +foo (void) /* The function's address is assumed as valid.  */

s/as valid/to be valid/

> +
> +  /* This call site is not checked for control-flow validness.  */

s/validness/validity/g

> +  (*foo1)();
> +
> +  foo1 = foo2;
> +  /* This call site is still not checked for control-flow validness.  */
> +  (*foo1)();
> +
> +  /* This call site is checked for control-flow validness.  */
> +  (*foo2)();
> +
> +  foo2 = foo1;
> +  /* This call site is still checked for control-flow validness.  */
> +  (*foo2)();
> +
> +  return 0;
> +@}
> +@end smallexample
> +
>  @end table
>
>  On the x86, the inliner does not inline a
> diff --git a/gcc/doc/gimple.texi b/gcc/doc/gimple.texi
> index 635abd3..b6d9149 100644
> --- a/gcc/doc/gimple.texi
> +++ b/gcc/doc/gimple.texi
> @@ -1310,9 +1310,11 @@ operand is validated with @code{is_gimple_operand}).
>  @end deftypefn
>
>
> -@deftypefn {GIMPLE function} gcall *gimple_build_call_from_tree (tree call_expr)
> -Build a @code{GIMPLE_CALL} from a @code{CALL_EXPR} node.  The arguments and the
> -function are taken from the expression directly.  This routine
> +@deftypefn {GIMPLE function} gcall *gimple_build_call_from_tree (tree call_expr, @
> +tree fnptrtype)
> +Build a @code{GIMPLE_CALL} from a @code{CALL_EXPR} node.  The arguments and
> +the function are taken from the expression directly.  The type is set from
> +the second parameter passed by a caller.  This routine
>  assumes that @code{call_expr} is already in GIMPLE form.  That is, its
>  operands are GIMPLE values and the function call needs no further
>  simplification.  All the call flags in @code{call_expr} are copied over
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index e4cacf2..578bc25 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -460,6 +460,7 @@ Objective-C and Objective-C++ Dialects}.
>  -fchkp-check-read  -fchkp-check-write  -fchkp-store-bounds @gol
>  -fchkp-instrument-calls  -fchkp-instrument-marked-only @gol
>  -fchkp-use-wrappers  -fchkp-flexible-struct-trailing-arrays@gol
> +-fcf-protection=@r{[}@var{full}|@var{branch}|@var{return}|@var{none}@r{]} @gol

Are full/branch/return/none supposed to be literal strings?  @var is the 
wrong markup for that.

>  -fstack-protector  -fstack-protector-all  -fstack-protector-strong @gol
>  -fstack-protector-explicit  -fstack-check @gol
>  -fstack-limit-register=@var{reg}  -fstack-limit-symbol=@var{sym} @gol
> @@ -11348,6 +11349,35 @@ is used to link a program, the GCC driver automatically links
>  against @file{libmpxwrappers}.  See also @option{-static-libmpxwrappers}.
>  Enabled by default.
>
> +@item -fcf-protection=@r{[}@var{full}|@var{branch}|@var{return}|@var{none}@r{]}

Again, markup?

> +@opindex fcf-protection
> +Enable code instrumentation of control-flow transfers to increase
> +a program security by checking a target address of control-flow

s/a program/program/
s/checking a target address/checking that target addresses/

> +transfer instructions (such as indirect function call, function return,
> +indirect jump) are valid targets.  This prevents diverting the control

s/are valid targets/are valid/

> +flow instructions from its original target address to a new undesigned

s/a new undesigned/an unintended/ ??
(not sure what you're trying to say here).

> +target.  This is intended to protect against such theats as

s/theats/threats

> +Return-oriented Programming (ROP), and similarly call/jmp-oriented
> +programming (COP/JOP).
> +
> +Each compiler, which will support the control-flow instrumentation,
> +is supposed to have its own target specific implementation of the
> +control-flow instrumentation and in case of absence of such
> +implementation the usage of @option{-fcf-protection} will
> +cause an error message.

That whole paragraph is very long-winded.  Are you trying to say here

Currently GCC only supports this option on [...] targets.

??

> +
> +The value @var{branch} tells the compiler to implement checking of

Again, wrong markup?  If it's a literal, use @code{branch}.

> +validness of control-flow trasfer for at the point of indirect branch

s/validness/validity/
s/trasfer for/transfer/

> +instructions, i.e. call/jmp instructions.  The value @var{return}

Again, wrong use of @var markup?

> +implements checking of validness at the point of returning from a
> +function.  The value @var{full} is an alias for specifying both
> +'branch' and 'return'. The value @var{none} turns off instrumentation.

Here too.  And do not use literal quotes for markup.

> +This value may be used for those architectures where @option{-fcf-protection}
> +is switched on by default.

Which architectures are those?

> +A user also has a control through the @code{nocf_check} attribute to identify

Users are the readers of this document and should be addressed directly:

You can also use the @code{nocf_check} attribute....

> +which functions and calls should be skipped from instrumentation.
> +

Please add a cross-reference here.

>  @item -fstack-protector
>  @opindex fstack-protector
>  Emit extra code to check for buffer overflows, such as stack smashing
> diff --git a/gcc/doc/rtl.texi b/gcc/doc/rtl.texi
> index 12355c2..f023067 100644
> --- a/gcc/doc/rtl.texi
> +++ b/gcc/doc/rtl.texi
> @@ -4040,6 +4040,21 @@ is used in place of the actual insn pattern.  This is done in cases where
>  the pattern is either complex or misleading.
>  @end table
>
> +The note @code{REG_CALL_NOCF_CHECK} describe the information connected to
> +the code instrumentation which is done when @code{-fcf-protection=branch}
> +option is specified.

Hmmm, how about rewriting this as

The note @code{REG_CALL_NOCF_CHECK} is used in conjunction with the
@option{-fcf-protection=branch} option.

> The note is set if a @code{nocf_check} attribute is
> +specified.  The note is stored in the @code{REG_NOTES} field of an insn.

> +@table @code
> +@findex REG_CALL_NOCF_CHECK
> +@item REG_CALL_NOCF_CHECK
> +A user has a control through the @code{nocf_check} attribute to identify
> +which call to a function should be skipped from control-flow instrumentation
> +when the option @code{-fcf-protection=branch} is specified.  The compiler

@option markup instead of @code on options, please.

> +puts a @samp{REG_CALL_NO_VERIFY} note on @samp{CALL_INSN} instruction,
> +which has a function type marked with a @code{nocf_check} attribute.
> +@end table
> +
>  For convenience, the machine mode in an @code{insn_list} or
>  @code{expr_list} is printed using these symbolic codes in debugging dumps.
>
> --
> 1.8.3.1

-Sandra