[PATCH] nptl: Open libgcc.so with RTLD_NOW during pthread_cancel

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] nptl: Open libgcc.so with RTLD_NOW during pthread_cancel

Florian Weimer-5
Disabling lazy binding reduces stack usage during unwinding.

Note that RTLD_NOW only makes a difference if libgcc.so has not
already been loaded, so this is only a partial fix.

2018-01-10  Florian Weimer  <[hidden email]>

        [BZ #22636]
        * sysdeps/nptl/unwind-forcedunwind.c (pthread_cancel_init): Open
        libgcc.so with RTLD_NOW, to avoid lazy binding during unwind.

diff --git a/sysdeps/nptl/unwind-forcedunwind.c b/sysdeps/nptl/unwind-forcedunwind.c
index ab4350de99..67b8e74b53 100644
--- a/sysdeps/nptl/unwind-forcedunwind.c
+++ b/sysdeps/nptl/unwind-forcedunwind.c
@@ -49,7 +49,7 @@ pthread_cancel_init (void)
       return;
     }
 
-  handle = __libc_dlopen (LIBGCC_S_SO);
+  handle = __libc_dlopen_mode (LIBGCC_S_SO, RTLD_NOW | __RTLD_DLOPEN);
 
   if (handle == NULL
       || (resume = __libc_dlsym (handle, "_Unwind_Resume")) == NULL
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] nptl: Open libgcc.so with RTLD_NOW during pthread_cancel

Adhemerval Zanella-2


On 10/01/2018 08:49, Florian Weimer wrote:

> Disabling lazy binding reduces stack usage during unwinding.
>
> Note that RTLD_NOW only makes a difference if libgcc.so has not
> already been loaded, so this is only a partial fix.
>
> 2018-01-10  Florian Weimer  <[hidden email]>
>
> [BZ #22636]
> * sysdeps/nptl/unwind-forcedunwind.c (pthread_cancel_init): Open
> libgcc.so with RTLD_NOW, to avoid lazy binding during unwind.

Reviewed-by: Adhemerval Zanella <[hidden email]>

>
> diff --git a/sysdeps/nptl/unwind-forcedunwind.c b/sysdeps/nptl/unwind-forcedunwind.c
> index ab4350de99..67b8e74b53 100644
> --- a/sysdeps/nptl/unwind-forcedunwind.c
> +++ b/sysdeps/nptl/unwind-forcedunwind.c
> @@ -49,7 +49,7 @@ pthread_cancel_init (void)
>        return;
>      }
>  
> -  handle = __libc_dlopen (LIBGCC_S_SO);
> +  handle = __libc_dlopen_mode (LIBGCC_S_SO, RTLD_NOW | __RTLD_DLOPEN);
>  
>    if (handle == NULL
>        || (resume = __libc_dlsym (handle, "_Unwind_Resume")) == NULL
>
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] nptl: Open libgcc.so with RTLD_NOW during pthread_cancel

Carlos O'Donell-6
In reply to this post by Florian Weimer-5
On 01/10/2018 02:49 AM, Florian Weimer wrote:

> Disabling lazy binding reduces stack usage during unwinding.
>
> Note that RTLD_NOW only makes a difference if libgcc.so has not
> already been loaded, so this is only a partial fix.
>
> 2018-01-10  Florian Weimer  <[hidden email]>
>
> [BZ #22636]
> * sysdeps/nptl/unwind-forcedunwind.c (pthread_cancel_init): Open
> libgcc.so with RTLD_NOW, to avoid lazy binding during unwind.
>
> diff --git a/sysdeps/nptl/unwind-forcedunwind.c b/sysdeps/nptl/unwind-forcedunwind.c
> index ab4350de99..67b8e74b53 100644
> --- a/sysdeps/nptl/unwind-forcedunwind.c
> +++ b/sysdeps/nptl/unwind-forcedunwind.c
> @@ -49,7 +49,7 @@ pthread_cancel_init (void)
>        return;
>      }
>  
> -  handle = __libc_dlopen (LIBGCC_S_SO);
> +  handle = __libc_dlopen_mode (LIBGCC_S_SO, RTLD_NOW | __RTLD_DLOPEN);
>  
>    if (handle == NULL
>        || (resume = __libc_dlsym (handle, "_Unwind_Resume")) == NULL
>

Should we not also do this for sysdeps/gnu/unwind-resume.c ?

--
Cheers,
Carlos.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] nptl: Open libgcc.so with RTLD_NOW during pthread_cancel

Florian Weimer-5
On 01/10/2018 08:09 PM, Carlos O'Donell wrote:

