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

[Tsimbalist, Igor V]

> -Original Message-
> From: Sandra Loosemore [mailto:san...@codesourcery.com]
> Sent: Friday, September 29, 2017 6:57 AM
> To: Tsimbalist, Igor V <igor.v.tsimbal...@intel.com>; 'gcc-
> patc...@gcc.gnu.org' <gcc-patches@gcc.gnu.org>
> Cc: Jeff Law <l...@redhat.com>
> Subject: Re: 0002-Part-2.-Document-finstrument-control-flow-and-notrack
> attribute
> 
> On 09/27/2017 06:27 AM, Tsimbalist, Igor V wrote:
> > Updated version #4.
> >
> > [snip]
> > @@ -11348,6 +11349,31 @@ 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{[}full@r{|}branch@r{|}return@r{|}none@r{]}
> > +@opindex fcf-protection
> > +Enable code instrumentation of control-flow transfers to increase
> > +program security by checking that target addresses of control-flow
> > +transfer instructions (such as indirect function call, function
> > +return, indirect jump) are valid.  This prevents diverting the flow
> > +of control to an unexpected target.  This is intended to protect
> > +against such threats as Return-oriented Programming (ROP), and
> > +similarly call/jmp-oriented programming (COP/JOP).
> > +
> > +For all targets, which do not support the @option{-fcf-protection}
> > +option, the option usage results in an error message.
> 
> Please take this sentence out.  It's ungrammatical and verbose and
> unnecessary.

Removed.

> Note that several of the other options described in this section are not
> enabled on all targets either.  E.g., I've just been looking at fixing the 
> nios2
> backend to make -fstack-protector work, and there is nothing in the manual
> to say that GCC issues an error if there's no target support, even though
> that's what it does.
> 
> The patch is OK to commit with that change.

Thanks,
Igor

> -Sandra

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

[Sandra Loosemore]

On 09/27/2017 06:27 AM, Tsimbalist, Igor V wrote:

Updated version #4.

[snip]
@@ -11348,6 +11349,31 @@ 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{[}full@r{|}branch@r{|}return@r{|}none@r{]}
+@opindex fcf-protection
+Enable code instrumentation of control-flow transfers to increase
+program security by checking that target addresses of control-flow
+transfer instructions (such as indirect function call, function return,
+indirect jump) are valid.  This prevents diverting the flow of control
+to an unexpected target.  This is intended to protect against such
+threats as Return-oriented Programming (ROP), and similarly
+call/jmp-oriented programming (COP/JOP).
+
+For all targets, which do not support the @option{-fcf-protection}
+option, the option usage results in an error message.


Please take this sentence out.  It's ungrammatical and verbose and 
unnecessary.


Note that several of the other options described in this section are not 
enabled on all targets either.  E.g., I've just been looking at fixing 
the nios2 backend to make -fstack-protector work, and there is nothing 
in the manual to say that GCC issues an error if there's no target 
support, even though that's what it does.


The patch is OK to commit with that change.

-Sandra

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

[Jeff Law]

On 09/27/2017 06:27 AM, Tsimbalist, Igor V wrote:

> 
> 
> 0002-Add-documentation-for-fcf-protection-option-and-nocf.patch
> 
> 
> From bc896670fef5eb7324c0e0134747696f3ed66553 Mon Sep 17 00:00:00 2001
> From: Igor Tsimbalist 
> Date: Sun, 17 Sep 2017 14:57:29 +0300
> Subject: [PATCH 2/5] Add documentation for fcf-protection option and
>  nocf_check attribute
> 
> 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.
s/documenation/documentation

Otherwise this is fine once Sandra gives her OK.

jeff

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

[Tsimbalist, Igor V]

Updated version #4.

