On Mon, Jul 25, 2016 at 2:06 PM, Nathan Sidwell <nat...@acm.org> wrote: > On 07/25/16 04:14, Martin Liška wrote: >> >> Hello. >> >> I've just analyzed PR68080, which exposes 2 interesting problems we have: >> >> 1) Majority of instrumented profiling code is not thread-safe, for >> instance edge profiler: >> >> PROF_edge_counter_11 = >> __gcov0._ZL20__gthread_mutex_lockP15pthread_mutex_t[0]; >> PROF_edge_counter_12 = PROF_edge_counter_11 + 1; >> __gcov0._ZL20__gthread_mutex_lockP15pthread_mutex_t[0] = >> PROF_edge_counter_12; >> >> I'm thinking about introducing a param (maybe option), that would >> instrument counter manipulation >> in a thread-safe way: >> >> __sync_fetch_and_add_8 >> (&__gcov0._ZL20__gthread_mutex_lockP15pthread_mutex_t[0], 1); > > > Agreed. (I'm frankly surprised people have got by without it for so long). > Perhaps the existence of an existing threading flag on the command line is > sufficient to clue the gcov machinery? I dislike inventing new knobs to > twiddle. > >> 2) The test-case creates a detached thread which is not properly joined. >> In such situation, the main >> thread reaches gcov_exit, calculates for instance PROFILE_SUMMARY and >> serializes counters to a file. >> However, as the created thread is still running, counters are still >> updated. The leads to a corrupted >> profile file. >> >> I'm wondering whether to prevent that? I can imagine we can lock the >> streaming code, where any profile >> updating thread would not be allowed to do that. > > > An interesting problem. Conceptually one would want the detatched thread to > stop instrumenting -- or instrument to its own set of counters. Neither's > really doable within the existing machinery. Grabbing a lock for every > counter update is going to be very expensive. > > Your phrasing of the problem might be enlightening. 'which is not > *properly* joined'. I.e. don't do that.
There's pthread_detach () - do we wrap that appropriately? That said, another way to make counters thread-safe is to allocate per-thread counters and update those (for example by making the counters __TLS). The interesting part is then to merge them with the main set of counters during thread exit (properly locked or atomically of course). This would also solve the above mentioned issue but have quite a cost on the memory side. Richard. > > nathan
