[3/3] elf/tst-dl_find_object: Disable subtests for non-contiguous maps (bug 28732)
Checks
Context |
Check |
Description |
dj/TryBot-apply_patch |
success
|
Patch applied to master at the time it was sent
|
dj/TryBot-32bit |
success
|
Build for i686
|
Commit Message
---
elf/tst-dl_find_object.c | 29 +++++++++++++++++------------
1 file changed, 17 insertions(+), 12 deletions(-)
Comments
On Mon, Jan 3, 2022 at 9:13 AM Florian Weimer via Libc-alpha
<libc-alpha@sourceware.org> wrote:
>
> ---
> elf/tst-dl_find_object.c | 29 +++++++++++++++++------------
> 1 file changed, 17 insertions(+), 12 deletions(-)
>
> diff --git a/elf/tst-dl_find_object.c b/elf/tst-dl_find_object.c
> index 21cdc0f848..2ad1924088 100644
> --- a/elf/tst-dl_find_object.c
> +++ b/elf/tst-dl_find_object.c
> @@ -71,19 +71,24 @@ check (void *address,
> __FILE__, line, address,
> actual.dlfo_flags, expected->dlfo_flags);
> }
> - if (actual.dlfo_flags != expected->dlfo_flags)
> + if (expected->dlfo_link_map->l_contiguous)
> {
> - support_record_failure ();
> - printf ("%s:%d: error: %p: map start is %p, expected %p\n",
> - __FILE__, line,
> - address, actual.dlfo_map_start, expected->dlfo_map_start);
> - }
> - if (actual.dlfo_map_end != expected->dlfo_map_end)
> - {
> - support_record_failure ();
> - printf ("%s:%d: error: %p: map end is %p, expected %p\n",
> - __FILE__, line,
> - address, actual.dlfo_map_end, expected->dlfo_map_end);
> + /* If the mappings are not contiguous, the actual and execpted
> + mappings may differ, so this subtest will not work. */
> + if (actual.dlfo_flags != expected->dlfo_flags)
> + {
> + support_record_failure ();
> + printf ("%s:%d: error: %p: map start is %p, expected %p\n",
> + __FILE__, line,
> + address, actual.dlfo_map_start, expected->dlfo_map_start);
> + }
> + if (actual.dlfo_map_end != expected->dlfo_map_end)
> + {
> + support_record_failure ();
> + printf ("%s:%d: error: %p: map end is %p, expected %p\n",
> + __FILE__, line,
> + address, actual.dlfo_map_end, expected->dlfo_map_end);
> + }
> }
> if (actual.dlfo_link_map != expected->dlfo_link_map)
> {
> --
> 2.33.1
>
I still see
FAIL: elf/tst-dl_find_object
even when using the new linker with the fix for
https://sourceware.org/bugzilla/show_bug.cgi?id=28743
to remove the 1-page gap. Which file doesn't have
non-contiguous mapping?
On Fri, Jan 14, 2022 at 7:06 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Mon, Jan 3, 2022 at 9:13 AM Florian Weimer via Libc-alpha
> <libc-alpha@sourceware.org> wrote:
> >
> > ---
> > elf/tst-dl_find_object.c | 29 +++++++++++++++++------------
> > 1 file changed, 17 insertions(+), 12 deletions(-)
> >
> > diff --git a/elf/tst-dl_find_object.c b/elf/tst-dl_find_object.c
> > index 21cdc0f848..2ad1924088 100644
> > --- a/elf/tst-dl_find_object.c
> > +++ b/elf/tst-dl_find_object.c
> > @@ -71,19 +71,24 @@ check (void *address,
> > __FILE__, line, address,
> > actual.dlfo_flags, expected->dlfo_flags);
> > }
> > - if (actual.dlfo_flags != expected->dlfo_flags)
> > + if (expected->dlfo_link_map->l_contiguous)
> > {
> > - support_record_failure ();
> > - printf ("%s:%d: error: %p: map start is %p, expected %p\n",
> > - __FILE__, line,
> > - address, actual.dlfo_map_start, expected->dlfo_map_start);
> > - }
> > - if (actual.dlfo_map_end != expected->dlfo_map_end)
> > - {
> > - support_record_failure ();
> > - printf ("%s:%d: error: %p: map end is %p, expected %p\n",
> > - __FILE__, line,
> > - address, actual.dlfo_map_end, expected->dlfo_map_end);
> > + /* If the mappings are not contiguous, the actual and execpted
> > + mappings may differ, so this subtest will not work. */
> > + if (actual.dlfo_flags != expected->dlfo_flags)
> > + {
> > + support_record_failure ();
> > + printf ("%s:%d: error: %p: map start is %p, expected %p\n",
> > + __FILE__, line,
> > + address, actual.dlfo_map_start, expected->dlfo_map_start);
> > + }
> > + if (actual.dlfo_map_end != expected->dlfo_map_end)
> > + {
> > + support_record_failure ();
> > + printf ("%s:%d: error: %p: map end is %p, expected %p\n",
> > + __FILE__, line,
> > + address, actual.dlfo_map_end, expected->dlfo_map_end);
> > + }
> > }
> > if (actual.dlfo_link_map != expected->dlfo_link_map)
> > {
> > --
> > 2.33.1
> >
>
> I still see
>
> FAIL: elf/tst-dl_find_object
>
> even when using the new linker with the fix for
>
> https://sourceware.org/bugzilla/show_bug.cgi?id=28743
>
> to remove the 1-page gap. Which file doesn't have
> non-contiguous mapping?
>
The linker bug isn't really fixed:
[21] .eh_frame PROGBITS 0000000000005ef0 005ef0
000758 00 A 0 0 8
[22] .init_array INIT_ARRAY 0000000000007000 007000
000010 08 WA 0 0 8
[23] .fini_array FINI_ARRAY 0000000000007010 007010
000008 08 WA 0 0 8
[24] .data.rel.ro PROGBITS 0000000000007020 007020
0000a0 00 WA 0 0 32
[25] .dynamic DYNAMIC 00000000000070c0 0070c0
000200 10 WA 8 0 8
[26] .got PROGBITS 00000000000072c0 0072c0
000078 08 WA 0 0 8
[27] .got.plt PROGBITS 0000000000008000 008000
0001d0 08 WA 0 0 8
* H. J. Lu:
> On Mon, Jan 3, 2022 at 9:13 AM Florian Weimer via Libc-alpha
> <libc-alpha@sourceware.org> wrote:
>>
>> ---
>> elf/tst-dl_find_object.c | 29 +++++++++++++++++------------
>> 1 file changed, 17 insertions(+), 12 deletions(-)
>>
>> diff --git a/elf/tst-dl_find_object.c b/elf/tst-dl_find_object.c
>> index 21cdc0f848..2ad1924088 100644
>> --- a/elf/tst-dl_find_object.c
>> +++ b/elf/tst-dl_find_object.c
>> @@ -71,19 +71,24 @@ check (void *address,
>> __FILE__, line, address,
>> actual.dlfo_flags, expected->dlfo_flags);
>> }
>> - if (actual.dlfo_flags != expected->dlfo_flags)
>> + if (expected->dlfo_link_map->l_contiguous)
>> {
>> - support_record_failure ();
>> - printf ("%s:%d: error: %p: map start is %p, expected %p\n",
>> - __FILE__, line,
>> - address, actual.dlfo_map_start, expected->dlfo_map_start);
>> - }
>> - if (actual.dlfo_map_end != expected->dlfo_map_end)
>> - {
>> - support_record_failure ();
>> - printf ("%s:%d: error: %p: map end is %p, expected %p\n",
>> - __FILE__, line,
>> - address, actual.dlfo_map_end, expected->dlfo_map_end);
>> + /* If the mappings are not contiguous, the actual and execpted
>> + mappings may differ, so this subtest will not work. */
>> + if (actual.dlfo_flags != expected->dlfo_flags)
>> + {
>> + support_record_failure ();
>> + printf ("%s:%d: error: %p: map start is %p, expected %p\n",
>> + __FILE__, line,
>> + address, actual.dlfo_map_start, expected->dlfo_map_start);
>> + }
>> + if (actual.dlfo_map_end != expected->dlfo_map_end)
>> + {
>> + support_record_failure ();
>> + printf ("%s:%d: error: %p: map end is %p, expected %p\n",
>> + __FILE__, line,
>> + address, actual.dlfo_map_end, expected->dlfo_map_end);
>> + }
>> }
>> if (actual.dlfo_link_map != expected->dlfo_link_map)
>> {
>> --
>> 2.33.1
>>
>
> I still see
>
> FAIL: elf/tst-dl_find_object
>
> even when using the new linker with the fix for
>
> https://sourceware.org/bugzilla/show_bug.cgi?id=28743
>
> to remove the 1-page gap. Which file doesn't have
> non-contiguous mapping?
We never set l_contiguous for the main executable, so it doesn't matter
what the link editor does. And none of the glibc fixes went in so far.
Thanks,
Florian
On Fri, Jan 14, 2022 at 7:10 AM Florian Weimer <fweimer@redhat.com> wrote:
>
> * H. J. Lu:
>
> > On Mon, Jan 3, 2022 at 9:13 AM Florian Weimer via Libc-alpha
> > <libc-alpha@sourceware.org> wrote:
> >>
> >> ---
> >> elf/tst-dl_find_object.c | 29 +++++++++++++++++------------
> >> 1 file changed, 17 insertions(+), 12 deletions(-)
> >>
> >> diff --git a/elf/tst-dl_find_object.c b/elf/tst-dl_find_object.c
> >> index 21cdc0f848..2ad1924088 100644
> >> --- a/elf/tst-dl_find_object.c
> >> +++ b/elf/tst-dl_find_object.c
> >> @@ -71,19 +71,24 @@ check (void *address,
> >> __FILE__, line, address,
> >> actual.dlfo_flags, expected->dlfo_flags);
> >> }
> >> - if (actual.dlfo_flags != expected->dlfo_flags)
> >> + if (expected->dlfo_link_map->l_contiguous)
> >> {
> >> - support_record_failure ();
> >> - printf ("%s:%d: error: %p: map start is %p, expected %p\n",
> >> - __FILE__, line,
> >> - address, actual.dlfo_map_start, expected->dlfo_map_start);
> >> - }
> >> - if (actual.dlfo_map_end != expected->dlfo_map_end)
> >> - {
> >> - support_record_failure ();
> >> - printf ("%s:%d: error: %p: map end is %p, expected %p\n",
> >> - __FILE__, line,
> >> - address, actual.dlfo_map_end, expected->dlfo_map_end);
> >> + /* If the mappings are not contiguous, the actual and execpted
> >> + mappings may differ, so this subtest will not work. */
> >> + if (actual.dlfo_flags != expected->dlfo_flags)
> >> + {
> >> + support_record_failure ();
> >> + printf ("%s:%d: error: %p: map start is %p, expected %p\n",
> >> + __FILE__, line,
> >> + address, actual.dlfo_map_start, expected->dlfo_map_start);
> >> + }
> >> + if (actual.dlfo_map_end != expected->dlfo_map_end)
> >> + {
> >> + support_record_failure ();
> >> + printf ("%s:%d: error: %p: map end is %p, expected %p\n",
> >> + __FILE__, line,
> >> + address, actual.dlfo_map_end, expected->dlfo_map_end);
> >> + }
> >> }
> >> if (actual.dlfo_link_map != expected->dlfo_link_map)
> >> {
> >> --
> >> 2.33.1
> >>
> >
> > I still see
> >
> > FAIL: elf/tst-dl_find_object
> >
> > even when using the new linker with the fix for
> >
> > https://sourceware.org/bugzilla/show_bug.cgi?id=28743
> >
> > to remove the 1-page gap. Which file doesn't have
> > non-contiguous mapping?
>
> We never set l_contiguous for the main executable, so it doesn't matter
> what the link editor does. And none of the glibc fixes went in so far.
>
> Thanks,
> Florian
>
Should l_contiguous be set on the main executable? If not, why?
* H. J. Lu:
>> We never set l_contiguous for the main executable, so it doesn't matter
>> what the link editor does. And none of the glibc fixes went in so far.
>>
>> Thanks,
>> Florian
>>
>
> Should l_contiguous be set on the main executable? If not, why?
I think it should be set even if the kernel loads the main executable.
See patch 2:
[PATCH 2/3] elf: Set l_contiguous to 1 for the main map in more cases
<https://sourceware.org/pipermail/libc-alpha/2022-January/134894.html>
Thanks,
Florian
On Fri, Jan 14, 2022 at 7:39 AM Florian Weimer <fweimer@redhat.com> wrote:
>
> * H. J. Lu:
>
> >> We never set l_contiguous for the main executable, so it doesn't matter
> >> what the link editor does. And none of the glibc fixes went in so far.
> >>
> >> Thanks,
> >> Florian
> >>
> >
> > Should l_contiguous be set on the main executable? If not, why?
>
> I think it should be set even if the kernel loads the main executable.
> See patch 2:
>
> [PATCH 2/3] elf: Set l_contiguous to 1 for the main map in more cases
> <https://sourceware.org/pipermail/libc-alpha/2022-January/134894.html>
If we have the second patch, do we still need the 3rd one?
* H. J. Lu:
> On Fri, Jan 14, 2022 at 7:39 AM Florian Weimer <fweimer@redhat.com> wrote:
>>
>> * H. J. Lu:
>>
>> >> We never set l_contiguous for the main executable, so it doesn't matter
>> >> what the link editor does. And none of the glibc fixes went in so far.
>> >>
>> >> Thanks,
>> >> Florian
>> >>
>> >
>> > Should l_contiguous be set on the main executable? If not, why?
>>
>> I think it should be set even if the kernel loads the main executable.
>> See patch 2:
>>
>> [PATCH 2/3] elf: Set l_contiguous to 1 for the main map in more cases
>> <https://sourceware.org/pipermail/libc-alpha/2022-January/134894.html>
>
> If we have the second patch, do we still need the 3rd one?
I think gaps are expected on some architectures for main executable
(ia64?).
Thanks,
Florian
On Fri, Jan 14, 2022 at 7:51 AM Florian Weimer <fweimer@redhat.com> wrote:
>
> * H. J. Lu:
>
> > On Fri, Jan 14, 2022 at 7:39 AM Florian Weimer <fweimer@redhat.com> wrote:
> >>
> >> * H. J. Lu:
> >>
> >> >> We never set l_contiguous for the main executable, so it doesn't matter
> >> >> what the link editor does. And none of the glibc fixes went in so far.
> >> >>
> >> >> Thanks,
> >> >> Florian
> >> >>
> >> >
> >> > Should l_contiguous be set on the main executable? If not, why?
> >>
> >> I think it should be set even if the kernel loads the main executable.
> >> See patch 2:
> >>
> >> [PATCH 2/3] elf: Set l_contiguous to 1 for the main map in more cases
> >> <https://sourceware.org/pipermail/libc-alpha/2022-January/134894.html>
> >
> > If we have the second patch, do we still need the 3rd one?
>
> I think gaps are expected on some architectures for main executable
> (ia64?).
>
Please add a header to indicate that the gap is expected only on some
architectures.
* H. J. Lu:
> On Fri, Jan 14, 2022 at 7:51 AM Florian Weimer <fweimer@redhat.com> wrote:
>>
>> * H. J. Lu:
>>
>> > On Fri, Jan 14, 2022 at 7:39 AM Florian Weimer <fweimer@redhat.com> wrote:
>> >>
>> >> * H. J. Lu:
>> >>
>> >> >> We never set l_contiguous for the main executable, so it doesn't matter
>> >> >> what the link editor does. And none of the glibc fixes went in so far.
>> >> >>
>> >> >> Thanks,
>> >> >> Florian
>> >> >>
>> >> >
>> >> > Should l_contiguous be set on the main executable? If not, why?
>> >>
>> >> I think it should be set even if the kernel loads the main executable.
>> >> See patch 2:
>> >>
>> >> [PATCH 2/3] elf: Set l_contiguous to 1 for the main map in more cases
>> >> <https://sourceware.org/pipermail/libc-alpha/2022-January/134894.html>
>> >
>> > If we have the second patch, do we still need the 3rd one?
>>
>> I think gaps are expected on some architectures for main executable
>> (ia64?).
>>
>
> Please add a header to indicate that the gap is expected only on some
> architectures.
Sorry, do you mean there should be a .h file to indicate whether the
architecture expects a gap?
How do I tell whether the gap is expected or the result of binutils
bugs?
Thanks,
Florian
On Fri, Jan 14, 2022 at 7:56 AM Florian Weimer <fweimer@redhat.com> wrote:
>
> * H. J. Lu:
>
> > On Fri, Jan 14, 2022 at 7:51 AM Florian Weimer <fweimer@redhat.com> wrote:
> >>
> >> * H. J. Lu:
> >>
> >> > On Fri, Jan 14, 2022 at 7:39 AM Florian Weimer <fweimer@redhat.com> wrote:
> >> >>
> >> >> * H. J. Lu:
> >> >>
> >> >> >> We never set l_contiguous for the main executable, so it doesn't matter
> >> >> >> what the link editor does. And none of the glibc fixes went in so far.
> >> >> >>
> >> >> >> Thanks,
> >> >> >> Florian
> >> >> >>
> >> >> >
> >> >> > Should l_contiguous be set on the main executable? If not, why?
> >> >>
> >> >> I think it should be set even if the kernel loads the main executable.
> >> >> See patch 2:
> >> >>
> >> >> [PATCH 2/3] elf: Set l_contiguous to 1 for the main map in more cases
> >> >> <https://sourceware.org/pipermail/libc-alpha/2022-January/134894.html>
> >> >
> >> > If we have the second patch, do we still need the 3rd one?
> >>
> >> I think gaps are expected on some architectures for main executable
> >> (ia64?).
> >>
> >
> > Please add a header to indicate that the gap is expected only on some
> > architectures.
>
> Sorry, do you mean there should be a .h file to indicate whether the
> architecture expects a gap?
>
> How do I tell whether the gap is expected or the result of binutils
> bugs?
>
You can add a linker test to check for the linker bug.
* H. J. Lu:
> You can add a linker test to check for the linker bug.
The linker bug could appear randomly, depending on the input files and
their contents.
Thanks,
Florian
On Mon, Jan 3, 2022 at 9:13 AM Florian Weimer via Libc-alpha
<libc-alpha@sourceware.org> wrote:
>
> ---
> elf/tst-dl_find_object.c | 29 +++++++++++++++++------------
> 1 file changed, 17 insertions(+), 12 deletions(-)
>
> diff --git a/elf/tst-dl_find_object.c b/elf/tst-dl_find_object.c
> index 21cdc0f848..2ad1924088 100644
> --- a/elf/tst-dl_find_object.c
> +++ b/elf/tst-dl_find_object.c
> @@ -71,19 +71,24 @@ check (void *address,
> __FILE__, line, address,
> actual.dlfo_flags, expected->dlfo_flags);
> }
> - if (actual.dlfo_flags != expected->dlfo_flags)
> + if (expected->dlfo_link_map->l_contiguous)
> {
> - support_record_failure ();
> - printf ("%s:%d: error: %p: map start is %p, expected %p\n",
> - __FILE__, line,
> - address, actual.dlfo_map_start, expected->dlfo_map_start);
> - }
> - if (actual.dlfo_map_end != expected->dlfo_map_end)
> - {
> - support_record_failure ();
> - printf ("%s:%d: error: %p: map end is %p, expected %p\n",
> - __FILE__, line,
> - address, actual.dlfo_map_end, expected->dlfo_map_end);
> + /* If the mappings are not contiguous, the actual and execpted
> + mappings may differ, so this subtest will not work. */
> + if (actual.dlfo_flags != expected->dlfo_flags)
> + {
> + support_record_failure ();
> + printf ("%s:%d: error: %p: map start is %p, expected %p\n",
> + __FILE__, line,
> + address, actual.dlfo_map_start, expected->dlfo_map_start);
> + }
> + if (actual.dlfo_map_end != expected->dlfo_map_end)
> + {
> + support_record_failure ();
> + printf ("%s:%d: error: %p: map end is %p, expected %p\n",
> + __FILE__, line,
> + address, actual.dlfo_map_end, expected->dlfo_map_end);
> + }
> }
> if (actual.dlfo_link_map != expected->dlfo_link_map)
> {
> --
> 2.33.1
>
LGTM.
Reviewed-by: H.J. Lu <hjl.tools@gmail.com>
Thanks.
@@ -71,19 +71,24 @@ check (void *address,
__FILE__, line, address,
actual.dlfo_flags, expected->dlfo_flags);
}
- if (actual.dlfo_flags != expected->dlfo_flags)
+ if (expected->dlfo_link_map->l_contiguous)
{
- support_record_failure ();
- printf ("%s:%d: error: %p: map start is %p, expected %p\n",
- __FILE__, line,
- address, actual.dlfo_map_start, expected->dlfo_map_start);
- }
- if (actual.dlfo_map_end != expected->dlfo_map_end)
- {
- support_record_failure ();
- printf ("%s:%d: error: %p: map end is %p, expected %p\n",
- __FILE__, line,
- address, actual.dlfo_map_end, expected->dlfo_map_end);
+ /* If the mappings are not contiguous, the actual and execpted
+ mappings may differ, so this subtest will not work. */
+ if (actual.dlfo_flags != expected->dlfo_flags)
+ {
+ support_record_failure ();
+ printf ("%s:%d: error: %p: map start is %p, expected %p\n",
+ __FILE__, line,
+ address, actual.dlfo_map_start, expected->dlfo_map_start);
+ }
+ if (actual.dlfo_map_end != expected->dlfo_map_end)
+ {
+ support_record_failure ();
+ printf ("%s:%d: error: %p: map end is %p, expected %p\n",
+ __FILE__, line,
+ address, actual.dlfo_map_end, expected->dlfo_map_end);
+ }
}
if (actual.dlfo_link_map != expected->dlfo_link_map)
{