> -Original Message-
> From: Sandra Loosemore [mailto:san...@codesourcery.com]
> Sent: Wednesday, September 27, 2017 5:11 AM
> To: Tsimbalist, Igor V <igor.v.tsimbal...@intel.com>; 'gcc-
> patc...@gcc.gnu.org' <gcc-patches@gcc.gnu.org>
> Cc: Jeff Law <l...@redhat.com>
> Subject: Re: 0002-Part-2.-Document-finstrument-control-flow-and-notrack
> attribute
> 
> On 09/26/2017 07:45 AM, Tsimbalist, Igor V wrote:
> > Here is the updated version (version#3). All comments below are fixed.
> 
> This still needs more work.  Specific comments below:
> 
> > +The @code{nocf_check} attribute is applied to an object's type.
> > +In case of assignment of a function address or a function pointer to
> > +another pointer, the attribute is not carried over from the
> > +right-hand object's type, the type of left-hand object stays
> > +unchanged.  The
> 
> s/object's type,/object's type;/


Fixed.

> > @@ -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{[}full@r{|}branch@r{|}return@r{|}none@r{]}
> > +@opindex fcf-protection
> > +Enable code instrumentation of control-flow transfers to increase
> > +program security by checking that target addresses of control-flow
> > +transfer instructions (such as indirect function call, function
> > +return, indirect jump) are valid.  This prevents diverting the
> > +control flow instructions from its original target address to a new
> > +undesigned
> 
> s/control flow instructions/control-flow instructions/
> 
> I'd rewrite the next sentence as
> 
> This prevents diverting the flow of control to an unexpected target.

I used your suggestion.

> > +target.  This is intended to protect against such threats as
> > +Return-oriented Programming (ROP), and similarly call/jmp-oriented
> > +programming (COP/JOP).
> > +
> > +Each compiler target, which is going to support the control-flow
> > +instrumentation, is supposed to have its own target specific
> > +implementation. For all targets where an implementation is absent the
> > +usage of @option{-fcf-protection} option causes an error message.
> 
> I would really prefer that you list the targets this works on here instead.

Another patch you are reviewing now (its name starts with 0005-Part-5)
has the statement you would like to put here. The important point here is
an error issuing. When I commit the first patch none of target platforms 
supports
the option and an error is printed when the option is specified. I removed the
first sentence but keep the second one:

For all targets, which do not support the @option{-fcf-protection}
option, the option usage results in an error message.

> > +The value @code{branch} tells the compiler to implement checking of
> > +validity of control-flow transfer at the point of indirect branch
> > +instructions, i.e. call/jmp instructions.  The value @code{return}
> > +implements checking of validity at the point of returning from a
> > +function.  The value @code{full} is an alias for specifying both
> > +@code{branch} and @code{return}. The value @code{none} turns off
> > +instrumentation.  This value may be used for future architectures
> > +where @option{-fcf-protection} option is switched on by default.
> 
> I don't think we need to document GCC's future behavior for future
> architectures (I'm always going around removing useless discussion from
> 20 years ago of possible extensions that never got implemented).  I assume
> that this is just provided for completeness and to override a previous -fcf-
> protection option on the command line.

Ok, removed the last sentence.

> > +You can also use the @code{nocf_check} attribute to identify which
> > +functions and calls should be skipped from instrumentation
> > +(@pxref{Function Attributes}).
> > +
> >  @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..b4fc5f3 100644
> > --- a/gcc/doc/rtl.texi
> > +++ b/gcc/doc/rtl.texi
> > @@ -4040,6 +4040,22 @@ 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} is used in conjunction with
> the
> > +@option{-fcf-protection=branch} option.  The note is set if a
> > +@code{nocf_check} attribute is specified for a funct

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

[Sandra Loosemore]

On 09/26/2017 07:45 AM, Tsimbalist, Igor V wrote:

