[Bug libstdc++/62313] Data race in debug iterators

[François Dumont]

On 22/09/2014 00:04, Jonathan Wakely wrote:
> On 10/09/14 22:55 +0200, François Dumont wrote:
>> Hi
>>
>>    Here is a proposal to fix this data race issue.
>>
>>    I finally generalized bitset approach to fix it by inheriting from 
>> the normal iterator first and then the _Safe_iterator_base type. None 
>> of the libstdc++ iterator types are final so it is fine. 
>> Surprisingly, despite inheritance being private gcc got confused 
>> between _Safe_iterator_base _M_next and forward_list _M_next so I 
>> need to adapt some code to make usage of _Safe_iterator_base _M_next 
>> explicit.
>
> Access control in C++ is not related to visibility, name lookup still
> finds private members, but it is an error to use them.

Ok, tricky.

>
>>    I also consider any operator where normal iterator is being 
>> modified while the safe iterator is linked to the list of iterators. 
>> This is necessary to make sure that thread sanatizer won't consider a 
>> race condition. I didn't touch to bitset::reference because list 
>> references are only access on bitset destruction which is clearly not 
>> an operation allowed to do while playing with references in another 
>> thread.
>>
>>    Do you see any way to check for this problem in the testsuite ? Is 
>> there a thread sanitizer we could use ?
>
> GCC's -fsanitize=thread option, although using it in the testsuite
> would need something like dg-require-tsan so the test doesn't run on
> platforms where it doesn't work, or if GCC was built without
> libsanitizer.
>
> Have you run some tests using -fsanitize=thread, even if they are not
> in the testsuite?

No I hadn't and try since but without success. When I build with 
-fsanitize=thread the produced binary just segfault at startup. It 
complained about using -fPIE at compilation time and -lpie at link time 
but even with those options it segfault. Don't know what is going wrong. 
Maybe Dmitry who reported the bug could give it a try. I will ask for 
this on the bug ticket

>
>> Index: include/debug/safe_iterator.h
> Same renaming here please, to _Iter_base.
>
> Apart from those minor adjustments I think this looks good, but I'd
> like to know that it has been tested with -fsanitize=thread, even if
> only lightly tested.
>
>

I fixed the vocabulary problems. Just need a slight test then.

François