[v5,15/22] elf: Run constructors if executable has a soname of a dependency
Checks
Context |
Check |
Description |
dj/TryBot-apply_patch |
success
|
Patch applied to master at the time it was sent
|
Commit Message
The DSO constructor should not be ignored if the main executable
has the SONAME set to a dependency. It fixes the case where
(using the scripts/dso-ordering-test.py definition):
{}->a->b->c;soname({})=c
Where the constructors should return
c>b>a>{}<a<b<c
Checked on x86_64-linux-gnu.
---
elf/dl-load.c | 9 +++++++--
elf/dso-sort-tests-1.def | 5 ++++-
2 files changed, 11 insertions(+), 3 deletions(-)
Comments
* Adhemerval Zanella:
> The DSO constructor should not be ignored if the main executable
> has the SONAME set to a dependency. It fixes the case where
> (using the scripts/dso-ordering-test.py definition):
>
> {}->a->b->c;soname({})=c
>
> Where the constructors should return
>
> c>b>a>{}<a<b<c
>
> Checked on x86_64-linux-gnu.
> ---
> elf/dl-load.c | 9 +++++++--
> elf/dso-sort-tests-1.def | 5 ++++-
> 2 files changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/elf/dl-load.c b/elf/dl-load.c
> index 4a0ff9d010..d585e1795d 100644
> --- a/elf/dl-load.c
> +++ b/elf/dl-load.c
> @@ -1998,13 +1998,18 @@ _dl_map_object (struct link_map *loader, const char *name,
> assert (nsid >= 0);
> assert (nsid < GL(dl_nns));
>
> + /* Special case: trying to map itself. */
> + if (name[0] == '\0')
> + return GL(dl_ns)[nsid]._ns_loaded;
Can you expand the comment a bit? As far as I can see, "" corresponds
to a NULL argument for dlopen, and we interpret that as returning the
head object of the namespace. The head link map must exist because the
dlopen call with NULL/"" has to come from some.
Do you know if we have explict tests for this behavior?
The change itself looks okay. The reference count is updated by the
caller in elf/dl-open.c.
Thanks,
Florian
On 11/11/2021 09:30, Florian Weimer wrote:
> * Adhemerval Zanella:
>
>> The DSO constructor should not be ignored if the main executable
>> has the SONAME set to a dependency. It fixes the case where
>> (using the scripts/dso-ordering-test.py definition):
>>
>> {}->a->b->c;soname({})=c
>>
>> Where the constructors should return
>>
>> c>b>a>{}<a<b<c
>>
>> Checked on x86_64-linux-gnu.
>> ---
>> elf/dl-load.c | 9 +++++++--
>> elf/dso-sort-tests-1.def | 5 ++++-
>> 2 files changed, 11 insertions(+), 3 deletions(-)
>>
>> diff --git a/elf/dl-load.c b/elf/dl-load.c
>> index 4a0ff9d010..d585e1795d 100644
>> --- a/elf/dl-load.c
>> +++ b/elf/dl-load.c
>> @@ -1998,13 +1998,18 @@ _dl_map_object (struct link_map *loader, const char *name,
>> assert (nsid >= 0);
>> assert (nsid < GL(dl_nns));
>>
>> + /* Special case: trying to map itself. */
>> + if (name[0] == '\0')
>> + return GL(dl_ns)[nsid]._ns_loaded;
>
> Can you expand the comment a bit? As far as I can see, "" corresponds
> to a NULL argument for dlopen, and we interpret that as returning the
> head object of the namespace. The head link map must exist because the
> dlopen call with NULL/"" has to come from some.
I have changed to:
/* Special case: trying to map itself. An empty name correspond to
a NULL or "" argument for dlopen(), and it returns the head object
of the namespace. */
>
> Do you know if we have explict tests for this behavior?
Yes, dlfcn/tststatic4 (I have to add this special case for this).
>
> The change itself looks okay. The reference count is updated by the
> caller in elf/dl-open.c.
>
> Thanks,
> Florian
>
@@ -1998,13 +1998,18 @@ _dl_map_object (struct link_map *loader, const char *name,
assert (nsid >= 0);
assert (nsid < GL(dl_nns));
+ /* Special case: trying to map itself. */
+ if (name[0] == '\0')
+ return GL(dl_ns)[nsid]._ns_loaded;
+
/* Look for this name among those already loaded. */
for (l = GL(dl_ns)[nsid]._ns_loaded; l; l = l->l_next)
{
/* If the requested name matches the soname of a loaded object,
use that object. Elide this check for names that have not
- yet been opened. */
- if (__glibc_unlikely ((l->l_faked | l->l_removed) != 0))
+ yet been opened or the executable itself. */
+ if (__glibc_unlikely ((l->l_faked | l->l_removed) != 0
+ || l->l_type == lt_executable))
continue;
if (!_dl_name_match_p (name, l))
{
@@ -50,7 +50,10 @@ output: e>d>c>b>a>{}<a<b<c<d<e
# Test if init/fini ordering behavior is proper, despite main program with
# an soname that may cause confusion
tst-dso-ordering10: {}->a->b->c;soname({})=c
-output: b>a>{}<a<b
+output: c>b>a>{}<a<b<c
+# Same as before, but setting the soname as a direct dependency.
+tst-dso-ordering11: {}->a->b->c;soname({})=a
+output: c>b>a>{}<a<b<c
# Complex example from Bugzilla #15311, under-linked and with circular
# relocation(dynamic) dependencies. While this is technically unspecified, the