Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd/link: valgrind compatible output #782

Closed
alberts opened this issue May 14, 2010 · 26 comments
Closed

cmd/link: valgrind compatible output #782

alberts opened this issue May 14, 2010 · 26 comments

Comments

@alberts
Copy link
Contributor

alberts commented May 14, 2010

RFE:

As discussed here:

http://groups.google.com/group/golang-
nuts/browse_thread/thread/c7d548974abdda92

it would be very useful if 6g could be modified to produce binaries that can 
be run with Valgrind.
@rsc
Copy link
Contributor

rsc commented May 21, 2010

Comment 1:

Not likely to happen.  Valgrind is really a tool for C programs.
It assumes it can replace malloc with its own copy, it assumes
the standard C stack model, I'm sure it assumes other things
specific to that world too.
Equally important, Valgrind doesn't lend much benefit to Go.
Go has no uninitialized variables and no dangling pointers.
I understand that it could be useful for debugging problems
with cgo, but I'm not sure that it would be in practice.
If anyone wants to work on this, patches welcome.

Labels changed: added priority-low, expertneeded, removed priority-medium.

Status changed to LongTerm.

@alberts
Copy link
Contributor Author

alberts commented Jan 12, 2011

Comment 2:

With Go binaries from tip, Valgrind 3.5.0 can actually start up and run for a while
before segfaulting. I don't know if it's Valgrind dying or the Go binary dying because
Valgrind is doing funny things to it, but it seems progress has been made.

@alberts
Copy link
Contributor Author

alberts commented Jan 12, 2011

Comment 3:

Just a note: if things start working better and the C code (maybe the GC) does touch
uninitialized memory, valgrind/memcheck.h has a macro called VALGRIND_MAKE_MEM_DEFINED
which can be used to tell Valgrind that this is expected behaviour.

@alberts
Copy link
Contributor Author

alberts commented Jan 12, 2011

Comment 4:

Valgrind bug reported here:
https://bugs.kde.org/show_bug.cgi?id=262916

@alberts
Copy link
Contributor Author

alberts commented Mar 2, 2011

Comment 5:

Valgrind 3.6.1 more or less runs a Go binary compiled with tip, but it says:
WARNING: Serious error when reading debug info
Can't make sense of .got section mapping
and later it causes the Go binary to crash.

@alberts
Copy link
Contributor Author

alberts commented Jul 28, 2011

Comment 6:

With the recent ELF fixes, the .got warning is gone.
Valgrind from SVN runs for a while and actually seems to produce some sensible output
and then segfaults. I'm going to try reporting the issue on the Valgrind mailing list.

@alberts
Copy link
Contributor Author

alberts commented Jul 28, 2011

Comment 7:

The latest Valgrind from SVN actually works. I think I was still running 3.6.1 in my
previous test.
Valgrind produces some of the following warnings:
Conditional jump or move depends on uninitialised value(s)
Use of uninitialised value of size x
Invalid read/write of size x
I'll take a closer look at these now.

@alberts
Copy link
Contributor Author

alberts commented Jul 29, 2011

@rsc
Copy link
Contributor

rsc commented Dec 9, 2011

Comment 9:

Labels changed: added priority-someday.

@rsc
Copy link
Contributor

rsc commented Sep 12, 2012

Comment 11:

Labels changed: added go1.1.

@rsc
Copy link
Contributor

rsc commented Dec 9, 2012

Comment 12:

I don't understand what is left to do in this bug. fullung@, can you clarify what you
believe is left for the Go developers to do?

Labels changed: removed go1.1.

@alberts
Copy link
Contributor Author

alberts commented Dec 9, 2012

Comment 13:

I think further debugging is needed with the latest version of Valgrind, and Go
developers are probably better equipped to understand out what Valgrind is doing wrong
(if anything), even if Valgrind developers fix it then. Finally, it probably leads to
some VALGRIND_MAKE_MEM_DEFINED bits in the Go runtime code.
I will test the state of things with Go tip and the latest Valgrind and report back.

@alberts
Copy link
Contributor Author

alberts commented Feb 2, 2013

Comment 14:

