Insufficient control over RUNNING_IN_VALGRIND, should only be active with --tool=memcheck

gperftools-2.7

When running in valgrind with --tool=callgrind or --tool=none the tcmalloc library is still disabling the function calls to the malloc extensions although with these tools the malloc calls are not intercepted by valgrind.

Current control is this :

static int GetRunningOnValgrind(void) { #ifdef RUNNING_ON_VALGRIND if (RUNNING_ON_VALGRIND) return 1; #endif const char *running_on_valgrind_str = TCMallocGetenvSafe("RUNNING_ON_VALGRIND"); if (running_on_valgrind_str) { return strcmp(running_on_valgrind_str, "0") != 0; } return 0; }

This means that if the library was built with -DRUNNING_ON_VALGRIND then the dynamic check for running on valgrind makes a (final) decision, and those who set an env variable RUNNING_ON_VALGRIND can only influence this decision when built without -DRUNNING_ON_VALGRIND.

The build with -DRUNNING_ON_VALGRIND is definitely better to avoid that people that are unaware of the config get puzzled by the valgrind intercepts breaking the malloc extensions, but it is worse in the sense that is takes away all control over the behaviour for those who are aware of the situation.

As explained above, this implies that in the former case one can never measure performance of the tcmalloc library and still output heap usage statistics in this mode.

There is a trivial fix : revert the two conditions such that those who set the environment variable will always have full control.

If you want to be fully compatible with the old behaviour (in case some people would count on the environment variable having no effect) a slightly larger fix is to introduce another variable, like this:

static int GetRunningOnValgrind(void) { const char *force_running_on_valgrind_str = TCMallocGetenvSafe("FORCE_RUNNING_ON_VALGRIND"); if (force_running_on_valgrind_str) { return strcmp(force_running_on_valgrind_str, "0") != 0; } #ifdef RUNNING_ON_VALGRIND if (RUNNING_ON_VALGRIND) return 1; #endif const char *running_on_valgrind_str = TCMallocGetenvSafe("RUNNING_ON_VALGRIND"); if (running_on_valgrind_str) { return strcmp(running_on_valgrind_str, "0") != 0; } return 0; }

But in my opinion it would be unlikely that people would set RUNNING_ON_VALGRIND=0 with a different intent than preserving the malloc extensions (i.e. unlikely this would be used in combination with --tool=memcheck or --tool=massif).

These are just the two most trivial ways to solve the problem, possibly there are better ways, but current behavior is not ideal and there is currently no way to dynamically check if valgrind is replacing the heap interface, so I would recommend to simply swap the two conditions.

All the best, Stef