diff mbox

libgo patch committed: Implement reflect.MakeFunc for 386

Message ID yddzjqr91f9.fsf@lokon.CeBiTec.Uni-Bielefeld.DE
State New
Headers show

Commit Message

Rainer Orth Oct. 2, 2013, 2:45 p.m. UTC 
Ian Lance Taylor <iant@google.com> writes:

> On Mon, Sep 30, 2013 at 7:07 AM, Rainer Orth
> <ro@cebitec.uni-bielefeld.de> wrote:
>> Ian Lance Taylor <iant@google.com> writes:
>>
>>> On Mon, Sep 30, 2013 at 6:07 AM, Rainer Orth
>>> <ro@cebitec.uni-bielefeld.de> wrote:
>>>> Ian Lance Taylor <iant@google.com> writes:
>>>>
>>>>> Following up on my earlier patch, this patch implements the
>>>>> reflect.MakeFunc function for 386.
>>>>>
>>>>> Tom Tromey pointed out to me that the libffi closure support can
>>>>> probably be used for this.  I was not aware of that support.  It
>>>>> supports a lot more processors, and I should probably start using it.
>>>>> The approach I am using does have a couple of advantages: it's more
>>>>> efficient, and it doesn't require any type of writable executable
>>>>> memory.  I can get away with that because indirect calls in Go always
>>>>> pass a closure value.  So even when and if I do change to using libffi,
>>>>> I might still keep this code for amd64 and 386.
>>>>
>>>> Unfortunately, this patch (and undoubtedly the corresponding amd64 one)
>>>> break Solaris/x86 libgo bootstrap with native as:
>>>
>>> Unfortunately I think I'll have to somehow disable this functionality
>>> on systems with assemblers that do not understand the .cfi directives,
>>> as otherwise calling panic in a function created with MakeFunc will
>>> not work.
>>
>> Alternatively, one could hand-craft the .eh_frame section for such
>> systems along the lines of libffi/src/x86/sysv.S: ugly, but doable.
>
> Yeah.  I'm not going to do that myself.  But I would be happy to
> approve a patch for that if somebody else wants to write it.

Here's what I came up with.  As I said, it is inspired by the libffi
code, but a bit simplified since e.g. stuff like no .ascii support
aren't relevant on the Solaris versions supported on mainline and 4.8
branch.

Bootstrapped on x86_64-unknown-linux-gnu and i386-pc-solaris2.10 with
Sun as and gas.  I've also compared the readelf --debug-dump=frames
output for the 32 and 64-bit makefunc.o, both PIC and non-PIC.  64-bit
is completely unchanged, while for 32-bit there are FDE encoding changes
as expected from the FDE_ENCODING/FDE_ENCODE macros.

	Rainer


2013-10-01  Rainer Orth  <ro@CeBiTec.Uni-Bielefeld.DE>

	* configure.ac (libgo_cv_ro_eh_frame): New test.
	(libgo_cv_as_comdat_gnu): Likewise.
	(libgo_cv_as_x86_pcrel): Likewise.
	(libgo_cv_as_x86_64_unwind_section_type): Likewise.
	* configure: Regenerate.
	* config.h.in: Regenerate.
	* go/reflect/makefunc_386.S: Replace CFI directives by hand-coded
	.eh_frame section.
	Restrict .note.* sections to Linux.
	* go/reflect/makefunc_amd64.S: Likewise.

Comments

