powerpc64: Remove support for !HAVE_INLINED_SYSCALLS

Message ID 20180820144034.B630C4028A147@oldenburg.str.redhat.com
State New, archived
Headers

Commit Message

Florian Weimer Aug. 20, 2018, 2:40 p.m. UTC
  HAVE_INLINED_SYSCALLS is always defined on Linux.

2018-08-20  Florian Weimer  <fweimer@redhat.com>

	* sysdeps/powerpc/powerpc64/dl-machine.h (DL_STARTING_UP_DEF):
	Remove macro.
	(RTLD_START): Adjust.
  

Comments

Adhemerval Zanella Netto Aug. 20, 2018, 4:29 p.m. UTC | #1
On 20/08/2018 11:40, Florian Weimer wrote:
> HAVE_INLINED_SYSCALLS is always defined on Linux.
> 
> 2018-08-20  Florian Weimer  <fweimer@redhat.com>
> 
> 	* sysdeps/powerpc/powerpc64/dl-machine.h (DL_STARTING_UP_DEF):
> 	Remove macro.
> 	(RTLD_START): Adjust.

This file is not Linux specific, so it can potentially be used on a port
which defines HAVE_INLINED_SYSCALLS.  Is the plan to get rid of 
HAVE_INLINED_SYSCALLS altogether? 

> 
> diff --git a/sysdeps/powerpc/powerpc64/dl-machine.h b/sysdeps/powerpc/powerpc64/dl-machine.h
> index 99a83d0c82..a768a08cd7 100644
> --- a/sysdeps/powerpc/powerpc64/dl-machine.h
> +++ b/sysdeps/powerpc/powerpc64/dl-machine.h
> @@ -122,16 +122,6 @@ elf_machine_dynamic (void)
>  #define elf_machine_relplt elf_machine_rela
>  
>  
> -#ifdef HAVE_INLINED_SYSCALLS
> -/* We do not need _dl_starting_up.  */
> -# define DL_STARTING_UP_DEF
> -#else
> -# define DL_STARTING_UP_DEF \
> -".LC__dl_starting_up:\n"  \
> -"	.tc __GI__dl_starting_up[TC],__GI__dl_starting_up\n"
> -#endif
> -
> -
>  /* Initial entry point code for the dynamic linker.  The C function
>     `_dl_start' is the real entry point; its return value is the user
>     program's entry point.  */
> @@ -165,7 +155,6 @@ BODY_PREFIX "_start:\n"							\
>  "	.align 2\n"							\
>  "	" END_2(_start) "\n"						\
>  "	.pushsection	\".toc\",\"aw\"\n"				\
> -DL_STARTING_UP_DEF							\
>  ".LC__rtld_local:\n"							\
>  "	.tc _rtld_local[TC],_rtld_local\n"				\
>  ".LC__dl_argc:\n"							\
>
  
Florian Weimer Aug. 20, 2018, 4:48 p.m. UTC | #2
On 08/20/2018 06:29 PM, Adhemerval Zanella wrote:
> 
> 
> On 20/08/2018 11:40, Florian Weimer wrote:
>> HAVE_INLINED_SYSCALLS is always defined on Linux.
>>
>> 2018-08-20  Florian Weimer  <fweimer@redhat.com>
>>
>> 	* sysdeps/powerpc/powerpc64/dl-machine.h (DL_STARTING_UP_DEF):
>> 	Remove macro.
>> 	(RTLD_START): Adjust.
> 
> This file is not Linux specific, so it can potentially be used on a port
> which defines HAVE_INLINED_SYSCALLS.  Is the plan to get rid of
> HAVE_INLINED_SYSCALLS altogether?

Do you plan to start a Hurd port for powerpc? 8-)

The condition looks wrong to me—it probably should be 
RTLD_PRIVATE_ERRNO, not HAVE_INLINED_SYSCALLS.  And I plan to get rid of 
the private errno for ld.so eventually on Linux.  I'm not sure if this 
comment is correct anymore:

/* The dynamic linker uses its own private errno variable.
    All access to errno inside the dynamic linker is serialized,
    so a single (hidden) global variable is all it needs.  */

Thanks,
Florian
  
Tulio Magno Quites Machado Filho Aug. 20, 2018, 6:22 p.m. UTC | #3
Florian Weimer <fweimer@redhat.com> writes:

