valgrind errors for programs compiled with profiling

[GoogleCodeExporter]

What steps will reproduce the problem?
1. echo 'int main() { }' > prof-vg-test.c
2. gcc -o prof-vg-test prof-vg-test.c -lprofiler -g
3. valgrind ./prof-vg-test

What is the expected output? What do you see instead?
==12077== Invalid read of size 1
==12077==    at 0x4C47380: base::VDSOSupport::ElfMemImage::Init(void
const*) (vdso_support.cc:211)
==12077==    by 0x4C47912: base::VDSOSupport::Init() (vdso_support.cc:379)
==12077==    by 0x4C49535: ??? (in /usr/lib64/libprofiler.so.0.0.0)
==12077==    by 0x4C43742: ??? (in /usr/lib64/libprofiler.so.0.0.0)
==12077==  Address 0x7fff2abd7000 is not stack'd, malloc'd or (recently) free'd

What version of the product are you using? On what operating system?

google-perftools-1.4-1.fc12.x86_64

Fedora 12

Linux psi.localdomain 2.6.31.5-127.fc12.x86_64 #1 SMP Sat Nov 7 21:11:14
EST 2009 x86_64 x86_64 x86_64 GNU/Linux

Please provide any additional information below.

Original issue reported on code.google.com by dstah...@gmail.com on 28 Nov 2009 at 4:04

[GoogleCodeExporter]

The full valgrind output...

Original comment by dstah...@gmail.com on 28 Nov 2009 at 4:09

Attachments:

• vg.txt

[GoogleCodeExporter]

Hmm, none of this code should be executed; it should all be short-circuited by 
this
code in vdso_support.cc:
    if (RunningOnValgrind()) {
      vdso_base_ = NULL;
      getcpu_fn_ = &GetCPUViaSyscall;
      return NULL;
    }

