Re: 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling

[Jeff Law]

On 10/12/2017 02:12 AM, Tsimbalist, Igor V wrote:
>> Seems reasonable.  As a result something like
>> check_missing_nocf_check_attribute is going to just go away along with the
>> code in *-typeck.c which called it, right?  If so that seems like a nice 
>> cleanup.
> Yes, you are right.
> 
> Updated patch is attached.
> 
> 
> High-level design.
> --
> 
> A proposal is to introduce a target independent flag
> -fcf-protection=[none|branch|return|full] with a semantic to
> instrument a code to control validness or integrity of control-flow
> transfers using jump and call instructions. The main goal is to detect
> and block a possible malware execution through transfer the execution
> to unknown target address. Implementation could be either software or
> target based. Any target platforms can provide their implementation
> for instrumentation under this option.
> 
> When the -fcf-protection flag is set each implementation has
> to check if a support exists for a target platform and report an error
> if no support is found.
> 
> The compiler should instrument any control-flow transfer points in a
> program (ex. call/jmp/ret) as well as any landing pads, which are
> targets of control-flow transfers.
> 
> A new 'nocf_check' attribute is introduced to provide hand tuning
> support. The attribute directs the compiler to skip a call to a
> function and a function's landing pad from instrumentation. The
> attribute can be used for function and pointer to function types,
> otherwise it will be ignored. The attribute is saved in a type and
> propagated to a GIMPLE call statement and later to a call instruction.
> 
> Currently all platforms except i386 will report the error and do no
> instrumentation. i386 will provide the implementation based on a
> specification published by Intel for a new technology called
> Control-flow Enforcement Technology (CET).
> 
> gcc/c-family/
>   * c-attribs.c (handle_nocf_check_attribute): New function.
>   (c_common_attribute_table): Add 'nocf_check' handling.
> 
> gcc/c/
>   * gimple-parser.c: Add second argument NULL to
>   gimple_build_call_from_tree.
> 
> gcc/
>   * attrib.c (comp_type_attributes): Check nocf_check attribute.
>   * cfgexpand.c (expand_call_stmt): Set REG_CALL_NOCF_CHECK for
>   call insn.
>   * combine.c (distribute_notes): Add REG_CALL_NOCF_CHECK handling.
>   * common.opt: Add fcf-protection flag.
>   * emit-rtl.c (try_split): Add REG_CALL_NOCF_CHECK handling.
>   * flag-types.h: Add enum cf_protection_level.
>   * gimple.c (gimple_build_call_from_tree): Add second parameter.
>   Add 'nocf_check' attribute propagation to gimple call.
>   * gimple.h (gf_mask): Add GF_CALL_NOCF_CHECK.
>   (gimple_build_call_from_tree): Update prototype.
>   (gimple_call_nocf_check_p): New function.
>   (gimple_call_set_nocf_check): Likewise.
>   * gimplify.c: Add second argument to gimple_build_call_from_tree.
>   * ipa-icf.c: Add nocf_check attribute in statement hash.
>   * recog.c (peep2_attempt): Add REG_CALL_NOCF_CHECK handling.
>   * reg-notes.def: Add REG_NOTE (CALL_NOCF_CHECK).
>   * toplev.c (process_options): Add flag_cf_protection handling.
OK.  Sorry about the long delays.

jeff

RE: 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling

[Tsimbalist, Igor V]

> Seems reasonable.  As a result something like
> check_missing_nocf_check_attribute is going to just go away along with the
> code in *-typeck.c which called it, right?  If so that seems like a nice 
> cleanup.
Yes, you are right.

Updated patch is attached.

Igor

> -Original Message-
> From: gcc-patches-ow...@gcc.gnu.org [mailto:gcc-patches-
> ow...@gcc.gnu.org] On Behalf Of Jeff Law
> Sent: Thursday, October 12, 2017 8:07 AM
> To: Tsimbalist, Igor V <igor.v.tsimbal...@intel.com>; gcc-
> patc...@gcc.gnu.org
> Cc: richard.guent...@gmail.com
> Subject: Re: 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling
> 
> On 10/05/2017 04:20 AM, Tsimbalist, Igor V wrote:
> > I would like to implement the patch in a bit different way depending
> > on answers I will get for my following proposals:
> >
> > - I propose to make a type with 'nocf_check' attribute to be different from
> type w/o the attribute.
> >The reason is that the type with 'nocf_check' attribute implies different
> code generation. It will be
> >done by setting affects_type_identity field to true for the attribute. 
> > That
> in turn will trigger
> >needed or expected type checking;
> Seems reasonable.  As a result something like
> check_missing_nocf_check_attribute is going to just go away along with the
> code in *-typeck.c which called it, right?  If so that seems like a nice 
> cleanup.
> 
> 
> >
> > - I propose to ignore the 'nocf_check' attribute if 'fcf-protection' option 
> > is
> not specified and output
> >the warning about this. If there is no instrumentation the type with
> attribute should not be treated
> >differently from type w/o the attribute (see previous item) and should
> not be recorded into the
> >type.
> Seems reasonable.
> >
> > If it's ok, please ignore my previous patch (version#3) and I will post the
> updated patch shortly.
> OK.  FWIW, I think we're ready to ACK on this.  So get it posted :-)
> 
> jeff


0001-Add-generic-part-for-Intel-CET-enabling-fcf-protecti.patch
Description: 0001-Add-generic-part-for-Intel-CET-enabling-fcf-protecti.patch

Re: 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling

[Jeff Law]

On 10/05/2017 04:20 AM, Tsimbalist, Igor V wrote:
> I would like to implement the patch in a bit different way depending on 
> answers I will get for
> my following proposals:
> 
> - I propose to make a type with 'nocf_check' attribute to be different from 
> type w/o the attribute.
>The reason is that the type with 'nocf_check' attribute implies different 
> code generation. It will be
>done by setting affects_type_identity field to true for the attribute. 
> That in turn will trigger
>needed or expected type checking;
Seems reasonable.  As a result something like
check_missing_nocf_check_attribute is going to just go away along with
the code in *-typeck.c which called it, right?  If so that seems like a
nice cleanup.


> 
> - I propose to ignore the 'nocf_check' attribute if 'fcf-protection' option 
> is not specified and output
>the warning about this. If there is no instrumentation the type with 
> attribute should not be treated
>differently from type w/o the attribute (see previous item) and should not 
> be recorded into the
>type.
Seems reasonable.
> 
> If it's ok, please ignore my previous patch (version#3) and I will post the 
> updated patch shortly.
OK.  FWIW, I think we're ready to ACK on this.  So get it posted :-)

jeff

RE: 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling

[Tsimbalist, Igor V]

I would like to implement the patch in a bit different way depending on answers 
I will get for
my following proposals:

- I propose to make a type with 'nocf_check' attribute to be different from 
type w/o the attribute.
   The reason is that the type with 'nocf_check' attribute implies different 
code generation. It will be
   done by setting affects_type_identity field to true for the attribute. That 
in turn will trigger
   needed or expected type checking;

- I propose to ignore the 'nocf_check' attribute if 'fcf-protection' option is 
not specified and output
   the warning about this. If there is no instrumentation the type with 
attribute should not be treated
   differently from type w/o the attribute (see previous item) and should not 
be recorded into the
   type.

If it's ok, please ignore my previous patch (version#3) and I will post the 
updated patch shortly.

Igor