> On 08/20/2018 06:29 PM, Adhemerval Zanella wrote:
>> 
>> 
>> On 20/08/2018 11:40, Florian Weimer wrote:
>>> HAVE_INLINED_SYSCALLS is always defined on Linux.
>>>
>>> 2018-08-20  Florian Weimer  <fweimer@redhat.com>
>>>
>>> 	* sysdeps/powerpc/powerpc64/dl-machine.h (DL_STARTING_UP_DEF):
>>> 	Remove macro.
>>> 	(RTLD_START): Adjust.
>> 
>> This file is not Linux specific, so it can potentially be used on a port
>> which defines HAVE_INLINED_SYSCALLS.  Is the plan to get rid of
>> HAVE_INLINED_SYSCALLS altogether?
>
> Do you plan to start a Hurd port for powerpc? 8-)

I agree with Adhemerval here, even if there isn't a plan to port powerpc to
Hurd.  :-D
But I do agree to remove it if this is causing a problem for you (and an
explanation of the reason in the commit message ;-) ).

> The condition looks wrong to me—it probably should be 
> RTLD_PRIVATE_ERRNO, not HAVE_INLINED_SYSCALLS.

Do you also plan to modify _dl_starting_up usages in other places?
e.g. elf/init.c
They're using _dl_starting_up based on HAVE_INLINED_SYSCALLS too.

> And I plan to get rid of the private errno for ld.so eventually on Linux.

If Linux always defines HAVE_INLINED_SYSCALLS, DL_STARTING_UP_DEF will be
defined to nothing.  How is this patch helping your work on the private errno
removal?
  
Florian Weimer Aug. 21, 2018, 1:17 p.m. UTC | #4
On 08/20/2018 08:22 PM, Tulio Magno Quites Machado Filho wrote:
> Florian Weimer <fweimer@redhat.com> writes:
> 
>> On 08/20/2018 06:29 PM, Adhemerval Zanella wrote:
>>>
>>>
>>> On 20/08/2018 11:40, Florian Weimer wrote:
>>>> HAVE_INLINED_SYSCALLS is always defined on Linux.
>>>>
>>>> 2018-08-20  Florian Weimer  <fweimer@redhat.com>
>>>>
>>>> 	* sysdeps/powerpc/powerpc64/dl-machine.h (DL_STARTING_UP_DEF):
>>>> 	Remove macro.
>>>> 	(RTLD_START): Adjust.
>>>
>>> This file is not Linux specific, so it can potentially be used on a port
>>> which defines HAVE_INLINED_SYSCALLS.  Is the plan to get rid of
>>> HAVE_INLINED_SYSCALLS altogether?
>>
>> Do you plan to start a Hurd port for powerpc? 8-)
> 
> I agree with Adhemerval here, even if there isn't a plan to port powerpc to
> Hurd.  :-D
> But I do agree to remove it if this is causing a problem for you (and an
> explanation of the reason in the commit message ;-) ).

I'm trying to simplify the powerpc startup code, to align it more with 
what is done on other architectures.  The additional preprocessor 
conditionals contribute to cognitive load when dealing with such changes.

This is related to the minimization patch for the startup code, but 
given the testing and review effort required for this patch anyway, I 
plan to go one step further and remove the always-NULL function 
arguments as well.

All in all, this should allow us to get rid of struct startup_info for 
new binaries.

Thanks,
Florian
  

Patch

diff --git a/sysdeps/powerpc/powerpc64/dl-machine.h b/sysdeps/powerpc/powerpc64/dl-machine.h
index 99a83d0c82..a768a08cd7 100644
--- a/sysdeps/powerpc/powerpc64/dl-machine.h
+++ b/sysdeps/powerpc/powerpc64/dl-machine.h
@@ -122,16 +122,6 @@  elf_machine_dynamic (void)
 #define elf_machine_relplt elf_machine_rela
 
 
-#ifdef HAVE_INLINED_SYSCALLS
-/* We do not need _dl_starting_up.  */
-# define DL_STARTING_UP_DEF
-#else
-# define DL_STARTING_UP_DEF \
-".LC__dl_starting_up:\n"  \
-"	.tc __GI__dl_starting_up[TC],__GI__dl_starting_up\n"
-#endif
-
-
 /* Initial entry point code for the dynamic linker.  The C function
    `_dl_start' is the real entry point; its return value is the user
    program's entry point.  */
@@ -165,7 +155,6 @@  BODY_PREFIX "_start:\n"							\
 "	.align 2\n"							\
 "	" END_2(_start) "\n"						\
 "	.pushsection	\".toc\",\"aw\"\n"				\
-DL_STARTING_UP_DEF							\
 ".LC__rtld_local:\n"							\
 "	.tc _rtld_local[TC],_rtld_local\n"				\
 ".LC__dl_argc:\n"							\