Introducing valgrind to detect memory leaks

[Constellation]

In C, memory leaks are major issues of a project.
To detect memory leaks and test it continuously, I suggest introducing a test with valgrind to detect memory leaks in CI.

[jwerle]

+1 I agree

[stephenmathieson]

Yes, very good idea. I've been running all of our deps through valgrind and slowing fixing leaks. I don't have valgrind on my normal development machine (OSX and valgrind aren't friends), so the process takes a bit longer than it should.

[stephenmathieson]

Perhaps we could just change this line to valgrind --leak-check=full ./$t on systems which valgrind supports?

[Constellation]

I have Ubuntu 13.10 machine :). OK, I'll check it out.

[stephenmathieson]

Also, we may have issues failing builds when leaks occur. Valgrind won't change the exit code of the program it's running, even if leaks are detected. Any suggestions / hacks you've used before?

[Constellation]

I think --error-exitcode option works fine.

[stephenmathieson]

I may have been using it wrong, but --error-exitcode=1 was always exiting with 1, regardless of leaks.

[Constellation]

Oops, you're right. I'll investigate it.

[Constellation]

Ah, I misunderstood.
executing valgrind --error-exitcode=1 <command> works.

[jwerle]

wait, why isn't this working ?

from man valgrind

       --error-exitcode=<number> [default: 0]
           Specifies an alternative exit code to return if Valgrind reported any errors in the run. When set to the default value (zero), the return value from Valgrind will always be
           the return value of the process being simulated. When set to a nonzero value, that value is returned instead, if Valgrind detects any errors. This is useful for using
           Valgrind as part of an automated test suite, since it makes it easy to detect test cases for which Valgrind has reported errors, just by inspecting return codes.

[jwerle]

yay :]

[stephenmathieson]

Ah, okay. We should get the test.sh running leak checks then.

[Constellation]

Investingating the result and I've found,

• Since script executes clib commands, --trace-children=yes is needed.
• Even if --trace-children=yes is enabled, --error-exitcode works for only the top process.

[stephenmathieson]

We may need to test subcommands separately then. Perhaps something like this:

valgrind --leak-check=full --error-exitcode=2 ./clib-install foo/bar foo/bar2
EXIT_CODE=$?

[ $EXIT_CODE -eq 2] && {
  echo >&2 "Memory leaks detected!";
  exit 1
}

[ $EXIT_CODE -eq 0 ] || {
  echo >&2 "Test failed for non-memory related reasons";
  exit 1
}

EDIT: or write our tests in C.

[Constellation]

What do you think of dumping a result to file and grep it to generate error code?

[stephenmathieson]

grepping the results would probably be fine. Can we leave it in the stream rather than writing to file (valgrind ./clib-install | grep ...)?

[jwerle]

what would the result dump look like ?

[Constellation]

For example, executing sudo valgrind --tool=memcheck --show-reachable=yes --leak-check=full --trace-children=yes --error-exitcode=1 ./help.sh 2> dump.log generates https://gist.github.com/Constellation/d8a25a5c0219740ce7dc

[Constellation]

Maybe, we can check errors by grepping such a ERROR SUMMARY format. https://gist.github.com/Constellation/d8a25a5c0219740ce7dc#file-dump-log-L106

[stephenmathieson]

I've just fixed a couple leaks (see #43 and #44), but haven't thought of a good way to automate leak checking. Have either of you (@Constellation @jwerle) thought of anything?

Looping in @larzconwell, as he's been a great help tracking down and fixing leaks on other projects.

[larzconwell]

You can also grep for this line https://gist.github.com/Constellation/d8a25a5c0219740ce7dc#file-dump-log-L127

Also valgrind catches cases where you try to free static or stack memory. The output is a lot different though.

[stephenmathieson]

Hmm so check for successful output rather than errors? That may be a bit more robust.

[larzconwell]

Well you can just check if the grep results are empty and go from there. I don't have much else to give, I've haven't tried automating C stuff yet.

[mbucc]

I had a couple ideas on this. One is to build and run the unit tests on OpenBSD with their malloc options turned on full debug. That can shake some bugs out.

Another approach is to take over malloc, free, etc and keep track. I found four projects on Github that do this.

https://github.com/imsut/malloc-tracer
https://github.com/zunda/count-malloc
https://github.com/meetrp/memory-wrapper
https://github.com/Ryandev/MemoryTracker

I tried imsut on OSX, since it was the only one that didn't require source code changes. It built fine, but when I ran it there was no log produced.

[jwerle]

Closing due to inactivity. Happy to start a conversation around this again