This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] [PR target/83641] Fix incorrect CFI for stack clash protected noreturn function on x86/x86_64


On 01/03/2018 01:36 PM, Jakub Jelinek wrote:
> On Wed, Jan 03, 2018 at 12:57:10PM -0700, Jeff Law wrote:
>>> Ok for trunk if it passes bootstrap/regtest on x86_64 and i686-linux?
>>>
>>> 2018-01-03  Jakub Jelinek  <jakub@redhat.com>
>>>
>>> 	PR target/83641
>>> 	* config/i386/i386.c (ix86_adjust_stack_and_probe_stack_clash): For
>>> 	noreturn probe, use gen_pop instead of ix86_emit_restore_reg_using_pop,
>>> 	only set RTX_FRAME_RELATED_P on both the push and pop if cfa_reg is sp
>>> 	and add REG_CFA_ADJUST_CFA notes in that case to both insns.
>> I still question how useful this part is, but not enough to object given
> 
> Reduces bloat in .eh_frame and when unwinding less work is needed.  The
> patch isn't that large after all.
> 
>> you've done the work.  I'll go ahead and commit both as a single unit.
> 
> BTW, I've now bootstrapped/regtested on x86_64-linux and i686-linux your and
> my patch, the only regression caused by your patch is:
> +FAIL: gcc.target/i386/stack-check-12.c scan-assembler popl\\t%esi
> +FAIL: gcc.target/i386/stack-check-12.c scan-assembler pushl\\t%esi
> on i686-linux, quite understandably, that is exactly the case you are
> changing, there is %edi clobbered in the function and thus needs to be saved
> and restored and so no push/pop of %esi is needed.
> So, this testcase should be tweaked.
I think the ASM can just be dropped.  It doesn't contribute anything to
what Florian was trying to show.   Once we drop the ASM we should see
those probes return.  I'll have to check that obviously.

jeff


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]