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.


> 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.

> 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.

> 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.

> 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.


Thanks,
Roland


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