Bug 84530 - -mfunction-return=thunk does not work for simple_return_pop_internal insn
Summary: -mfunction-return=thunk does not work for simple_return_pop_internal insn
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 8.0.1
: P3 normal
Target Milestone: 8.0
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks: 84072
  Show dependency treegraph
 
Reported: 2018-02-23 15:04 UTC by Martin Liška
Modified: 2019-02-20 18:44 UTC (History)
2 users (show)

See Also:
Host:
Target: i586
Build:
Known to work:
Known to fail:
Last reconfirmed: 2018-02-24 00:00:00


Attachments
A patch (2.97 KB, patch)
2018-02-24 14:53 UTC, H.J. Lu
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Liška 2018-02-23 15:04:05 UTC
i386.md contains:

 13100  (define_insn "simple_return_pop_internal"
 13101    [(simple_return)
 13102     (use (match_operand:SI 0 "const_int_operand"))]
 13103    "reload_completed"
 13104    "%!ret\t%0"
 13105    [(set_attr "length" "3")
 13106     (set_attr "atom_unit" "jeu")
 13107     (set_attr "length_immediate" "2")
 13108     (set_attr "modrm" "0")
 13109     (set_attr "maybe_prefix_bnd" "1")])

Thus:

$ cat stdarg.c 
struct s { _Complex unsigned short x; };
struct s gs = { 100 + 200i };
struct s __attribute__((noinline)) foo (void) { return gs; }

$ gcc stdarg.c -S -O2 -m32 -o/dev/stdout  -mfunction-return=thunk
	.file	"stdarg.c"
	.text
	.p2align 4,,15
	.globl	foo
	.type	foo, @function
foo:
.LFB0:
	.cfi_startproc
	movl	4(%esp), %eax
	movl	gs, %edx
	movl	%edx, (%eax)
	ret	$4
	.cfi_endproc
.LFE0:
	.size	foo, .-foo
	.globl	gs
	.data
	.align 4
	.type	gs, @object
	.size	gs, 4
gs:
	.value	100
	.value	200
	.ident	"GCC: (GNU) 8.0.1 20180223 (experimental)"
	.section	.note.GNU-stack,"",@progbits

While x86-64 works:
gcc stdarg.c -S -O2  -o/dev/stdout  -mfunction-return=thunk
	.file	"stdarg.c"
	.text
	.p2align 4,,15
	.globl	foo
	.type	foo, @function
foo:
.LFB0:
	.cfi_startproc
	movzwl	gs+2(%rip), %eax
	sall	$16, %eax
	movl	%eax, %edx
	movzwl	gs(%rip), %eax
	orl	%edx, %eax
	jmp	__x86_return_thunk
	.cfi_endproc
.LFE0:
	.size	foo, .-foo
	.globl	gs
	.data
	.align 2
	.type	gs, @object
	.size	gs, 4
gs:
	.value	100
	.value	200
	.section	.text.__x86_indirect_thunk,"axG",@progbits,__x86_indirect_thunk,comdat
	.globl	__x86_indirect_thunk
	.hidden	__x86_indirect_thunk
	.type	__x86_indirect_thunk, @function
__x86_indirect_thunk:
	.set	__x86_return_thunk,__x86_indirect_thunk
	.globl	__x86_return_thunk
	.hidden	__x86_return_thunk
.LFB1:
	.cfi_startproc
	call	.LIND1
.LIND0:
	pause
	lfence
	jmp	.LIND0
.LIND1:
	lea	8(%rsp), %rsp
	ret
	.cfi_endproc
.LFE1:
	.ident	"GCC: (GNU) 8.0.1 20180223 (experimental)"
	.section	.note.GNU-stack,"",@progbits
Comment 1 H.J. Lu 2018-02-24 14:53:40 UTC
Created attachment 43494 [details]
A patch

I am testing this patch.
Comment 2 H.J. Lu 2018-02-25 01:38:21 UTC
Comment on attachment 43494 [details]
A patch

The patch is posted at

https://gcc.gnu.org/ml/gcc-patches/2018-02/msg01375.html
Comment 3 hjl@gcc.gnu.org 2018-02-26 15:30:02 UTC
Author: hjl
Date: Mon Feb 26 15:29:30 2018
New Revision: 257992

URL: https://gcc.gnu.org/viewcvs?rev=257992&root=gcc&view=rev
Log:
i386: Update -mfunction-return= for return with pop

When -mfunction-return= is used, simple_return_pop_internal should pop
return address into ECX register, adjust stack by bytes to pop from stack
and jump to the return thunk via ECX register.