Here is the updated version (version#3). All comments below are fixed.


This still needs more work.  Specific comments below:


+The @code{nocf_check} attribute is applied to an object's type.
+In case of assignment of a function address or a function pointer to
+another pointer, the attribute is not carried over from the right-hand
+object's type, the type of left-hand object stays unchanged.  The


s/object's type,/object's type;/


@@ -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{[}full@r{|}branch@r{|}return@r{|}none@r{]}
+@opindex fcf-protection
+Enable code instrumentation of control-flow transfers to increase
+program security by checking that target addresses of control-flow
+transfer instructions (such as indirect function call, function return,
+indirect jump) are valid.  This prevents diverting the control
+flow instructions from its original target address to a new undesigned


s/control flow instructions/control-flow instructions/

I'd rewrite the next sentence as

This prevents diverting the flow of control to an unexpected target.


+target.  This is intended to protect against such threats as
+Return-oriented Programming (ROP), and similarly call/jmp-oriented
+programming (COP/JOP).
+
+Each compiler target, which is going to support the control-flow
+instrumentation, is supposed to have its own target specific
+implementation. For all targets where an implementation is absent the
+usage of @option{-fcf-protection} option causes an error message.


I would really prefer that you list the targets this works on here instead.


+The value @code{branch} tells the compiler to implement checking of
+validity of control-flow transfer at the point of indirect branch
+instructions, i.e. call/jmp instructions.  The value @code{return}
+implements checking of validity at the point of returning from a
+function.  The value @code{full} is an alias for specifying both
+@code{branch} and @code{return}. The value @code{none} turns off
+instrumentation.  This value may be used for future architectures
+where @option{-fcf-protection} option is switched on by default.


I don't think we need to document GCC's future behavior for future 
architectures (I'm always going around removing useless discussion from 
20 years ago of possible extensions that never got implemented).  I 
assume that this is just provided for completeness and to override a 
previous -fcf-protection option on the command line.



+You can also use the @code{nocf_check} attribute to identify
+which functions and calls should be skipped from instrumentation
+(@pxref{Function Attributes}).
+
 @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..b4fc5f3 100644
--- a/gcc/doc/rtl.texi
+++ b/gcc/doc/rtl.texi
@@ -4040,6 +4040,22 @@ 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} is used in conjunction with the
+@option{-fcf-protection=branch} option.  The note is set if a
+@code{nocf_check} attribute is specified for a function type or a
+pointer to function type.  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


S/A user has a control/Users have control/


+which call to a function should be skipped from control-flow instrumentation


s/call/calls/


+when the option @option{-fcf-protection=branch} is specified.  The compiler
+puts a @code{REG_CALL_NOCF_CHECK} note on @code{CALL_INSN} instruction,
+which has a function type marked with a @code{nocf_check} attribute.


s/@code{CALL_INSN} instruction, which/each @code{CALL_INSN} instruction 
that/


-Sandra

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

[Tsimbalist, Igor V]

