Don't use INTDEF/INTUSE with _dl_argv (bug 14132)
Commit Message
Continuing the removal of the obsolete INTDEF / INTUSE mechanism, this
patch replaces its use for _dl_argv with rtld_hidden_data_def and
rtld_hidden_proto. Some places in .S files that previously used
_dl_argv_internal or INTUSE(_dl_argv) now use __GI__dl_argv directly
(there are plenty of existing examples of such direct use of __GI_*).
A single place in rtld.c previously used _dl_argv without INTUSE,
apparently accidentally, while the rtld_hidden_proto mechanism avoids
such accidential omissions. As a consequence, this patch *does*
change the contents of stripped ld.so. However, the installed
stripped shared libraries are identical to those you get if instead of
this patch you change that single _dl_argv use to use INTUSE, without
any other changes.
Tested for x86_64 (testsuite as well as comparison of installed
stripped shared libraries as described above).
2014-10-21 Joseph Myers <joseph@codesourcery.com>
[BZ #14132]
* sysdeps/generic/ldsodefs.h (_dl_argv): Use rtld_hidden_proto.
[IS_IN_rtld] (_dl_argv_internal): Do not declare.
(rtld_progname): Make macro definition unconditional.
* elf/rtld.c (_dl_argv): Use rtld_hidden_data_def instead of
INTDEF.
(dlmopen_doit): Do not use INTUSE with _dl_argv.
(dl_main): Likewise.
* elf/dl-sysdep.c (_dl_sysdep_start): Likewise.
* sysdeps/alpha/dl-machine.h (RTLD_START): Use __GI__dl_argv
instead of _dl_argv_internal.
* sysdeps/powerpc/powerpc32/dl-start.S (_dl_start_user): Use
__GI__dl_argv instead of INTUSE(_dl_argv).
* sysdeps/powerpc/powerpc64/dl-machine.h (RTLD_START): Use
__GI__dl_argv instead of _dl_argv_internal.
Comments
Shouldn't sysdeps/ia64/dl-machine.h be using __GI__dl_argv as well?
Andreas.
On Tue, 21 Oct 2014, Andreas Schwab wrote:
> Shouldn't sysdeps/ia64/dl-machine.h be using __GI__dl_argv as well?
I don't know. Since it's not using _dl_argv_internal or INTUSE(_dl_argv)
at present I see no reason my patch should break it (the aim of the patch
isn't to make _dl_argv uses more efficient than at present, simply to
remove use of an obsolete mechanism without making anything worse).
"Joseph S. Myers" <joseph@codesourcery.com> writes:
> On Tue, 21 Oct 2014, Andreas Schwab wrote:
>
>> Shouldn't sysdeps/ia64/dl-machine.h be using __GI__dl_argv as well?
>
> I don't know. Since it's not using _dl_argv_internal or INTUSE(_dl_argv)
> at present I see no reason my patch should break it
It's not about breaking it, it's about making it consistent.
Andreas.
On Tue, 21 Oct 2014, Andreas Schwab wrote:
> "Joseph S. Myers" <joseph@codesourcery.com> writes:
>
> > On Tue, 21 Oct 2014, Andreas Schwab wrote:
> >
> >> Shouldn't sysdeps/ia64/dl-machine.h be using __GI__dl_argv as well?
> >
> > I don't know. Since it's not using _dl_argv_internal or INTUSE(_dl_argv)
> > at present I see no reason my patch should break it
>
> It's not about breaking it, it's about making it consistent.
That's beyond the scope of what this patch (or patch series) is intended
to do. Followups with other cleanups are of course welcome.
"Joseph S. Myers" <joseph@codesourcery.com> writes:
> On Tue, 21 Oct 2014, Andreas Schwab wrote:
>
>> "Joseph S. Myers" <joseph@codesourcery.com> writes:
>>
>> > On Tue, 21 Oct 2014, Andreas Schwab wrote:
>> >
>> >> Shouldn't sysdeps/ia64/dl-machine.h be using __GI__dl_argv as well?
>> >
>> > I don't know. Since it's not using _dl_argv_internal or INTUSE(_dl_argv)
>> > at present I see no reason my patch should break it
>>
>> It's not about breaking it, it's about making it consistent.
>
> That's beyond the scope of what this patch (or patch series) is intended
> to do.
It's perfectly fitting, just like the other bare _dl_argv reference.
Andreas.
On Tue, 21 Oct 2014, Andreas Schwab wrote:
> "Joseph S. Myers" <joseph@codesourcery.com> writes:
>
> > On Tue, 21 Oct 2014, Andreas Schwab wrote:
> >
> >> "Joseph S. Myers" <joseph@codesourcery.com> writes:
> >>
> >> > On Tue, 21 Oct 2014, Andreas Schwab wrote:
> >> >
> >> >> Shouldn't sysdeps/ia64/dl-machine.h be using __GI__dl_argv as well?
> >> >
> >> > I don't know. Since it's not using _dl_argv_internal or INTUSE(_dl_argv)
> >> > at present I see no reason my patch should break it
> >>
> >> It's not about breaking it, it's about making it consistent.
> >
> > That's beyond the scope of what this patch (or patch series) is intended
> > to do.
>
> It's perfectly fitting, just like the other bare _dl_argv reference.
This patch is intended to be minimal - that is, containing *only* the
changes that are necessarily part of eliminating this particular use of
INTDEF/INTUSE. Such a change to sysdeps/ia64/dl-machine.h is clearly not
necessary for this patch.
Ping. This patch
<https://sourceware.org/ml/libc-alpha/2014-10/msg00443.html> is pending
review.
Ping^2. This patch
<https://sourceware.org/ml/libc-alpha/2014-10/msg00443.html> is still
pending review.
On Tue, Oct 21, 2014 at 12:45:50AM +0000, Joseph S. Myers wrote:
> Continuing the removal of the obsolete INTDEF / INTUSE mechanism, this
> patch replaces its use for _dl_argv with rtld_hidden_data_def and
> rtld_hidden_proto. Some places in .S files that previously used
> _dl_argv_internal or INTUSE(_dl_argv) now use __GI__dl_argv directly
> (there are plenty of existing examples of such direct use of __GI_*).
>
> A single place in rtld.c previously used _dl_argv without INTUSE,
> apparently accidentally, while the rtld_hidden_proto mechanism avoids
> such accidential omissions. As a consequence, this patch *does*
> change the contents of stripped ld.so. However, the installed
> stripped shared libraries are identical to those you get if instead of
> this patch you change that single _dl_argv use to use INTUSE, without
> any other changes.
>
> Tested for x86_64 (testsuite as well as comparison of installed
> stripped shared libraries as described above).
>
> 2014-10-21 Joseph Myers <joseph@codesourcery.com>
>
> [BZ #14132]
> * sysdeps/generic/ldsodefs.h (_dl_argv): Use rtld_hidden_proto.
> [IS_IN_rtld] (_dl_argv_internal): Do not declare.
> (rtld_progname): Make macro definition unconditional.
> * elf/rtld.c (_dl_argv): Use rtld_hidden_data_def instead of
> INTDEF.
> (dlmopen_doit): Do not use INTUSE with _dl_argv.
> (dl_main): Likewise.
> * elf/dl-sysdep.c (_dl_sysdep_start): Likewise.
> * sysdeps/alpha/dl-machine.h (RTLD_START): Use __GI__dl_argv
> instead of _dl_argv_internal.
> * sysdeps/powerpc/powerpc32/dl-start.S (_dl_start_user): Use
> __GI__dl_argv instead of INTUSE(_dl_argv).
> * sysdeps/powerpc/powerpc64/dl-machine.h (RTLD_START): Use
> __GI__dl_argv instead of _dl_argv_internal.
>
Looks good to me.
Siddhesh
On Tue, 4 Nov 2014, Siddhesh Poyarekar wrote:
> Looks good to me.
Thanks, committed. Andreas, feel free to follow up with the ia64 change
you wanted.
@@ -108,7 +108,7 @@ _dl_sysdep_start (void **start_argptr,
#endif
__libc_stack_end = DL_STACK_END (start_argptr);
- DL_FIND_ARG_COMPONENTS (start_argptr, _dl_argc, INTUSE(_dl_argv), _environ,
+ DL_FIND_ARG_COMPONENTS (start_argptr, _dl_argc, _dl_argv, _environ,
GLRO(dl_auxv));
user_entry = (ElfW(Addr)) ENTRY_POINT;
@@ -83,7 +83,7 @@ int _dl_argc attribute_relro attribute_hidden;
char **_dl_argv attribute_relro = NULL;
unsigned int _dl_skip_args attribute_relro attribute_hidden;
#endif
-INTDEF(_dl_argv)
+rtld_hidden_data_def (_dl_argv)
#ifndef THREAD_SET_STACK_GUARD
/* Only exported for architectures that don't store the stack guard canary
@@ -492,7 +492,7 @@ dlmopen_doit (void *a)
args->map = _dl_open (args->fname,
(RTLD_LAZY | __RTLD_DLOPEN | __RTLD_AUDIT
| __RTLD_SECURE),
- dl_main, LM_ID_NEWLM, _dl_argc, INTUSE(_dl_argv),
+ dl_main, LM_ID_NEWLM, _dl_argc, _dl_argv,
__environ);
}
@@ -804,55 +804,55 @@ dl_main (const ElfW(Phdr) *phdr,
GL(dl_rtld_map).l_name = rtld_progname;
while (_dl_argc > 1)
- if (! strcmp (INTUSE(_dl_argv)[1], "--list"))
+ if (! strcmp (_dl_argv[1], "--list"))
{
mode = list;
GLRO(dl_lazy) = -1; /* This means do no dependency analysis. */
++_dl_skip_args;
--_dl_argc;
- ++INTUSE(_dl_argv);
+ ++_dl_argv;
}
- else if (! strcmp (INTUSE(_dl_argv)[1], "--verify"))
+ else if (! strcmp (_dl_argv[1], "--verify"))
{
mode = verify;
++_dl_skip_args;
--_dl_argc;
- ++INTUSE(_dl_argv);
+ ++_dl_argv;
}
- else if (! strcmp (INTUSE(_dl_argv)[1], "--inhibit-cache"))
+ else if (! strcmp (_dl_argv[1], "--inhibit-cache"))
{
GLRO(dl_inhibit_cache) = 1;
++_dl_skip_args;
--_dl_argc;
- ++INTUSE(_dl_argv);
+ ++_dl_argv;
}
- else if (! strcmp (INTUSE(_dl_argv)[1], "--library-path")
+ else if (! strcmp (_dl_argv[1], "--library-path")
&& _dl_argc > 2)
{
- library_path = INTUSE(_dl_argv)[2];
+ library_path = _dl_argv[2];
_dl_skip_args += 2;
_dl_argc -= 2;
- INTUSE(_dl_argv) += 2;
+ _dl_argv += 2;
}
- else if (! strcmp (INTUSE(_dl_argv)[1], "--inhibit-rpath")
+ else if (! strcmp (_dl_argv[1], "--inhibit-rpath")
&& _dl_argc > 2)
{
- GLRO(dl_inhibit_rpath) = INTUSE(_dl_argv)[2];
+ GLRO(dl_inhibit_rpath) = _dl_argv[2];
_dl_skip_args += 2;
_dl_argc -= 2;
- INTUSE(_dl_argv) += 2;
+ _dl_argv += 2;
}
- else if (! strcmp (INTUSE(_dl_argv)[1], "--audit") && _dl_argc > 2)
+ else if (! strcmp (_dl_argv[1], "--audit") && _dl_argc > 2)
{
- process_dl_audit (INTUSE(_dl_argv)[2]);
+ process_dl_audit (_dl_argv[2]);
_dl_skip_args += 2;
_dl_argc -= 2;
- INTUSE(_dl_argv) += 2;
+ _dl_argv += 2;
}
else
break;
@@ -886,7 +886,7 @@ of this helper program; chances are you did not intend to run this program.\n\
++_dl_skip_args;
--_dl_argc;
- ++INTUSE(_dl_argv);
+ ++_dl_argv;
/* The initialization of _dl_stack_flags done below assumes the
executable's PT_GNU_STACK may have been honored by the kernel, and
@@ -1800,7 +1800,7 @@ ERROR: ld.so: object '%s' cannot be loaded as audit interface: %s; ignored.\n",
ElfW(Addr) loadbase;
lookup_t result;
- result = _dl_lookup_symbol_x (INTUSE(_dl_argv)[i], main_map,
+ result = _dl_lookup_symbol_x (_dl_argv[i], main_map,
&ref, main_map->l_scope,
NULL, ELF_RTYPE_CLASS_PLT,
DL_LOOKUP_ADD_DEPENDENCY, NULL);
@@ -1808,7 +1808,7 @@ ERROR: ld.so: object '%s' cannot be loaded as audit interface: %s; ignored.\n",
loadbase = LOOKUP_VALUE_ADDRESS (result);
_dl_printf ("%s found at 0x%0*Zd in object at 0x%0*Zd\n",
- INTUSE(_dl_argv)[i],
+ _dl_argv[i],
(int) sizeof ref->st_value * 2,
(size_t) ref->st_value,
(int) sizeof loadbase * 2, (size_t) loadbase);
@@ -184,16 +184,16 @@ $fixup_stack: \n\
/* Adjust the stack pointer to skip _dl_skip_args words.\n\
This involves copying everything down, since the \n\
stack pointer must always be 16-byte aligned. */ \n\
- ldah $7, _dl_argv_internal($gp) !gprelhigh \n\
+ ldah $7, __GI__dl_argv($gp) !gprelhigh \n\
ldq $2, 0($sp) \n\
- ldq $5, _dl_argv_internal($7) !gprellow \n\
+ ldq $5, __GI__dl_argv($7) !gprellow \n\
subq $31, $1, $6 \n\
subq $2, $1, $2 \n\
s8addq $6, $5, $5 \n\
mov $sp, $4 \n\
s8addq $1, $sp, $3 \n\
stq $2, 0($sp) \n\
- stq $5, _dl_argv_internal($7) !gprellow \n\
+ stq $5, __GI__dl_argv($7) !gprellow \n\
/* Copy down argv. */ \n\
0: ldq $5, 8($3) \n\
addq $4, 8, $4 \n\
@@ -641,6 +641,7 @@ extern char **_dl_argv
attribute_relro
#endif
;
+rtld_hidden_proto (_dl_argv)
#ifdef IS_IN_rtld
extern unsigned int _dl_skip_args attribute_hidden
# ifndef DL_ARGV_NOT_RELRO
@@ -652,15 +653,8 @@ extern unsigned int _dl_skip_args_internal attribute_hidden
attribute_relro
# endif
;
-extern char **_dl_argv_internal attribute_hidden
-# ifndef DL_ARGV_NOT_RELRO
- attribute_relro
-# endif
- ;
-# define rtld_progname (INTUSE(_dl_argv)[0])
-#else
-# define rtld_progname _dl_argv[0]
#endif
+#define rtld_progname _dl_argv[0]
/* Flag set at startup and cleared when the last initializer has run. */
extern int _dl_starting_up;
@@ -54,7 +54,7 @@ _dl_start_user:
/* &_dl_argc in 29, &_dl_argv in 27, and _dl_loaded in 28. */
lwz r28,_rtld_local@got(r31)
lwz r29,_dl_argc@got(r31)
- lwz r27,INTUSE(_dl_argv)@got(r31)
+ lwz r27,__GI__dl_argv@got(r31)
/* Call _dl_init (_dl_loaded, _dl_argc, _dl_argv, _dl_argv+_dl_argc+1). */
lwz r3,0(r28)
@@ -169,7 +169,7 @@ DL_STARTING_UP_DEF \
".LC__dl_argc:\n" \
" .tc _dl_argc[TC],_dl_argc\n" \
".LC__dl_argv:\n" \
-" .tc _dl_argv_internal[TC],_dl_argv_internal\n" \
+" .tc __GI__dl_argv[TC],__GI__dl_argv\n" \
".LC__dl_fini:\n" \
" .tc _dl_fini[TC],_dl_fini\n" \
" .popsection\n" \