|
From: David F. <fa...@kd...> - 2014-06-08 17:09:25
Attachments:
testcase_local_static.cpp
|
Hello,
I'm using helgrind quite a lot these days, and I love it.
However I wonder if it doesn't give me false positives for the case of reading
a value from a static object, which was set in the constructor.
Given that gcc does indeed implement "threadsafe statics" as per C++11 (but
even before C++11 came out), one can assume that gcc does "something like" a
mutex around the creation of the object, and therefore that there is a
"happens before" relation between the end of the constructor and the use of
this object later on, right?
In that case it would seem that helgrind needs to learn that, to avoid many
false positives.
Testcase attached.
The assembly code says
call __cxa_guard_acquire
testl %eax, %eax
je .L3
.loc 1 16 0 discriminator 2
movl $_ZZ11threadStartPvE9singleton, %edi
call _ZN9SingletonC1Ev
movl $_ZGVZ11threadStartPvE9singleton, %edi
call __cxa_guard_release
.L3:
IIRC __cxa_guard_acquire/release is the protection around the static, but I'm
not sure exactly what this means. Is there an actual happens-before relation
here?
helgrind log:
==31469== Possible data race during read of size 4 at 0x602068 by thread #3
==31469== Locks held: none
==31469== at 0x400ADF: threadStart(void*) (testcase_local_static.cpp:17)
==31469== by 0x4C2D151: mythread_wrapper (hg_intercepts.c:233)
==31469== by 0x4E3C0DA: start_thread (pthread_create.c:309)
==31469== by 0x595B90C: clone (clone.S:111)
==31469==
==31469== This conflicts with a previous write of size 4 by thread #2
==31469== Locks held: none
==31469== at 0x400BC6: Singleton::Singleton() (testcase_local_static.cpp:9)
==31469== by 0x400AD4: threadStart(void*) (testcase_local_static.cpp:16)
==31469== by 0x4C2D151: mythread_wrapper (hg_intercepts.c:233)
==31469== by 0x4E3C0DA: start_thread (pthread_create.c:309)
==31469== by 0x595B90C: clone (clone.S:111)
--
David Faure, fa...@kd..., http://www.davidfaure.fr
Working on KDE Frameworks 5
|
|
From: Bart V. A. <bva...@ac...> - 2014-06-09 07:53:45
|
On 06/08/14 19:09, David Faure wrote:
> I'm using helgrind quite a lot these days, and I love it.
>
> However I wonder if it doesn't give me false positives for the case of reading
> a value from a static object, which was set in the constructor.
>
> Given that gcc does indeed implement "threadsafe statics" as per C++11 (but
> even before C++11 came out), one can assume that gcc does "something like" a
> mutex around the creation of the object, and therefore that there is a
> "happens before" relation between the end of the constructor and the use of
> this object later on, right?
>
> In that case it would seem that helgrind needs to learn that, to avoid many
> false positives.
>
> Testcase attached.
>
> The assembly code says
> call __cxa_guard_acquire
> testl %eax, %eax
> je .L3
> .loc 1 16 0 discriminator 2
> movl $_ZZ11threadStartPvE9singleton, %edi
> call _ZN9SingletonC1Ev
> movl $_ZGVZ11threadStartPvE9singleton, %edi
> call __cxa_guard_release
> .L3:
>
> IIRC __cxa_guard_acquire/release is the protection around the static, but I'm
> not sure exactly what this means. Is there an actual happens-before relation
> here?
I think g++ will have to be modified to allow Helgrind and DRD to
recognize thread-safe statics on architectures that do not have relaxed
memory consistency. From the gcc source file gcc/cp/decl.c I derived
that on architectures without relaxed memory consistency that g++
protects static initialization as follows:
static <type> guard;
if (!guard.first_byte) {
if (__cxa_guard_acquire (&guard)) {
bool flag = false;
try {
// Do initialization.
flag = true; __cxa_guard_release (&guard);
// Register variable for destruction at end of program.
} catch {
if (!flag) __cxa_guard_abort (&guard);
}
}
If g++ would be modified such that the "if (!guard.first_byte)" test can
be skipped at run-time then it would become possible for Helgrind and
DRD to recognize static initialization by intercepting the
__cxa_guard_*() functions. However, I'm not sure which mechanism the gcc
maintainers will prefer for disabling that if-test at run-time.
Bart.
|
|
From: Bart V. A. <bva...@ac...> - 2014-06-09 13:42:15
|
On 06/09/14 13:16, Olivier Goffart wrote: > On Monday 09 June 2014 09:53:37 Bart Van Assche wrote: >> If g++ would be modified such that the "if (!guard.first_byte)" test can >> be skipped at run-time then it would become possible for Helgrind and >> DRD to recognize static initialization by intercepting the >> __cxa_guard_*() functions. However, I'm not sure which mechanism the gcc >> maintainers will prefer for disabling that if-test at run-time. > > Just an idea: Is it not possible to record guard's address on > __cxa_guard_release, and then consider that an access to guard as an happens > after? > I don't know the internals of helgrind, so I can't say if it is possible, or > if it would be too slow. > Or is __cxa_guard_* used for more than the static initialization? This is certainly possible but I don't see how to implement this without slowing down Helgrind and DRD significantly. Bart. |
|
From: Patrick J. L. <lop...@gm...> - 2014-06-09 15:39:52
|
On Mon, Jun 9, 2014 at 12:53 AM, Bart Van Assche <bva...@ac...> wrote:
>
> I think g++ will have to be modified to allow Helgrind and DRD to
> recognize thread-safe statics on architectures that do not have relaxed
> memory consistency. From the gcc source file gcc/cp/decl.c I derived
> that on architectures without relaxed memory consistency that g++
> protects static initialization as follows:
>
> static <type> guard;
> if (!guard.first_byte) {
> if (__cxa_guard_acquire (&guard)) {
> bool flag = false;
> try {
> // Do initialization.
> flag = true; __cxa_guard_release (&guard);
> // Register variable for destruction at end of program.
> } catch {
> if (!flag) __cxa_guard_abort (&guard);
> }
> }
Interesting! So on x86 and similar, they implement thread-safe Meyers
singletons via the double-checked locking anti-pattern... Which is
actually safe thanks to Intel's not-exactly-relaxed memory model.
> If g++ would be modified such that the "if (!guard.first_byte)" test can
> be skipped at run-time then it would become possible for Helgrind and
> DRD to recognize static initialization
If I understand you correctly (?), you plan to ask the g++ maintainers
to tamper with their fast path to make life easier for Helgrind (?)
If so, I would suggest having a Plan B...
Would it make sense to re-think the happens-before/happens-after
annotation macros for C++11?
Or -- just thinking out loud here -- devise a way to encode this sort
of information in the debug section, and convince the GCC+Clang folks
to implement it (e.g. "-gannotate-memory-model" or somesuch)?
Here is my concern. C++11 provides a fairly elaborate threading and
memory model, with lots of built-in synchronization primitives, memory
barriers, various flavors of ordered loads and stores, etc. As more
and more of these get direct (open-coded) compiler support, and get
used for interesting lock-free algorithms and such, it is going to get
increasingly difficult for Helgrind to figure out what is going on.
Fixing this in general, if possible, would be better than playing
whack-a-mole.
Or am I way off base?
- Pat
|
|
From: David F. <fa...@kd...> - 2014-06-20 19:23:52
|
On Monday 09 June 2014 08:39:45 Patrick J. LoPresti wrote: > Interesting! So on x86 and similar, they implement thread-safe Meyers > singletons via the double-checked locking anti-pattern... Which is > actually safe thanks to Intel's not-exactly-relaxed memory model. Interesting indeed. > > If g++ would be modified such that the "if (!guard.first_byte)" test can > > be skipped at run-time then it would become possible for Helgrind and > > DRD to recognize static initialization > > If I understand you correctly (?), you plan to ask the g++ maintainers > to tamper with their fast path to make life easier for Helgrind (?) Yeah, I can't really see that happening, either. Unless some of you have a really good relationship with the gcc maintainers :-) > If so, I would suggest having a Plan B... > > Would it make sense to re-think the happens-before/happens-after > annotation macros for C++11? Not sure what you mean exactly, so at the risk of asking the same question: would it be possible for me to annotate a global static with some special macros that make helgrind understand what's happening? I know, annotating all global statics one by one sounds horrible, but actually in my case they're already encapsulated in a macro, and I just need a way to remove all these false positives in order for helgrind to be usable. -- David Faure, fa...@kd..., http://www.davidfaure.fr Working on KDE Frameworks 5 |