> On 01/10/2018 02:49 AM, Florian Weimer wrote:
>> Disabling lazy binding reduces stack usage during unwinding.
>>
>> Note that RTLD_NOW only makes a difference if libgcc.so has not
>> already been loaded, so this is only a partial fix.
>>
>> 2018-01-10  Florian Weimer  <[hidden email]>
>>
>> [BZ #22636]
>> * sysdeps/nptl/unwind-forcedunwind.c (pthread_cancel_init): Open
>> libgcc.so with RTLD_NOW, to avoid lazy binding during unwind.
>>
>> diff --git a/sysdeps/nptl/unwind-forcedunwind.c b/sysdeps/nptl/unwind-forcedunwind.c
>> index ab4350de99..67b8e74b53 100644
>> --- a/sysdeps/nptl/unwind-forcedunwind.c
>> +++ b/sysdeps/nptl/unwind-forcedunwind.c
>> @@ -49,7 +49,7 @@ pthread_cancel_init (void)
>>         return;
>>       }
>>  
>> -  handle = __libc_dlopen (LIBGCC_S_SO);
>> +  handle = __libc_dlopen_mode (LIBGCC_S_SO, RTLD_NOW | __RTLD_DLOPEN);
>>  
>>     if (handle == NULL
>>         || (resume = __libc_dlsym (handle, "_Unwind_Resume")) == NULL
>>
>
> Should we not also do this for sysdeps/gnu/unwind-resume.c ?

Interesting.

_Unwind_Resume is only called from a landing pad after unwinding has
already begun, so the code never actually loads libgcc_s, and RTLD_NOW
does not have any effect at this point (even with lazy binding, most of
the unwinder will already have been bound by this point).  In contrast,
the nptl unwinding code used by pthread_exit/pthread_cancel actually
initiates unwinding.

There is certainly a cleanup opportunity here, but changing the dlopen
flags is not needed at this point.

Thanks,
Florian
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] nptl: Open libgcc.so with RTLD_NOW during pthread_cancel

Carlos O'Donell-6
On 01/10/2018 11:20 AM, Florian Weimer wrote:

> On 01/10/2018 08:09 PM, Carlos O'Donell wrote:
>> On 01/10/2018 02:49 AM, Florian Weimer wrote:
>>> Disabling lazy binding reduces stack usage during unwinding.
>>>
>>> Note that RTLD_NOW only makes a difference if libgcc.so has not
>>> already been loaded, so this is only a partial fix.
>>>
>>> 2018-01-10  Florian Weimer  <[hidden email]>
>>>
>>>     [BZ #22636]
>>>     * sysdeps/nptl/unwind-forcedunwind.c (pthread_cancel_init): Open
>>>     libgcc.so with RTLD_NOW, to avoid lazy binding during unwind.
>>>
>>> diff --git a/sysdeps/nptl/unwind-forcedunwind.c b/sysdeps/nptl/unwind-forcedunwind.c
>>> index ab4350de99..67b8e74b53 100644
>>> --- a/sysdeps/nptl/unwind-forcedunwind.c
>>> +++ b/sysdeps/nptl/unwind-forcedunwind.c
>>> @@ -49,7 +49,7 @@ pthread_cancel_init (void)
>>>         return;
>>>       }
>>>   -  handle = __libc_dlopen (LIBGCC_S_SO);
>>> +  handle = __libc_dlopen_mode (LIBGCC_S_SO, RTLD_NOW | __RTLD_DLOPEN);
>>>       if (handle == NULL
>>>         || (resume = __libc_dlsym (handle, "_Unwind_Resume")) == NULL
>>>
>>
>> Should we not also do this for sysdeps/gnu/unwind-resume.c ?
>
> Interesting.
>
> _Unwind_Resume is only called from a landing pad after unwinding has
> already begun, so the code never actually loads libgcc_s, and
> RTLD_NOW does not have any effect at this point (even with lazy
> binding, most of the unwinder will already have been bound by this
> point).  In contrast, the nptl unwinding code used by
> pthread_exit/pthread_cancel actually initiates unwinding.
>
> There is certainly a cleanup opportunity here, but changing the
> dlopen flags is not needed at this point.
What I want to avoid is two similar pieces of code that do distinct
things. Is there any reason we shouldn't just checkin a change here
that makes both the same, thus avoiding differences?

--
Cheers,
Carlos.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] nptl: Open libgcc.so with RTLD_NOW during pthread_cancel

Florian Weimer-5
On 01/10/2018 08:29 PM, Carlos O'Donell wrote:

> What I want to avoid is two similar pieces of code that do distinct
> things. Is there any reason we shouldn't just checkin a change here
> that makes both the same, thus avoiding differences?

Okay, then let's add a comment.

Thanks,
Florian

unwind.patch (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] nptl: Open libgcc.so with RTLD_NOW during pthread_cancel

Carlos O'Donell-6
On 01/10/2018 12:03 PM, Florian Weimer wrote:
> On 01/10/2018 08:29 PM, Carlos O'Donell wrote:
>
>> What I want to avoid is two similar pieces of code that do distinct
>> things. Is there any reason we shouldn't just checkin a change here
>> that makes both the same, thus avoiding differences?
>
> Okay, then let's add a comment.

Perfect. LGTM.

Reviewed-by: Carlos O'Donell <[hidden email]>

--
Cheers,
Carlos.