Hmm, I think the problem may be a bit later in this function.  Try editing
vdso_support.cc to change this code:
  if (vdso_base_) {
    VDSOSupport vdso;
    SymbolInfo info;
so the first line says
  if (vdso_base_ != kInvalidBase) {

Does that fix the valgrind reports for you?

Original comment by csilv...@gmail.com on 30 Nov 2009 at 6:17

• Added labels: Priority-Medium, Type-Defect

[GoogleCodeExporter]

No, that didn't fix it, but I've attached a new valgrind report now that I've
compiled with '-O0'.

Original comment by dstah...@gmail.com on 16 Dec 2009 at 5:14

Attachments:

• vg.txt

[GoogleCodeExporter]

Is it possible for you to throw a printf in there or something, to see if
RunningOnValgrind() is returning true for you, when you're running under 
valgrind? 
I'm confused why you're executing the code in question at all.

Oh wait, try making this change as well:
    if (RunningOnValgrind()) {
      vdso_base_ = NULL;

change it to
      vsdo_base_ = kInvalidBase;

There may a few other places we use NULL instead of kInvalidBase in there, but
hopefully these are the important ones.

Original comment by csilv...@gmail.com on 18 Dec 2009 at 6:30

[GoogleCodeExporter]

Hi -

I'm having similar issues where tcmalloc 1.5 does not appear to correctly 
detect the 
presence of valgrind. I changed src/base/dynamic_annotations.cc's definition of 
RunningOnValgrind to print some diagnostics:

extern "C" int RunningOnValgrind() {
  printf("Using dynamic_annotations.cc version of RunningOnValgrind\n");
  static int running_on_valgrind = GetRunningOnValgrind();
  if (running_on_valgrind) {
    printf("We are using valgrind\n");
  } else {
    printf("We are not using valgrind\n");
  }
  return running_on_valgrind;
}

I still see this message printed even when valgrind is present:

LD_PRELOAD=libtcmalloc_minimal.so valgrind ls /dev/null
==12299== Memcheck, a memory error detector
==12299== Copyright (C) 2002-2009, and GNU GPL'd, by Julian Seward et al.
==12299== Using Valgrind-3.6.0.SVN-Debian and LibVEX; rerun with -h for 
copyright 
info
==12299== Command: ls /dev/null
==12299== 
Using dynamic_annotations.cc version of RunningOnValgrind
We are not using valgrind
/dev/null

So it doesn't appear that valgrind's version of RunningOnValgrind is being 
executed 
in this case.

However, If I change the definition of GetRunningOnValgrind to use the 
RUNNING_ON_VALGRIND macro from valgrind.h:

static int GetRunningOnValgrind() {
  const char *running_on_valgrind_str = GetenvBeforeMain("RUNNING_ON_VALGRIND");
  if (running_on_valgrind_str) {
    return strcmp(running_on_valgrind_str, "0") != 0;
  }
  return RUNNING_ON_VALGRIND;  // from valgrind.h
}

then it does detect the presence of valgrind correctly:

LD_PRELOAD=libtcmalloc_minimal.so valgrind ls /dev/null
==16425== Memcheck, a memory error detector
==16425== Copyright (C) 2002-2009, and GNU GPL'd, by Julian Seward et al.
==16425== Using Valgrind-3.6.0.SVN-Debian and LibVEX; rerun with -h for 
copyright 
info
==16425== Command: ls /dev/null
==16425== 
Using dynamic_annotations.cc version of RunningOnValgrind
We are using valgrind
/dev/null

The above tests were run on an x86_64 Ubuntu Lucid alpha install, against the 
SVN 
head of google-perftools. On that machine, valgrind --version reports:

valgrind-3.6.0.SVN-Debian

google-perftools was built with the system gcc (Ubuntu 4.4.3-3ubuntu1), and 
configured with the following flags:

--disable-libtool-lock --with-gnu-ld --with-pic --disable-static --disable-
dependency-tracking --enable-minimal

The --enable-minimal is there since there is not a working libunwind available.

Original comment by andrew.c.morrow@gmail.com on 3 Mar 2010 at 11:11

[GoogleCodeExporter]

Hmm, curious, it looks like your valgrind isn't setting RUNNING_ON_VALGRIND, or 
maybe 
it's setting it in a way that your binary can't see it.  What happens if you 
replace 
the GetenvBeforeMain() call with just a normal call to getenv().  Does it work 
for 
you then?

Your solution is good, but it would be nice to not have to depend on 
valgrind.h.  For 
instance, my development machine has valgrind installed, but not valgrind.h (so 
far 
as I can tell).

} The --enable-minimal is there since there is not a working libunwind 
available.

This is unrelated, but you can compile with --enable-frame-pointers instead.

Original comment by csilv...@gmail.com on 3 Mar 2010 at 11:36

[GoogleCodeExporter]

Thanks for the suggestion. I tried changing it to use getenv, rather than 
GetenvBeforeMain, and it did not change the behavior.

Perhaps I've misunderstood how this is supposed to work:

I interpreted the comment above RunningOnValgrind in dynamic_annotations.cc to 
mean 
that when run under valgrind's memcheck tool, all calls to the function 
"RunningOnValgrind' would be intercepted by the valgrind runtime and appear to 
return 
a non-zero value. In that case the dynamic_annotations.cc version of 
RunningOnValgrind would not be called at all, so whether it calls getenv or 
GetenvBeforeMain shouldn't make a difference.

I can confirm that my version of valgrind does not set the variable 
RUNNING_ON_VALGRIND in the environment of the target process, and does not 
intercept 
RunningOnValgrind, but does honor the RUNNING_ON_VALGRIND symbol defined in 
valgrind.h:

~/tmp$ cat ./check_for_valgrind.cc 
#include <cstdio>
#include <cstdlib>
#include <valgrind/valgrind.h>

bool OurRovWasCalled = false;
extern "C" int RunningOnValgrind() {
    OurRovWasCalled = true;
    return false;
}

int main(int argc, char* argv[]) {
   const char* var = getenv("RUNNING_ON_VALGRIND");
   if (var) {
     printf("envvar RUNNING_ON_VALGRIND is set, has value: %s.\n", var);
   } else {
     printf("envvar RUNNING_ON_VALGRIND is not set.\n");
   }

   if(RunningOnValgrind()) {
       printf("RunningOnValgrind() evaluates to 'true'.\n");
   } else {
       printf("RunningOnValgrind() evaluates to 'false'.\n");
   }

   if(OurRovWasCalled) {
       printf("RunningOnValgrind was not intercepted by valgrind.\n");
   } else {
       printf("RunningOnValgrind was intercepted by valgrind.\n");
   }

   if(RUNNING_ON_VALGRIND) {
       printf("symbol RUNNING_ON_VALGRIND evaluates to 'true'.\n");
   } else {
       printf("symbol RUNNING_ON_VALGRIND evaluates to 'false'.\n");
   }

   return 0;
}
~/tmp$ g++ ./check_for_valgrind.cc 
~/tmp$ ./a.out 
envvar RUNNING_ON_VALGRIND is not set.
RunningOnValgrind() evaluates to 'false'.
RunningOnValgrind was not intercepted by valgrind.
symbol RUNNING_ON_VALGRIND evaluates to 'false'.

~/tmp$ valgrind ./a.out 
==6855== Memcheck, a memory error detector
==6855== Copyright (C) 2002-2009, and GNU GPL'd, by Julian Seward et al.
==6855== Using Valgrind-3.6.0.SVN-Debian and LibVEX; rerun with -h for 
copyright info
==6855== Command: ./a.out
==6855== 
envvar RUNNING_ON_VALGRIND is not set.
RunningOnValgrind() evaluates to 'false'.
RunningOnValgrind was not intercepted by valgrind.
symbol RUNNING_ON_VALGRIND evaluates to 'true'.
==6855== 
==6855== HEAP SUMMARY:
==6855==     in use at exit: 0 bytes in 0 blocks
==6855==   total heap usage: 0 allocs, 0 frees, 0 bytes allocated
==6855== 
==6855== All heap blocks were freed -- no leaks are possible
==6855== 
==6855== For counts of detected and suppressed errors, rerun with: -v
==6855== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 4 from 4)

~/tmp$ valgrind --tool=callgrind ./a.out 
==6858== Callgrind, a call-graph generating cache profiler
==6858== Copyright (C) 2002-2009, and GNU GPL'd, by Josef Weidendorfer et al.
==6858== Using Valgrind-3.6.0.SVN-Debian and LibVEX; rerun with -h for 
copyright info
==6858== Command: ./a.out
==6858== 
==6858== For interactive control, run 'callgrind_control -h'.
envvar RUNNING_ON_VALGRIND is not set.
RunningOnValgrind() evaluates to 'false'.
RunningOnValgrind was not intercepted by valgrind.
symbol RUNNING_ON_VALGRIND evaluates to 'true'.
==6858== 
==6858== Events    : Ir
==6858== Collected : 1288263
==6858== 
==6858== I   refs:      1,288,263

So it looks as if the valgrind.h defined RUNNING_ON_VALGRIND symbol is the only 
way 
to detect valgrind, at least on my development machine. I could work around 
this by 
explicitly setting RUNNING_ON_VALGRIND in my environment every time I want to 
use 
valgrind, but that is not a great solution, and people would forget to do it. 
I'd 
really prefer to see google-perftools valgrind auto-detection do the right 
thing. I 
do understand your objection to introducing a dependency on the valgrind 
headers to 
base though.

Re libunwind and frame pointers:

Thanks for suggesting --enable-frame-pointers. This comment in the 
google-perftools 
INSTALL file scared me away from using it:

"If you are on x86-64 system, know that you have a set of system
libraries with frame-pointers enabled, and compile all your
applications with -fno-omit-frame-pointer, then you can enable the
built-in perftools stack unwinder by passing the
--enable-frame-pointers flag to configure."

Obviously I can build google-perftools with -fno-omit-frame-pointer. I could 
probably 
get my applications building with that flag as well, though my understanding is 
that 
maintaining the frame pointer has performance implications. But as far as I 
know 
Ubuntu x86_64 ships with no frame pointers in system libraries and other 
development 
packages, and that seems to preclude using --enable-frame-pointers to build 
google-
perftools. Please let me know if I'm misinterpreting things.

Thanks again for your help.

Original comment by andrew.c.morrow@gmail.com on 4 Mar 2010 at 5:59

[GoogleCodeExporter]

} Thanks for suggesting --enable-frame-pointers. This comment in the 
google-perftools 
} INSTALL file scared me away from using it:

