Re: libstdc++ and race detectors

[Paolo Carlini <paolo dot carlini at oracle dot com>]

Hi,
> On Wed, Jul 14, 2010 at 1:58 PM, Paolo Carlini <paolo.carlini@oracle.com> wrote:
>   
>> On 07/14/2010 10:57 AM, Jonathan Wakely wrote:
>>     
>>> My preference would be to do it like that rather than adding yet
>>> another level of indirection to our atomics.
>>>
>>>       
>> Agreed. But then I'd like to see a one-line comment at each appearance
>> of _GLIBCXX_SYNCHRONIZATION_HAPPENS_* pointing to c++config.
>>     
> Do you mean that _GLIBCXX_SYNCHRONIZATION_HAPPENS_* should be defined
> and explained in ./bits/c++config instead of ext/atomicity.h?
>   
Oops, yes, actually I mean both ;) But really, I didn't consciously mean
the point about having the macro in c++config. As a matter of fact it
seems a little more appropriate, because it's just a configuration
macro, not actual code.
>   
>> And a few
>> lines in the doc
>>     
> Which part of doc?
> Will it make sense to add links to the actual race detection tools in the doc?
>   
About the most appropriate part, I'm not sure. If you can prepare a
small blurb, including indeed a (reasonably stable) link to race
detection tools and your external analysis of these issues, I'm pretty
sure Jon can find a place with you. I would say it should be split in
two parts: the first one general and the second one including the quirks
with actually implementing the idea in the current GCC library, thus the
extern template solution for basic_string and the remaining problem with
the usages of atomic operations in exported non-template code.
>>  too would be appreciated, also because normally the
>> _GLIBCXX_* macros are set automatically at library build time and this
>> case may be different.
>>
>> Anyway, I think we need also to add the macros to rc_string_base.h,
>>     
> Right. Will do.
> Also, what about bits/locale_classes.h?
>     void
>     _M_remove_reference() const throw()
>     {
>       if (__gnu_cxx::__exchange_and_add_dispatch(&_M_refcount, -1) ==
> 1)
>         {
>           __try
>             { delete this; }
>   
I think adding the macro in this, and the other already mentioned cases,
would be effective only if the library is rebuilt, because
_M_remove_reference seems (please double check) used only by functions
exported by the *.so, isn't user-visible per se (the throw() exception
specification has been added most definitely by Honza because it helps
the code generation of stuff in the .so)

The *other* _M_remove_reference in that file seems different, because is
called by user-visible templates, like locale(const locale& __other,
_Facet* __f).
>
>   
>> where everything is in templates without instantiations in the .so - no
>> problem - like the shared_ptr case. I think we are going to have
>> troubles for some locale classes and also for ios_base::Init::~Init.
>>     
> no luck indeed, warnings here (if any) will have to be suppressed...
>   
You know, my idea would be, adding the macros for consistency
*everywhere* an atomic counter is decreased (I gather this is the
technical core of the problem). And then, in the docs, point out which
are effective (that is, able to actually suppress the warnings) with a
normally built library, which require the extern template trick on the
command line (I think only basic_string), which require a library
rebuild. I'm not saying you have to provide the whole package at once,
for instance you can split out the documentation work, but please commit
to something consistent for 4.6.x, we don't want to deliver half cooked
stuff, in particular nothing missing a serious documentation, because
these issues can be tricky for the ""average"" user.

Paolo.