> -----Original Message----- > From: Jeff Law [mailto:l...@redhat.com] > Sent: Wednesday, November 8, 2017 8:06 PM > To: Tsimbalist, Igor V <igor.v.tsimbal...@intel.com>; gcc- > patc...@gcc.gnu.org > Cc: i...@airs.com > Subject: Re: [PATCH 08/22] Add Intel CET support for EH in libgcc. > > On 11/04/2017 06:43 AM, Tsimbalist, Igor V wrote: > >> -----Original Message----- > >> From: Jeff Law [mailto:l...@redhat.com] > >> Sent: Tuesday, October 31, 2017 5:49 AM > >> To: Tsimbalist, Igor V <igor.v.tsimbal...@intel.com>; gcc- > >> patc...@gcc.gnu.org > >> Cc: i...@airs.com > >> Subject: Re: [PATCH 08/22] Add Intel CET support for EH in libgcc. > >> > >> On 10/12/2017 01:56 PM, Tsimbalist, Igor V wrote: > >>> Control-flow Enforcement Technology (CET), published by Intel, > Introduces > >>> the Shadow Stack feature, which ensures a return from a function is > done > >>> to exactly the same location from where the function was called. When > EH > >>> is present the control-flow transfer may skip some stack frames and the > >>> shadow stack has to be adjusted not to signal a violation of a > >>> control-flow transfer. It's done by counting a number of skipping frames > >>> and adjusting shadow stack pointer by this number. > >>> > >>> gcc/ > >>> * config/i386/i386.c (ix86_expand_epilogue): Change simple > >>> return to indirect jump for EH return. Change explicit 'false' > >>> argument in pro_epilogue_adjust_stack with a value of > >>> flag_cf_protection. > >>> * config/i386/i386.md (simple_return_indirect_internal): Remove > >>> SImode restriction to support 64-bit. > >>> > >>> libgcc/ > >>> * config/i386/linux-unwind.h: Include > >>> config/i386/shadow-stack-unwind.h. > >>> * config/i386/shadow-stack-unwind.h: New file. > >>> * unwind-dw2.c: (uw_install_context): Add a FRAMES argument and > >>> pass it to _Unwind_Frames_Extra. > >>> * unwind-generic.h (FRAMES_P_DECL): New. > >>> (FRAMES_VAR): Likewise. > >>> (FRAMES_VAR_P): Likewise. > >>> (FRAMES_VAR_DECL): Likewise. > >>> (FRAMES_VAR_DECL_1): Likewise. > >>> (FRAMES_VAR_INC): Likewise. > >>> (FRAMES_P_UPDATE): Likewise. > >>> (_Unwind_Frames_Extra): Likewise. > >>> * unwind.inc (_Unwind_RaiseException_Phase2): Use > >> FRAMES_P_DECL, > >>> FRAMES_VAR_DECL_1, FRAMES_VAR_INC and FRAMES_P_UPDATE. > >>> (_Unwind_RaiseException): Use FRAMES_VAR_DECL, > >> FRAMES_VAR_P and > >>> FRAMES_VAR. > >>> (_Unwind_ForcedUnwind_Phase2): Use FRAMES_P_DECL, > >>> FRAMES_VAR_DECL_1, FRAMES_VAR_INC, FRAMES_P_UPDATE. > >>> (_Unwind_ForcedUnwind): Use FRAMES_VAR_DECL, > >> FRAMES_VAR_P and > >>> FRAMES_VAR. > >>> (_Unwind_Resume): Use FRAMES_VAR_DECL, FRAMES_VAR_P and > >>> FRAMES_VAR. > >>> (_Unwind_Resume_or_Rethrow): Use FRAMES_VAR_DECL, > >> FRAMES_VAR_P > >>> and FRAMES_VAR. > >>> > >>> Igor > >>> > >>> > >>> > >>> 0008-Add-Intel-CET-support-for-EH-in-libgcc.patch > >>> > >>> > >>> From 16eb1d0d9239e039fba28f1ae71762f19061b157 Mon Sep 17 > 00:00:00 > >> 2001 > >>> From: Igor Tsimbalist <igor.v.tsimbal...@intel.com> > >>> Date: Wed, 19 Jul 2017 03:04:46 +0300 > >>> Subject: [PATCH 08/22] Add Intel CET support for EH in libgcc. > >>> > >>> Control-flow Enforcement Technology (CET), published by Intel, > >>> introduces > >>> the Shadow Stack feature, which ensures a return from a function is > done > >>> to exactly the same location from where the function was called. When > EH > >>> is present the control-flow transfer may skip some stack frames and the > >>> shadow stack has to be adjusted not to signal a violation of a > >>> control-flow transfer. It's done by counting a number of skiping frames > >>> and adjasting shadow stack pointer by this number. > >>> > >>> Having new semantic of the 'ret' instruction if CET is supported in HW > >>> the 'ret' instruction cannot be generated in ix86_expand_epilogue when > >>> we are returning after EH is processed. Added a code in > >>> ix86_expand_epilogue to adjust Shadow Stack pointer and to generate > an > >>> indirect jump instead of 'ret'. As sp register is used during this > >>> adjustment thus the argument in pro_epilogue_adjust_stack is changed > >>> to update cfa_reg based on whether control-flow instrumentation is set. > >>> Without updating the cfa_reg field there is an assert later in dwarf2 > >>> pass related to mismatch the stack register and cfa_reg value. > >>> > >>> gcc/ > >>> * config/i386/i386.c (ix86_expand_epilogue): Change simple > >>> return to indirect jump for EH return. Change explicit 'false' > >>> argument in pro_epilogue_adjust_stack with a value of > >>> flag_cf_protection. > >>> * config/i386/i386.md (simple_return_indirect_internal): Remove > >>> SImode restriction to support 64-bit. > >>> > >>> libgcc/ > >>> * config/i386/linux-unwind.h: Include > >>> config/i386/shadow-stack-unwind.h. > >>> * config/i386/shadow-stack-unwind.h: New file. > >>> * unwind-dw2.c: (uw_install_context): Add a FRAMES argument and > >>> pass it to _Unwind_Frames_Extra. > >>> * unwind-generic.h (FRAMES_P_DECL): New. > >>> (FRAMES_VAR): Likewise. > >>> (FRAMES_VAR_P): Likewise. > >>> (FRAMES_VAR_DECL): Likewise. > >>> (FRAMES_VAR_DECL_1): Likewise. > >>> (FRAMES_VAR_INC): Likewise. > >>> (FRAMES_P_UPDATE): Likewise. > >>> (_Unwind_Frames_Extra): Likewise. > >>> * unwind.inc (_Unwind_RaiseException_Phase2): Use > >> FRAMES_P_DECL, > >>> FRAMES_VAR_DECL_1, FRAMES_VAR_INC and FRAMES_P_UPDATE. > >>> (_Unwind_RaiseException): Use FRAMES_VAR_DECL, > >> FRAMES_VAR_P and > >>> FRAMES_VAR. > >>> (_Unwind_ForcedUnwind_Phase2): Use FRAMES_P_DECL, > >>> FRAMES_VAR_DECL_1, FRAMES_VAR_INC, FRAMES_P_UPDATE. > >>> (_Unwind_ForcedUnwind): Use FRAMES_VAR_DECL, > >> FRAMES_VAR_P and > >>> FRAMES_VAR. > >>> (_Unwind_Resume): Use FRAMES_VAR_DECL, FRAMES_VAR_P and > >>> FRAMES_VAR. > >>> (_Unwind_Resume_or_Rethrow): Use FRAMES_VAR_DECL, > >> FRAMES_VAR_P > >>> and FRAMES_VAR. > >>> > >>> > >> > > >> I must say that I'm not happy with all the macro games we're playing in > >> this patch. Is there no cleaner way to get the desired behavior? > >> > >> Are there any ABI/API implications of this patch? Ie, does the > >> signature of any exported function change? > > There is no ABI/API implications. It's done through macro to keep existing > infrastructure > > and functions. Otherwise a copy of unwind functions have to be introduced > and modified > > for CET support. > It just seems to me there ought to be a way to handle this without > making a copy and without the macro games. > > So for example, why can't we make the handling of FRAMES_VAR_* > unconditional. Just go ahead and declare the local variable and compute > it. Pass its address to Unwind_RaiseException_Phase2 unconditionally. > The parameter might need to be marked as ATTRIBUTE_UNUSED. > > Then the only macro games we need is _Unwind_Frames_Extra. > > We end up with some potentially dead code at the source level (for cases > where _Unwind_Frames_Extra does nothing). Ideally the IPA bits would > realize the code is dead and specialize UnwindRaiseException_Phase2 > removing the unnecessary overhead. But even if IPA didn't do that I > think the result is just so much cleaner that a bit of overhead on the > exception/unwinding path is warranted.
Ok, I will implement your suggestion. I agree that is cleaner. > > > >> Has this been tested anywhere other than x86/x86_64 linux? > > Yes, I tested it on arm64 system. I applied 2 patches, previous 07/22 and > this one 08/22. Everything > > was built successfully. Further to the building I did testing also. No new > fails. > So how does that reconcile with H-P's note about the calls to > uw_install_context when FRAMES_VAR is defined as nothinng? > > + uw_install_context (&this_context, &cur_context, FRAMES_VAR); > > Doesn't that create a syntax error when FRAMES_VAR is defined, but with > no content? It doesn't give a syntax error. HJ has given an example in this thread. Igor > Jeff
