* [PATCH] mips: fix elf/tst-dlmopen4
@ 2022-01-28 18:54 Xi Ruoyao
2022-01-28 19:04 ` Carlos O'Donell
0 siblings, 1 reply; 7+ messages in thread
From: Xi Ruoyao @ 2022-01-28 18:54 UTC (permalink / raw)
To: libc-alpha; +Cc: Jiaxun Yang, syq, Joseph Myers, Carlos O'Donell
MIPS uses DT_MIPS_RLD_MAP and DT_MIPS_RLD_MAP_REL instead of DT_DEBUG,
to provide access to r_debug.
---
elf/tst-dlmopen4.c | 29 +++++++++++++++++++----------
1 file changed, 19 insertions(+), 10 deletions(-)
diff --git a/elf/tst-dlmopen4.c b/elf/tst-dlmopen4.c
index d8bcf7e9d5..de67a30a4b 100644
--- a/elf/tst-dlmopen4.c
+++ b/elf/tst-dlmopen4.c
@@ -25,17 +25,26 @@
#include <support/check.h>
#include <support/test-driver.h>
-#ifndef ELF_MACHINE_GET_R_DEBUG
-# define ELF_MACHINE_GET_R_DEBUG(d) \
- (__extension__ ({ \
- struct r_debug_extended *debug; \
- if ((d)->d_tag == DT_DEBUG) \
- debug = (struct r_debug_extended *) (d)->d_un.d_ptr; \
- else \
- debug = NULL; \
- debug; }))
+static struct r_debug_extended *
+elf_get_r_debug (ElfW(Dyn) *d)
+{
+#ifdef __mips__
+ if (d->d_tag == DT_MIPS_RLD_MAP_REL)
+ {
+ char *ptr = (char *) d;
+ ptr += d->d_un.d_val;
+ return *(struct r_debug_extended **) ptr;
+ }
+ else if (d->d_tag == DT_MIPS_RLD_MAP)
+ return *(struct r_debug_extended **) d->d_un.d_ptr;
+#else
+ if (d->d_tag == DT_DEBUG)
+ return (struct r_debug_extended *) d->d_un.d_ptr;
#endif
+ return NULL;
+}
+
static int
do_test (void)
{
@@ -44,7 +53,7 @@ do_test (void)
for (d = _DYNAMIC; d->d_tag != DT_NULL; ++d)
{
- debug = ELF_MACHINE_GET_R_DEBUG (d);
+ debug = elf_get_r_debug (d);
if (debug != NULL)
break;
}
--
2.35.0
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] mips: fix elf/tst-dlmopen4
2022-01-28 18:54 [PATCH] mips: fix elf/tst-dlmopen4 Xi Ruoyao
@ 2022-01-28 19:04 ` Carlos O'Donell
2022-01-28 20:12 ` Xi Ruoyao
2022-01-28 20:41 ` H.J. Lu
0 siblings, 2 replies; 7+ messages in thread
From: Carlos O'Donell @ 2022-01-28 19:04 UTC (permalink / raw)
To: Xi Ruoyao, libc-alpha; +Cc: syq, Joseph Myers, Jiaxun Yang
On 1/28/22 13:54, Xi Ruoyao via Libc-alpha wrote:
> MIPS uses DT_MIPS_RLD_MAP and DT_MIPS_RLD_MAP_REL instead of DT_DEBUG,
> to provide access to r_debug.
This looks correct to me. OK for glibc 2.35.
I'm going to push this today if there are no other objections.
Reviewed-by: Carlos O'Donell <carlos@redhat.com>
> ---
> elf/tst-dlmopen4.c | 29 +++++++++++++++++++----------
> 1 file changed, 19 insertions(+), 10 deletions(-)
>
> diff --git a/elf/tst-dlmopen4.c b/elf/tst-dlmopen4.c
> index d8bcf7e9d5..de67a30a4b 100644
> --- a/elf/tst-dlmopen4.c
> +++ b/elf/tst-dlmopen4.c
> @@ -25,17 +25,26 @@
> #include <support/check.h>
> #include <support/test-driver.h>
>
> -#ifndef ELF_MACHINE_GET_R_DEBUG
> -# define ELF_MACHINE_GET_R_DEBUG(d) \
> - (__extension__ ({ \
> - struct r_debug_extended *debug; \
> - if ((d)->d_tag == DT_DEBUG) \
> - debug = (struct r_debug_extended *) (d)->d_un.d_ptr; \
> - else \
> - debug = NULL; \
> - debug; }))
> +static struct r_debug_extended *
> +elf_get_r_debug (ElfW(Dyn) *d)
> +{
> +#ifdef __mips__
> + if (d->d_tag == DT_MIPS_RLD_MAP_REL)
> + {
> + char *ptr = (char *) d;
> + ptr += d->d_un.d_val;
> + return *(struct r_debug_extended **) ptr;
> + }
> + else if (d->d_tag == DT_MIPS_RLD_MAP)
> + return *(struct r_debug_extended **) d->d_un.d_ptr;
OK. Matches the logic in sysdeps/mips/dl-debug.h
> +#else
> + if (d->d_tag == DT_DEBUG)
> + return (struct r_debug_extended *) d->d_un.d_ptr;
OK.
> #endif
>
> + return NULL;
OK.
> +}
> +
> static int
> do_test (void)
> {
> @@ -44,7 +53,7 @@ do_test (void)
>
> for (d = _DYNAMIC; d->d_tag != DT_NULL; ++d)
> {
> - debug = ELF_MACHINE_GET_R_DEBUG (d);
> + debug = elf_get_r_debug (d);
OK.
> if (debug != NULL)
> break;
> }
--
Cheers,
Carlos.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] mips: fix elf/tst-dlmopen4
2022-01-28 19:04 ` Carlos O'Donell
@ 2022-01-28 20:12 ` Xi Ruoyao
2022-01-28 20:41 ` H.J. Lu
1 sibling, 0 replies; 7+ messages in thread
From: Xi Ruoyao @ 2022-01-28 20:12 UTC (permalink / raw)
To: Carlos O'Donell, libc-alpha; +Cc: syq, Joseph Myers, Jiaxun Yang
On Fri, 2022-01-28 at 14:04 -0500, Carlos O'Donell wrote:
> On 1/28/22 13:54, Xi Ruoyao via Libc-alpha wrote:
> > MIPS uses DT_MIPS_RLD_MAP and DT_MIPS_RLD_MAP_REL instead of
> > DT_DEBUG,
> > to provide access to r_debug.
>
> This looks correct to me. OK for glibc 2.35.
>
> I'm going to push this today if there are no other objections.
>
> Reviewed-by: Carlos O'Donell <carlos@redhat.com>
Thanks!
Please review my patch for pldd on MIPS
(https://sourceware.org/pipermail/libc-alpha/2022-January/135821.html).
The logic is same.
--
Xi Ruoyao <xry111@mengyan1223.wang>
School of Aerospace Science and Technology, Xidian University
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] mips: fix elf/tst-dlmopen4
2022-01-28 19:04 ` Carlos O'Donell
2022-01-28 20:12 ` Xi Ruoyao
@ 2022-01-28 20:41 ` H.J. Lu
2022-01-28 20:49 ` Carlos O'Donell
1 sibling, 1 reply; 7+ messages in thread
From: H.J. Lu @ 2022-01-28 20:41 UTC (permalink / raw)
To: Carlos O'Donell
Cc: Xi Ruoyao, GNU C Library, syq, Jiaxun Yang, Joseph Myers
On Fri, Jan 28, 2022 at 11:04 AM Carlos O'Donell via Libc-alpha
<libc-alpha@sourceware.org> wrote:
>
> On 1/28/22 13:54, Xi Ruoyao via Libc-alpha wrote:
> > MIPS uses DT_MIPS_RLD_MAP and DT_MIPS_RLD_MAP_REL instead of DT_DEBUG,
> > to provide access to r_debug.
>
> This looks correct to me. OK for glibc 2.35.
>
> I'm going to push this today if there are no other objections.
>
> Reviewed-by: Carlos O'Donell <carlos@redhat.com>
>
> > ---
> > elf/tst-dlmopen4.c | 29 +++++++++++++++++++----------
> > 1 file changed, 19 insertions(+), 10 deletions(-)
> >
> > diff --git a/elf/tst-dlmopen4.c b/elf/tst-dlmopen4.c
> > index d8bcf7e9d5..de67a30a4b 100644
> > --- a/elf/tst-dlmopen4.c
> > +++ b/elf/tst-dlmopen4.c
> > @@ -25,17 +25,26 @@
> > #include <support/check.h>
> > #include <support/test-driver.h>
> >
> > -#ifndef ELF_MACHINE_GET_R_DEBUG
> > -# define ELF_MACHINE_GET_R_DEBUG(d) \
> > - (__extension__ ({ \
> > - struct r_debug_extended *debug; \
> > - if ((d)->d_tag == DT_DEBUG) \
> > - debug = (struct r_debug_extended *) (d)->d_un.d_ptr; \
> > - else \
> > - debug = NULL; \
> > - debug; }))
> > +static struct r_debug_extended *
> > +elf_get_r_debug (ElfW(Dyn) *d)
> > +{
> > +#ifdef __mips__
Shouldn't MIPS simply define a proper ELF_MACHINE_GET_R_DEBUG?
It can be done with a header file.
> > + if (d->d_tag == DT_MIPS_RLD_MAP_REL)
> > + {
> > + char *ptr = (char *) d;
> > + ptr += d->d_un.d_val;
> > + return *(struct r_debug_extended **) ptr;
> > + }
> > + else if (d->d_tag == DT_MIPS_RLD_MAP)
> > + return *(struct r_debug_extended **) d->d_un.d_ptr;
>
> OK. Matches the logic in sysdeps/mips/dl-debug.h
>
> > +#else
> > + if (d->d_tag == DT_DEBUG)
> > + return (struct r_debug_extended *) d->d_un.d_ptr;
>
> OK.
>
> > #endif
> >
> > + return NULL;
>
> OK.
>
> > +}
> > +
> > static int
> > do_test (void)
> > {
> > @@ -44,7 +53,7 @@ do_test (void)
> >
> > for (d = _DYNAMIC; d->d_tag != DT_NULL; ++d)
> > {
> > - debug = ELF_MACHINE_GET_R_DEBUG (d);
> > + debug = elf_get_r_debug (d);
>
> OK.
>
> > if (debug != NULL)
> > break;
> > }
>
>
> --
> Cheers,
> Carlos.
>
--
H.J.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] mips: fix elf/tst-dlmopen4
2022-01-28 20:41 ` H.J. Lu
@ 2022-01-28 20:49 ` Carlos O'Donell
2022-01-28 21:20 ` H.J. Lu
2022-01-28 21:23 ` Xi Ruoyao
0 siblings, 2 replies; 7+ messages in thread
From: Carlos O'Donell @ 2022-01-28 20:49 UTC (permalink / raw)
To: H.J. Lu; +Cc: Xi Ruoyao, GNU C Library, syq, Jiaxun Yang, Joseph Myers
On 1/28/22 15:41, H.J. Lu wrote:
> On Fri, Jan 28, 2022 at 11:04 AM Carlos O'Donell via Libc-alpha
> <libc-alpha@sourceware.org> wrote:
>>
>> On 1/28/22 13:54, Xi Ruoyao via Libc-alpha wrote:
>>> MIPS uses DT_MIPS_RLD_MAP and DT_MIPS_RLD_MAP_REL instead of DT_DEBUG,
>>> to provide access to r_debug.
>>
>> This looks correct to me. OK for glibc 2.35.
>>
>> I'm going to push this today if there are no other objections.
>>
>> Reviewed-by: Carlos O'Donell <carlos@redhat.com>
>>
>>> ---
>>> elf/tst-dlmopen4.c | 29 +++++++++++++++++++----------
>>> 1 file changed, 19 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/elf/tst-dlmopen4.c b/elf/tst-dlmopen4.c
>>> index d8bcf7e9d5..de67a30a4b 100644
>>> --- a/elf/tst-dlmopen4.c
>>> +++ b/elf/tst-dlmopen4.c
>>> @@ -25,17 +25,26 @@
>>> #include <support/check.h>
>>> #include <support/test-driver.h>
>>>
>>> -#ifndef ELF_MACHINE_GET_R_DEBUG
>>> -# define ELF_MACHINE_GET_R_DEBUG(d) \
>>> - (__extension__ ({ \
>>> - struct r_debug_extended *debug; \
>>> - if ((d)->d_tag == DT_DEBUG) \
>>> - debug = (struct r_debug_extended *) (d)->d_un.d_ptr; \
>>> - else \
>>> - debug = NULL; \
>>> - debug; }))
>>> +static struct r_debug_extended *
>>> +elf_get_r_debug (ElfW(Dyn) *d)
>>> +{
>>> +#ifdef __mips__
>
> Shouldn't MIPS simply define a proper ELF_MACHINE_GET_R_DEBUG?
> It can be done with a header file.
The only use of ELF_MACHINE_GET_R_DEBUG is in the test, and the static function
is easier to read, edit, and debug, all of which is better for a test?
>>> + if (d->d_tag == DT_MIPS_RLD_MAP_REL)
>>> + {
>>> + char *ptr = (char *) d;
>>> + ptr += d->d_un.d_val;
>>> + return *(struct r_debug_extended **) ptr;
>>> + }
>>> + else if (d->d_tag == DT_MIPS_RLD_MAP)
>>> + return *(struct r_debug_extended **) d->d_un.d_ptr;
>>
>> OK. Matches the logic in sysdeps/mips/dl-debug.h
>>
>>> +#else
>>> + if (d->d_tag == DT_DEBUG)
>>> + return (struct r_debug_extended *) d->d_un.d_ptr;
>>
>> OK.
>>
>>> #endif
>>>
>>> + return NULL;
>>
>> OK.
>>
>>> +}
>>> +
>>> static int
>>> do_test (void)
>>> {
>>> @@ -44,7 +53,7 @@ do_test (void)
>>>
>>> for (d = _DYNAMIC; d->d_tag != DT_NULL; ++d)
>>> {
>>> - debug = ELF_MACHINE_GET_R_DEBUG (d);
>>> + debug = elf_get_r_debug (d);
>>
>> OK.
>>
>>> if (debug != NULL)
>>> break;
>>> }
>>
>>
>> --
>> Cheers,
>> Carlos.
>>
>
>
--
Cheers,
Carlos.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] mips: fix elf/tst-dlmopen4
2022-01-28 20:49 ` Carlos O'Donell
@ 2022-01-28 21:20 ` H.J. Lu
2022-01-28 21:23 ` Xi Ruoyao
1 sibling, 0 replies; 7+ messages in thread
From: H.J. Lu @ 2022-01-28 21:20 UTC (permalink / raw)
To: Carlos O'Donell
Cc: Xi Ruoyao, GNU C Library, syq, Jiaxun Yang, Joseph Myers
On Fri, Jan 28, 2022 at 12:49 PM Carlos O'Donell <carlos@redhat.com> wrote:
>
> On 1/28/22 15:41, H.J. Lu wrote:
> > On Fri, Jan 28, 2022 at 11:04 AM Carlos O'Donell via Libc-alpha
> > <libc-alpha@sourceware.org> wrote:
> >>
> >> On 1/28/22 13:54, Xi Ruoyao via Libc-alpha wrote:
> >>> MIPS uses DT_MIPS_RLD_MAP and DT_MIPS_RLD_MAP_REL instead of DT_DEBUG,
> >>> to provide access to r_debug.
> >>
> >> This looks correct to me. OK for glibc 2.35.
> >>
> >> I'm going to push this today if there are no other objections.
> >>
> >> Reviewed-by: Carlos O'Donell <carlos@redhat.com>
> >>
> >>> ---
> >>> elf/tst-dlmopen4.c | 29 +++++++++++++++++++----------
> >>> 1 file changed, 19 insertions(+), 10 deletions(-)
> >>>
> >>> diff --git a/elf/tst-dlmopen4.c b/elf/tst-dlmopen4.c
> >>> index d8bcf7e9d5..de67a30a4b 100644
> >>> --- a/elf/tst-dlmopen4.c
> >>> +++ b/elf/tst-dlmopen4.c
> >>> @@ -25,17 +25,26 @@
> >>> #include <support/check.h>
> >>> #include <support/test-driver.h>
> >>>
> >>> -#ifndef ELF_MACHINE_GET_R_DEBUG
> >>> -# define ELF_MACHINE_GET_R_DEBUG(d) \
> >>> - (__extension__ ({ \
> >>> - struct r_debug_extended *debug; \
> >>> - if ((d)->d_tag == DT_DEBUG) \
> >>> - debug = (struct r_debug_extended *) (d)->d_un.d_ptr; \
> >>> - else \
> >>> - debug = NULL; \
> >>> - debug; }))
> >>> +static struct r_debug_extended *
> >>> +elf_get_r_debug (ElfW(Dyn) *d)
> >>> +{
> >>> +#ifdef __mips__
> >
> > Shouldn't MIPS simply define a proper ELF_MACHINE_GET_R_DEBUG?
> > It can be done with a header file.
>
> The only use of ELF_MACHINE_GET_R_DEBUG is in the test, and the static function
> is easier to read, edit, and debug, all of which is better for a test?
I don't think __mips__ should be in generic code. There
another place where we need to handle _r_debug differences.
A single header file can cover both.
>
> >>> + if (d->d_tag == DT_MIPS_RLD_MAP_REL)
> >>> + {
> >>> + char *ptr = (char *) d;
> >>> + ptr += d->d_un.d_val;
> >>> + return *(struct r_debug_extended **) ptr;
> >>> + }
> >>> + else if (d->d_tag == DT_MIPS_RLD_MAP)
> >>> + return *(struct r_debug_extended **) d->d_un.d_ptr;
> >>
> >> OK. Matches the logic in sysdeps/mips/dl-debug.h
> >>
> >>> +#else
> >>> + if (d->d_tag == DT_DEBUG)
> >>> + return (struct r_debug_extended *) d->d_un.d_ptr;
> >>
> >> OK.
> >>
> >>> #endif
> >>>
> >>> + return NULL;
> >>
> >> OK.
> >>
> >>> +}
> >>> +
> >>> static int
> >>> do_test (void)
> >>> {
> >>> @@ -44,7 +53,7 @@ do_test (void)
> >>>
> >>> for (d = _DYNAMIC; d->d_tag != DT_NULL; ++d)
> >>> {
> >>> - debug = ELF_MACHINE_GET_R_DEBUG (d);
> >>> + debug = elf_get_r_debug (d);
> >>
> >> OK.
> >>
> >>> if (debug != NULL)
> >>> break;
> >>> }
> >>
> >>
> >> --
> >> Cheers,
> >> Carlos.
> >>
> >
> >
>
>
> --
> Cheers,
> Carlos.
>
--
H.J.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] mips: fix elf/tst-dlmopen4
2022-01-28 20:49 ` Carlos O'Donell
2022-01-28 21:20 ` H.J. Lu
@ 2022-01-28 21:23 ` Xi Ruoyao
1 sibling, 0 replies; 7+ messages in thread
From: Xi Ruoyao @ 2022-01-28 21:23 UTC (permalink / raw)
To: Carlos O'Donell, H.J. Lu
Cc: GNU C Library, syq, Jiaxun Yang, Joseph Myers
On Fri, 2022-01-28 at 15:49 -0500, Carlos O'Donell wrote:
> On 1/28/22 15:41, H.J. Lu wrote:
> > On Fri, Jan 28, 2022 at 11:04 AM Carlos O'Donell via Libc-alpha
> > <libc-alpha@sourceware.org> wrote:
> > >
> > > On 1/28/22 13:54, Xi Ruoyao via Libc-alpha wrote:
> > > > MIPS uses DT_MIPS_RLD_MAP and DT_MIPS_RLD_MAP_REL instead of
> > > > DT_DEBUG,
> > > > to provide access to r_debug.
> > >
> > > This looks correct to me. OK for glibc 2.35.
> > >
> > > I'm going to push this today if there are no other objections.
> > >
> > > Reviewed-by: Carlos O'Donell <carlos@redhat.com>
> > >
> > > > ---
> > > > elf/tst-dlmopen4.c | 29 +++++++++++++++++++----------
> > > > 1 file changed, 19 insertions(+), 10 deletions(-)
> > > >
> > > > diff --git a/elf/tst-dlmopen4.c b/elf/tst-dlmopen4.c
> > > > index d8bcf7e9d5..de67a30a4b 100644
> > > > --- a/elf/tst-dlmopen4.c
> > > > +++ b/elf/tst-dlmopen4.c
> > > > @@ -25,17 +25,26 @@
> > > > #include <support/check.h>
> > > > #include <support/test-driver.h>
> > > >
> > > > -#ifndef ELF_MACHINE_GET_R_DEBUG
> > > > -# define ELF_MACHINE_GET_R_DEBUG(d) \
> > > > - (__extension__
> > > > ({ \
> > > > - struct r_debug_extended
> > > > *debug; \
> > > > - if ((d)->d_tag == DT_DEBUG) \
> > > > - debug = (struct r_debug_extended *) (d)->d_un.d_ptr; \
> > > > - else \
> > > > - debug = NULL; \
> > > > - debug; }))
> > > > +static struct r_debug_extended *
> > > > +elf_get_r_debug (ElfW(Dyn) *d)
> > > > +{
> > > > +#ifdef __mips__
> >
> > Shouldn't MIPS simply define a proper ELF_MACHINE_GET_R_DEBUG?
> > It can be done with a header file.
>
> The only use of ELF_MACHINE_GET_R_DEBUG is in the test, and the static
> function
> is easier to read, edit, and debug, all of which is better for a test?
I don't want to add a new header for a test. And among the existing
headers, it seems the alternative definition can only go in
sysdeps/mips/bits/link.h, which is a part of the public API.
(And I'm really not a fan of "macros" like "{...; retval;}" :).
--
Xi Ruoyao <xry111@mengyan1223.wang>
School of Aerospace Science and Technology, Xidian University
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-01-28 21:23 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-28 18:54 [PATCH] mips: fix elf/tst-dlmopen4 Xi Ruoyao
2022-01-28 19:04 ` Carlos O'Donell
2022-01-28 20:12 ` Xi Ruoyao
2022-01-28 20:41 ` H.J. Lu
2022-01-28 20:49 ` Carlos O'Donell
2022-01-28 21:20 ` H.J. Lu
2022-01-28 21:23 ` Xi Ruoyao
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).