> -Original Message-
> From: Tsimbalist, Igor V
> Sent: Friday, September 29, 2017 6:04 PM
> To: Jeff Law <l...@redhat.com>; gcc-patches@gcc.gnu.org
> Cc: richard.guent...@gmail.com; Tsimbalist, Igor V
> <igor.v.tsimbal...@intel.com>
> Subject: RE: 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling
> 
> Updated patch, version #3.
> 
> Igor
> 
> 
> > -Original Message-
> > From: Tsimbalist, Igor V
> > Sent: Friday, September 29, 2017 4:32 PM
> > To: Jeff Law <l...@redhat.com>; gcc-patches@gcc.gnu.org
> > Cc: richard.guent...@gmail.com; Tsimbalist, Igor V
> > <igor.v.tsimbal...@intel.com>
> > Subject: RE: 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling
> >
> > > -Original Message-
> > > From: Jeff Law [mailto:l...@redhat.com]
> > > Sent: Friday, September 29, 2017 12:44 AM
> > > To: Tsimbalist, Igor V <igor.v.tsimbal...@intel.com>; gcc-
> > > patc...@gcc.gnu.org
> > > Cc: richard.guent...@gmail.com
> > > Subject: Re: 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling
> > >
> > > On 09/19/2017 07:39 AM, Tsimbalist, Igor V wrote:
> > > > Here is an updated patch (version #2). The main differences are:
> > > >
> > > > - Change attribute and option names;
> > > > - Add additional parameter to gimple_build_call_from_tree by
> > > > adding a
> > > type parameter and
> > > >   use it 'nocf_check' attribute propagation;
> > > > - Reimplement fixes in expand_call_stmt to propagate 'nocf_check'
> > > > attribute;
> > > > - Consider 'nocf_check' attribute in Identical Code Folding (ICF)
> > > > optimization;
> > > > - Add warning for type inconsistency regarding 'nocf_check'
> > > > attribute;
> > > > - Many small fixes;
> > > >
> > > > gcc/c-family/
> > > > * c-attribs.c (handle_nocf_check_attribute): New function.
> > > > (c_common_attribute_table): Add 'nocf_check' handling.
> > > > * c-common.c (check_missing_format_attribute): New function.
> > > > * c-common.h: Likewise.
> > > >
> > > > gcc/c/
> > > > * c-typeck.c (convert_for_assignment): Add check for nocf_check
> > > > attribute.
> > > > * gimple-parser.c: Add second argument NULL to
> > > > gimple_build_call_from_tree.
> > > >
> > > > gcc/cp/
> > > > * typeck.c (convert_for_assignment): Add check for nocf_check
> > > > attribute.
> > > >
> > > > gcc/
> > > > * cfgexpand.c (expand_call_stmt): Set REG_CALL_NOCF_CHECK for
> > > > call insn.
> > > > * combine.c (distribute_notes): Add REG_CALL_NOCF_CHECK
> > > handling.
> > > > * common.opt: Add fcf-protection flag.
> > > > * emit-rtl.c (try_split): Add REG_CALL_NOCF_CHECK handling.
> > > > * flag-types.h: Add enum cf_protection_level.
> > > > * gimple.c (gimple_build_call_from_tree): Add second parameter.
> > > > Add 'nocf_check' attribute propagation to gimple call.
> > > > * gimple.h (gf_mask): Add GF_CALL_NOCF_CHECK.
> > > > (gimple_call_nocf_check_p): New function.
> > > > (gimple_call_set_nocf_check): Likewise.
> > > > * gimplify.c: Add second argument to 
> > > > gimple_build_call_from_tree.
> > > > * ipa-icf.c: Add nocf_check attribute in statement hash.
> > > > * recog.c (peep2_attempt): Add REG_CALL_NOCF_CHECK handling.
> > > > * r

RE: 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling

[Tsimbalist, Igor V]

Updated patch, version #3.

Igor


> -Original Message-
> From: Tsimbalist, Igor V
> Sent: Friday, September 29, 2017 4:32 PM
> To: Jeff Law <l...@redhat.com>; gcc-patches@gcc.gnu.org
> Cc: richard.guent...@gmail.com; Tsimbalist, Igor V
> <igor.v.tsimbal...@intel.com>
> Subject: RE: 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling
> 
> > -Original Message-
> > From: Jeff Law [mailto:l...@redhat.com]
> > Sent: Friday, September 29, 2017 12:44 AM
> > To: Tsimbalist, Igor V <igor.v.tsimbal...@intel.com>; gcc-
> > patc...@gcc.gnu.org
> > Cc: richard.guent...@gmail.com
> > Subject: Re: 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling
> >
> > On 09/19/2017 07:39 AM, Tsimbalist, Igor V wrote:
> > > Here is an updated patch (version #2). The main differences are:
> > >
> > > - Change attribute and option names;
> > > - Add additional parameter to gimple_build_call_from_tree by adding
> > > a
> > type parameter and
> > >   use it 'nocf_check' attribute propagation;
> > > - Reimplement fixes in expand_call_stmt to propagate 'nocf_check'
> > > attribute;
> > > - Consider 'nocf_check' attribute in Identical Code Folding (ICF)
> > > optimization;
> > > - Add warning for type inconsistency regarding 'nocf_check'
> > > attribute;
> > > - Many small fixes;
> > >
> > > gcc/c-family/
> > >   * c-attribs.c (handle_nocf_check_attribute): New function.
> > >   (c_common_attribute_table): Add 'nocf_check' handling.
> > >   * c-common.c (check_missing_format_attribute): New function.
> > >   * c-common.h: Likewise.
> > >
> > > gcc/c/
> > >   * c-typeck.c (convert_for_assignment): Add check for nocf_check
> > >   attribute.
> > >   * gimple-parser.c: Add second argument NULL to
> > >   gimple_build_call_from_tree.
> > >
> > > gcc/cp/
> > >   * typeck.c (convert_for_assignment): Add check for nocf_check
> > >   attribute.
> > >
> > > gcc/
> > >   * cfgexpand.c (expand_call_stmt): Set REG_CALL_NOCF_CHECK for
> > >   call insn.
> > >   * combine.c (distribute_notes): Add REG_CALL_NOCF_CHECK
> > handling.
> > >   * common.opt: Add fcf-protection flag.
> > >   * emit-rtl.c (try_split): Add REG_CALL_NOCF_CHECK handling.
> > >   * flag-types.h: Add enum cf_protection_level.
> > >   * gimple.c (gimple_build_call_from_tree): Add second parameter.
> > >   Add 'nocf_check' attribute propagation to gimple call.
> > >   * gimple.h (gf_mask): Add GF_CALL_NOCF_CHECK.
> > >   (gimple_call_nocf_check_p): New function.
> > >   (gimple_call_set_nocf_check): Likewise.
> > >   * gimplify.c: Add second argument to gimple_build_call_from_tree.
> > >   * ipa-icf.c: Add nocf_check attribute in statement hash.
> > >   * recog.c (peep2_attempt): Add REG_CALL_NOCF_CHECK handling.
> > >   * reg-notes.def: Add REG_NOTE (CALL_NOCF_CHECK).
> > >   * toplev.c (process_options): Add flag_cf_protection handling.
> > >
> > > Is it ok for trunk?
> > >
> > > Thanks,
> > > Igor
> > >
> > >
> >
> >
> > >
> > > diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c
> > > index
> > > 0337537..77d1909 100644
> > > --- a/gcc/c-family/c-attribs.c
> > > +++ b/gcc/c-family/c-attribs.c
> > > @@ -65,6 +65,7 @@ static tree handle_asan_odr_indicator_attribute
> > > (tree *, tree, tree, int,  static tree
> > > handle_stack_protect_attribute (tree *, tree, tree, int, bool *);
> > > static tree handle_noinline_attribute (tree *, tree, tree, int, bool
> > > *);  static tree handle_noclone_attribute (tree *, tree, tree, int,
> > > bool *);
> > > +static tree handle_nocf_check_attribute (tree *, tree, tree, int,
> > > +bool *);
> > >  static tree handle_noicf_attribute (tree *, tree, tree, int, bool
> > > *); static tree handle_noipa_attribute (tree *, tree, tree, int,
> > > bool *); static tree handle_leaf_attribute (tree *, tree, tree, int,
> > > bool *); @@ -367,6 +368,8 @@ const struct attribute_spec
> > c_common_attribute_table[] =
> > >{ "patchable_function_entry",  1, 2, true, false, false,
> > > handle_patchable_function_entry_attribute,
> > > false },
> > > +  { "nocf_check",  0, 0, false, true, true,
> > > +   handle_nocf_che

RE: 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling

[Tsimbalist, Igor V]

> -Original Message-
> From: Jeff Law [mailto:l...@redhat.com]
> Sent: Friday, September 29, 2017 12:44 AM
> To: Tsimbalist, Igor V <igor.v.tsimbal...@intel.com>; gcc-
> patc...@gcc.gnu.org
> Cc: richard.guent...@gmail.com
> Subject: Re: 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling
> 
> On 09/19/2017 07:39 AM, Tsimbalist, Igor V wrote:
> > Here is an updated patch (version #2). The main differences are:
> >
> > - Change attribute and option names;
> > - Add additional parameter to gimple_build_call_from_tree by adding a
> type parameter and
> >   use it 'nocf_check' attribute propagation;
> > - Reimplement fixes in expand_call_stmt to propagate 'nocf_check'
> > attribute;
> > - Consider 'nocf_check' attribute in Identical Code Folding (ICF)
> > optimization;
> > - Add warning for type inconsistency regarding 'nocf_check' attribute;
> > - Many small fixes;
> >
> > gcc/c-family/
> > * c-attribs.c (handle_nocf_check_attribute): New function.
> > (c_common_attribute_table): Add 'nocf_check' handling.
> > * c-common.c (check_missing_format_attribute): New function.
> > * c-common.h: Likewise.
> >
> > gcc/c/
> > * c-typeck.c (convert_for_assignment): Add check for nocf_check
> > attribute.
> > * gimple-parser.c: Add second argument NULL to
> > gimple_build_call_from_tree.
> >
> > gcc/cp/
> > * typeck.c (convert_for_assignment): Add check for nocf_check
> > attribute.
> >
> > gcc/
> > * cfgexpand.c (expand_call_stmt): Set REG_CALL_NOCF_CHECK for
> > call insn.
> > * combine.c (distribute_notes): Add REG_CALL_NOCF_CHECK
> handling.
> > * common.opt: Add fcf-protection flag.
> > * emit-rtl.c (try_split): Add REG_CALL_NOCF_CHECK handling.
> > * flag-types.h: Add enum cf_protection_level.
> > * gimple.c (gimple_build_call_from_tree): Add second parameter.
> > Add 'nocf_check' attribute propagation to gimple call.
> > * gimple.h (gf_mask): Add GF_CALL_NOCF_CHECK.
> > (gimple_call_nocf_check_p): New function.
> > (gimple_call_set_nocf_check): Likewise.
> > * gimplify.c: Add second argument to gimple_build_call_from_tree.
> > * ipa-icf.c: Add nocf_check attribute in statement hash.
> > * recog.c (peep2_attempt): Add REG_CALL_NOCF_CHECK handling.
> > * reg-notes.def: Add REG_NOTE (CALL_NOCF_CHECK).
> > * toplev.c (process_options): Add flag_cf_protection handling.
> >
> > Is it ok for trunk?
> >
> > Thanks,
> > Igor
> >
> >
> 
> 
> >
> > diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c index
> > 0337537..77d1909 100644
> > --- a/gcc/c-family/c-attribs.c
> > +++ b/gcc/c-family/c-attribs.c
> > @@ -65,6 +65,7 @@ static tree handle_asan_odr_indicator_attribute
> > (tree *, tree, tree, int,  static tree handle_stack_protect_attribute
> > (tree *, tree, tree, int, bool *);  static tree
> > handle_noinline_attribute (tree *, tree, tree, int, bool *);  static
> > tree handle_noclone_attribute (tree *, tree, tree, int, bool *);
> > +static tree handle_nocf_check_attribute (tree *, tree, tree, int,
> > +bool *);
> >  static tree handle_noicf_attribute (tree *, tree, tree, int, bool *);
> > static tree handle_noipa_attribute (tree *, tree, tree, int, bool *);
> > static tree handle_leaf_attribute (tree *, tree, tree, int, bool *);
> > @@ -367,6 +368,8 @@ const struct attribute_spec
> c_common_attribute_table[] =
> >{ "patchable_function_entry",1, 2, true, false, false,
> >   handle_patchable_function_entry_attribute,
> >   false },
> > +  { "nocf_check",0, 0, false, true, true,
> > + handle_nocf_check_attribute, false },
> >{ NULL, 0, 0, false, false, false, NULL, false }
> >  };
> >
> > @@ -783,6 +786,26 @@ handle_noclone_attribute (tree *node, tree
> name,
> >return NULL_TREE;
> >  }
> >
> > +/* Handle a "nocf_check" attribute; arguments as in
> > +   struct attribute_spec.handler.  */
> > +
> > +static tree
> > +handle_nocf_check_attribute (tree *node, tree name,
> > + tree ARG_UNUSED (args),
> > + int ARG_UNUSED (flags), bool *no_add_attrs) {
> > +  if (TREE_CODE (*node) != FUNCTION_TYPE
> > +  && TREE_CODE (*node) != METHOD_TYPE
> > +  && TREE_CODE (*node) != FIELD_DECL
> > +  && 

Re: 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling

[Jeff Law]

On 09/19/2017 07:39 AM, Tsimbalist, Igor V wrote:
> Here is an updated patch (version #2). The main differences are:
> 
> - Change attribute and option names;
> - Add additional parameter to gimple_build_call_from_tree by adding a type 
> parameter and
>   use it 'nocf_check' attribute propagation;
> - Reimplement fixes in expand_call_stmt to propagate 'nocf_check' attribute;
> - Consider 'nocf_check' attribute in Identical Code Folding (ICF) 
> optimization;
> - Add warning for type inconsistency regarding 'nocf_check' attribute;
> - Many small fixes;
> 
> gcc/c-family/
>   * c-attribs.c (handle_nocf_check_attribute): New function.
>   (c_common_attribute_table): Add 'nocf_check' handling.
>   * c-common.c (check_missing_format_attribute): New function.
>   * c-common.h: Likewise.
> 
> gcc/c/
>   * c-typeck.c (convert_for_assignment): Add check for nocf_check
>   attribute.
>   * gimple-parser.c: Add second argument NULL to
>   gimple_build_call_from_tree.
> 
> gcc/cp/
>   * typeck.c (convert_for_assignment): Add check for nocf_check
>   attribute.
> 
> gcc/
>   * cfgexpand.c (expand_call_stmt): Set REG_CALL_NOCF_CHECK for
>   call insn.
>   * combine.c (distribute_notes): Add REG_CALL_NOCF_CHECK handling.
>   * common.opt: Add fcf-protection flag.
>   * emit-rtl.c (try_split): Add REG_CALL_NOCF_CHECK handling.
>   * flag-types.h: Add enum cf_protection_level.
>   * gimple.c (gimple_build_call_from_tree): Add second parameter.
>   Add 'nocf_check' attribute propagation to gimple call.
>   * gimple.h (gf_mask): Add GF_CALL_NOCF_CHECK.
>   (gimple_call_nocf_check_p): New function.
>   (gimple_call_set_nocf_check): Likewise.
>   * gimplify.c: Add second argument to gimple_build_call_from_tree.
>   * ipa-icf.c: Add nocf_check attribute in statement hash.
>   * recog.c (peep2_attempt): Add REG_CALL_NOCF_CHECK handling.
>   * reg-notes.def: Add REG_NOTE (CALL_NOCF_CHECK).
>   * toplev.c (process_options): Add flag_cf_protection handling.
> 
> Is it ok for trunk?
> 
> Thanks,
> Igor
> 
> 


> 
> diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c
> index 0337537..77d1909 100644
> --- a/gcc/c-family/c-attribs.c
> +++ b/gcc/c-family/c-attribs.c
> @@ -65,6 +65,7 @@ static tree handle_asan_odr_indicator_attribute (tree *, 
> tree, tree, int,
>  static tree handle_stack_protect_attribute (tree *, tree, tree, int, bool *);
>  static tree handle_noinline_attribute (tree *, tree, tree, int, bool *);
>  static tree handle_noclone_attribute (tree *, tree, tree, int, bool *);
> +static tree handle_nocf_check_attribute (tree *, tree, tree, int, bool *);
>  static tree handle_noicf_attribute (tree *, tree, tree, int, bool *);
>  static tree handle_noipa_attribute (tree *, tree, tree, int, bool *);
>  static tree handle_leaf_attribute (tree *, tree, tree, int, bool *);
> @@ -367,6 +368,8 @@ const struct attribute_spec c_common_attribute_table[] =
>{ "patchable_function_entry",  1, 2, true, false, false,
> handle_patchable_function_entry_attribute,
> false },
> +  { "nocf_check",  0, 0, false, true, true,
> +   handle_nocf_check_attribute, false },
>{ NULL, 0, 0, false, false, false, NULL, false }
>  };
>  
> @@ -783,6 +786,26 @@ handle_noclone_attribute (tree *node, tree name,
>return NULL_TREE;
>  }
>  
> +/* Handle a "nocf_check" attribute; arguments as in
> +   struct attribute_spec.handler.  */
> +
> +static tree
> +handle_nocf_check_attribute (tree *node, tree name,
> +   tree ARG_UNUSED (args),
> +   int ARG_UNUSED (flags), bool *no_add_attrs)
> +{
> +  if (TREE_CODE (*node) != FUNCTION_TYPE
> +  && TREE_CODE (*node) != METHOD_TYPE
> +  && TREE_CODE (*node) != FIELD_DECL
> +  && TREE_CODE (*node) != TYPE_DECL)
So curious, is this needed for FIELD_DECL and TYPE_DECL?  ISTM the
attribute is applied to function/method types.

If we do need to handle FIELD_DECL and TYPE_DECL here, can you add a
quick comment why?

> diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
> index b3ec3a0..78a730e 100644
> --- a/gcc/c-family/c-common.c
> +++ b/gcc/c-family/c-common.c
> @@ -7253,6 +7253,26 @@ check_missing_format_attribute (tree ltype, tree rtype)
>  return false;
>  }
>  
> +/* Check for missing nocf_check attributes on function pointers.  LTYPE is
> +   the new type or left-hand side type.  RTYPE is the old type or
> +   right-hand side type.  Returns TRUE if LTYPE is missing the desired
> +   attribute.  */
> +
> +bool
> +check_missing_nocf_check_attribute (tree ltype, tree rtype)
> +{
> +  tree const ttr = TREE_TYPE (rtype), ttl = TREE_TYPE (ltype);
> +  tree ra, la;
> +
> +  for (ra = TYPE_ATTRIBUTES (ttr); ra; ra = TREE_CHAIN (ra))
> +if (is_attribute_p ("nocf_check", TREE_PURPOSE (ra)))
> +  break;
> +  

RE: 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling

[Tsimbalist, Igor V]

Here is an updated patch (version #2). The main differences are:

- Change attribute and option names;
- Add additional parameter to gimple_build_call_from_tree by adding a type 
parameter and
  use it 'nocf_check' attribute propagation;
- Reimplement fixes in expand_call_stmt to propagate 'nocf_check' attribute;
- Consider 'nocf_check' attribute in Identical Code Folding (ICF) optimization;
- Add warning for type inconsistency regarding 'nocf_check' attribute;
- Many small fixes;

gcc/c-family/
* c-attribs.c (handle_nocf_check_attribute): New function.
(c_common_attribute_table): Add 'nocf_check' handling.
* c-common.c (check_missing_format_attribute): New function.
* c-common.h: Likewise.

gcc/c/
* c-typeck.c (convert_for_assignment): Add check for nocf_check
attribute.
* gimple-parser.c: Add second argument NULL to
gimple_build_call_from_tree.

gcc/cp/
* typeck.c (convert_for_assignment): Add check for nocf_check
attribute.

gcc/
* cfgexpand.c (expand_call_stmt): Set REG_CALL_NOCF_CHECK for
call insn.
* combine.c (distribute_notes): Add REG_CALL_NOCF_CHECK handling.
* common.opt: Add fcf-protection flag.
* emit-rtl.c (try_split): Add REG_CALL_NOCF_CHECK handling.
* flag-types.h: Add enum cf_protection_level.
* gimple.c (gimple_build_call_from_tree): Add second parameter.
Add 'nocf_check' attribute propagation to gimple call.
* gimple.h (gf_mask): Add GF_CALL_NOCF_CHECK.
(gimple_call_nocf_check_p): New function.
(gimple_call_set_nocf_check): Likewise.
* gimplify.c: Add second argument to gimple_build_call_from_tree.
* ipa-icf.c: Add nocf_check attribute in statement hash.
* recog.c (peep2_attempt): Add REG_CALL_NOCF_CHECK handling.
* reg-notes.def: Add REG_NOTE (CALL_NOCF_CHECK).
* toplev.c (process_options): Add flag_cf_protection handling.

Is it ok for trunk?

Thanks,
Igor


> -Original Message-
> From: Richard Biener [mailto:richard.guent...@gmail.com]
> Sent: Friday, September 15, 2017 2:14 PM
> To: Tsimbalist, Igor V <igor.v.tsimbal...@intel.com>
> Cc: gcc-patches@gcc.gnu.org
> Subject: Re: 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling
> 
> On Fri, Sep 15, 2017 at 1:12 PM, Tsimbalist, Igor V
> <igor.v.tsimbal...@intel.com> wrote:
> >> -Original Message-
> >> From: Tsimbalist, Igor V
> >> Sent: Tuesday, September 12, 2017 5:35 PM
> >> To: 'Richard Biener' <richard.guent...@gmail.com>
> >> Cc: 'gcc-patches@gcc.gnu.org' <gcc-patches@gcc.gnu.org>; Tsimbalist,
> >> Igor V <igor.v.tsimbal...@intel.com>
> >> Subject: RE: 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling
> >>
> >> > -Original Message-
> >> > From: Tsimbalist, Igor V
> >> > Sent: Friday, August 18, 2017 4:43 PM
> >> > To: 'Richard Biener' <richard.guent...@gmail.com>
> >> > Cc: gcc-patches@gcc.gnu.org; Tsimbalist, Igor V
> >> > <igor.v.tsimbal...@intel.com>
> >> > Subject: RE: 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling
> >> >
> >> > > -Original Message-----
> >> > > From: Richard Biener [mailto:richard.guent...@gmail.com]
> >> > > Sent: Friday, August 18, 2017 3:53 PM
> >> > > To: Tsimbalist, Igor V <igor.v.tsimbal...@intel.com>
> >> > > Cc: gcc-patches@gcc.gnu.org
> >> > > Subject: Re: 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling
> >> > >
> >> > > On Fri, Aug 18, 2017 at 3:11 PM, Tsimbalist, Igor V
> >> > > <igor.v.tsimbal...@intel.com> wrote:
> >> > > >> -Original Message-
> >> > > >> From: Richard Biener [mailto:richard.guent...@gmail.com]
> >> > > >> Sent: Tuesday, August 15, 2017 3:43 PM
> >> > > >> To: Tsimbalist, Igor V <igor.v.tsimbal...@intel.com>
> >> > > >> Cc: gcc-patches@gcc.gnu.org
> >> > > >> Subject: Re:
> >> > > >> 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling
> >> > > >>
> >> > > >> On Tue, Aug 1, 2017 at 10:56 AM, Tsimbalist, Igor V
> >> > > >> <igor.v.tsimbal...@intel.com> wrote:
> >> > > >> > Part#1. Add generic part for Intel CET enabling.
> >> > > >> >
> >> > > >> > The spec is available at
> >> > > >> >
> >> > > >> > https://software.intel.com/sites/default/files/managed/4d/2a
> >> > > >> > /co nt ro l-f

Re: 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling

[Richard Biener]

On Fri, Sep 15, 2017 at 1:12 PM, Tsimbalist, Igor V
<igor.v.tsimbal...@intel.com> wrote:
>> -Original Message-
>> From: Tsimbalist, Igor V
>> Sent: Tuesday, September 12, 2017 5:35 PM
>> To: 'Richard Biener' <richard.guent...@gmail.com>
>> Cc: 'gcc-patches@gcc.gnu.org' <gcc-patches@gcc.gnu.org>; Tsimbalist, Igor V
>> <igor.v.tsimbal...@intel.com>
>> Subject: RE: 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling
>>
>> > -Original Message-
>> > From: Tsimbalist, Igor V
>> > Sent: Friday, August 18, 2017 4:43 PM
>> > To: 'Richard Biener' <richard.guent...@gmail.com>
>> > Cc: gcc-patches@gcc.gnu.org; Tsimbalist, Igor V
>> > <igor.v.tsimbal...@intel.com>
>> > Subject: RE: 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling
>> >
>> > > -Original Message-
>> > > From: Richard Biener [mailto:richard.guent...@gmail.com]
>> > > Sent: Friday, August 18, 2017 3:53 PM
>> > > To: Tsimbalist, Igor V <igor.v.tsimbal...@intel.com>
>> > > Cc: gcc-patches@gcc.gnu.org
>> > > Subject: Re: 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling
>> > >
>> > > On Fri, Aug 18, 2017 at 3:11 PM, Tsimbalist, Igor V
>> > > <igor.v.tsimbal...@intel.com> wrote:
>> > > >> -----Original Message-
>> > > >> From: Richard Biener [mailto:richard.guent...@gmail.com]
>> > > >> Sent: Tuesday, August 15, 2017 3:43 PM
>> > > >> To: Tsimbalist, Igor V <igor.v.tsimbal...@intel.com>
>> > > >> Cc: gcc-patches@gcc.gnu.org
>> > > >> Subject: Re: 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling
>> > > >>
>> > > >> On Tue, Aug 1, 2017 at 10:56 AM, Tsimbalist, Igor V
>> > > >> <igor.v.tsimbal...@intel.com> wrote:
>> > > >> > Part#1. Add generic part for Intel CET enabling.
>> > > >> >
>> > > >> > The spec is available at
>> > > >> >
>> > > >> > https://software.intel.com/sites/default/files/managed/4d/2a/co
>> > > >> > nt ro l-f low-enforcement-technology-preview.pdf
>
> <..skipped..>
>
>> > > >> I think 'notrack' is somewhat unspecific of a name, what
>> > > >> prevented you to use 'nocet'?
>> > > >
>> > > > Actually it's specific. The HW will have a prefix with exactly
>> > > > this name and
>> > > the same meaning. And I think, what is more important, 'track/notrack'
>> > > gives better semantic for a user. CET is a name bound with Intel
>> > > specific technology.
>> > >
>> > > But 'tracking' something is quite unspecific.  Tracking for what?
>> > > 'no_verify_cf' (aka do not verify control flow) maybe?
>> >
>> > The name just  has to suggest the right semantic. 'no_verify_cf' is
>> > good, let's use it unless different name appears.
>> I have renamed all newly introduced function and macro names to use
>> 'noverify_cf'. But I still keep the attribute name as 'notrack'. 
>> Historically the
>> attribute name follows the public CET specification, which uses 'no-track
>> prefix' wording. Is it ok to keep such attribute name?
>
> Here is an updated proposal about option name and attribute name.
>
> The new option has values to let a user to choose what control-flow 
> protection to activate.
>
> -fcf-protection=[full|branch|return|none]
>   branch - do control-flow protection for indirect jumps and calls
>   return - do control-flow protection for function returns
>   full - alias to specify both branch + return
>   none - turn off protection. This value is needed when/if cf-protection is 
> turned on by default by driver in future
>
> Attribute name is the most tough one. Here are several names to evaluate: 
> 'nocf_verify' or 'nocf_check', or to be more specific and to mimic option 
> name 'nocf_branch_verify' or 'nocf_branch_check'. I would prefer 'nocf_check' 
> as it applies to functions and function pointers so it's definitely related 
> to a branch and it's a smaller one.
>
> If you ok with the new proposal I'll implement it in a general parts (code, 
> documentation and tests) and resend these patches for review.

nocf_check sounds fine to me.

Richard.

> Thanks,
> Igor
>

RE: 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling

[Tsimbalist, Igor V]

> -Original Message-
> From: Tsimbalist, Igor V
> Sent: Tuesday, September 12, 2017 5:35 PM
> To: 'Richard Biener' <richard.guent...@gmail.com>
> Cc: 'gcc-patches@gcc.gnu.org' <gcc-patches@gcc.gnu.org>; Tsimbalist, Igor V
> <igor.v.tsimbal...@intel.com>
> Subject: RE: 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling
> 
> > -Original Message-
> > From: Tsimbalist, Igor V
> > Sent: Friday, August 18, 2017 4:43 PM
> > To: 'Richard Biener' <richard.guent...@gmail.com>
> > Cc: gcc-patches@gcc.gnu.org; Tsimbalist, Igor V
> > <igor.v.tsimbal...@intel.com>
> > Subject: RE: 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling
> >
> > > -Original Message-
> > > From: Richard Biener [mailto:richard.guent...@gmail.com]
> > > Sent: Friday, August 18, 2017 3:53 PM
> > > To: Tsimbalist, Igor V <igor.v.tsimbal...@intel.com>
> > > Cc: gcc-patches@gcc.gnu.org
> > > Subject: Re: 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling
> > >
> > > On Fri, Aug 18, 2017 at 3:11 PM, Tsimbalist, Igor V
> > > <igor.v.tsimbal...@intel.com> wrote:
> > > >> -Original Message-
> > > >> From: Richard Biener [mailto:richard.guent...@gmail.com]
> > > >> Sent: Tuesday, August 15, 2017 3:43 PM
> > > >> To: Tsimbalist, Igor V <igor.v.tsimbal...@intel.com>
> > > >> Cc: gcc-patches@gcc.gnu.org
> > > >> Subject: Re: 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling
> > > >>
> > > >> On Tue, Aug 1, 2017 at 10:56 AM, Tsimbalist, Igor V
> > > >> <igor.v.tsimbal...@intel.com> wrote:
> > > >> > Part#1. Add generic part for Intel CET enabling.
> > > >> >
> > > >> > The spec is available at
> > > >> >
> > > >> > https://software.intel.com/sites/default/files/managed/4d/2a/co
> > > >> > nt ro l-f low-enforcement-technology-preview.pdf

<..skipped..>

> > > >> I think 'notrack' is somewhat unspecific of a name, what
> > > >> prevented you to use 'nocet'?
> > > >
> > > > Actually it's specific. The HW will have a prefix with exactly
> > > > this name and
> > > the same meaning. And I think, what is more important, 'track/notrack'
> > > gives better semantic for a user. CET is a name bound with Intel
> > > specific technology.
> > >
> > > But 'tracking' something is quite unspecific.  Tracking for what?
> > > 'no_verify_cf' (aka do not verify control flow) maybe?
> >
> > The name just  has to suggest the right semantic. 'no_verify_cf' is
> > good, let's use it unless different name appears.
> I have renamed all newly introduced function and macro names to use
> 'noverify_cf'. But I still keep the attribute name as 'notrack'. Historically 
> the
> attribute name follows the public CET specification, which uses 'no-track
> prefix' wording. Is it ok to keep such attribute name?

Here is an updated proposal about option name and attribute name.

The new option has values to let a user to choose what control-flow protection 
to activate.

-fcf-protection=[full|branch|return|none]
  branch - do control-flow protection for indirect jumps and calls
  return - do control-flow protection for function returns
  full - alias to specify both branch + return
  none - turn off protection. This value is needed when/if cf-protection is 
turned on by default by driver in future

Attribute name is the most tough one. Here are several names to evaluate: 
'nocf_verify' or 'nocf_check', or to be more specific and to mimic option name 
'nocf_branch_verify' or 'nocf_branch_check'. I would prefer 'nocf_check' as it 
applies to functions and function pointers so it's definitely related to a 
branch and it's a smaller one.

If you ok with the new proposal I'll implement it in a general parts (code, 
documentation and tests) and resend these patches for review.

Thanks,
Igor

Re: 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling

[Jeff Law]

On 09/12/2017 09:40 AM, Tsimbalist, Igor V wrote:
> 
>> -Original Message-
>> From: Jeff Law [mailto:l...@redhat.com]
>> Sent: Friday, August 25, 2017 10:32 PM
>> To: Richard Biener <richard.guent...@gmail.com>; Tsimbalist, Igor V
>> <igor.v.tsimbal...@intel.com>
>> Cc: gcc-patches@gcc.gnu.org
>> Subject: Re: 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling
>>
>> On 08/15/2017 07:42 AM, Richard Biener wrote:
>>>
>>> Please change the names to omit 'with_', thus just notrack and
>>> GF_CALL_NOTRACK.
>>>
>>> I think 'notrack' is somewhat unspecific of a name, what prevented you
>>> to use 'nocet'?
>> I think we should look for something better than notrack.  I think "control
>> flow enforcement/CFE" is commonly used for this stuff.  CET is an Intel
>> marketing name IIRC.
>>
>> The tracking is for indirect branch/call targets.  So some combination of 
>> cfe,
>> branch/call and track should be sufficient.
> Still remaining question from me - is it ok to use 'notrack' as the attribute 
> name. I've asked Richard
> about this in this thread.
I tend to agree with Richi that "track" is a bit too generic.  no_cfe
might be better.  Or no_cfi, but cfi is commonly used to represent
call-frame-info :-)

jeff

Re: 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling

[Jeff Law]

On 09/13/2017 11:07 AM, Tsimbalist, Igor V wrote:
>> -Original Message-
>> From: Tsimbalist, Igor V
>> Sent: Tuesday, September 12, 2017 5:59 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: 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling
>>
>>
>>> -Original Message-
>>> From: Jeff Law [mailto:l...@redhat.com]
>>> Sent: Friday, August 25, 2017 10:50 PM
>>> To: Tsimbalist, Igor V <igor.v.tsimbal...@intel.com>; 'gcc-
>>> patc...@gcc.gnu.org' <gcc-patches@gcc.gnu.org>
>>> Subject: Re: 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling
>>>
>>> On 08/01/2017 02:56 AM, Tsimbalist, Igor V wrote:
>>>> Part#1. Add generic part for Intel CET enabling.
>>>>
>>
>>> Q. Do we need to do anything with ICF (identical code folding) and CFE?
>>> Given two functions which have the same implementation in gimple,
>>> except that one has a notrack indirect call and the other has a
>>> tracked indirect call, what is proper behavior?  I think we'd keep
>>> them separate which implies we need to make sure the notrack attribute
>>> is part of the ICF hashing implementation.  It'd probably even be
>>> worth building a test for this :-)
>> Are you talking about a case when such two functions are inlined? Or there is
>> a possibility to merge function bodies if they are identical?
>>
>> I agree with you that the functions should be kept separate. I haven't looked
>> into such optimization in gcc so I need to learn it.
> I thought over this case and my conclusion is that nothing has to be done 
> regarding ICF.
> 
> First of all let's sync on a case we are talking about. A code template could 
> look like
> 
> fn1 definition
> {
>   
> }
> 
> fn2 definition with notrack attribute
> {
>   
> }
> 
> func definition
> {
>   ...
> }
> 
> Is it the case you are talking about? Let's consider different scenarios:
> 
> 1) calls to fn1 and fn2 are direct calls. In that case 'notrack' has no 
> effect on direct calls as they are
> assumed to be save (it applies to indirect calls only). ICF can be done here;
> 2) one of calls is an indirect call or both calls are indirect calls. If 
> compiler can prove what exact functions
> are called then indirect call(s) can be replaced by direct call(s) and that 
> gives us the case 1);
> 3) if compiler cannot prove what function is called it will keep the indirect 
> call and so there is nothing
> to do for ICF here. 
No, not the case I'm worried about.  Instead

fn1()
{
  indirect call where the signature is marked with notrack
}

fn2()
{
  indirect call where the signature is not marked with notrack
}


fn1 and fn2 would be subject to ICF which I think is wrong.

Essentially we're carrying semantic information in attributes that are
part of the type of the function pointer.  I think we need to include
those attributes when we hash and compare two objects for equality
within ICF.

Jeff

Re: 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling

[Jeff Law]

On 09/12/2017 09:59 AM, Tsimbalist, Igor V wrote:

> 
>> Q. Do we need to do anything with ICF (identical code folding) and CFE?
>> Given two functions which have the same implementation in gimple, except
>> that one has a notrack indirect call and the other has a tracked indirect 
>> call,
>> what is proper behavior?  I think we'd keep them separate which implies we
>> need to make sure the notrack attribute is part of the ICF hashing
>> implementation.  It'd probably even be worth building a test for this :-)
> Are you talking about a case when such two functions are inlined? Or there is 
> a possibility to merge
> function bodies if they are identical?
The latter.  The compiler has a couple strategies when it finds
identical bodies.  I'm over-simplifying, but given two functions, A and
B.  If ICF hashing finds they are identical, then only one function
definition would be emitted.

So given two functions A & B.  They are identical except that A has an
indirect call and the signature of the call target has the notrack
attribute while B has an indirect call and the signature of the call
target does not have the notrack attribute.

A & B would be subject to ICF, but I don't think that's the right/safe
thing to do.  Or am I missing something?

jeff

RE: 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling

[Tsimbalist, Igor V]

> -Original Message-
> From: Tsimbalist, Igor V
> Sent: Tuesday, September 12, 2017 5:59 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: 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling
> 
> 
> > -Original Message-
> > From: Jeff Law [mailto:l...@redhat.com]
> > Sent: Friday, August 25, 2017 10:50 PM
> > To: Tsimbalist, Igor V <igor.v.tsimbal...@intel.com>; 'gcc-
> > patc...@gcc.gnu.org' <gcc-patches@gcc.gnu.org>
> > Subject: Re: 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling
> >
> > On 08/01/2017 02:56 AM, Tsimbalist, Igor V wrote:
> > > Part#1. Add generic part for Intel CET enabling.
> > >
>
> > Q. Do we need to do anything with ICF (identical code folding) and CFE?
> > Given two functions which have the same implementation in gimple,
> > except that one has a notrack indirect call and the other has a
> > tracked indirect call, what is proper behavior?  I think we'd keep
> > them separate which implies we need to make sure the notrack attribute
> > is part of the ICF hashing implementation.  It'd probably even be
> > worth building a test for this :-)
> Are you talking about a case when such two functions are inlined? Or there is
> a possibility to merge function bodies if they are identical?
> 
> I agree with you that the functions should be kept separate. I haven't looked
> into such optimization in gcc so I need to learn it.
I thought over this case and my conclusion is that nothing has to be done 
regarding ICF.

First of all let's sync on a case we are talking about. A code template could 
look like

fn1 definition
{
  
}

fn2 definition with notrack attribute
{
  
}

func definition
{
  ...
}

Is it the case you are talking about? Let's consider different scenarios:

1) calls to fn1 and fn2 are direct calls. In that case 'notrack' has no effect 
on direct calls as they are
assumed to be save (it applies to indirect calls only). ICF can be done here;
2) one of calls is an indirect call or both calls are indirect calls. If 
compiler can prove what exact functions
are called then indirect call(s) can be replaced by direct call(s) and that 
gives us the case 1);
3) if compiler cannot prove what function is called it will keep the indirect 
call and so there is nothing
to do for ICF here. 