Here is the updated version (version#3). All comments below are fixed.

Igor


> -Original Message-
> From: Tsimbalist, Igor V
> Sent: Monday, September 25, 2017 11:57 PM
> To: Sandra Loosemore <san...@codesourcery.com>; 'gcc-
> patc...@gcc.gnu.org' <gcc-patches@gcc.gnu.org>
> Cc: Jeff Law <l...@redhat.com>; Tsimbalist, Igor V
> <igor.v.tsimbal...@intel.com>
> Subject: RE: 0002-Part-2.-Document-finstrument-control-flow-and-notrack
> attribute
> 
> > -Original Message-
> > From: Sandra Loosemore [mailto:san...@codesourcery.com]
> > Sent: Monday, September 25, 2017 5:07 AM
> > To: Tsimbalist, Igor V <igor.v.tsimbal...@intel.com>; 'gcc-
> > patc...@gcc.gnu.org' <gcc-patches@gcc.gnu.org>
> > Cc: Jeff Law <l...@redhat.com>
> > Subject: Re:
> > 0002-Part-2.-Document-finstrument-control-flow-and-notrack
> > attribute
> >
> > 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/
> 
> Fixed.
> 
> > > +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...
> 
> Fixed.
> 
> > > +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.
> 
> The second sentence is removed. The third sentence is re-written as
> 
> In case of assignment of a function address or a function pointer to another
> pointer, the attribute is not carried over from the right-hand object's type,
> the type of left-hand object stays unchanged.  The compiler checks for
> @code{nocf_check} attribute mismatch and reports a warning in case of
> mismatch.
> 
> > > +
> > > +@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/
> 
> Fixed.
> 
> > > +
> > > +  /* This call site is not checked for control-flow validness.  */
> >
> > s/validness/validity/g
> 
> Fixed.
> 
> > > +  (*foo1)();
> > > +
> > > +  foo1 = foo2;
> > > +  /* This call site is still not checked for control-flow validness.

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

[Tsimbalist, Igor V]

> -Original Message-
> From: Sandra Loosemore [mailto:san...@codesourcery.com]
> Sent: Monday, September 25, 2017 5:07 AM
> To: Tsimbalist, Igor V <igor.v.tsimbal...@intel.com>; 'gcc-
> patc...@gcc.gnu.org' <gcc-patches@gcc.gnu.org>
> Cc: Jeff Law <l...@redhat.com>
> Subject: Re: 0002-Part-2.-Document-finstrument-control-flow-and-notrack
> attribute
> 
> 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/

Fixed.

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

Fixed.

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

The second sentence is removed. The third sentence is re-written as

In case of assignment of a function address or a function pointer to
another pointer, the attribute is not carried over from the right-hand
object's type, the type of left-hand object stays unchanged.  The
compiler checks for @code{nocf_check} attribute mismatch and reports
a warning in case of mismatch.

> > +
> > +@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/

Fixed.

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

Fixed.

> > +  (*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
> > dire

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

[Sandra Loosemore]

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 

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

[Tsimbalist, Igor V]

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?

Thanks,
Igor


> -Original Message-
> From: Tsimbalist, Igor V
> Sent: Friday, September 15, 2017 5:14 PM
> To: 'Jeff Law' <l...@redhat.com>; 'gcc-patches@gcc.gnu.org'  patc...@gcc.gnu.org>
> Cc: Tsimbalist, Igor V <igor.v.tsimbal...@intel.com>
> Subject: RE: 0002-Part-2.-Document-finstrument-control-flow-and-notrack
> attribute
> 
> > -Original Message-
> > From: Jeff Law [mailto:l...@redhat.com]
> > Sent: Friday, August 25, 2017 10:59 PM
> > To: Tsimbalist, Igor V <igor.v.tsimbal...@intel.com>; 'gcc-
> > patc...@gcc.gnu.org' <gcc-patches@gcc.gnu.org>
> > Subject: Re:
> > 0002-Part-2.-Document-finstrument-control-flow-and-notrack
> > attribute
> >
> > On 08/01/2017 02:56 AM, Tsimbalist, Igor V wrote:
> > > Part#2. Document -finstrument-control-flow and notrack attribute.
> > >
> > >
> > > 0002-Part-2.-Document-finstrument-control-flow-and-notrac.patch
> > >
> > >
> > > From c3e45c80731672e74d638f787e80ba975279b9b9 Mon Sep 17 00:00:00
> > 2001
> > > From: Igor Tsimbalist <igor.v.tsimbal...@intel.com>
> > > Date: Mon, 3 Jul 2017 17:12:49 +0300
> > > Subject: [PATCH 2/9] Part#2. Document -finstrument-control-flow and
> > > notrack  attribute.
> > >
> > > gcc/
> > >   * doc/extend.texi: Add 'notrack' documentation.
> > >   * doc/invoke.texi: Add -finstrument-control-flow documentation.
> > >   * doc/rtl.texi: Add REG_CALL_NOTRACK documenation.
> > > ---
> > >  gcc/doc/extend.texi | 52
> > > 
> > >  gcc/doc/invoke.texi | 22 ++
> > >  gcc/doc/rtl.texi| 15 +++
> > >  3 files changed, 89 insertions(+)
> > >
> > > diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi index
> > > 6934b4c..80de8a7 100644
> > > --- a/gcc/doc/extend.texi
> > > +++ b/gcc/doc/extend.texi
> > > @@ -5632,6 +5632,58 @@ 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 notrack
> > > +@cindex @code{notrack} function attribute The @code{notrack}
> > > +attribute on a function is used to inform the compiler that the
> > > +function's prolog should not be instrumented when compiled with the
> > > +@option{-finstrument-control-flow} option.  The compiler assumes
> > > +that the function's address is a valid target for a control-flow 
> > > transfer.
> > Is the default to instrument everything when -finstrument-control-flow
> > is enabled?  Or can we avoid instrumentation on a function that never
> > has its address taken (ie, it is only called via a call instruction?)
> The instrumentation is on by default but for all platform except of x86 it 
> does
> nothing as the implementation is not supported. For x86 the implementation
> is lightweight and just increase a bit code size due to 'endbranch' 
> instruction.
> 
> Given a function decl is there an information already available if an address
> was taken from the function? I plan to do what you suggested later as an
> optimization especially for global function where ipa is required.
> 
> > > +
> > > +The @code{notrack} 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{-finstrument-control-flow} 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 save call thus direct calls will not be
> > > +instrumented by the compiler.
> > s/save/safe/
> >
> > FWIW, I think putting the attribute into in the type system is a good
> > thing :-)
> >
> > > +
> > > +The @code{notrack} attribute is applied to an object's type.  A The
> > > +@code{notrack} attribute is transfered to a call instruction at the
> > >

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

[Tsimbalist, Igor V]

> -Original Message-
> From: Jeff Law [mailto:l...@redhat.com]
> Sent: Friday, August 25, 2017 10:59 PM
> To: Tsimbalist, Igor V <igor.v.tsimbal...@intel.com>; 'gcc-
> patc...@gcc.gnu.org' <gcc-patches@gcc.gnu.org>
> Subject: Re: 0002-Part-2.-Document-finstrument-control-flow-and-notrack
> attribute
> 
> On 08/01/2017 02:56 AM, Tsimbalist, Igor V wrote:
> > Part#2. Document -finstrument-control-flow and notrack attribute.
> >
> >
> > 0002-Part-2.-Document-finstrument-control-flow-and-notrac.patch
> >
> >
> > From c3e45c80731672e74d638f787e80ba975279b9b9 Mon Sep 17 00:00:00
> 2001
> > From: Igor Tsimbalist <igor.v.tsimbal...@intel.com>
> > Date: Mon, 3 Jul 2017 17:12:49 +0300
> > Subject: [PATCH 2/9] Part#2. Document -finstrument-control-flow and
> > notrack  attribute.
> >
> > gcc/
> > * doc/extend.texi: Add 'notrack' documentation.
> > * doc/invoke.texi: Add -finstrument-control-flow documentation.
> > * doc/rtl.texi: Add REG_CALL_NOTRACK documenation.
> > ---
> >  gcc/doc/extend.texi | 52
> > 
> >  gcc/doc/invoke.texi | 22 ++
> >  gcc/doc/rtl.texi| 15 +++
> >  3 files changed, 89 insertions(+)
> >
> > diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi index
> > 6934b4c..80de8a7 100644
> > --- a/gcc/doc/extend.texi
> > +++ b/gcc/doc/extend.texi
> > @@ -5632,6 +5632,58 @@ 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 notrack
> > +@cindex @code{notrack} function attribute The @code{notrack}
> > +attribute on a function is used to inform the compiler that the
> > +function's prolog should not be instrumented when compiled with the
> > +@option{-finstrument-control-flow} option.  The compiler assumes that
> > +the function's address is a valid target for a control-flow transfer.
> Is the default to instrument everything when -finstrument-control-flow is
> enabled?  Or can we avoid instrumentation on a function that never has its
> address taken (ie, it is only called via a call instruction?)
The instrumentation is on by default but for all platform except of x86 it does 
nothing as
the implementation is not supported. For x86 the implementation is lightweight 
and just
increase a bit code size due to 'endbranch' instruction.

Given a function decl is there an information already available if an address 
was taken from
the function? I plan to do what you suggested later as an optimization 
especially for global
function where ipa is required. 

> > +
> > +The @code{notrack} 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{-finstrument-control-flow} 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 save call thus direct calls will not be
> > +instrumented by the compiler.
> s/save/safe/
> 
> FWIW, I think putting the attribute into in the type system is a good thing 
> :-)
> 
> > +
> > +The @code{notrack} attribute is applied to an object's type.  A The
> > +@code{notrack} attribute is transfered to a call instruction at the
> > +GIMPLE and RTL translation phases.  The attribute is not propagated
> > +through assignment, store and load.
> > +
> > +@smallexample
> > +@{
> > +void (*foo)(void) __attribute__(notrack); void (*foo1)(void)
> > +__attribute__(notrack); void (*foo2)(void);
> > +
> > +int
> > +foo (void) /* The function's address is not tracked.  */
> > +
> > +  /* This call site is not tracked for
> > + control-flow instrumentation.  */  (*foo1)();
> > +  foo1 = foo2;
> > +  /* This call site is still not tracked for
> > + control-flow instrumentation.  */  (*foo1)();
> > +
> > +  /* This call site is tracked for
> > + control-flow instrumentation.  */  (*foo2)();
> > +  foo2 = foo1;
> > +  /* This call site is still tracked for
> > + control-flow instrumentation.  */  (*foo2)();
> > +
> > +  return 0;
> > +@}
> > +@end smallexample
> Given the notrack attribute is part of the type system, could we issue a
> warning on the foo1 = foo2 assignment since we're discarding tracking 

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

