elf: Remove redundant add_to_global_resize_failure call from dl_open_args

Message ID 87y2ql9mqc.fsf@mid.deneb.enyo.de
State Committed
Delegated to: Carlos O'Donell
Headers
Series elf: Remove redundant add_to_global_resize_failure call from dl_open_args |

Commit Message

Florian Weimer April 24, 2020, 10:55 a.m. UTC
  The second call does not do anything because the data structures have
already been resized by the call that comes before the demarcation
point.  Fixes commit a509eb117fac1d764b15eba64993f4bdb63d7f3c
("Avoid late dlopen failure due to scope, TLS slotinfo updates
[BZ #25112]").

-----
Tested on i686-linux-gnu and x86_64-linux-gnu.

 elf/dl-open.c | 5 -----
 1 file changed, 5 deletions(-)
  

Comments

Carlos O'Donell May 12, 2020, 2:47 a.m. UTC | #1
On 4/24/20 6:55 AM, Florian Weimer wrote:
> The second call does not do anything because the data structures have
> already been resized by the call that comes before the demarcation
> point.  Fixes commit a509eb117fac1d764b15eba64993f4bdb63d7f3c
> ("Avoid late dlopen failure due to scope, TLS slotinfo updates
> [BZ #25112]").

No regressions on x86_64 and i686.

OK for master.

Reviewed-by: Carlos O'Donell <carlos@redhat.com>

Question for you:
`git-pw patch apply` treats everything, including text after the
"-----" as part of the commit message, this is because git-am
considers a divider only if it is exactly 3 dashes. Did you add
the 5 dashes? Did you intend everything after the dashes to be
included in the commit message?	This is OK for master if you don't
commit the diffstat as part of your commit message.

> -----
> Tested on i686-linux-gnu and x86_64-linux-gnu.
> 
>  elf/dl-open.c | 5 -----
>  1 file changed, 5 deletions(-)
> 
> diff --git a/elf/dl-open.c b/elf/dl-open.c
> index 7b3b177aa6..d9fdb0bdce 100644
> --- a/elf/dl-open.c
> +++ b/elf/dl-open.c
> @@ -738,11 +738,6 @@ dl_open_worker (void *a)
>    DL_STATIC_INIT (new);
>  #endif
>  
> -  /* Perform the necessary allocations for adding new global objects
> -     to the global scope below, via add_to_global_update.  */
> -  if (mode & RTLD_GLOBAL)
> -    add_to_global_resize (new);
> -

OK.

Notes:

The call before the demarcation point must by definition have set
the size to the correct size and allocated enough space for it, otherwise
what would be the point of further extending and needing to call malloc
to do such an extension (and possibly fail).

We call these before we called the removed add_to_global_resize:
activate_nodelete
update_scopes
update_tls_Slotinfo
_dl_call_libc_early_init
DL_STATIC_INIT

None of these should add new objects.

Then we would call:
call_dl_init

This calls all the foreign functions (constructors etc) and these might
add objects to the global scope. We explicitly do not let them see the
currently constructed object because construction is not done yet.

Then we call:
add_to_global_update

This adds the loaded objects into the global scope.

If during a constructor we added new objects to the global scope (dlopen) that
would be OK, we would have called add_to_global_resize and add_to_global_update
as part of that dlopen, so the global size would already be large enough (part
of the recursive dlopen logic handling).

>    /* Run the initializer functions of new objects.  Temporarily
>       disable the exception handler, so that lazy binding failures are
>       fatal.  */
>
  
Florian Weimer May 12, 2020, 5:07 p.m. UTC | #2
* Carlos O'Donell via Libc-alpha:

> On 4/24/20 6:55 AM, Florian Weimer wrote:
>> The second call does not do anything because the data structures have
>> already been resized by the call that comes before the demarcation
>> point.  Fixes commit a509eb117fac1d764b15eba64993f4bdb63d7f3c
>> ("Avoid late dlopen failure due to scope, TLS slotinfo updates
>> [BZ #25112]").
>
> No regressions on x86_64 and i686.
>
> OK for master.
>
> Reviewed-by: Carlos O'Donell <carlos@redhat.com>
>
> Question for you:
> `git-pw patch apply` treats everything, including text after the
> "-----" as part of the commit message, this is because git-am
> considers a divider only if it is exactly 3 dashes. Did you add
> the 5 dashes? Did you intend everything after the dashes to be
> included in the commit message?

No, I think it used to be three dashes?

Or maybe my patch posting script has always been wrong.
  

Patch

diff --git a/elf/dl-open.c b/elf/dl-open.c
index 7b3b177aa6..d9fdb0bdce 100644
--- a/elf/dl-open.c
+++ b/elf/dl-open.c
@@ -738,11 +738,6 @@  dl_open_worker (void *a)
   DL_STATIC_INIT (new);
 #endif
 
-  /* Perform the necessary allocations for adding new global objects
-     to the global scope below, via add_to_global_update.  */
-  if (mode & RTLD_GLOBAL)
-    add_to_global_resize (new);
-
   /* Run the initializer functions of new objects.  Temporarily
      disable the exception handler, so that lazy binding failures are
      fatal.  */