MT-safe annotations for gcvt and related functions

On Dec 16, 2014, Florian Weimer <fweimer@redhat.com> wrote:

> On 12/16/2014 08:25 PM, Alexandre Oliva wrote:
>> AFAICT, whereas __printf_fp does access _NL_CURRENT multiple times, but
>> they all access the same locale object, because they all use the result
>> of dereferencing the pointer to the current locale a single time, as a
>> result of compiler optimization, and as such it doesn't mandate the
>> “locale” annotation, although it certainly wouldn't hurt to add it.

> Interesting, I wasn't aware that the rules for these annotations work
> out that way.  I thought all access to the global locale object were
> racy.

Here's a patch that appends some more comments to the definition of
locale discussing this point.

Ok to install?


for ChangeLog

	* manual/intro.texi (locale): Expand, in comments, rationale
	on not marking single dereferences of the locale pointer.
---
 manual/intro.texi |   16 ++++++++++++++++
 1 file changed, 16 insertions(+)

On 12/18/2014 06:41 AM, Alexandre Oliva wrote:
> diff --git a/manual/intro.texi b/manual/intro.texi
> index d4045f2..6cd5776 100644
> --- a/manual/intro.texi
> +++ b/manual/intro.texi
> @@ -730,6 +730,22 @@ constant in these contexts, which makes the former safe.
>  @c locale multiple times may invoke all sorts of undefined behavior
>  @c because of the unexpected locale changes.
>  
> +@c The “multiple times” above is relevant.  Functions that dereference
> +@c the locale pointer only once, even if to access the locale object
> +@c multiple times (_NL_CURRENT's design enables this sort of compiler
> +@c optimization, and current compilers can be counted on to perform
> +@c them), will behave consistently with that one locale object they
> +@c reference throughout.  So, we will generally not annotate such
> +@c functions with the @code{locale} keyword, even though calling such a
> +@c function multiple times in sequence will not ensure the same locale
> +@c object is used.
> +
> +@c We have to take care, however, of our functions that *internally*
> +@c call such functions multiple times: these have to be marked with
> +@c locale, because each internal call may end up using a different
> +@c locale and thus the internal calling function will behave
> +@c inconsistently overall.

I think this contradicts glibc's goal of “no internal data races” that
was discussed some time ago.  Maybe add this aspect as well?  Then it
seems I fine addition to me.

On Fri, 2015-01-30 at 15:03 +0100, Florian Weimer wrote:
> On 12/18/2014 06:41 AM, Alexandre Oliva wrote:
> > diff --git a/manual/intro.texi b/manual/intro.texi
> > index d4045f2..6cd5776 100644
> > --- a/manual/intro.texi
> > +++ b/manual/intro.texi
> > @@ -730,6 +730,22 @@ constant in these contexts, which makes the former safe.
> >  @c locale multiple times may invoke all sorts of undefined behavior
> >  @c because of the unexpected locale changes.
> >  
> > +@c The “multiple times” above is relevant.  Functions that dereference
> > +@c the locale pointer only once, even if to access the locale object
> > +@c multiple times (_NL_CURRENT's design enables this sort of compiler
> > +@c optimization, and current compilers can be counted on to perform
> > +@c them), will behave consistently with that one locale object they
> > +@c reference throughout.  So, we will generally not annotate such
> > +@c functions with the @code{locale} keyword, even though calling such a
> > +@c function multiple times in sequence will not ensure the same locale
> > +@c object is used.
> > +
> > +@c We have to take care, however, of our functions that *internally*
> > +@c call such functions multiple times: these have to be marked with
> > +@c locale, because each internal call may end up using a different
> > +@c locale and thus the internal calling function will behave
> > +@c inconsistently overall.
> 
> I think this contradicts glibc's goal of “no internal data races” that
> was discussed some time ago.  Maybe add this aspect as well?  Then it
> seems I fine addition to me.

I think there are two things here:  First, it is possible to synchronize
in a way in which you grab a pointer to an immutable piece of data once,
and then use that one safely.
Second, when we want to synchronize in that way, we need to tell the
compiler that we're doing this -- in a reliable way.  This means using
atomic operations, and depending on how locale is initialized,
potentially memory_order_acquire too.  AFAIK we're not doing this yet,
but I haven't looked at the code.