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
