theRockLiu / thread-sanitizer

Automatically exported from code.google.com/p/thread-sanitizer
0 stars 0 forks source link

Tsan reports many issues for GLib that seem to be false positives #83

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
This bug was originally filed for GLib here: 
https://bugzilla.gnome.org/show_bug.cgi?id=737345

What seems to be happening, at least according to a cppcon presentation on 
tsan, is that tsan won't detect synchronisation operations for shared libraries 
that use their own locking primitives. GLib does exactly this which means that 
tsan will report issues on it on every app that uses GLib, which on a modern 
Linux system is almost all of them (both GTK+ and Qt use the GLib mainloop, for 
example).

From what I understood of the documentation (and I may be totally wrong about 
this) is that tsan compiler instrumentation definitions would need to be put in 
the actual source code rather than headers. This would mean that everyone would 
need to compile their own GLib in order to use tsan efficiently, which does not 
provide a pleasant development experience. It would be nice if tsan would work 
with this use case without the users having to do any special setup.

Original issue reported on code.google.com by jussi.pa...@canonical.com on 29 Oct 2014 at 1:08

GoogleCodeExporter commented 9 years ago
Yea. recompiling glib is the best choice. 
glider, are we doing that for Chromium? 

If all stack traces are inside glib, you may also try suppressions.
https://code.google.com/p/thread-sanitizer/wiki/CppManual#Suppressing_Reports
This may or may not help, and also suppressions have a performance penalty. 

Original comment by konstant...@gmail.com on 31 Oct 2014 at 10:49

GoogleCodeExporter commented 9 years ago
Ah, and I've just realized that your address is @canonical.com. 
You can actually help here by making Ubuntu packages compile with 
asan/tsan/msan out of the box. We did this work for 50+ packages in Ubuntu 
12.04 and are finishing it for Ubuntu 14.04, but without help from 
Debian/Ubuntu these will remain "local patches". 

earthdok, please add details if you have any. 

Original comment by konstant...@gmail.com on 31 Oct 2014 at 10:53

GoogleCodeExporter commented 9 years ago
FWIW right now we don't test Chromium regularly with glib compiled with TSan.

Original comment by gli...@google.com on 6 Nov 2014 at 9:15

GoogleCodeExporter commented 9 years ago
I have talked to some people about distro wide tsan/asan/msan/etc support and 
there is interest in getting it done. The main enemies are familiar: time 
tables and resources.

Original comment by jussi.pa...@canonical.com on 6 Nov 2014 at 9:59

GoogleCodeExporter commented 9 years ago
I'm currently in a room with the GMutex maintainer. Similar to Qt & QMutex in 
issue 53 he's not a fan of having the annotations unconditionally enabled.

On the Qt bug there was a suggestion that we could use a LD_PRELOADed library 
to add an annotated wrapper. Since you need to both compile and link with 
-fsanitize=thread, it seems that maybe we could provide a way to have Qt and 
GLib (and whoever else feels like implementing their own mutexes) add 
annotations in a debug ELF section and automatically generate the relevant 
annotated wrapper either at compile or link time?

That sounds like a fun project.

Original comment by christop...@canonical.com on 3 Feb 2015 at 4:49

GoogleCodeExporter commented 9 years ago
I don't question that it's a fun project. But it sounds too complex to be 
practical and reliable.

The simplest solution is provide a separate build of a library, just as lots of 
libraries have debug and release builds.
For MemorySanitizer a special build is a must.
I am not sure what is our status with libc++, but the idea is that a compiler 
provides asan/tsan/msan builds of libstdc++/libc++ and driver chooses the 
relevant build. Ideally the same is done for other binary-distributed libraries.

Original comment by dvyu...@google.com on 3 Feb 2015 at 4:57

GoogleCodeExporter commented 9 years ago
That's certainly a simpler solution; what's the suggestion for how the driver 
chooses the relevant build? Automatically postfixing -tsan, -asan, to the link 
resolution? That would be easy enough to provide at the distro layer.

Original comment by christop...@canonical.com on 3 Feb 2015 at 5:43

GoogleCodeExporter commented 9 years ago
Perhaps more along the lines of prepending /tsan/ directory component?
It's better to keep soname unchanged to prevent simultaneously linking 2 
versions of the same library.

Original comment by euge...@google.com on 3 Feb 2015 at 5:46

GoogleCodeExporter commented 9 years ago
+kcc

Adding an additional lib directory to search paths looks like a good solution. 
But I don't know all the options and consequences.
We need to loop in clang and gcc people, so that they agree on a single 
solution.

Original comment by dvyu...@google.com on 3 Feb 2015 at 5:58

GoogleCodeExporter commented 9 years ago
Hm. Either way I think that leaves things which use dlopen() out in the cold.

If we wanted to support *that* we could look at either extending NT_GNU_ABI_TAG 
or abuse the hardware capabilities system to get ld.so to resolve the correct 
thing at runtime.

Original comment by christop...@canonical.com on 4 Feb 2015 at 1:54