Re: [Valgrind-developers] Valgrind: r14313 - in /trunk: coregrind/ coregrind/m_scheduler/ docs/xml/ helgrind/docs/ helgrind/tests/ none/tests/

[Philippe W. <phi...@sk...>]

On Thu, 2014-08-21 at 09:06 +0200, Bart Van Assche wrote:
> The newly added sim-hint relies on glibc internals. That means that the
> code that implements that sim-hint makes Valgrind harder to port and to
> maintain instead of easier. As you know in the Valgrind project
> OS-independent algorithms are preferred over algorithms that either
> depend on a particular OS or on a particular C library implementation.

For sure, OS or C lib independent algorithms are preferrable.

But in this case, this is a glibc specific kludge to solve a glibc
specific problem (ie. that glibc uses an internal cache of recycled
memory and that helgrind (or drd) cannot observe the synchronisation
primitives used to manage this cache.

So, there is no need to port this kludge to other OS, or to other C
libraries, as they are unlikely to suffer from the same original
problem, which is caused by the specific way glibc handles
some tls related data structure and stack cache.
(there are other similar glibc only kludges, when deemed appropriate
e.g. __libc_freeres handling).

Replicating something non trivial (at least I could not
understand all the subtilities), in the core algorithm of helgrind
(with e.g. risks of false negative) to solve a glibc specific problem
does not attract me a lot.

The sim-hint is simple to develop, does not have
to be ported to other OS or other lib, is easy to understand,
and is by default not enabled (as not everybody encounters
these problems) and when enabled has a very small impact
on behaviour/performance.

The kludgy aspect is that it might be broken by an internal
change in glibc.

I have tested this with glibc 2.11, 2.16, 2.18 and/or on x86, amd64,
ppc64, s390x.
I also looked at the 2.20 code, which is still the same. So, should
still work with the next glibc.

So, it looks not *that* quickly broken.

And I will (try to) repair, if the kludge is broken or not as good as I
wish (e.g. I will soon improve it so that disabling the cache
happens earlier, as glibc is recycling memory for detached threads
before valgrind sees their termination).

Hoping the above clarifies the choice made. Be sure it was
not made in a hurry (initial discussions and trials some months
ago, investigation on how drd handles this, discussions with glibc
developers, etc).

Philippe