[Jeff Law]

On 08/01/2017 02:56 AM, Tsimbalist, Igor V wrote:
> Part#2. Document -finstrument-control-flow and notrack attribute.
> 
> 
> 0002-Part-2.-Document-finstrument-control-flow-and-notrac.patch
> 
> 
> From c3e45c80731672e74d638f787e80ba975279b9b9 Mon Sep 17 00:00:00 2001
> From: Igor Tsimbalist <igor.v.tsimbal...@intel.com>
> Date: Mon, 3 Jul 2017 17:12:49 +0300
> Subject: [PATCH 2/9] Part#2. Document -finstrument-control-flow and notrack
>  attribute.
> 
> gcc/
>   * doc/extend.texi: Add 'notrack' documentation.
>   * doc/invoke.texi: Add -finstrument-control-flow documentation.
>   * doc/rtl.texi: Add REG_CALL_NOTRACK documenation.
> ---
>  gcc/doc/extend.texi | 52 
>  gcc/doc/invoke.texi | 22 ++
>  gcc/doc/rtl.texi| 15 +++
>  3 files changed, 89 insertions(+)
> 
> diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
> index 6934b4c..80de8a7 100644
> --- a/gcc/doc/extend.texi
> +++ b/gcc/doc/extend.texi
> @@ -5632,6 +5632,58 @@ 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 notrack
> +@cindex @code{notrack} function attribute
> +The @code{notrack} attribute on a function is used to inform the
> +compiler that the function's prolog should not be instrumented when
> +compiled with the @option{-finstrument-control-flow} option.  The
> +compiler assumes that the function's address is a valid target for a
> +control-flow transfer.
Is the default to instrument everything when -finstrument-control-flow
is enabled?  Or can we avoid instrumentation on a function that never
has its address taken (ie, it is only called via a call instruction?)