Ian Lance Taylor Oct. 2, 2013, 5:32 p.m. UTC | #1 
On Wed, Oct 2, 2013 at 7:45 AM, Rainer Orth <ro@cebitec.uni-bielefeld.de> wrote:
>
> Here's what I came up with.  As I said, it is inspired by the libffi
> code, but a bit simplified since e.g. stuff like no .ascii support
> aren't relevant on the Solaris versions supported on mainline and 4.8
> branch.
>
> Bootstrapped on x86_64-unknown-linux-gnu and i386-pc-solaris2.10 with
> Sun as and gas.  I've also compared the readelf --debug-dump=frames
> output for the 32 and 64-bit makefunc.o, both PIC and non-PIC.  64-bit
> is completely unchanged, while for 32-bit there are FDE encoding changes
> as expected from the FDE_ENCODING/FDE_ENCODE macros.
>
>         Rainer
>
>
> 2013-10-01  Rainer Orth  <ro@CeBiTec.Uni-Bielefeld.DE>
>
>         * configure.ac (libgo_cv_ro_eh_frame): New test.
>         (libgo_cv_as_comdat_gnu): Likewise.
>         (libgo_cv_as_x86_pcrel): Likewise.
>         (libgo_cv_as_x86_64_unwind_section_type): Likewise.
>         * configure: Regenerate.
>         * config.h.in: Regenerate.
>         * go/reflect/makefunc_386.S: Replace CFI directives by hand-coded
>         .eh_frame section.
>         Restrict .note.* sections to Linux.
>         * go/reflect/makefunc_amd64.S: Likewise.

Great, thanks for working on this.  Committed to trunk and 4.8 branch.

Ian
diff mbox

Patch

# HG changeset patch
# Parent afdb60d4178e74141fa8d3ad8dfd756d009a209c
Avoid CFI directives in makefunc_*.S

diff --git a/libgo/configure.ac b/libgo/configure.ac
--- a/libgo/configure.ac
+++ b/libgo/configure.ac
@@ -757,6 +757,68 @@  if test "$libgo_cv_lib_setcontext_clobbe
 	    [Define if setcontext clobbers TLS variables])
 fi
 
+AC_CACHE_CHECK([whether .eh_frame section should be read-only],
+libgo_cv_ro_eh_frame, [
+libgo_cv_ro_eh_frame=no
+echo 'extern void foo (void); void bar (void) { foo (); foo (); }' > conftest.c
+if $CC $CFLAGS -S -fpic -fexceptions -o conftest.s conftest.c > /dev/null 2>&1; then
+  if grep '.section.*eh_frame.*"a"' conftest.s > /dev/null; then
+    libgo_cv_ro_eh_frame=yes
+  elif grep '.section.*eh_frame.*#alloc' conftest.c \
+       | grep -v '#write' > /dev/null; then
+    libgo_cv_ro_eh_frame=yes
+  fi
+fi
+rm -f conftest.*
+])
+if test "x$libgo_cv_ro_eh_frame" = xyes; then
+  AC_DEFINE(EH_FRAME_FLAGS, "a",
+	    [Define to the flags needed for the .section .eh_frame directive.])
+else
+  AC_DEFINE(EH_FRAME_FLAGS, "aw",
+	    [Define to the flags needed for the .section .eh_frame directive.])
+fi
+
+AC_CACHE_CHECK([if assembler supports GNU comdat group syntax],
+libgo_cv_as_comdat_gnu, [
+echo '.section .text,"axG",@progbits,.foo,comdat' > conftest.s
+if $CC $CFLAGS -c conftest.s > /dev/null 2>&1; then
+  libgo_cv_as_comdat_gnu=yes
+else
+  libgo_cv_as_comdat_gnu=no
+fi
+])
+if test "x$libgo_cv_as_comdat_gnu" = xyes; then
+  AC_DEFINE(HAVE_AS_COMDAT_GAS, 1,
+	    [Define if your assembler supports GNU comdat group syntax.])
+fi
+
+AC_CACHE_CHECK([assembler supports pc related relocs],
+libgo_cv_as_x86_pcrel, [
+libgo_cv_as_x86_pcrel=yes
+echo '.text; foo: nop; .data; .long foo-.; .text' > conftest.s
+if $CC $CFLAGS -c conftest.s 2>&1 | $EGREP -i 'illegal|warning' > /dev/null; then
+    libgo_cv_as_x86_pcrel=no
+fi
+])
+if test "x$libgo_cv_as_x86_pcrel" = xyes; then
+  AC_DEFINE(HAVE_AS_X86_PCREL, 1,
+	    [Define if your assembler supports PC relative relocs.])
+fi
+
+AC_CACHE_CHECK([assembler supports unwind section type],
+libgo_cv_as_x86_64_unwind_section_type, [
+libgo_cv_as_x86_64_unwind_section_type=yes
+echo '.section .eh_frame,"a",@unwind' > conftest.s
+if $CC $CFLAGS -c conftest.s 2>&1 | grep -i warning > /dev/null; then
+    libgo_cv_as_x86_64_unwind_section_type=no
+fi
+])
+if test "x$libgo_cv_as_x86_64_unwind_section_type" = xyes; then
+  AC_DEFINE(HAVE_AS_X86_64_UNWIND_SECTION_TYPE, 1,
+	    [Define if your assembler supports unwind section type.])
+fi
+
 AC_CACHE_SAVE
 
 if test ${multilib} = yes; then