Tested on i686 and x86-64.

	PR target/84530
	* config/i386/i386-protos.h (ix86_output_indirect_jmp): Remove
	the bool argument.
	(ix86_output_indirect_function_return): New prototype.
	(ix86_split_simple_return_pop_internal): Likewise.
	* config/i386/i386.c (indirect_return_via_cx): New.
	(indirect_return_via_cx_bnd): Likewise.
	(indirect_thunk_name): Handle return va CX_REG.
	(output_indirect_thunk_function): Create alias for
	__x86_return_thunk_[re]cx and __x86_return_thunk_[re]cx_bnd.
	(ix86_output_indirect_jmp): Remove the bool argument.
	(ix86_output_indirect_function_return): New function.
	(ix86_split_simple_return_pop_internal): Likewise.
	* config/i386/i386.md (*indirect_jump): Don't pass false
	to ix86_output_indirect_jmp.
	(*tablejump_1): Likewise.
	(simple_return_pop_internal): Change it to define_insn_and_split.
	Call ix86_split_simple_return_pop_internal to split it for
	-mfunction-return=.
	(simple_return_indirect_internal): Call
	ix86_output_indirect_function_return instead of
	ix86_output_indirect_jmp.

gcc/testsuite/

	PR target/84530
	* gcc.target/i386/ret-thunk-22.c: New test.
	* gcc.target/i386/ret-thunk-23.c: Likewise.
	* gcc.target/i386/ret-thunk-24.c: Likewise.
	* gcc.target/i386/ret-thunk-25.c: Likewise.
	* gcc.target/i386/ret-thunk-26.c: Likewise.

Added:
    trunk/gcc/testsuite/gcc.target/i386/ret-thunk-22.c
    trunk/gcc/testsuite/gcc.target/i386/ret-thunk-23.c
    trunk/gcc/testsuite/gcc.target/i386/ret-thunk-24.c
    trunk/gcc/testsuite/gcc.target/i386/ret-thunk-25.c
    trunk/gcc/testsuite/gcc.target/i386/ret-thunk-26.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/i386/i386-protos.h
    trunk/gcc/config/i386/i386.c
    trunk/gcc/config/i386/i386.md
    trunk/gcc/testsuite/ChangeLog
Comment 4 hjl@gcc.gnu.org 2018-03-02 13:05:49 UTC
Author: hjl
Date: Fri Mar  2 13:05:18 2018
New Revision: 258134

URL: https://gcc.gnu.org/viewcvs?rev=258134&root=gcc&view=rev
Log:
i386: Update -mfunction-return= for return with pop

When -mfunction-return= is used, simple_return_pop_internal should pop
return address into ECX register, adjust stack by bytes to pop from stack
and jump to the return thunk via ECX register.

Revision 257992 removed the bool argument from ix86_output_indirect_jmp.
Update comments to reflect it.

Tested on i686 and x86-64.

gcc/

	Backport from mainline
	2018-02-26  H.J. Lu  <hongjiu.lu@intel.com>

	* config/i386/i386.c (ix86_output_indirect_jmp): Update comments.

	2018-02-26  H.J. Lu  <hongjiu.lu@intel.com>

	PR target/84530
	* config/i386/i386-protos.h (ix86_output_indirect_jmp): Remove
	the bool argument.
	(ix86_output_indirect_function_return): New prototype.
	(ix86_split_simple_return_pop_internal): Likewise.
	* config/i386/i386.c (indirect_return_via_cx): New.
	(indirect_return_via_cx_bnd): Likewise.
	(indirect_thunk_name): Handle return va CX_REG.
	(output_indirect_thunk_function): Create alias for
	__x86_return_thunk_[re]cx and __x86_return_thunk_[re]cx_bnd.
	(ix86_output_indirect_jmp): Remove the bool argument.
	(ix86_output_indirect_function_return): New function.
	(ix86_split_simple_return_pop_internal): Likewise.
	* config/i386/i386.md (*indirect_jump): Don't pass false
	to ix86_output_indirect_jmp.
	(*tablejump_1): Likewise.
	(simple_return_pop_internal): Change it to define_insn_and_split.
	Call ix86_split_simple_return_pop_internal to split it for
	-mfunction-return=.
	(simple_return_indirect_internal): Call
	ix86_output_indirect_function_return instead of
	ix86_output_indirect_jmp.

gcc/testsuite/

	Backport from mainline
	2018-02-26  H.J. Lu  <hongjiu.lu@intel.com>

	PR target/84530
	* gcc.target/i386/ret-thunk-22.c: New test.
	* gcc.target/i386/ret-thunk-23.c: Likewise.
	* gcc.target/i386/ret-thunk-24.c: Likewise.
	* gcc.target/i386/ret-thunk-25.c: Likewise.
	* gcc.target/i386/ret-thunk-26.c: Likewise.

Added:
    branches/gcc-7-branch/gcc/testsuite/gcc.target/i386/ret-thunk-22.c
    branches/gcc-7-branch/gcc/testsuite/gcc.target/i386/ret-thunk-23.c
    branches/gcc-7-branch/gcc/testsuite/gcc.target/i386/ret-thunk-24.c
    branches/gcc-7-branch/gcc/testsuite/gcc.target/i386/ret-thunk-25.c
    branches/gcc-7-branch/gcc/testsuite/gcc.target/i386/ret-thunk-26.c