> +
> +The @code{notrack} 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{-finstrument-control-flow} 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 save call thus direct calls will not be
> +instrumented by the compiler.
s/save/safe/

FWIW, I think putting the attribute into in the type system is a good
thing :-)

> +
> +The @code{notrack} attribute is applied to an object's type.  A
> +The @code{notrack} attribute is transfered to a call instruction at
> +the GIMPLE and RTL translation phases.  The attribute is not propagated
> +through assignment, store and load.
> +
> +@smallexample
> +@{
> +void (*foo)(void) __attribute__(notrack);
> +void (*foo1)(void) __attribute__(notrack);
> +void (*foo2)(void);
> +
> +int
> +foo (void) /* The function's address is not tracked.  */
> +
> +  /* This call site is not tracked for
> + control-flow instrumentation.  */
> +  (*foo1)();
> +  foo1 = foo2;
> +  /* This call site is still not tracked for
> + control-flow instrumentation.  */
> +  (*foo1)();
> +
> +  /* This call site is tracked for
> + control-flow instrumentation.  */
> +  (*foo2)();
> +  foo2 = foo1;
> +  /* This call site is still tracked for
> + control-flow instrumentation.  */
> +  (*foo2)();
> +
> +  return 0;
> +@}
> +@end smallexample
Given the notrack attribute is part of the type system, could we issue a
warning on the foo1 = foo2 assignment since we're discarding tracking
that's implicit on foo2?

> +
>  @end table
>  
>  On the x86, the inliner does not inline a
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index 5ae9dc4..ff2ce92 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -459,6 +459,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
> +-finstrument-control-flow @gol
>  -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
> @@ -11284,6 +11285,27 @@ is used to link a program, the GCC driver 
> automatically links
>  against @file{libmpxwrappers}.  See also @option{-static-libmpxwrappers}.
>  Enabled by default.
>  
> +@item -finstrument-control-flow
> +@opindex finstrument-control-flow
> +@opindex fno-instrument-control-flow
> +Enable code instrumentation of control-flow transfers to increase
> +a program se

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

[Tsimbalist, Igor V]

Part#2. Document -finstrument-control-flow and notrack attribute.



0002-Part-2.-Document-finstrument-control-flow-and-notrac.patch
Description: 0002-Part-2.-Document-finstrument-control-flow-and-notrac.patch