diff --git a/libgo/go/reflect/makefunc_386.S b/libgo/go/reflect/makefunc_386.S
--- a/libgo/go/reflect/makefunc_386.S
+++ b/libgo/go/reflect/makefunc_386.S
@@ -4,6 +4,8 @@ 
 
 # MakeFunc 386 assembly code.
 
+#include "config.h"
+
 	.global reflect.makeFuncStub
 
 #ifdef __ELF__
@@ -11,7 +13,7 @@ 
 #endif
 
 reflect.makeFuncStub:
-	.cfi_startproc
+.LFB1:
 
 	# Go does not provide any equivalent to the regparm function
 	# attribute, so on Go we do not need to worry about passing
@@ -27,15 +29,12 @@  reflect.makeFuncStub:
 	# }
 
 	pushl	%ebp
-	.cfi_def_cfa_offset 8
-	.cfi_offset %ebp, -8
+.LCFI0:
 	movl	%esp, %ebp
-	.cfi_def_cfa_register %ebp
-
+.LCFI1:
 	pushl	%ebx		# In case this is PIC.
-
 	subl	$36, %esp	# Enough for args and to align stack.
-	.cfi_offset %ebx, -12
+.LCFI2:
 
 #ifdef __PIC__
 	call	__x86.get_pc_thunk.bx
@@ -75,36 +74,123 @@  reflect.makeFuncStub:
 
 	addl	$36, %esp
 	popl	%ebx
-	.cfi_restore %ebx
+.LCFI3:
 	popl	%ebp
-	.cfi_restore %ebp
-	.cfi_def_cfa %esp, 4
-
+.LCFI4:
 	ret
-	.cfi_endproc
-
+.LFE1:
 #ifdef __ELF__
 	.size	reflect.makeFuncStub, . - reflect.makeFuncStub
 #endif
 
 #ifdef __PIC__
+#ifdef HAVE_AS_COMDAT_GAS
 	.section	.text.__x86.get_pc_thunk.bx,"axG",@progbits,__x86.get_pc_thunk.bx,comdat
+#else
+	/* Sun as needs a different syntax.  */
+	.section	.text.__x86.get_pc_thunk.bx%__x86.get_pc_thunk.bx,"ax",@progbits
+	.group		__x86.get_pc_thunk.bx,.text.__x86.get_pc_thunk.bx%__x86.get_pc_thunk.bx,#comdat
+#endif
 	.globl	__x86.get_pc_thunk.bx
 	.hidden	__x86.get_pc_thunk.bx
 #ifdef __ELF__
 	.type	__x86.get_pc_thunk.bx, @function
 #endif
 __x86.get_pc_thunk.bx:
-	.cfi_startproc
+.LFB2:
 	movl	(%esp), %ebx
 	ret
-	.cfi_endproc
+.LFE2:
 #ifdef __ELF__
 	.size	__x86.get_pc_thunk.bx, . - __x86.get_pc_thunk.bx
 #endif
 #endif
 
 #ifdef __ELF__
