[PATCH 2/2] manual: Document __libc_single_threaded

[Rich Felker]

On Fri, May 22, 2020 at 07:02:01PM +0200, Florian Weimer wrote:
> * Rich Felker:
> 
> >> This still has consequences for setxid safety which is why musl now
> >> fully synchronizes the existing threads list. But if you're not using
> >> the thread count for that, it's not an issue. Indeed I think
> >> SYS_membarrier is a solution here, but if it's not supported or
> >> blocked by seccomp then __libc_single_threaded must not be made true
> >> again at this time.
> >
> > Uhg, SYS_membarrier is *not* a solution here. The problem is far
> > worse, because the user of __libc_single_threaded potentially lacks
> > *compiler barriers* too.
> >
> > Consider something like:
> >
> > 	if (!__libc_single_threaded) { lock(); need_unlock=1; }
> > 	x = *p;
> > 	if (need_unlock) unlock();
> > 	/* ... */
> > 	if (!__libc_single_threaded) { lock(); need_unlock=1; }
> > 	x = *p;
> > 	if (need_unlock) unlock();
> >
> > Here, in the case where __libc_single_threaded is true the second time
> > around, there is no (memory or compiler) acquire barrier between the
> > first access to *p and the second. Thus the compiler can (and actually
> > does! I don't have a minimal PoC but musl actually just hit a bug very
> > close to this) omit the second load from memory, and uses the cached
> > value, which may be incorrect because the exiting thread modified it.
> >
> > This could potentially be avoided with complex contracts about
> > barriers needed to use __libc_single_threaded, but it seems highly
> > error-prone.
> 
> Well, yes.  It's clearly a data race if the implementation sets
> __libc_single_threaded directly from an exiting thread.  I don't see a
> way around that.
> 
> Our discussion focused on the problem that observing a thread count of 1
> in pthread_join does not necessarily mean that it is safe to assume at
> this point that the process is single-threaded, in glibc's
> implementation that uses a simple __nptl_nthreads counter decremented on
> the thread itself.  This does not cause a low-level data race directly,
> but is potentially still incorrect (I'm not quite sure yet).

pthread_join necessarily has an acquire barrier (this is a fundamental
requirement of the interface contract; join is acquiring the results
of the thread) so under some weak assumptions on unsynchronized memory
access (e.g. non-tearing, not seeing a value that wasn't stored
sometime between the last and next acquire barriers on the observer's
side) I think observing it from pthread_join is safe.

On the other hand I'm skeptical of the utility. In a program that
only makes small use of threads, the join may happen long after the
thread exits, during which time many operations may have been slowed
down by inability to skip locks.

> In glibc, we annotate many functions with __attribute__ ((leaf)),
> implicitly via __THROW.  None of these functions may reset
> __libc_single_threaded.

I don't think leaf (at least the gcc attribute leaf) is the actual
issue here; it's more complicated. Nothing about leaf forbids stores
to global variables. It just means the compiler can assume it has a
fuller picture for escape analysis. But any update to
__libc_single_threaded would require acquire barriers (e.g., after
acquiring a lock, using the value of __nptl_threads to infer that
future lock cycles can be skipped), and these barriers would in turn
preclude any invalid transformation by the compiler. (Leaf does not
negate barriers.)

Rich