Message ID | 20180608102748.GA110450@intel.com |
---|---|
State | New |
Headers | show |
Series | i386; Add indirect_return function attribute | expand |
On Fri, Jun 8, 2018 at 3:27 AM, H.J. Lu <hongjiu.lu@intel.com> wrote: > On x86, swapcontext may return via indirect branch when shadow stack > is enabled. To support code instrumentation of control-flow transfers > with -fcf-protection, add indirect_return function attribute to inform > compiler that a function may return via indirect branch. > > Note: Unlike setjmp, swapcontext only returns once. Mark it return > twice will unnecessarily disable compiler optimization. > > OK for trunk? > > H.J. > ---- > gcc/ > > PR target/85620 > * config/i386/i386.c (rest_of_insert_endbranch): Also generate > ENDBRANCH for non-tail call which may return via indirect branch. > * doc/extend.texi: Document indirect_return attribute. > > gcc/testsuite/ > > PR target/85620 > * gcc.target/i386/pr85620-1.c: New test. > * gcc.target/i386/pr85620-2.c: Likewise. > Here is the updated patch with a testcase to show the impact of returns_twice attribute. Jan, Uros, can you take a look? Thanks.
On Tue, Jul 3, 2018 at 5:32 PM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Fri, Jun 8, 2018 at 3:27 AM, H.J. Lu <hongjiu.lu@intel.com> wrote: >> On x86, swapcontext may return via indirect branch when shadow stack >> is enabled. To support code instrumentation of control-flow transfers >> with -fcf-protection, add indirect_return function attribute to inform >> compiler that a function may return via indirect branch. >> >> Note: Unlike setjmp, swapcontext only returns once. Mark it return >> twice will unnecessarily disable compiler optimization. >> >> OK for trunk? >> >> H.J. >> ---- >> gcc/ >> >> PR target/85620 >> * config/i386/i386.c (rest_of_insert_endbranch): Also generate >> ENDBRANCH for non-tail call which may return via indirect branch. >> * doc/extend.texi: Document indirect_return attribute. >> >> gcc/testsuite/ >> >> PR target/85620 >> * gcc.target/i386/pr85620-1.c: New test. >> * gcc.target/i386/pr85620-2.c: Likewise. >> > > Here is the updated patch with a testcase to show the impact of > returns_twice attribute. > > Jan, Uros, can you take a look? LGTM for the implementation, can't say if attribute is really needed or not. +@item indirect_return +@cindex @code{indirect_return} function attribute, x86 + +The @code{indirect_return} attribute on a function is used to inform +the compiler that the function may return via indiret branch. s/indiret/indirect/ Uros.
On Tue, Jul 3, 2018 at 9:12 AM, Uros Bizjak <ubizjak@gmail.com> wrote: > On Tue, Jul 3, 2018 at 5:32 PM, H.J. Lu <hjl.tools@gmail.com> wrote: >> On Fri, Jun 8, 2018 at 3:27 AM, H.J. Lu <hongjiu.lu@intel.com> wrote: >>> On x86, swapcontext may return via indirect branch when shadow stack >>> is enabled. To support code instrumentation of control-flow transfers >>> with -fcf-protection, add indirect_return function attribute to inform >>> compiler that a function may return via indirect branch. >>> >>> Note: Unlike setjmp, swapcontext only returns once. Mark it return >>> twice will unnecessarily disable compiler optimization. >>> >>> OK for trunk? >>> >>> H.J. >>> ---- >>> gcc/ >>> >>> PR target/85620 >>> * config/i386/i386.c (rest_of_insert_endbranch): Also generate >>> ENDBRANCH for non-tail call which may return via indirect branch. >>> * doc/extend.texi: Document indirect_return attribute. >>> >>> gcc/testsuite/ >>> >>> PR target/85620 >>> * gcc.target/i386/pr85620-1.c: New test. >>> * gcc.target/i386/pr85620-2.c: Likewise. >>> >> >> Here is the updated patch with a testcase to show the impact of >> returns_twice attribute. >> >> Jan, Uros, can you take a look? > > LGTM for the implementation, can't say if attribute is really needed or not. This gives programmers more flexibly. > +@item indirect_return > +@cindex @code{indirect_return} function attribute, x86 > + > +The @code{indirect_return} attribute on a function is used to inform > +the compiler that the function may return via indiret branch. > > s/indiret/indirect/ Fixed. Here is the updated patch. Thanks.
On 07/03/2018 10:53 AM, H.J. Lu wrote: > On Tue, Jul 3, 2018 at 9:12 AM, Uros Bizjak <ubizjak@gmail.com> wrote: >> On Tue, Jul 3, 2018 at 5:32 PM, H.J. Lu <hjl.tools@gmail.com> wrote: >>> On Fri, Jun 8, 2018 at 3:27 AM, H.J. Lu <hongjiu.lu@intel.com> wrote: >>>> On x86, swapcontext may return via indirect branch when shadow stack >>>> is enabled. To support code instrumentation of control-flow transfers >>>> with -fcf-protection, add indirect_return function attribute to inform >>>> compiler that a function may return via indirect branch. >>>> >>>> Note: Unlike setjmp, swapcontext only returns once. Mark it return >>>> twice will unnecessarily disable compiler optimization. >>>> >>>> OK for trunk? >>>> >>>> H.J. >>>> ---- >>>> gcc/ >>>> >>>> PR target/85620 >>>> * config/i386/i386.c (rest_of_insert_endbranch): Also generate >>>> ENDBRANCH for non-tail call which may return via indirect branch. >>>> * doc/extend.texi: Document indirect_return attribute. >>>> >>>> gcc/testsuite/ >>>> >>>> PR target/85620 >>>> * gcc.target/i386/pr85620-1.c: New test. >>>> * gcc.target/i386/pr85620-2.c: Likewise. >>>> >>> Here is the updated patch with a testcase to show the impact of >>> returns_twice attribute. >>> >>> Jan, Uros, can you take a look? >> LGTM for the implementation, can't say if attribute is really needed or not. > This gives programmers more flexibly. > >> +@item indirect_return >> +@cindex @code{indirect_return} function attribute, x86 >> + >> +The @code{indirect_return} attribute on a function is used to inform >> +the compiler that the function may return via indiret branch. >> >> s/indiret/indirect/ > Fixed. Here is the updated patch. > > Thanks. > > -- H.J. > > > 0001-i386-Add-indirect_return-function-attribute.patch > > > From bb98f6a31801659ae3c6689d6d31af33a3c28bb2 Mon Sep 17 00:00:00 2001 > From: "H.J. Lu" <hjl.tools@gmail.com> > Date: Thu, 7 Jun 2018 20:05:15 -0700 > Subject: [PATCH] i386; Add indirect_return function attribute > > On x86, swapcontext may return via indirect branch when shadow stack > is enabled. To support code instrumentation of control-flow transfers > with -fcf-protection, add indirect_return function attribute to inform > compiler that a function may return via indirect branch. > > Note: Unlike setjmp, swapcontext only returns once. Mark it return > twice will unnecessarily disable compiler optimization as shown in > the testcase here. > > gcc/ > > PR target/85620 > * config/i386/i386.c (rest_of_insert_endbranch): Also generate > ENDBRANCH for non-tail call which may return via indirect branch. > * doc/extend.texi: Document indirect_return attribute. OK jeff
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c index b95f0612562..3fb79178138 100644 --- a/gcc/config/i386/i386.c +++ b/gcc/config/i386/i386.c @@ -2625,7 +2625,26 @@ rest_of_insert_endbranch (void) { if (CALL_P (insn)) { - if (find_reg_note (insn, REG_SETJMP, NULL) == NULL) + bool need_endbr; + need_endbr = find_reg_note (insn, REG_SETJMP, NULL) != NULL; + if (!need_endbr && !SIBLING_CALL_P (insn)) + { + rtx call = get_call_rtx_from (insn); + rtx fnaddr = XEXP (call, 0); + + /* Also generate ENDBRANCH for non-tail call which + may return via indirect branch. */ + if (MEM_P (fnaddr) + && GET_CODE (XEXP (fnaddr, 0)) == SYMBOL_REF) + { + tree fndecl = SYMBOL_REF_DECL (XEXP (fnaddr, 0)); + if (fndecl + && lookup_attribute ("indirect_return", + DECL_ATTRIBUTES (fndecl))) + need_endbr = true; + } + } + if (!need_endbr) continue; /* Generate ENDBRANCH after CALL, which can return more than twice, setjmp-like functions. */ @@ -46769,6 +46788,8 @@ static const struct attribute_spec ix86_attribute_table[] = ix86_handle_fndecl_attribute, NULL }, { "function_return", 1, 1, true, false, false, false, ix86_handle_fndecl_attribute, NULL }, + { "indirect_return", 0, 0, true, false, false, false, + ix86_handle_fndecl_attribute, NULL }, /* End element. */ { NULL, 0, 0, false, false, false, false, NULL, NULL } diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi index 3e6c98a554a..ddd50b0da3e 100644 --- a/gcc/doc/extend.texi +++ b/gcc/doc/extend.texi @@ -5878,6 +5878,12 @@ foo (void) @} @end smallexample +@item indirect_return +@cindex @code{indirect_return} function attribute, x86 + +The @code{indirect_return} attribute on a function is used to inform +the compiler that the function may return via indiret branch. + @end table On the x86, the inliner does not inline a diff --git a/gcc/testsuite/gcc.target/i386/pr85620-1.c b/gcc/testsuite/gcc.target/i386/pr85620-1.c new file mode 100644 index 00000000000..32efb08e59e --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/pr85620-1.c @@ -0,0 +1,15 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fcf-protection" } */ +/* { dg-final { scan-assembler-times {\mendbr} 2 } } */ + +struct ucontext; + +extern int bar (struct ucontext *) __attribute__((__indirect_return__)); + +extern int res; + +void +foo (struct ucontext *oucp) +{ + res = bar (oucp); +} diff --git a/gcc/testsuite/gcc.target/i386/pr85620-2.c b/gcc/testsuite/gcc.target/i386/pr85620-2.c new file mode 100644 index 00000000000..b2e680fa1fe --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/pr85620-2.c @@ -0,0 +1,13 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fcf-protection" } */ +/* { dg-final { scan-assembler-times {\mendbr} 1 } } */ + +struct ucontext; + +extern int bar (struct ucontext *) __attribute__((__indirect_return__)); + +int +foo (struct ucontext *oucp) +{ + return bar (oucp); +}