Re: [PATCH v32] Avoid cancellable I/O primitives in ld.so.

[Adhemerval Zanella <adhemerval dot zanella at linaro dot org>]

On 06/06/2018 17:28, Zack Weinberg wrote:
> On Wed, Jun 6, 2018 at 4:00 PM, Adhemerval Zanella
> <adhemerval.zanella@linaro.org> wrote:
>> On 02/06/2018 19:08, Zack Weinberg wrote:
>>> Neither the <dlfcn.h> entry points, nor lazy symbol resolution, nor
>>> initial shared library load-up, are cancellation points, so ld.so
>>> should exclusively use I/O primitives that are not cancellable; in
>>> addition to streamlining low-level operations, this means elf/Makefile
>>> does not think we require a copy of unwind.c in ld.so.
> ...
>> Patch LGTM with only one adjustment on
>> sysdeps/unix/sysv/linux/Makefile.
> 
> Thanks for reviewing.
> 
>> One thing that it is good your have started is to avoid use non-LFS
>> interfaces within glibc, ideally it shouldn't require to add non-LFS
>> nocancel interfaces (but I think we can make it a following cleanup
>> patch).
> 
> Yeah, let's leave this as a future cleanup.  I'm not altogether sold
> on explicit __foo64 within glibc, considering that the preferred style
> for applications is _FILE_OFFSET_BITS=64 and the Hurd seems to have
> always had 64-bit off_t going with the unadorned names.

I think using explicit __foo64 on glibc simplifies the possible messed
handling of defining _FILE_OFFSET_BITS and the non-LFS wrappers building
(we will need to either not define it on non-LFS wrappers or undefine
them before including the headers).

> 
> [...]
>>> @@ -199,4 +204,6 @@ tests += tst-align-clone tst-getpid1 \
>>>       tst-thread-affinity-pthread tst-thread-affinity-pthread2 \
>>>       tst-thread-affinity-sched
>>>  tests-internal += tst-setgetname
>>> +pthread-compat-wrappers += close_nocancel fcntl_nocancel nanosleep_nocancel \
>>> +     open64_nocancel pause_nocancel read_nocancel write_nocancel
>>>  endif
>>
>> I think the rule 'pthread-compat-wrappers' is misleading here, since the nocancel
>> variations is used not to provide compatibility wrappers.  I think it is better
>> to use 'libpthread-routines' instead.
> 
> In terms of the ABI, they *are* compatibility wrappers.  Formerly,
> __close_nocancel was defined by the same object file as close,
> __fcntl_nocancel was defined by the same object file as fcntl, and so
> on.  Therefore, putting 'close' in pthread-compat-wrappers would have
> the effect of making *both* 'close' and '__close_nocancel' pthread
> compatibility symbols.  If I put these in libpthread-routines, they
> will get the wrong treatment from nptl/Makefile, namely, they'll be
> put into libpthread.a unnecessarily.
> 
> ... Having said that, x86-64 at least doesn't export any _nocancel
> functions from libpthread, which may mean we don't need these at all.
> Will experiment and report back.
> 
> zw
> 

The nocancel variants are used on libpthread symbols, not really to on
wrappers:

  * __open64_nocancel, __read_nocancel, __write_nocancel, __close_nocancel
    are used on unix/sysv/linux/pthread_{get,set}name.c.
  * __pause_nocancel is used on nptl/pthread_mutex_lock.c.
  * __nanosleep_nocancel is used on nptl/pthread_mutex_timedlock.c.

I think adding them on libpthread-routines is exactly what we want.