Thanks,
Igor

> 
> > >  }
> > >
> > >
> > > +/* Return true if call GS is marked as no-track.  */
> > > +
> > > +static inline bool
> > > +gimple_call_with_notrack_p (const gcall *gs) {
> > > +  return (gs->subcode & GF_CALL_WITH_NOTRACK) != 0; }
> > > +
> > > +static inline bool
> > > +gimple_call_with_notrack_p (const gimple *gs) {
> > > +  const gcall *gc = GIMPLE_CHECK2 (gs);
> > > +  return gimple_call_with_notrack_p (gc); }
> > Agree with Richi WRT avoiding gimple * overloads.
> Fixed.
> 
> Thanks,
> Igor
> 
> >
> >
> > Jeff

RE: 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling

[Tsimbalist, Igor V]

> -Original Message-
> From: Jeff Law [mailto:l...@redhat.com]
> Sent: Friday, August 25, 2017 10:50 PM
> To: Tsimbalist, Igor V <igor.v.tsimbal...@intel.com>; 'gcc-
> patc...@gcc.gnu.org' <gcc-patches@gcc.gnu.org>
> Subject: Re: 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling
> 
> On 08/01/2017 02:56 AM, Tsimbalist, Igor V wrote:
> > Part#1. Add generic part for Intel CET enabling.
> >
> > The spec is available at
> >
> > https://software.intel.com/sites/default/files/managed/4d/2a/control-f
> > low-enforcement-technology-preview.pdf
> >
> > High-level design.
> > --
> >
> > A proposal is to introduce a target independent flag
> > -finstrument-control-flow with a semantic to instrument a code to
> > control validness or integrity of control-flow transfers using jump
> > and call instructions. The main goal is to detect and block a possible
> > malware execution through transfer the execution to unknown target
> > address. Implementation could be either software or target based. Any
> > target platforms can provide their implementation for instrumentation
> > under this option.
> >
> > When the -finstrument-control-flow flag is set each implementation has
> > to check if a support exists for a target platform and report an error
> > if no support is found.
> >
> > The compiler should instrument any control-flow transfer points in a
> > program (ex. call/jmp/ret) as well as any landing pads, which are
> > targets of for control-flow transfers.
> >
> > A new 'notrack' attribute is introduced to provide hand tuning support.
> > The attribute directs the compiler to skip a call to a function and a
> > function's landing pad from instrumentation (tracking). The attribute
> > can be used for function and pointer to function types, otherwise it
> > will be ignored. The attribute is saved in a type and propagated to a
> > GIMPLE call statement and later to a call instruction.
> >
> > Currently all platforms except i386 will report the error and do no
> > instrumentation. i386 will provide the implementation based on a
> > specification published by Intel for a new technology called
> > Control-flow Enforcement Technology (CET).
> >
> >
> > 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling.patch
> >
> >
> > From 403fc8239fb1f690cc378287b4def57dcc9d25bf Mon Sep 17 00:00:00
> 2001
> > From: Igor Tsimbalist <igor.v.tsimbal...@intel.com>
> > Date: Mon, 3 Jul 2017 17:11:58 +0300
> > Subject: [PATCH 1/9] Part#1. Add generic part for Intel CET enabling.
> >
> > The spec is available at
> >
> > https://software.intel.com/sites/default/files/managed/4d/2a/control-f
> > low-enforcement-technology-preview.pdf
> >
> > High-level design.
> > --
> >
> > A proposal is to introduce a target independent flag
> > -finstrument-control-flow with a semantic to instrument a code to
> > control validness or integrity of control-flow transfers using jump
> > and call instructions. The main goal is to detect and block a possible
> > malware execution through transfer the execution to unknown target
> > address. Implementation could be either software or target based. Any
> > target platforms can provide their implementation for instrumentation
> > under this option.
> >
> > When the -finstrument-control-flow flag is set each implementation has
> > to check if a support exists for a target platform and report an error
> > if no support is found.
> >
> > The compiler should instrument any control-flow transfer points in a
> > program (ex. call/jmp/ret) as well as any landing pads, which are
> > targets of for control-flow transfers.
> >
> > A new 'notrack' attribute is introduced to provide hand tuning support.
> > The attribute directs the compiler to skip a call to a function and a
> > function's landing pad from instrumentation (tracking). The attribute
> > can be used for function and pointer to function types, otherwise it
> > will be ignored. The attribute is saved in a type and propagated to a
> > GIMPLE call statement and later to a call instruction.
> >
> > Currently all platforms except i386 will report the error and do no
> > instrumentation. i386 will provide the implementation based on a
> > specification published by Intel for a new technology called
> > Control-flow Enforcement Technology (CET).
> >
> > gcc/c-family/
> >
> > * c-attribs.c (handle_notrack_attribute): New function.
> > (c_common_attribut

RE: 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling

[Tsimbalist, Igor V]

> -Original Message-
> From: Jeff Law [mailto:l...@redhat.com]
> Sent: Friday, August 25, 2017 10:32 PM
> To: Richard Biener <richard.guent...@gmail.com>; Tsimbalist, Igor V
> <igor.v.tsimbal...@intel.com>
> Cc: gcc-patches@gcc.gnu.org
> Subject: Re: 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling
> 
> On 08/15/2017 07:42 AM, Richard Biener wrote:
> >
> > Please change the names to omit 'with_', thus just notrack and
> > GF_CALL_NOTRACK.
> >
> > I think 'notrack' is somewhat unspecific of a name, what prevented you
> > to use 'nocet'?
> I think we should look for something better than notrack.  I think "control
> flow enforcement/CFE" is commonly used for this stuff.  CET is an Intel
> marketing name IIRC.
> 
> The tracking is for indirect branch/call targets.  So some combination of cfe,
> branch/call and track should be sufficient.
Still remaining question from me - is it ok to use 'notrack' as the attribute 
name. I've asked Richard
about this in this thread.

Thanks,
Igor

> 
> > Any idea how to implement a software-based solution efficiently?
> > Creating a table of valid destination addresses in a special section
> > should be possible without too much work, am I right in that only
> > indirect control transfer is checked?  Thus CET assumes the code
> > itself cannot be changed (and thus the stack isn't executable)?
> Well, there's two broad areas that have to be addressed.
> 
> First you need to separate the call stack from the rest of the call frame, or 
> at
> least the parts of the call frame that are potentially vulnerable to overruns.
> LLVM has some code to do this.  Essentially any object in the stack that is 
> not
> proven to be safely accessed gets put into a separate stack.  That roughly
> duplicates the shadow stack capability.  I think their implementation is just
> x86 and IIRC doesn't work in some circumstances -- I'd consider it a proof of
> concept, not something ready for production use.
> 
> 
> Bernd and I also spec'd a couple more approaches to protect the return
> address.  Essentially, the return address turns into a cookie that a 
> particular
> caller can use to lookup/map to a real return address.  We didn't take any of
> this to completion because it was pretty clear the ROP mitigation landscape
> was going to change and make software only solutions less appealing.
> 
> Second you need the indirect branch/call tracking.  I spec'd something out in
> this space with Intel's engineers years ago.  Essentially building tables of 
> valid
> targets for indirect branches and checking
> instrumentation.   You can have a global table, per-DSO tables or you
> can have a per-branch table.  It gets a little hairy in mixed mode
> environments.  But it basically works how you'd expect.  Indirect
> branches/calls turn into something considerably more complex as do the
> branch/call targets if you have access to something like a last-taken-branch.
> 
> Jeff

RE: 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling

[Tsimbalist, Igor V]

> -Original Message-
> From: Tsimbalist, Igor V
> Sent: Friday, August 18, 2017 4:43 PM
> To: 'Richard Biener' <richard.guent...@gmail.com>
> Cc: gcc-patches@gcc.gnu.org; Tsimbalist, Igor V
> <igor.v.tsimbal...@intel.com>
> Subject: RE: 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling
> 
> > -Original Message-
> > From: Richard Biener [mailto:richard.guent...@gmail.com]
> > Sent: Friday, August 18, 2017 3:53 PM
> > To: Tsimbalist, Igor V <igor.v.tsimbal...@intel.com>
> > Cc: gcc-patches@gcc.gnu.org
> > Subject: Re: 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling
> >
> > On Fri, Aug 18, 2017 at 3:11 PM, Tsimbalist, Igor V
> > <igor.v.tsimbal...@intel.com> wrote:
> > >> -Original Message-
> > >> From: Richard Biener [mailto:richard.guent...@gmail.com]
> > >> Sent: Tuesday, August 15, 2017 3:43 PM
> > >> To: Tsimbalist, Igor V <igor.v.tsimbal...@intel.com>
> > >> Cc: gcc-patches@gcc.gnu.org
> > >> Subject: Re: 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling
> > >>
> > >> On Tue, Aug 1, 2017 at 10:56 AM, Tsimbalist, Igor V
> > >> <igor.v.tsimbal...@intel.com> wrote:
> > >> > Part#1. Add generic part for Intel CET enabling.
> > >> >
> > >> > The spec is available at
> > >> >
> > >> > https://software.intel.com/sites/default/files/managed/4d/2a/cont
> > >> > ro l-f low-enforcement-technology-preview.pdf
> > >> >
> > >> > High-level design.
> > >> > --
> > >> >
> > >> > A proposal is to introduce a target independent flag
> > >> > -finstrument-control-flow with a semantic to instrument a code to
> > >> > control validness or integrity of control-flow transfers using
> > >> > jump and call instructions. The main goal is to detect and block
> > >> > a possible malware execution through transfer the execution to
> > >> > unknown target address. Implementation could be either software
> > >> > or target based. Any target platforms can provide their
> > >> > implementation for instrumentation under this option.
> > >> >
> > >> > When the -finstrument-control-flow flag is set each
> > >> > implementation has to check if a support exists for a target
> > >> > platform and report an error if no support is found.
> > >> >
> > >> > The compiler should instrument any control-flow transfer points
> > >> > in a program (ex. call/jmp/ret) as well as any landing pads,
> > >> > which are targets of for control-flow transfers.
> > >> >
> > >> > A new 'notrack' attribute is introduced to provide hand tuning
> support.
> > >> > The attribute directs the compiler to skip a call to a function
> > >> > and a function's landing pad from instrumentation (tracking). The
> > >> > attribute can be used for function and pointer to function types,
> > >> > otherwise it will be ignored. The attribute is saved in a type
> > >> > and propagated to a GIMPLE call statement and later to a call
> instruction.
> > >> >
> > >> > Currently all platforms except i386 will report the error and do
> > >> > no instrumentation. i386 will provide the implementation based on
> > >> > a specification published by Intel for a new technology called
> > >> > Control-flow Enforcement Technology (CET).
> > >>
> > >> diff --git a/gcc/gimple.c b/gcc/gimple.c index 479f90c..2e4ab2d
> > >> 100644
> > >> --- a/gcc/gimple.c
> > >> +++ b/gcc/gimple.c
> > >> @@ -378,6 +378,23 @@ gimple_build_call_from_tree (tree t)
> > >>gimple_set_no_warning (call, TREE_NO_WARNING (t));
> > >>gimple_call_set_with_bounds (call, CALL_WITH_BOUNDS_P (t));
> > >>
> > >> +  if (fndecl == NULL_TREE)
> > >> +{
> > >> +  /* Find the type of an indirect call.  */
> > >> +  tree addr = CALL_EXPR_FN (t);
> > >> +  if (TREE_CODE (addr) != FUNCTION_DECL)
> > >> +   {
> > >> + tree fntype = TREE_TYPE (addr);
> > >> + gcc_assert (POINTER_TYPE_P (fntype));
> > >> + fntype = TREE_TYPE (fntype);
> > >> +
> > >> + /* Check if its type has the no-track attribute and propagate

Re: 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling

[Jeff Law]

On 08/01/2017 02:56 AM, Tsimbalist, Igor V wrote:
> Part#1. Add generic part for Intel CET enabling.
> 
> The spec is available at
> 
> https://software.intel.com/sites/default/files/managed/4d/2a/control-flow-enforcement-technology-preview.pdf
> 
> High-level design.
> --
> 
> A proposal is to introduce a target independent flag
> -finstrument-control-flow with a semantic to instrument a code to
> control validness or integrity of control-flow transfers using jump
> and call instructions. The main goal is to detect and block a possible
> malware execution through transfer the execution to unknown target
> address. Implementation could be either software or target based. Any
> target platforms can provide their implementation for instrumentation
> under this option.
> 
> When the -finstrument-control-flow flag is set each implementation has
> to check if a support exists for a target platform and report an error
> if no support is found.
> 
> The compiler should instrument any control-flow transfer points in a
> program (ex. call/jmp/ret) as well as any landing pads, which are
> targets of for control-flow transfers.
> 
> A new 'notrack' attribute is introduced to provide hand tuning support.
> The attribute directs the compiler to skip a call to a function and a
> function's landing pad from instrumentation (tracking). The attribute
> can be used for function and pointer to function types, otherwise it
> will be ignored. The attribute is saved in a type and propagated to a
> GIMPLE call statement and later to a call instruction.
> 
> Currently all platforms except i386 will report the error and do no
> instrumentation. i386 will provide the implementation based on a
> specification published by Intel for a new technology called
> Control-flow Enforcement Technology (CET).
> 
> 
> 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling.patch
> 
> 
> From 403fc8239fb1f690cc378287b4def57dcc9d25bf Mon Sep 17 00:00:00 2001
> From: Igor Tsimbalist 
> Date: Mon, 3 Jul 2017 17:11:58 +0300
> Subject: [PATCH 1/9] Part#1. Add generic part for Intel CET enabling.
> 
> The spec is available at
> 
> https://software.intel.com/sites/default/files/managed/4d/2a/control-flow-enforcement-technology-preview.pdf
> 
> High-level design.
> --
> 
> A proposal is to introduce a target independent flag
> -finstrument-control-flow with a semantic to instrument a code to
> control validness or integrity of control-flow transfers using jump
> and call instructions. The main goal is to detect and block a possible
> malware execution through transfer the execution to unknown target
> address. Implementation could be either software or target based. Any
> target platforms can provide their implementation for instrumentation
> under this option.
> 
> When the -finstrument-control-flow flag is set each implementation has
> to check if a support exists for a target platform and report an error
> if no support is found.
> 
> The compiler should instrument any control-flow transfer points in a
> program (ex. call/jmp/ret) as well as any landing pads, which are
> targets of for control-flow transfers.
> 
> A new 'notrack' attribute is introduced to provide hand tuning support.
> The attribute directs the compiler to skip a call to a function and a
> function's landing pad from instrumentation (tracking). The attribute
> can be used for function and pointer to function types, otherwise it
> will be ignored. The attribute is saved in a type and propagated to a
> GIMPLE call statement and later to a call instruction.
> 
> Currently all platforms except i386 will report the error and do no
> instrumentation. i386 will provide the implementation based on a
> specification published by Intel for a new technology called
> Control-flow Enforcement Technology (CET).
> 
> gcc/c-family/
> 
>   * c-attribs.c (handle_notrack_attribute): New function.
>   (c_common_attribute_table): Add 'notrack' handling.
> 
> gcc/
> 
>   * cfgexpand.c (expand_call_stmt): Set REG_CALL_NOTRACK.
>   * combine.c (distribute_notes): Add REG_CALL_NOTRACK handling.
>   * common.opt: Add finstrument-control-flow flag.
>   * emit-rtl.c (try_split): Add REG_CALL_NOTRACK handling.
>   * gimple.c (gimple_build_call_from_tree): Add 'notrack'
>   propogation.
>   * gimple.h (gf_mask): Add GF_CALL_WITH_NOTRACK.
>   (gimple_call_with_notrack_p): function.
>   (gimple_call_set_with_notrack): Likewise.
>   * recog.c (peep2_attempt): Add REG_CALL_NOTRACK handling.
>   * reg-notes.def: Add REG_NOTE (CALL_NOTRACK).
>   * toplev.c (process_options): Add flag_instrument_control_flow
>   handling.
> ---
>  gcc/c-family/c-attribs.c | 23 +++
>  gcc/cfgexpand.c  | 11 +++
>  gcc/combine.c|  1 +
>  gcc/common.opt   |  5 +
>  gcc/emit-rtl.c   |  1 +
>  gcc/gimple.c | 17 +
>  gcc/gimple.h  

Re: 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling

[Jeff Law]

On 08/15/2017 07:42 AM, Richard Biener wrote:
> 
> Please change the names to omit 'with_', thus just notrack
> and GF_CALL_NOTRACK.
> 
> I think 'notrack' is somewhat unspecific of a name, what prevented
> you to use 'nocet'?
I think we should look for something better than notrack.  I think
"control flow enforcement/CFE" is commonly used for this stuff.  CET is
an Intel marketing name IIRC.

The tracking is for indirect branch/call targets.  So some combination
of cfe, branch/call and track should be sufficient.



> Any idea how to implement a software-based solution efficiently?
> Creating a table of valid destination addresses in a special section
> should be possible without too much work, am I right in that only
> indirect control transfer is checked?  Thus CET assumes the
> code itself cannot be changed (and thus the stack isn't executable)?
Well, there's two broad areas that have to be addressed.

First you need to separate the call stack from the rest of the call
frame, or at least the parts of the call frame that are potentially
vulnerable to overruns.  LLVM has some code to do this.  Essentially any
object in the stack that is not proven to be safely accessed gets put
into a separate stack.  That roughly duplicates the shadow stack
capability.  I think their implementation is just x86 and IIRC doesn't
work in some circumstances -- I'd consider it a proof of concept, not
something ready for production use.


Bernd and I also spec'd a couple more approaches to protect the return
address.  Essentially, the return address turns into a cookie that a
particular caller can use to lookup/map to a real return address.  We
didn't take any of this to completion because it was pretty clear the
ROP mitigation landscape was going to change and make software only
solutions less appealing.

Second you need the indirect branch/call tracking.  I spec'd something
out in this space with Intel's engineers years ago.  Essentially
building tables of valid targets for indirect branches and checking
instrumentation.   You can have a global table, per-DSO tables or you
can have a per-branch table.  It gets a little hairy in mixed mode
environments.  But it basically works how you'd expect.  Indirect
branches/calls turn into something considerably more complex as do the
branch/call targets if you have access to something like a
last-taken-branch.

Jeff

RE: 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling

[Tsimbalist, Igor V]

> -Original Message-
> From: Richard Biener [mailto:richard.guent...@gmail.com]
> Sent: Friday, August 18, 2017 3:53 PM
> To: Tsimbalist, Igor V <igor.v.tsimbal...@intel.com>
> Cc: gcc-patches@gcc.gnu.org
> Subject: Re: 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling
> 
> On Fri, Aug 18, 2017 at 3:11 PM, Tsimbalist, Igor V
> <igor.v.tsimbal...@intel.com> wrote:
> >> -Original Message-
> >> From: Richard Biener [mailto:richard.guent...@gmail.com]
> >> Sent: Tuesday, August 15, 2017 3:43 PM
> >> To: Tsimbalist, Igor V <igor.v.tsimbal...@intel.com>
> >> Cc: gcc-patches@gcc.gnu.org
> >> Subject: Re: 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling
> >>
> >> On Tue, Aug 1, 2017 at 10:56 AM, Tsimbalist, Igor V
> >> <igor.v.tsimbal...@intel.com> wrote:
> >> > Part#1. Add generic part for Intel CET enabling.
> >> >
> >> > The spec is available at
> >> >
> >> > https://software.intel.com/sites/default/files/managed/4d/2a/contro
> >> > l-f low-enforcement-technology-preview.pdf
> >> >
> >> > High-level design.
> >> > --
> >> >
> >> > A proposal is to introduce a target independent flag
> >> > -finstrument-control-flow with a semantic to instrument a code to
> >> > control validness or integrity of control-flow transfers using jump
> >> > and call instructions. The main goal is to detect and block a
> >> > possible malware execution through transfer the execution to
> >> > unknown target address. Implementation could be either software or
> >> > target based. Any target platforms can provide their implementation
> >> > for instrumentation under this option.
> >> >
> >> > When the -finstrument-control-flow flag is set each implementation
> >> > has to check if a support exists for a target platform and report
> >> > an error if no support is found.
> >> >
> >> > The compiler should instrument any control-flow transfer points in
> >> > a program (ex. call/jmp/ret) as well as any landing pads, which are
> >> > targets of for control-flow transfers.
> >> >
> >> > A new 'notrack' attribute is introduced to provide hand tuning support.
> >> > The attribute directs the compiler to skip a call to a function and
> >> > a function's landing pad from instrumentation (tracking). The
> >> > attribute can be used for function and pointer to function types,
> >> > otherwise it will be ignored. The attribute is saved in a type and
> >> > propagated to a GIMPLE call statement and later to a call instruction.
> >> >
> >> > Currently all platforms except i386 will report the error and do no
> >> > instrumentation. i386 will provide the implementation based on a
> >> > specification published by Intel for a new technology called
> >> > Control-flow Enforcement Technology (CET).
> >>
> >> diff --git a/gcc/gimple.c b/gcc/gimple.c index 479f90c..2e4ab2d
> >> 100644
> >> --- a/gcc/gimple.c
> >> +++ b/gcc/gimple.c
> >> @@ -378,6 +378,23 @@ gimple_build_call_from_tree (tree t)
> >>gimple_set_no_warning (call, TREE_NO_WARNING (t));
> >>gimple_call_set_with_bounds (call, CALL_WITH_BOUNDS_P (t));
> >>
> >> +  if (fndecl == NULL_TREE)
> >> +{
> >> +  /* Find the type of an indirect call.  */
> >> +  tree addr = CALL_EXPR_FN (t);
> >> +  if (TREE_CODE (addr) != FUNCTION_DECL)
> >> +   {
> >> + tree fntype = TREE_TYPE (addr);
> >> + gcc_assert (POINTER_TYPE_P (fntype));
> >> + fntype = TREE_TYPE (fntype);
> >> +
> >> + /* Check if its type has the no-track attribute and propagate
> >> +it to the CALL insn.  */
> >> + if (lookup_attribute ("notrack", TYPE_ATTRIBUTES (fntype)))
> >> +   gimple_call_set_with_notrack (call, TRUE);
> >> +   }
> >> +}
> >>
> >> this means notrack is not recognized if fndecl is not NULL.  Note
> >> that only the two callers know the real function type in effect (they
> >> call gimple_call_set_fntype with it).  I suggest to pass down that
> >> type to gimple_build_call_from_tree and move the
> >> gimple_call_set_fntype call there as well.  And simply use the type for the
> above.
> >
> > The best way to s

Re: 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling

[Richard Biener]

On Fri, Aug 18, 2017 at 3:11 PM, Tsimbalist, Igor V
<igor.v.tsimbal...@intel.com> wrote:
>> -Original Message-
>> From: Richard Biener [mailto:richard.guent...@gmail.com]
>> Sent: Tuesday, August 15, 2017 3:43 PM
>> To: Tsimbalist, Igor V <igor.v.tsimbal...@intel.com>
>> Cc: gcc-patches@gcc.gnu.org
>> Subject: Re: 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling
>>
>> On Tue, Aug 1, 2017 at 10:56 AM, Tsimbalist, Igor V
>> <igor.v.tsimbal...@intel.com> wrote:
>> > Part#1. Add generic part for Intel CET enabling.
>> >
>> > The spec is available at
>> >
>> > https://software.intel.com/sites/default/files/managed/4d/2a/control-f
>> > low-enforcement-technology-preview.pdf
>> >
>> > High-level design.
>> > --
>> >
>> > A proposal is to introduce a target independent flag
>> > -finstrument-control-flow with a semantic to instrument a code to
>> > control validness or integrity of control-flow transfers using jump
>> > and call instructions. The main goal is to detect and block a possible
>> > malware execution through transfer the execution to unknown target
>> > address. Implementation could be either software or target based. Any
>> > target platforms can provide their implementation for instrumentation
>> > under this option.
>> >
>> > When the -finstrument-control-flow flag is set each implementation has
>> > to check if a support exists for a target platform and report an error
>> > if no support is found.
>> >
>> > The compiler should instrument any control-flow transfer points in a
>> > program (ex. call/jmp/ret) as well as any landing pads, which are
>> > targets of for control-flow transfers.
>> >
>> > A new 'notrack' attribute is introduced to provide hand tuning support.
>> > The attribute directs the compiler to skip a call to a function and a
>> > function's landing pad from instrumentation (tracking). The attribute
>> > can be used for function and pointer to function types, otherwise it
>> > will be ignored. The attribute is saved in a type and propagated to a
>> > GIMPLE call statement and later to a call instruction.
>> >
>> > Currently all platforms except i386 will report the error and do no
>> > instrumentation. i386 will provide the implementation based on a
>> > specification published by Intel for a new technology called
>> > Control-flow Enforcement Technology (CET).
>>
>> diff --git a/gcc/gimple.c b/gcc/gimple.c index 479f90c..2e4ab2d 100644
>> --- a/gcc/gimple.c
>> +++ b/gcc/gimple.c
>> @@ -378,6 +378,23 @@ gimple_build_call_from_tree (tree t)
>>gimple_set_no_warning (call, TREE_NO_WARNING (t));
>>gimple_call_set_with_bounds (call, CALL_WITH_BOUNDS_P (t));
>>
>> +  if (fndecl == NULL_TREE)
>> +{
>> +  /* Find the type of an indirect call.  */
>> +  tree addr = CALL_EXPR_FN (t);
>> +  if (TREE_CODE (addr) != FUNCTION_DECL)
>> +   {
>> + tree fntype = TREE_TYPE (addr);
>> + gcc_assert (POINTER_TYPE_P (fntype));
>> + fntype = TREE_TYPE (fntype);
>> +
>> + /* Check if its type has the no-track attribute and propagate
>> +it to the CALL insn.  */
>> + if (lookup_attribute ("notrack", TYPE_ATTRIBUTES (fntype)))
>> +   gimple_call_set_with_notrack (call, TRUE);
>> +   }
>> +}
>>
>> this means notrack is not recognized if fndecl is not NULL.  Note that only 
>> the
>> two callers know the real function type in effect (they call
>> gimple_call_set_fntype with it).  I suggest to pass down that type to
>> gimple_build_call_from_tree and move the gimple_call_set_fntype call
>> there as well.  And simply use the type for the above.
>
> The best way to say is notrack is not propagated if fndecl is not NULL. 
> Fndecl, if not NULL, is a direct call and notrack is not applicable for such 
> calls. I will add a comment before the if.

Hmm.  But what does this mean?  I guess direct calls are always
'notrack' then and thus we're fine
to ignore it.

> I would like to propose modifying the existing code without changing 
> interfaces. The idea is that at the time the notrack is propagated (the code 
> snippet above) the gimple call was created and the correct type was assigned 
> to the 'call' exactly by gimple_call_set_fntype. My proposal is to get the 
> type out of the gimple 'call' (like gimple_call_fntype) instead of the tree 
> 't'. Is it righ

RE: 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling

[Tsimbalist, Igor V]

> -Original Message-
> From: Richard Biener [mailto:richard.guent...@gmail.com]
> Sent: Tuesday, August 15, 2017 3:43 PM
> To: Tsimbalist, Igor V <igor.v.tsimbal...@intel.com>
> Cc: gcc-patches@gcc.gnu.org
> Subject: Re: 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling
> 
> On Tue, Aug 1, 2017 at 10:56 AM, Tsimbalist, Igor V
> <igor.v.tsimbal...@intel.com> wrote:
> > Part#1. Add generic part for Intel CET enabling.
> >
> > The spec is available at
> >
> > https://software.intel.com/sites/default/files/managed/4d/2a/control-f
> > low-enforcement-technology-preview.pdf
> >
> > High-level design.
> > --
> >
> > A proposal is to introduce a target independent flag
> > -finstrument-control-flow with a semantic to instrument a code to
> > control validness or integrity of control-flow transfers using jump
> > and call instructions. The main goal is to detect and block a possible
> > malware execution through transfer the execution to unknown target
> > address. Implementation could be either software or target based. Any
> > target platforms can provide their implementation for instrumentation
> > under this option.
> >
> > When the -finstrument-control-flow flag is set each implementation has
> > to check if a support exists for a target platform and report an error
> > if no support is found.
> >
> > The compiler should instrument any control-flow transfer points in a
> > program (ex. call/jmp/ret) as well as any landing pads, which are
> > targets of for control-flow transfers.
> >
> > A new 'notrack' attribute is introduced to provide hand tuning support.
> > The attribute directs the compiler to skip a call to a function and a
> > function's landing pad from instrumentation (tracking). The attribute
> > can be used for function and pointer to function types, otherwise it
> > will be ignored. The attribute is saved in a type and propagated to a
> > GIMPLE call statement and later to a call instruction.
> >
> > Currently all platforms except i386 will report the error and do no
> > instrumentation. i386 will provide the implementation based on a
> > specification published by Intel for a new technology called
> > Control-flow Enforcement Technology (CET).
> 
> diff --git a/gcc/gimple.c b/gcc/gimple.c index 479f90c..2e4ab2d 100644
> --- a/gcc/gimple.c
> +++ b/gcc/gimple.c
> @@ -378,6 +378,23 @@ gimple_build_call_from_tree (tree t)
>gimple_set_no_warning (call, TREE_NO_WARNING (t));
>gimple_call_set_with_bounds (call, CALL_WITH_BOUNDS_P (t));
> 
> +  if (fndecl == NULL_TREE)
> +{
> +  /* Find the type of an indirect call.  */
> +  tree addr = CALL_EXPR_FN (t);
> +  if (TREE_CODE (addr) != FUNCTION_DECL)
> +   {
> + tree fntype = TREE_TYPE (addr);
> + gcc_assert (POINTER_TYPE_P (fntype));
> + fntype = TREE_TYPE (fntype);
> +
> + /* Check if its type has the no-track attribute and propagate
> +it to the CALL insn.  */
> + if (lookup_attribute ("notrack", TYPE_ATTRIBUTES (fntype)))
> +   gimple_call_set_with_notrack (call, TRUE);
> +   }
> +}
> 
> this means notrack is not recognized if fndecl is not NULL.  Note that only 
> the
> two callers know the real function type in effect (they call
> gimple_call_set_fntype with it).  I suggest to pass down that type to
> gimple_build_call_from_tree and move the gimple_call_set_fntype call
> there as well.  And simply use the type for the above.

The best way to say is notrack is not propagated if fndecl is not NULL. Fndecl, 
if not NULL, is a direct call and notrack is not applicable for such calls. I 
will add a comment before the if.

I would like to propose modifying the existing code without changing 
interfaces. The idea is that at the time the notrack is propagated (the code 
snippet above) the gimple call was created and the correct type was assigned to 
the 'call' exactly by gimple_call_set_fntype. My proposal is to get the type 
out of the gimple 'call' (like gimple_call_fntype) instead of the tree 't'. Is 
it right?

> +static inline bool
> +gimple_call_with_notrack_p (const gimple *gs) {
> +  const gcall *gc = GIMPLE_CHECK2 (gs);
> +  return gimple_call_with_notrack_p (gc); }
> 
> please do not add gimple * overloads for new APIs, instead make sure to
> pass down gcalls at callers.

Ok, I will remove.

> Please change the names to omit 'with_', thus just notrack and
> GF_CALL_NOTRACK.

Ok, I will rename.

> I think 'notrack' is somewhat unspecific of a name, what prevented you to
> use 'nocet'?

Actually it's specific. The HW will have a prefi

Re: 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling

[Richard Biener]

On Tue, Aug 1, 2017 at 10:56 AM, Tsimbalist, Igor V
 wrote:
> Part#1. Add generic part for Intel CET enabling.
>
> The spec is available at
>
> https://software.intel.com/sites/default/files/managed/4d/2a/control-flow-enforcement-technology-preview.pdf
>
> High-level design.
> --
>
> A proposal is to introduce a target independent flag
> -finstrument-control-flow with a semantic to instrument a code to
> control validness or integrity of control-flow transfers using jump
> and call instructions. The main goal is to detect and block a possible
> malware execution through transfer the execution to unknown target
> address. Implementation could be either software or target based. Any
> target platforms can provide their implementation for instrumentation
> under this option.
>
> When the -finstrument-control-flow flag is set each implementation has
> to check if a support exists for a target platform and report an error
> if no support is found.
>
> The compiler should instrument any control-flow transfer points in a
> program (ex. call/jmp/ret) as well as any landing pads, which are
> targets of for control-flow transfers.
>
> A new 'notrack' attribute is introduced to provide hand tuning support.
> The attribute directs the compiler to skip a call to a function and a
> function's landing pad from instrumentation (tracking). The attribute
> can be used for function and pointer to function types, otherwise it
> will be ignored. The attribute is saved in a type and propagated to a
> GIMPLE call statement and later to a call instruction.
>
> Currently all platforms except i386 will report the error and do no
> instrumentation. i386 will provide the implementation based on a
> specification published by Intel for a new technology called
> Control-flow Enforcement Technology (CET).

diff --git a/gcc/gimple.c b/gcc/gimple.c
index 479f90c..2e4ab2d 100644
--- a/gcc/gimple.c
+++ b/gcc/gimple.c
@@ -378,6 +378,23 @@ gimple_build_call_from_tree (tree t)
   gimple_set_no_warning (call, TREE_NO_WARNING (t));
   gimple_call_set_with_bounds (call, CALL_WITH_BOUNDS_P (t));

+  if (fndecl == NULL_TREE)
+{
+  /* Find the type of an indirect call.  */
+  tree addr = CALL_EXPR_FN (t);
+  if (TREE_CODE (addr) != FUNCTION_DECL)
+   {
+ tree fntype = TREE_TYPE (addr);
+ gcc_assert (POINTER_TYPE_P (fntype));
+ fntype = TREE_TYPE (fntype);
+
+ /* Check if its type has the no-track attribute and propagate
+it to the CALL insn.  */
+ if (lookup_attribute ("notrack", TYPE_ATTRIBUTES (fntype)))
+   gimple_call_set_with_notrack (call, TRUE);
+   }
+}

this means notrack is not recognized if fndecl is not NULL.  Note that only
the two callers know the real function type in effect (they call
gimple_call_set_fntype with it).  I suggest to pass down that type
to gimple_build_call_from_tree and move the gimple_call_set_fntype
call there as well.  And simply use the type for the above.

+static inline bool
+gimple_call_with_notrack_p (const gimple *gs)
+{
+  const gcall *gc = GIMPLE_CHECK2 (gs);
+  return gimple_call_with_notrack_p (gc);
+}

please do not add gimple * overloads for new APIs, instead make
sure to pass down gcalls at callers.

Please change the names to omit 'with_', thus just notrack
and GF_CALL_NOTRACK.

I think 'notrack' is somewhat unspecific of a name, what prevented
you to use 'nocet'?

Any idea how to implement a software-based solution efficiently?
Creating a table of valid destination addresses in a special section
should be possible without too much work, am I right in that only
indirect control transfer is checked?  Thus CET assumes the
code itself cannot be changed (and thus the stack isn't executable)?

Thanks,
Richard.

0001-Part-1.-Add-generic-part-for-Intel-CET-enabling

[Tsimbalist, Igor V]

Part#1. Add generic part for Intel CET enabling.

The spec is available at

https://software.intel.com/sites/default/files/managed/4d/2a/control-flow-enforcement-technology-preview.pdf

High-level design.
--

A proposal is to introduce a target independent flag
-finstrument-control-flow with a semantic to instrument a code to
control validness or integrity of control-flow transfers using jump
and call instructions. The main goal is to detect and block a possible
malware execution through transfer the execution to unknown target
address. Implementation could be either software or target based. Any
target platforms can provide their implementation for instrumentation
under this option.

When the -finstrument-control-flow flag is set each implementation has
to check if a support exists for a target platform and report an error
if no support is found.

The compiler should instrument any control-flow transfer points in a
program (ex. call/jmp/ret) as well as any landing pads, which are
targets of for control-flow transfers.

A new 'notrack' attribute is introduced to provide hand tuning support.
The attribute directs the compiler to skip a call to a function and a
function's landing pad from instrumentation (tracking). The attribute
can be used for function and pointer to function types, otherwise it
will be ignored. The attribute is saved in a type and propagated to a
GIMPLE call statement and later to a call instruction.

Currently all platforms except i386 will report the error and do no
instrumentation. i386 will provide the implementation based on a
specification published by Intel for a new technology called
Control-flow Enforcement Technology (CET).


0001-Part-1.-Add-generic-part-for-Intel-CET-enabling.patch
Description: 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling.patch