With Valgrind 3.8.1 on any binary I get:
==6081== Warning: ignored attempt to set SIGRT32 handler in sigaction();
==6081==          the SIGRT32 signal is used internally by Valgrind
fatal error: rt_sigaction failure (this is from go)
So I think the runtime should skip setting up an action for SIGRT32? I will play with it
a bit.

@alberts
Copy link
Contributor Author

alberts commented Feb 2, 2013

Comment 15:

With the rt_sigaction error check disabled, this sometimes works:
valgrind --tool=none -v ./reflect.test -test.v
but also sometimes fails with nil pointer derefs, etc.
This points to some issues in the Valgrind runtime.
valgrind --tool=memcheck survives some packages but segfaults on reflect.
So next steps:
1. Fix the SIGRT32 sigaction thing in the Go runtime. Some guidance would be appreciated.
2. Fix the Valgrind runtime so that nullgrind can always run all the tests.
3. Tackle the memcheck segfault/stacktrace debugging task from the valgrind-developers
mailing list:
"""
You might use the Valgrind gdbserver to use gdb to do the stacktrace
just before the error is reported by Valgrind.
If gdb+Valgrind gdbserver can produce a stacktrace of the simulated cpu
and Valgrind "core" cannot make a stacktrace with the same simulated cpu
state, then it looks like the Valgrind stacktrace logic is to be
enhanced/corrected.
If both can't make a stacktrace, but the native gdb can do a stacktrace,
then that looks more like a Valgrind simulation bug.
"""
4. Add the stuff from
http://valgrind.org/docs/manual/manual-core-adv.html#manual-core-adv.clientreq to the Go
runtime code to hopefully cut down on spurious Valgrind warnings.

@gopherbot
Copy link

Comment 16 by robryk:

It actually makes more sense to run valgrind on go programs than is apparent at first
glance: Some fuzzy testing and similar tools (for example
https://code.google.com/p/avalanche) use valgrind to run the program being tested.

@rsc
Copy link
Contributor

rsc commented Dec 4, 2013

Comment 17:

Labels changed: added repo-main.

@rsc
Copy link
Contributor

rsc commented Mar 3, 2014

Comment 18:

Adding Release=None to all Priority=Someday bugs.

Labels changed: added release-none.

@ianlancetaylor
Copy link
Contributor

Comment 19:

Note that nil pointer dereferences are well-defined Go (they cause a panic).
I'm not at all sure that there is any meaningful way to run valgrind on a Go binary.

@dvyukov
Copy link
Member

dvyukov commented Mar 26, 2014

Comment 20:

robryk, what exactly valgrind tool do you mean?
memcheck makes no sense for Go, because Go does not have out-of-bounds, use-after-free
and leaks (in C sense)
helgrind could make sense, but helgrind does not support Go and w/o this support we
can't do anything; also Go has own race detector which covers most of helgrind
functionality
I propose to close this issue. It's un-actionable and confusing. Valgrind is for C/C++.

@ianlancetaylor
Copy link
Contributor

Comment 21:

According to comment #16, it would be useful if valgrind could run Go programs even if
it didn't detect any problems.
But I'm skeptical that there is any way this can be done reasonably.

@gopherbot
Copy link

Comment 22 by jtolds:

using valgrind on go code would be incredibly useful for helping with cgo libraries

@kodawah
Copy link

kodawah commented Apr 15, 2015

Right now (go 1.4) valgrind reports

bad executable (__PAGEZERO is not 4 GB)
valgrind: ./program: cannot execute binary file

@rsc rsc changed the title cmd/ld: valgrind compatible output cmd/link: valgrind compatible output Jun 8, 2015
@thefallentree
Copy link

any progress on this? use valgrind for CGo libs will be a big plus

@minux
Copy link
Member

minux commented Feb 10, 2017 via email

@rsc rsc closed this as completed Feb 10, 2017
@thefallentree
Copy link

thefallentree commented Feb 10, 2017 via email

@rsc
Copy link
Contributor

rsc commented Feb 10, 2017

I don't know. It would be better to bring this up with the memcheck authors. If you find out something simple that can be done, great.

Note that Go does work with https://github.com/google/sanitizers/wiki/MemorySanitizer and https://github.com/google/sanitizers/wiki/AddressSanitizer.

@golang golang locked and limited conversation to collaborators Feb 10, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

8 participants