This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH v3 1/3] elf: Allow dlopen of filter object to work [BZ #16272]
On 21/01/2020 12:59, David Kilroy wrote:
>> -----Original Message-----
>> From: David Kilroy
>>> -----Original Message-----
>>> From: Adhemerval Zanella <adhemerval.zanella@linaro.org>
>>>
>>>> diff --git a/elf/dl-deps.c b/elf/dl-deps.c
>>>> index c29b988..bb85c83 100644
>>>> --- a/elf/dl-deps.c
>>>> +++ b/elf/dl-deps.c
>>>> @@ -550,13 +550,14 @@ Filters not supported with
>>> LD_TRACE_PRELINKING"));
>>>> }
>>>>
>>>> /* Maybe we can remove some relocation dependencies now. */
>>>> - assert (map->l_searchlist.r_list[0] == map);
>>>
>>> Ok, the first entry is the filtee object.
>>>
>>>> struct link_map_reldeps *l_reldeps = NULL;
>>>> if (map->l_reldeps != NULL)
>>>> {
>>>> - for (i = 1; i < nlist; ++i)
>>>> + for (i = 0; i < nlist; ++i)
>>>> map->l_searchlist.r_list[i]->l_reserved = 1;
>>>>
>>>> + /* Avoid removing relocation dependencies of the main binary.
>>> */
>>>> + map->l_reserved = 0;
>>>> struct link_map **list = &map->l_reldeps->list[0];
>>>> for (i = 0; i < map->l_reldeps->act; ++i)
>>>> if (list[i]->l_reserved)
>>>> @@ -581,16 +582,32 @@ Filters not supported with
>>> LD_TRACE_PRELINKING"));
>>>> }
>>>> }
>>>>
>>>> - for (i = 1; i < nlist; ++i)
>>>> + for (i = 0; i < nlist; ++i)
>>>> map->l_searchlist.r_list[i]->l_reserved = 0;
>>>> }
>>>
>>> I am trying to understand why we can't skip first element here.
>> Neither
>>> of the tests actually exercise this code patch (they won't add a
>>> dependency on l_reldeps), so could you provide an example/testcase
>>> where it requires such change?
>>
>> I haven't observed the use case that this code handles. I just tried to
>> maintain the existing behaviour that we avoid doing this for the main
>> object.
>>
>> I've been trying to understand what triggers this, but not getting very
>> far.
>> Does anyone have any hints as to how I need to setup the test so that
>> we do
>> trigger relocation removals?
>
> OK, so I git blamed this code to commit c4bb124a75 from 2001. I adapted
> elf/relmod5 to include a filter library, and that is working. But it's not
> hitting the relocation removal code (checked by adding _dlerror_printfs).
> I then ran all tests in the elf directory. Only elf/tst-libc_dlvsym-static and
> elf/tst-dlmopen1 enter the for loop, but the l_reserved check is always false.
> In both of the above cases map is libc.so.6 and list[i] is libdl.so.2
>
> I verified the same is the case without any of my commits applied.
>
> Have changes elsewhere superseded this? Suggestions as to what I should do
> here appreciated :)
I am trying to understand how exactly this code is exercised, the
l_reldeps will be set by add_dependency function and this is
set only on _dl_lookup_symbol for some specific case. I will
probably need more time to dig into this, but my understanding
so far the this change should not change current semantic since
removing the main binary with 'map->l_reserved'.