Yeah, I agree the comment is a bit scary.  I was hoping to ward off bug reports 
of 
the form "stacktraces aren't working correctly," because they won't if all 
those 
conditions aren't met.

That said, even incomplete stacktraces are better than none at all, which is 
what you 
get with --enable-minimal, so I think I'd recomment --enable-frame-pointers 
over --
enable-minimal, for users sophisticated enough to be able to handle the 
incomplete 
frame pointers that result (basically, in your case, frames will stop when they 
get 
into libc).

As for the valgrind part, I admit I am not familiar with this part of the 
package, 
and don't know valgrind at all.  Assuming valgrind is supposed to intercept 
this 
function call, do you know why your valgrind isn't?  I don't see how it could, 
honestly.  And searching for RunningOnValgrind on google only shows hits for 
google-
perftools, so it doesn't look like it's a valgrind thing.

Poking around a bit more, I found this page:
   http://valgrind.org/docs/manual/manual-core-adv.html#manual-core-adv.clientreq
which says this:

} You are encouraged to copy the valgrind/*.h headers into your project's 
include
} directory, so your program doesn't have a compile-time dependency on Valgrind 
being 
} installed. The Valgrind headers, unlike most of the rest of the code, are 
under a
} BSD-style license so you may include them without worrying about license
} incompatibility.

So that sounds like a perfect solution here.  Can you attach your valgrind.h to 
this 
bug report?  (I don't have one.)  I'll see about incorporating it into the next 
release.

Original comment by csilv...@gmail.com on 4 Mar 2010 at 6:50

• Changed state: Accepted

[GoogleCodeExporter]

I've attached todays SVN head of include/valgrind.h. Unfortunately, the 
valgrind site 
doesn't seem to have a web-enabled SVN repository, so I can't just post a link. 
FWIW 

FWIW, chromium seems to have gone this path with doing valgrind detection as 
well. 
See:

http://src.chromium.org/viewvc/chrome/trunk/src/base/third_party/valgrind/valgri
nd.h?
view=markup&sortdir=down

http://src.chromium.org/viewvc/chrome/trunk/src/base/dynamic_annotations.cc?
view=markup&sortdir=down&pathrev=26576

I appreciate the clarification on the stack frames issue. I'm fine with partial 
frames as long as I can see my code, and it lets me avoid a dependency on 
libunwind 
too.

Thanks.

Original comment by andrew.c.morrow@gmail.com on 5 Mar 2010 at 3:43

Attachments:

• valgrind.h

[GoogleCodeExporter]

OK, we'll include valgrind.h in the next release (it's luckily 
license-compatible).

Original comment by csilv...@gmail.com on 17 Mar 2010 at 11:30

• Changed state: Started

[GoogleCodeExporter]

I'm glad to hear the simple solution of adding valgrind.h works out. Do you 
have any 
thoughts on when an official release containing this fix might become available?

Original comment by andrew.c.morrow@gmail.com on 23 Mar 2010 at 4:35

[GoogleCodeExporter]

Looks like the issue affects Chromium as well 
http://code.google.com/p/chromium/issues/detail?id=42257

Original comment by timurrrr@chromium.org on 28 Apr 2010 at 8:41

[GoogleCodeExporter]

This should be fixed in perftools 1.6, just released.

Original comment by csilv...@gmail.com on 5 Aug 2010 at 8:46

• Changed state: Fixed