+#if defined __PIC__
+# if defined __sun__ && defined __svr4__
+/* 32-bit Solaris 2/x86 uses datarel encoding for PIC.  GNU ld before 2.22
+   doesn't correctly sort .eh_frame_hdr with mixed encodings, so match this.  */
+#  define FDE_ENCODING		0x30	/* datarel */
+#  define FDE_ENCODE(X)		X@GOTOFF
+# else
+#  define FDE_ENCODING		0x1b	/* pcrel sdata4 */
+#  if defined HAVE_AS_X86_PCREL
+#   define FDE_ENCODE(X)	X-.
+#  else
+#   define FDE_ENCODE(X)	X@rel
+#  endif
+# endif
+#else
+# define FDE_ENCODING		0	/* absolute */
+# define FDE_ENCODE(X)		X
+#endif
+
+	.section	.eh_frame,EH_FRAME_FLAGS,@progbits
+.Lframe1:
+	.long	.LECIE1-.LSCIE1	/* Length of Common Information Entry */
+.LSCIE1:
+	.long	0x0	/* CIE Identifier Tag */
+	.byte	0x1	/* CIE Version */
+	.ascii "zR\0"	/* CIE Augmentation */
+	.byte	0x1	/* .uleb128 0x1; CIE Code Alignment Factor */
+	.byte	0x7c	/* .sleb128 -4; CIE Data Alignment Factor */
+	.byte	0x8	/* CIE RA Column */
+	.byte	0x1	/* .uleb128 0x1; Augmentation size */
+	.byte	FDE_ENCODING
+	.byte	0xc	/* DW_CFA_def_cfa */
+	.byte	0x4	/* .uleb128 0x4 */
+	.byte	0x4	/* .uleb128 0x4 */
+	.byte	0x88	/* DW_CFA_offset, column 0x8 */
+	.byte	0x1	/* .uleb128 0x1 */
+	.align 4
+.LECIE1:
+.LSFDE1:
+	.long	.LEFDE1-.LASFDE1	/* FDE Length */
+.LASFDE1:
+	.long	.LASFDE1-.Lframe1	/* FDE CIE offset */
+	.long	FDE_ENCODE(.LFB1)	/* FDE initial location */
+	.long	.LFE1-.LFB1		/* FDE address range */
+	.byte	0x0	/* .uleb128 0x0; Augmentation size */
+	.byte	0x4	/* DW_CFA_advance_loc4 */
+	.long	.LCFI0-.LFB1
+	.byte	0xe	/* DW_CFA_def_cfa_offset */
+	.byte	0x8	/* .uleb128 0x8 */
+	.byte	0x85	/* DW_CFA_offset, column 0x5 */
+	.byte	0x2	/* .uleb128 0x2 */
+	.byte	0x4	/* DW_CFA_advance_loc4 */
+	.long	.LCFI1-.LCFI0
+	.byte	0xd	/* DW_CFA_def_cfa_register */
+	.byte	0x5	/* .uleb128 0x5 */
+	.byte	0x4	/* DW_CFA_advance_loc4 */
+	.long	.LCFI2-.LCFI1
+	.byte	0x83	/* .DW_CFA_offset, column 0x3 */
+	.byte	0x3	/* .uleb128 0x3 */
+	.byte	0x4	/* DW_CFA_advance_loc4 */
+	.long	.LCFI3-.LCFI2
+	.byte	0xc3	/* DW_CFA_restore, column 0x3 */
+	.byte	0x4	/* DW_CFA_advance_loc4 */
+	.long	.LCFI4-.LCFI3
+	.byte	0xc5	/* DW_CFA_restore, column 0x5 */
+	.byte	0xc	/* DW_CFA_def_cfa */
+	.byte	0x4	/* .uleb128 0x4 */
+	.byte	0x4	/* .uleb128 0x4 */
+	.align 4
+.LEFDE1:
+#ifdef __PIC__
+.LSFDE2:
+	.long	.LEFDE2-.LASFDE2	/* FDE Length */
+.LASFDE2:
+	.long	.LASFDE2-.Lframe1	/* FDE CIE offset */
+	.long	FDE_ENCODE(.LFB2)	/* FDE initial location */
+	.long	.LFE2-.LFB2		/* FDE address range */
+	.byte	0x0	/* .uleb128 0x0; Augmentation size */
+	.align 4
+.LEFDE2:
+#endif /* __PIC__ */
+#endif /* __ELF__ */
+
+#if defined(__ELF__) && defined(__linux__)
 	.section	.note.GNU-stack,"",@progbits
 	.section	.note.GNU-split-stack,"",@progbits
 	.section	.note.GNU-no-split-stack,"",@progbits
