Page MenuHomePhabricator

Merge #17455: tests: Update valgrind suppressions
ClosedPublic

Authored by markblundeberg on Tue, Feb 11, 05:11.

Details

Reviewers
Fabien
Group Reviewers
Restricted Project
Commits
rABC686b53ce4188: Merge #17455: tests: Update valgrind suppressions
Summary

Backport PR17455


Backport note:
I've run the same generator and the only thing it turns up currently
is the LogInstance reachable leak (which is intentional, see logger.cpp).
However it's likely we'll see the other reachable leaks appear in
future backports, so might as well include them now.

Note that --child-silent-after-fork=yes is necessary on all valgrind
test_bitcoin runs after D5254, so comments have been updated accordingly.

Result valgrinding test_bitcoin shows no leaks after suppressions:

==28408== HEAP SUMMARY:
==28408==     in use at exit: 1,088 bytes in 5 blocks
==28408==   total heap usage: 182,020,544 allocs, 182,020,539 frees, 31,333,403,759 bytes allocated

We do have some leaks in bench-bitcoin CoinSelection test, likely
need PR14822.

Depends on D5254

Test Plan

Borrowed from the PR -- run from build dir

valgrind --suppressions=../contrib/valgrind.supp --child-silent-after-fork=yes src/test/test_bitcoin
valgrind --suppressions=../contrib/valgrind.supp src/bench/bitcoin-bench -evals=1 -scaling=0.0

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

markblundeberg created this revision.Tue, Feb 11, 05:11
Herald added a reviewer: Restricted Project. · View Herald TranscriptTue, Feb 11, 05:11
teamcity edited the summary of this revision. (Show Details)Tue, Feb 11, 05:12

[Bot Message]
One or more PR numbers were detected in the summary.
Links to those Bitcoin Core PRs have been inserted into the summary for reference.

markblundeberg edited the test plan for this revision. (Show Details)Tue, Feb 11, 05:12
markblundeberg edited the test plan for this revision. (Show Details)
markblundeberg added a reviewer: Fabien.

(the 5 remaining reachable leaked blocks are: 4 from leveldb, 1 from LogInstance)

update based on D5254

markblundeberg edited the summary of this revision. (Show Details)Tue, Feb 11, 09:32
markblundeberg edited the test plan for this revision. (Show Details)

update comments with --child-silent-after-fork=yes

markblundeberg edited the summary of this revision. (Show Details)Tue, Feb 11, 09:41
Fabien accepted this revision.Tue, Feb 11, 10:48
Fabien added inline comments.
contrib/valgrind.supp
70 ↗(On Diff #16260)

Nit: you can leave the glibc bug suppression at the end, it could help to avoid future merge conflicts a bit.

This revision is now accepted and ready to land.Tue, Feb 11, 10:48

update per nit