[PATCH 1/4] x86: Add -mindirect-branch=

Jan Hubicka hubicka@ucw.cz
Sat Jan 13 16:03:00 GMT 2018


> >> +/* Output a funtion with a call and return thunk for indirect branch.
> >> +   If BND_P is true, the BND prefix is needed.   If REGNO != -1,  the
> >> +   function address is in REGNO.  Otherwise, the function address is
> >> +   on the top of stack.  */
> >> +
> >> +static void
> >> +output_indirect_thunk_function (bool need_bnd_p, int regno)
> >> +{
> >> +  char name[32];
> >> +  tree decl;
> >> +
> >> +  /* Create __x86_indirect_thunk/__x86_indirect_thunk_bnd.  */
> >> +  indirect_thunk_name (name, regno, need_bnd_p);
> >> +  decl = build_decl (BUILTINS_LOCATION, FUNCTION_DECL,
> >> +                  get_identifier (name),
> >> +                  build_function_type_list (void_type_node, NULL_TREE));
> >> +  DECL_RESULT (decl) = build_decl (BUILTINS_LOCATION, RESULT_DECL,
> >> +                                NULL_TREE, void_type_node);
> >> +  TREE_PUBLIC (decl) = 1;
> >> +  TREE_STATIC (decl) = 1;
> >> +  DECL_IGNORED_P (decl) = 1;
> >
> > DECL_ARTIFICIAL as well?
> 
> This is done exactly the same way as PIC thunk.  I don't think we
> should change it here.
> >
> > Why do you need to output asm visibility directives by hand when you create
> > symbol for the function anyway?
> 
> This is done exactly the same way as PIC thunk.  I don't think we
> should change it here.

I see, it is pretty ancient code.  Perhaps you can at least commonize
the uglness so we don't duplicate the ifdefs for MACHO?

> 
> > I would expect that you can just use similar code as cgraph_node::expand_thunk
> > when calls output_mi_thunk and get this done in a way that is independent of
> > target assembler.
> 
> I took a look.  I don't see an easy to do it.   I'd like to keep it exactly the
> same as PIC thunk.  And we change both thunks together later if needed.

OK, lets keep it done same was as PIC thunk.
Next stage1 we could clean up both.
> > I suppose it may make sense to split the insn and at least explicitly represent
> > the fact that we (sometimes) push the target to stack.
> 
> Did you mean "split the function"?  I will break it into
> ix86_output_indirect_branch_via_reg and
> ix86_output_indirect_branch_via_push.

I mean define_split to insert push instruction into the instruction
stream rather then printing it as part of the indirect jump insn.
> 
> > Why the memory variant exists at first place?
> 
> When the function address is in memory, we can push it onto stack
> to save a register.   Also it is needed to cover "call foo" to "call [foo@GOT]"
> conversion.
> >
> > I think you also want to update type to "many" when we do more than just indirect branch.
> 
> Did you mean "multi"?  I will change to "multi".

Yes.

> 
> > We do not care much about this, but it feels wrong to have attributes off reality.
> >> diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
> >> index f3e4a63ab46..ddb6035be96 100644
> >> --- a/gcc/doc/extend.texi
> >> +++ b/gcc/doc/extend.texi
> >> @@ -5754,6 +5754,16 @@ Specify which floating-point unit to use.  You must specify the
> >>  @code{target("fpmath=sse+387")} because the comma would separate
> >>  different options.
> >>
> >> +@item indirect_branch("@var{choice}")
> >> +@cindex @code{indirect_branch} function attribute, x86
> >> +On x86 targets, the @code{indirect_branch} attribute causes the compiler
> >> +to convert indirect call and jump with @var{choice}.  @samp{keep}
> >> +keeps indirect call and jump unmodified.  @samp{thunk} converts indirect
> >> +call and jump to call and return thunk.  @samp{thunk-inline} converts
> >> +indirect call and jump to inlined call and return thunk.
> >> +@samp{thunk-extern} converts indirect call and jump to external call
> >> +and return thunk provided in a separate object file.
> >
> > Please expand the documentation in a way that random user who is not aware of the
> > issue will understand that those are security features that come at a cost.
> >
> >> +@opindex -mindirect-branch
> >> +Convert indirect call and jump with @var{choice}.  The default is
> >> +@samp{keep}, which keeps indirect call and jump unmodified.
> >> +@samp{thunk} converts indirect call and jump to call and return thunk.
> >> +@samp{thunk-inline} converts indirect call and jump to inlined call
> >> +and return thunk.  @samp{thunk-extern} converts indirect call and jump
> >> +to external call and return thunk provided in a separate object file.
> >> +You can control this behavior for a specific function by using the
> >> +function attribute @code{indirect_branch}.  @xref{Function Attributes}.
> >
> > Similarly here.
> 
> I will update documentation with user guide info  after Intel white
> paper is published.

OK,
thanks!
Honza
> 
> > Rest of the patch seems OK. We may want incrementally represent more of the
> > indirect jump/call seqeuence in RTL, but at this point probably keeping things
> > simple and localized is not a bad idea. This can be done incrementally.
> >
> > Please make updated patch and I would like to give others chance to comment today.
> >
> > Honza
> 
> 
> 
> -- 
> H.J.



More information about the Gcc-patches mailing list