Modified:
    branches/gcc-7-branch/gcc/ChangeLog
    branches/gcc-7-branch/gcc/config/i386/i386-protos.h
    branches/gcc-7-branch/gcc/config/i386/i386.c
    branches/gcc-7-branch/gcc/config/i386/i386.md
    branches/gcc-7-branch/gcc/testsuite/ChangeLog
Comment 5 H.J. Lu 2018-03-17 12:37:23 UTC
Fixed for GCC 8.
Comment 6 hjl@gcc.gnu.org 2018-04-16 19:07:04 UTC
Author: hjl
Date: Mon Apr 16 19:06:32 2018
New Revision: 259420

URL: https://gcc.gnu.org/viewcvs?rev=259420&root=gcc&view=rev
Log:
i386: Update -mfunction-return= for return with pop

When -mfunction-return= is used, simple_return_pop_internal should pop
return address into ECX register, adjust stack by bytes to pop from stack
and jump to the return thunk via ECX register.

Revision 257992 removed the bool argument from ix86_output_indirect_jmp.
Update comments to reflect it.

Tested on i686 and x86-64.

gcc/

	Backport from mainline
	2018-02-26  H.J. Lu  <hongjiu.lu@intel.com>

	* config/i386/i386.c (ix86_output_indirect_jmp): Update comments.

	2018-02-26  H.J. Lu  <hongjiu.lu@intel.com>

	PR target/84530
	* config/i386/i386-protos.h (ix86_output_indirect_jmp): Remove
	the bool argument.
	(ix86_output_indirect_function_return): New prototype.
	(ix86_split_simple_return_pop_internal): Likewise.
	* config/i386/i386.c (indirect_return_via_cx): New.
	(indirect_return_via_cx_bnd): Likewise.
	(indirect_thunk_name): Handle return va CX_REG.
	(output_indirect_thunk_function): Create alias for
	__x86_return_thunk_[re]cx and __x86_return_thunk_[re]cx_bnd.
	(ix86_output_indirect_jmp): Remove the bool argument.
	(ix86_output_indirect_function_return): New function.
	(ix86_split_simple_return_pop_internal): Likewise.
	* config/i386/i386.md (*indirect_jump): Don't pass false
	to ix86_output_indirect_jmp.
	(*tablejump_1): Likewise.
	(simple_return_pop_internal): Change it to define_insn_and_split.
	Call ix86_split_simple_return_pop_internal to split it for
	-mfunction-return=.
	(simple_return_indirect_internal): Call
	ix86_output_indirect_function_return instead of
	ix86_output_indirect_jmp.

gcc/testsuite/

	Backport from mainline
	2018-02-26  H.J. Lu  <hongjiu.lu@intel.com>

	PR target/84530
	* gcc.target/i386/ret-thunk-22.c: New test.
	* gcc.target/i386/ret-thunk-23.c: Likewise.
	* gcc.target/i386/ret-thunk-24.c: Likewise.
	* gcc.target/i386/ret-thunk-25.c: Likewise.
	* gcc.target/i386/ret-thunk-26.c: Likewise.

Added:
    branches/gcc-6-branch/gcc/testsuite/gcc.target/i386/ret-thunk-22.c
    branches/gcc-6-branch/gcc/testsuite/gcc.target/i386/ret-thunk-23.c
    branches/gcc-6-branch/gcc/testsuite/gcc.target/i386/ret-thunk-24.c
    branches/gcc-6-branch/gcc/testsuite/gcc.target/i386/ret-thunk-25.c
    branches/gcc-6-branch/gcc/testsuite/gcc.target/i386/ret-thunk-26.c
Modified:
    branches/gcc-6-branch/gcc/ChangeLog
    branches/gcc-6-branch/gcc/config/i386/i386-protos.h
    branches/gcc-6-branch/gcc/config/i386/i386.c
    branches/gcc-6-branch/gcc/config/i386/i386.md
    branches/gcc-6-branch/gcc/testsuite/ChangeLog
Comment 7 Ev Drikos 2019-02-20 18:44:43 UTC
Hello,

There is perhaps a Darwin specific problem reproduced in various versions, ie 8.2:

Not really sure if the problem I face is indeed related to this PR.

$ gcc8 -mfunction-return=thunk gcc/testsuite/gcc.target/i386/ret-thunk-26.c -arch i386
Undefined symbols for architecture i386:
  "__x86_return_thunk", referenced from:
      _foo in ccQ3DlYI.o
      _bar in ccQ3DlYI.o
      _main in ccQ3DlYI.o
     (maybe you meant: ___x86_return_thunk)
ld: symbol(s) not found for architecture i386
collect2: error: ld returned 1 exit status
$
$ gcc8 -v
Using built-in specs.
COLLECT_GCC=gcc8
COLLECT_LTO_WRAPPER=/opt/local/libexec/gcc/x86_64-apple-darwin17.5.0/8.2.0/lto-wrapper
Target: x86_64-apple-darwin17.5.0
Configured with: ../gcc-8.2.0/configure --prefix=/opt/local --program-suffix=8 --enable-languages=c,c++,fortran --enable-checking=release --disable-nls --with-system-zlib
Thread model: posix
gcc version 8.2.0 (GCC) 
$