diff mbox

_dl_init: Remove internal_function attribute

Message ID 20170814113318.6885840138EE3@oldenburg.str.redhat.com
State New
Headers show

Commit Message

Florian Weimer Aug. 14, 2017, 11:33 a.m. UTC
The function is called from the i386 startup code, which needs minor
adjustments due to the changed ABI.

2017-08-14  Florian Weimer  <fweimer@redhat.com>

	* elf/dl-init.c (_dl_init): Remove internal_function.
	* sysdeps/generic/ldsodefs.h (_dl_init): Likewise.
	* sysdeps/i386/dl-machine.h (RTLD_START): Adjust call to _dl_init.

Comments

H.J. Lu Aug. 14, 2017, 12:28 p.m. UTC | #1
On Mon, Aug 14, 2017 at 4:33 AM, Florian Weimer <fweimer@redhat.com> wrote:
> The function is called from the i386 startup code, which needs minor
> adjustments due to the changed ABI.
>
> 2017-08-14  Florian Weimer  <fweimer@redhat.com>
>
>         * elf/dl-init.c (_dl_init): Remove internal_function.
>         * sysdeps/generic/ldsodefs.h (_dl_init): Likewise.
>         * sysdeps/i386/dl-machine.h (RTLD_START): Adjust call to _dl_init.
>
> diff --git a/elf/dl-init.c b/elf/dl-init.c
> index 5c5f3de365..fb3734590b 100644
> --- a/elf/dl-init.c
> +++ b/elf/dl-init.c
> @@ -75,7 +75,6 @@ call_init (struct link_map *l, int argc, char **argv, char **env)
>
>
>  void
> -internal_function
>  _dl_init (struct link_map *main_map, int argc, char **argv, char **env)
>  {
>    ElfW(Dyn) *preinit_array = main_map->l_info[DT_PREINIT_ARRAY];
> diff --git a/sysdeps/generic/ldsodefs.h b/sysdeps/generic/ldsodefs.h
> index 4540096688..49e673dd24 100644
> --- a/sysdeps/generic/ldsodefs.h
> +++ b/sysdeps/generic/ldsodefs.h
> @@ -964,7 +964,7 @@ extern int _dl_check_map_versions (struct link_map *map, int verbose,
>  /* Initialize the object in SCOPE by calling the constructors with
>     ARGC, ARGV, and ENV as the parameters.  */
>  extern void _dl_init (struct link_map *main_map, int argc, char **argv,
> -                     char **env) internal_function attribute_hidden;
> +                     char **env) attribute_hidden;
>
>  /* Call the finalizer functions of all shared objects whose
>     initializer functions have completed.  */
> diff --git a/sysdeps/i386/dl-machine.h b/sysdeps/i386/dl-machine.h
> index 924de953b7..2e17eba5c0 100644
> --- a/sysdeps/i386/dl-machine.h
> +++ b/sysdeps/i386/dl-machine.h
> @@ -177,17 +177,20 @@ _dl_start_user:\n\
>         # switch stacks if it moves these contents over.\n\
>  " RTLD_START_SPECIAL_INIT "\n\
>         # Load the parameters again.\n\
> -       # (eax, edx, ecx, *--esp) = (_dl_loaded, argc, argv, envp)\n\
> +       # (eax, edx, ecx, esi) = (_dl_loaded, argc, argv, envp)\n\
>         movl _rtld_local@GOTOFF(%ebx), %eax\n\
>         leal 8(%esp,%edx,4), %esi\n\
>         leal 4(%esp), %ecx\n\
>         movl %esp, %ebp\n\
>         # Make sure _dl_init is run with 16 byte aligned stack.\n\
>         andl $-16, %esp\n\
> -       pushl %eax\n\
> -       pushl %eax\n\
> +        subl $12, %esp\n\
>         pushl %ebp\n\
> +        # Arguments for _dl_init.\n\
>         pushl %esi\n\
> +       pushl %ecx\n\
> +       pushl %edx\n\
> +       pushl %eax\n\
>         # Clear %ebp, so that even constructors have terminated backchain.\n\
>         xorl %ebp, %ebp\n\
>         # Call the function to run the initializers.\n\
> @@ -195,7 +198,7 @@ _dl_start_user:\n\
>         # Pass our finalizer function to the user in %edx, as per ELF ABI.\n\
>         leal _dl_fini@GOTOFF(%ebx), %edx\n\
>         # Restore %esp _start expects.\n\
> -       movl (%esp), %esp\n\
> +       movl 16(%esp), %esp\n\
>         # Jump to the user's entry point.\n\
>         jmp *%edi\n\
>         .previous\n\

CFI adjustments are missing.
Florian Weimer Aug. 14, 2017, 12:39 p.m. UTC | #2
On 08/14/2017 02:28 PM, H.J. Lu wrote:
>>         # Clear %ebp, so that even constructors have terminated backchain.\n\
>>         xorl %ebp, %ebp\n\

> CFI adjustments are missing.

The original code did not have any because we never unwind to this
frame, as indicated by the zero %ebp value.

I can add CFI annotations for completeness, but I'd consider that  a
separate change.  It is also not quite clear to me what the canonical
frame address for the startup function should be.  We reach into the
caller's stack frame, after all.

Thanks,
Florian
H.J. Lu Aug. 14, 2017, 12:42 p.m. UTC | #3
On Mon, Aug 14, 2017 at 5:39 AM, Florian Weimer <fweimer@redhat.com> wrote:
> On 08/14/2017 02:28 PM, H.J. Lu wrote:
>>>         # Clear %ebp, so that even constructors have terminated backchain.\n\
>>>         xorl %ebp, %ebp\n\
>
>> CFI adjustments are missing.
>
> The original code did not have any because we never unwind to this
> frame, as indicated by the zero %ebp value.
>
> I can add CFI annotations for completeness, but I'd consider that  a
> separate change.  It is also not quite clear to me what the canonical
> frame address for the startup function should be.  We reach into the
> caller's stack frame, after all.
>

The patch is fine then.

Thanks.
diff mbox

Patch

diff --git a/elf/dl-init.c b/elf/dl-init.c
index 5c5f3de365..fb3734590b 100644
--- a/elf/dl-init.c
+++ b/elf/dl-init.c
@@ -75,7 +75,6 @@  call_init (struct link_map *l, int argc, char **argv, char **env)
 
 
 void
-internal_function
 _dl_init (struct link_map *main_map, int argc, char **argv, char **env)
 {
   ElfW(Dyn) *preinit_array = main_map->l_info[DT_PREINIT_ARRAY];
diff --git a/sysdeps/generic/ldsodefs.h b/sysdeps/generic/ldsodefs.h
index 4540096688..49e673dd24 100644
--- a/sysdeps/generic/ldsodefs.h
+++ b/sysdeps/generic/ldsodefs.h
@@ -964,7 +964,7 @@  extern int _dl_check_map_versions (struct link_map *map, int verbose,
 /* Initialize the object in SCOPE by calling the constructors with
    ARGC, ARGV, and ENV as the parameters.  */
 extern void _dl_init (struct link_map *main_map, int argc, char **argv,
-		      char **env) internal_function attribute_hidden;
+		      char **env) attribute_hidden;
 
 /* Call the finalizer functions of all shared objects whose
    initializer functions have completed.  */
diff --git a/sysdeps/i386/dl-machine.h b/sysdeps/i386/dl-machine.h
index 924de953b7..2e17eba5c0 100644
--- a/sysdeps/i386/dl-machine.h
+++ b/sysdeps/i386/dl-machine.h
@@ -177,17 +177,20 @@  _dl_start_user:\n\
 	# switch stacks if it moves these contents over.\n\
 " RTLD_START_SPECIAL_INIT "\n\
 	# Load the parameters again.\n\
-	# (eax, edx, ecx, *--esp) = (_dl_loaded, argc, argv, envp)\n\
+	# (eax, edx, ecx, esi) = (_dl_loaded, argc, argv, envp)\n\
 	movl _rtld_local@GOTOFF(%ebx), %eax\n\
 	leal 8(%esp,%edx,4), %esi\n\
 	leal 4(%esp), %ecx\n\
 	movl %esp, %ebp\n\
 	# Make sure _dl_init is run with 16 byte aligned stack.\n\
 	andl $-16, %esp\n\
-	pushl %eax\n\
-	pushl %eax\n\
+        subl $12, %esp\n\
 	pushl %ebp\n\
+        # Arguments for _dl_init.\n\
 	pushl %esi\n\
+	pushl %ecx\n\
+	pushl %edx\n\
+	pushl %eax\n\
 	# Clear %ebp, so that even constructors have terminated backchain.\n\
 	xorl %ebp, %ebp\n\
 	# Call the function to run the initializers.\n\
@@ -195,7 +198,7 @@  _dl_start_user:\n\
 	# Pass our finalizer function to the user in %edx, as per ELF ABI.\n\
 	leal _dl_fini@GOTOFF(%ebx), %edx\n\
 	# Restore %esp _start expects.\n\
-	movl (%esp), %esp\n\
+	movl 16(%esp), %esp\n\
 	# Jump to the user's entry point.\n\
 	jmp *%edi\n\
 	.previous\n\