This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Always define _dl_starting_up.


On 03/14/2014 01:37 PM, Roland McGrath wrote:
>> If I'm reading this correctly the Linux implementation assumes the writev
>> syscall will serialize access to the fd and avoids taking the dl_load_lock. 
> 
> It is the case that writev's specified semantics is an atomic write so that
> the parts of the message can't be interleaved with writes from other
> threads (or processes).  But the code is not actually concerned with that
> issue.  As the comment in elf/dl-writev.h says, the sole purpose for taking
> the lock is to exclude another thread entering dynamic linker code where it
> might racily clobber the rtld-private errno (which is a simple global).
> The Linux implementation avoids that just by not using a syscall stub that
> touches errno, and that's its main purpose in existing.

OK.

>> In the case of elf/dl-writev.h(_dl_writev) it makes sense to use
>> _dl_starting_up because it allows you to skip the locking. I assume
>> that skipping the locking is a correctness issue I don't yet understand.
>> I wouldn't compilcate _dl_writev with a premature optimization like this.
> 
> As the comment in elf/dl-writev.h says, it's because it might be called
> before enough initialization has happened to make it safe to call the lock
> functions.  I'm not sure that there is any such initialization that matters
> for the locking functions in use on Linux, because they are direct
> CAS+futex uses without use of pthread infrastructure.  But even if that's
> so today it might not have been true when this code was written, and it's
> neither true nor guaranteed for the general case of whatever
> <bits/libc-lock.h> implementation might be in use.

Ageed.

>> Given [1] I think that elf/dl-writev.h is actually the only correct
>> implementation on Linux today, and deleting sysdeps/unix/sysv/linux/dl-writev.h.
>> is the solution.
> 
> Kernel bugs in writev atomicity are wholly irrelevant here, because we
> don't really care about that atomicity.

OK.

>> In the current master my expectation is that __libc_multiple_libcs will
>> always be zero because we remove the weak symbol _dl_starting_up from ld.so.
>> Therefore in Linux it seems that we assume libc will never be loaded more
>> than once which I don't think is true. With dlmopen we could certainly load
>> more than one libc to inspect it, and doing so today would result in that
>> libc resetting your FPU and possibly aborting if the kernel is too old
>> (things we skip doing via __libc_multiple_libcs).
> 
> I'm not sure about this.  The case that I recall motivation
> __libc_multiple_libcs originally was static linking, where the DSO you load
> (nss_*.so, for example) depends on libc.so and so you have the dynamic
> libc.so loaded after startup as well as the static libc in the program.
> 
> If you want to make a change here, you should write test cases that
> exercise both of those scenarios and prove that there is actually a bug
> lurking today.  If there isn't, then optimizing out the unnecessary
> variable is a good thing.

I agree. This requires testing both the dynamic and static-loading-dynamic
cases.

Cheers,
Carlos.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]