diff --git a/libgo/go/reflect/makefunc_amd64.S b/libgo/go/reflect/makefunc_amd64.S
--- a/libgo/go/reflect/makefunc_amd64.S
+++ b/libgo/go/reflect/makefunc_amd64.S
@@ -4,6 +4,8 @@ 
 
 # MakeFunc amd64 assembly code.
 
+#include "config.h"
+
 	.global	reflect.makeFuncStub
 
 #ifdef __ELF__
@@ -11,7 +13,7 @@ 
 #endif
 
 reflect.makeFuncStub:
-	.cfi_startproc
+.LFB1:
 
 	# Store all the parameter registers in a struct that looks
 	# like:
@@ -35,10 +37,9 @@  reflect.makeFuncStub:
 	# };
 
 	pushq	%rbp
-	.cfi_def_cfa_offset 16
-	.cfi_offset %rbp, -16
+.LCFI0:
 	movq	%rsp, %rbp
-	.cfi_def_cfa_register %rbp
+.LCFI1:
 
 	subq	$0xc0, %rsp		# Space for struct on stack.
 
@@ -91,16 +92,70 @@  reflect.makeFuncStub:
 	# double type.
 
 	leave
-	.cfi_def_cfa %rsp, 8
+.LCFI2:
 
 	ret
+.LFE1:
 
-	.cfi_endproc
 #ifdef __ELF__
 	.size	reflect.makeFuncStub, . - reflect.makeFuncStub
 #endif
 
 #ifdef __ELF__
+#ifdef HAVE_AS_X86_64_UNWIND_SECTION_TYPE
+	.section	.eh_frame,"a",@unwind
+#else
+	.section	.eh_frame,"a",@progbits
+#endif
+.Lframe1:
+	.long	.LECIE1-.LSCIE1	/* Length of Common Information Entry */
+.LSCIE1:
+	.long	0x0		/* CIE Identifier Tag */
+	.byte	0x1		/* CIE Version */
+	.ascii "zR\0"		/* CIE Augmentation */
+	.uleb128 1		/* CIE Code Alignment Factor */
+	.sleb128 -8		/* CIE Data Alignment Factor */
+	.byte	0x10		/* CIE RA Column */
+	.uleb128 1		/* Augmentation size */
+	.byte	0x1b		/* FDE Encoding (pcrel sdata4) */
+	.byte	0xc		/* DW_CFA_def_cfa, %rsp offset 8 */
+	.uleb128 7
+	.uleb128 8
+	.byte	0x80+16		/* DW_CFA_offset, %rip offset 1*-8 */
+	.uleb128 1
+	.align 8
+.LECIE1:
+.LSFDE1:
+	.long	.LEFDE1-.LASFDE1	/* FDE Length */
+.LASFDE1:
+	.long	.LASFDE1-.Lframe1	/* FDE CIE offset */
+#if HAVE_AS_X86_PCREL
+	.long	.LFB1-.			/* FDE initial location */
+#else
+	.long	.LFB1@rel
+#endif
+	.long	.LFE1-.LFB1		/* FDE address range */
+	.uleb128 0x0			/* Augmentation size */
+	.byte	0x4			/* DW_CFA_advance_loc4 */
+	.long	.LCFI0-.LFB1
+	.byte	0xe			/* DW_CFA_def_cfa_offset */
+	.uleb128 16
+	.byte	0x86			/* DW_CFA_offset, column 0x6 */
+	.uleb128 2
+	.byte	0x4			/* DW_CFA_advance_loc4 */
+	.long	.LCFI1-.LCFI0
+	.byte	0xd			/* DW_CFA_def_cfa_register */
+	.uleb128 6
+	.byte	0x2			/* DW_CFA_advance_loc1 */
+	.byte	.LCFI2-.LCFI1
+	.byte	0xc			/* DW_CFA_def_cfa */
+	.uleb128 7
+	.uleb128 8
+	.align 8
+.LEFDE1:
+#endif /* __ELF__ */
+
+#if defined(__ELF__) && defined(__linux__)
 	.section	.note.GNU-stack,"",@progbits
 	.section	.note.GNU-split-stack,"",@progbits
 	.section	.note.GNU-no-split-stack,"",@progbits