This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: Synchronizing auxiliary mutex data
On Tue, 2017-06-20 at 22:35 +0300, Alexander Monakov wrote:
> On Tue, 20 Jun 2017, Torvald Riegel wrote:
> > > Plain accesses to fields like __data.owner are fine as long as they all are
> > > within critical sections set up by LLL_MUTEX_{LOCK,UNLOCK}, but there are some
> > > outside of them. So e.g. in nptl/pthread_mutex_lock:
> > >
> > > 95 else if (__builtin_expect (PTHREAD_MUTEX_TYPE (mutex)
> > > 96 == PTHREAD_MUTEX_RECURSIVE_NP, 1))
> > > 97 {
> > > 98 /* Recursive mutex. */
> > > 99 pid_t id = THREAD_GETMEM (THREAD_SELF, tid);
> > > 100
> > > 101 /* Check whether we already hold the mutex. */
> > > 102 if (mutex->__data.__owner == id)
> > > 103 {
> > > 104 /* Just bump the counter. */
> > > 105 if (__glibc_unlikely (mutex->__data.__count + 1 == 0))
> > > 106 /* Overflow of the counter. */
> > > 107 return EAGAIN;
> > > 108
> > > 109 ++mutex->__data.__count;
> > > 110
> > > 111 return 0;
> > > 112 }
> > > 113
> > > 114 /* We have to get the mutex. */
> > > 115 LLL_MUTEX_LOCK (mutex);
> > > 116
> > > 117 assert (mutex->__data.__owner == 0);
> > >
> > > afaict the access at line 102 can invoke undefined behavior due to a data race.
> >
> > All of them can. Even if the access is in the critical section, because
> > it's not atomic, the compiler is technically allowed to use it as
> > scratch space for a while.
>
> No, the compiler could only use that scratch space within that critical section,
> and thus other threads wouldn't be able to observe that.
There are loads from __owner outside of the critical sections. That's
the data race we're talking about, and they would be able to observe
spurious writes in the critical sections.
> Generally the compiler can't introduce data races
I agree that it must not introduce data races, but that's always under
the precondition that the program is data-race-free itself.
> so in the quoted code snippet
> the compiler can't introduce spurious writes to __owner outside of the critical
> section (as the original code only reads it), and may introduce spurious writes
> to __count only under __owner==id condition.
The compiler is allowed to introduce spurious writes to __count in the
critical section if it writes to it nonatomically (again, because of
DRF) and if there's no other (potentially) synchronizing code (eg, the
atomic accesses to __lock) between the original and the spurious write
(because of DRF).
So, it could introduce spurious writes in the __owner==id branch
potentially, but in practice this would require whole-program analysis.