[contrib] Add Valgrind suppressions file #11035

pull practicalswift wants to merge 2 commits into bitcoin:master from practicalswift:valgrind-suppressions changing 2 files +58 −0
  1. practicalswift commented at 3:17 pm on August 12, 2017: contributor

    Includes known Valgrind warnings in our dependencies that cannot be fixed in-tree.

    Example use:

    0$ valgrind --suppressions=contrib/valgrind.supp src/test/test_bitcoin
1$ valgrind --suppressions=contrib/valgrind.supp --leak-check=full \
2      --show-leak-kinds=all src/test/test_bitcoin --log_level=test_suite

    Running with the suppressions file under Ubuntu 16.04:

    0$ valgrind --suppressions=contrib/valgrind.supp --leak-check=full --show-leak-kinds=all src/test/test_bitcoin --log_level=test_suite --run_test=wallet_crypto
1…
2==10769== LEAK SUMMARY:
3==10769==    definitely lost: 0 bytes in 0 blocks
4==10769==    indirectly lost: 0 bytes in 0 blocks
5==10769==      possibly lost: 0 bytes in 0 blocks
6==10769==    still reachable: 0 bytes in 0 blocks
7==10769==         suppressed: 72,704 bytes in 1 blocks

    Running without the suppressions file under Ubuntu 16.04:

     0$ valgrind --leak-check=full --show-leak-kinds=all src/test/test_bitcoin --log_level=test_suite --run_test=wallet_crypto
 1…
 2==10724== 72,704 bytes in 1 blocks are still reachable in loss record 1 of 1
 3==10724==    at 0x4C2DBF6: malloc (vg_replace_malloc.c:299)
 4==10724==    by 0x6F74EFF: ??? (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.21)
 5==10724==    by 0x40106B9: call_init.part.0 (dl-init.c:72)
 6==10724==    by 0x40107CA: call_init (dl-init.c:30)
 7==10724==    by 0x40107CA: _dl_init (dl-init.c:120)
 8==10724==    by 0x4000C69: ??? (in /lib/x86_64-linux-gnu/ld-2.23.so)
 9==10724==    by 0x2: ???
10==10724==    by 0x1FFF0006D2: ???
11==10724==    by 0x1FFF0006E8: ???
12==10724==    by 0x1FFF0006FF: ???
13==10724==
14==10724== LEAK SUMMARY:
15==10724==    definitely lost: 0 bytes in 0 blocks
16==10724==    indirectly lost: 0 bytes in 0 blocks
17==10724==      possibly lost: 0 bytes in 0 blocks
18==10724==    still reachable: 72,704 bytes in 1 blocks
19==10724==         suppressed: 0 bytes in 0 blocks
  2. practicalswift force-pushed on Aug 12, 2017
  3. MarcoFalke added the label Tests on Aug 12, 2017
  4. MarcoFalke added the label Scripts and tools on Aug 12, 2017
  5. practicalswift force-pushed on Aug 12, 2017
  6. practicalswift force-pushed on Aug 12, 2017
  7. practicalswift force-pushed on Aug 12, 2017
  8. TheBlueMatt commented at 2:55 pm on August 14, 2017: member
    Looks like this is Debian-specific? Is it possible to match libraries without a full, explicit path?
  9. practicalswift commented at 11:59 am on August 15, 2017: contributor

    @TheBlueMatt Good point! I’ve only verified the suppressions file under Ubuntu 16.04. What do you think about changing from …

    0obj:*/libstdc++.*
1obj:/lib/x86_64-linux-gnu/ld-*.so
2obj:/usr/lib/x86_64-linux-gnu/libdb_cxx-5.3.so

    … to …

    0obj:*/libstdc++.*
1obj:*/ld-*.so
2obj:*/libdb_cxx-5.3.so
  10. laanwj commented at 3:58 pm on August 15, 2017: member
    Concept ACK
  11. TheBlueMatt commented at 7:28 pm on August 16, 2017: member
    @practicalswift seems reasonable to me, is the bdb issue there specific to bdb 5.3?
  12. sipa commented at 7:32 pm on August 16, 2017: member
    I believe every BDB version has had this.
  13. practicalswift force-pushed on Aug 16, 2017
  14. practicalswift commented at 7:38 pm on August 16, 2017: contributor

    @TheBlueMatt @sipa Thanks for your input. I’ve now changed from …

    0obj:*/libstdc++.*
1obj:/lib/x86_64-linux-gnu/ld-*.so
2obj:/usr/lib/x86_64-linux-gnu/libdb_cxx-5.3.so

    … to …

    0obj:*/libstdc++.*
1obj:*/ld-*.so
2obj:*/libdb_cxx-*.so

    Looks good? :-)

  15. contrib: Add Valgrind suppressions file 
    Includes known Valgrind warnings in our dependencies that cannot be fixed in-tree.

Example use:

```
$ valgrind --suppressions=contrib/valgrind.supp src/test/test_bitcoin
$ valgrind --suppressions=contrib/valgrind.supp --leak-check=full \
      --show-leak-kinds=all src/test/test_bitcoin --log_level=test_suite
```
    84e2462ccf
  16. practicalswift force-pushed on Aug 16, 2017
  17. TheBlueMatt commented at 1:54 am on August 18, 2017: member

    utACK.

    On August 16, 2017 3:38:39 PM EDT, practicalswift notifications@github.com wrote:

    @TheBlueMatt @sipa Thanks for your input. I’ve now changed from …

    0obj:*/libstdc++.*
1obj:/lib/x86_64-linux-gnu/ld-*.so
2obj:/usr/lib/x86_64-linux-gnu/libdb_cxx-5.3.so

    … to …

    0obj:*/libstdc++.*
1obj:*/ld-*.so
2obj:*/libdb_cxx-*.so

    Looks good? :-)

    – You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/bitcoin/bitcoin/pull/11035#issuecomment-322876858

  18. sipa commented at 5:41 am on August 18, 2017: member
    Concept ACK
  19. luke-jr commented at 6:11 pm on August 19, 2017: member
    Shouldn’t there be a bunch for libsecp256k1 (maybe they ought to go in its repo)?
  20. sipa commented at 6:19 pm on August 19, 2017: member
    @luke-jr Does libbsecp256k1 have memory leaks?
  21. practicalswift commented at 4:12 pm on August 20, 2017: contributor
    @luke-jr I haven’t seen any indications of libbsecp256k1 leaks when running valgrind on test/test_bitcoin. Have you? :-)
  22. luke-jr commented at 5:35 pm on August 20, 2017: member 
     0==24451== Conditional jump or move depends on uninitialised value(s)
 1==24451==    at 0x2DD1A5F: secp256k1_ec_seckey_verify (secp256k1.c:399)
 2==24451==    by 0x2883913: CKey::Check(unsigned char const*) (key.cpp:123)
 3==24451==    by 0x2883A3E: CKey::MakeNewKey(bool) (key.cpp:129)
 4==24451==    by 0x2886F08: ECC_InitSanityCheck() (key.cpp:286)
 5==24451==    by 0x1FBADDC: InitSanityCheck() (init.cpp:703)
 6==24451==    by 0x1FDB034: AppInitSanityChecks() (init.cpp:1173)
 7==24451==    by 0x1F7642E: AppInit(int, char**) (bitcoind.cpp:142)
 8==24451==    by 0x1F77060: main (bitcoind.cpp:195)
 9
10==24451== Conditional jump or move depends on uninitialised value(s)
11==24451==    at 0x2DD1B73: secp256k1_ec_pubkey_create (secp256k1.c:418)
12==24451==    by 0x2883BCC: CKey::GetPubKey() const (key.cpp:152)
13==24451==    by 0x2886F15: ECC_InitSanityCheck() (key.cpp:287)
14==24451==    by 0x1FBADDC: InitSanityCheck() (init.cpp:703)
15==24451==    by 0x1FDB034: AppInitSanityChecks() (init.cpp:1173)
16==24451==    by 0x1F7642E: AppInit(int, char**) (bitcoind.cpp:142)
17==24451==    by 0x1F77060: main (bitcoind.cpp:195)
18
19==24451== Conditional jump or move depends on uninitialised value(s)
20==24451==    at 0x2DC9FE0: secp256k1_pubkey_load (secp256k1.c:132)
21==24451==    by 0x2DD06D3: secp256k1_ec_pubkey_serialize (secp256k1.c:179)
22==24451==    by 0x2883C60: CKey::GetPubKey() const (key.cpp:154)
23==24451==    by 0x2886F15: ECC_InitSanityCheck() (key.cpp:287)
24==24451==    by 0x1FBADDC: InitSanityCheck() (init.cpp:703)
25==24451==    by 0x1FDB034: AppInitSanityChecks() (init.cpp:1173)
26==24451==    by 0x1F7642E: AppInit(int, char**) (bitcoind.cpp:142)
27==24451==    by 0x1F77060: main (bitcoind.cpp:195)
28
29==24451== Conditional jump or move depends on uninitialised value(s)
30==24451==    at 0x2DC3D90: secp256k1_fe_normalize_var (field_5x52_impl.h:142)
31==24451==    by 0x2DD0716: secp256k1_eckey_pubkey_serialize (eckey_impl.h:40)
32==24451==    by 0x2DD0716: secp256k1_ec_pubkey_serialize (secp256k1.c:180)
33==24451==    by 0x2883C60: CKey::GetPubKey() const (key.cpp:154)
34==24451==    by 0x2886F15: ECC_InitSanityCheck() (key.cpp:287)
35==24451==    by 0x1FBADDC: InitSanityCheck() (init.cpp:703)
36==24451==    by 0x1FDB034: AppInitSanityChecks() (init.cpp:1173)
37==24451==    by 0x1F7642E: AppInit(int, char**) (bitcoind.cpp:142)
38==24451==    by 0x1F77060: main (bitcoind.cpp:195)
39
40==24451== Conditional jump or move depends on uninitialised value(s)
41==24451==    at 0x2DD179B: secp256k1_ecdsa_sign (secp256k1.c:361)
42==24451==    by 0x288505E: CKey::Sign(uint256 const&, std::vector<unsigned char, std::allocator<unsigned char> >&, unsigned int) const (key.cpp:168)
43==24451==    by 0x2885624: CKey::VerifyPubKey(CPubKey const&) const (key.cpp:185)
44==24451==    by 0x2886F22: ECC_InitSanityCheck() (key.cpp:288)
45==24451==    by 0x1FBADDC: InitSanityCheck() (init.cpp:703)
46==24451==    by 0x1FDB034: AppInitSanityChecks() (init.cpp:1173)
47==24451==    by 0x1F7642E: AppInit(int, char**) (bitcoind.cpp:142)
48==24451==    by 0x1F77060: main (bitcoind.cpp:195)

    These are not normal?

  23. practicalswift commented at 12:22 pm on August 21, 2017: contributor

    @luke-jr Interesting! These are not triggered when running valgrind on test/test_bitcoin.

    Assuming that these cannot be fixed without changes in our dependencies they should all be included in the suppressions file :-)

  24. luke-jr commented at 2:47 pm on August 21, 2017: member
    Not all systems have the same versions of dependencies, BTW. But I guess untriggered suppressions are harmless.
  25. practicalswift commented at 3:02 pm on August 21, 2017: contributor
    @luke-jr Very much so! Do you want to put together the suppressions (--gen-suppressions=all) for the ones you are encountering, or do you prefer that I give it a try? :-)
  26. practicalswift commented at 6:09 pm on August 23, 2017: contributor

    @luke-jr I’ve been unable to reproduce the warnings you posted. I tried …

    0$ valgrind -v --leak-check=full ./src/bitcoind -printtoconsole

    … against current master.

  27. practicalswift renamed this:
    contrib: Add Valgrind suppressions file
    [contrib] Add Valgrind suppressions file
    on Aug 23, 2017
  28. practicalswift commented at 7:56 pm on October 25, 2017: contributor
    Any chance of getting this merged? :-)
  29. laanwj commented at 12:25 pm on November 9, 2017: member
    I think this file should be referenced somewhere. Not sure from where, but not just in this issue. If there’s a place describing development/debugging practices, a section should be added on using valgrind w/ bitcoin core. If this is debian-specific, this can be documented there too.
  30. practicalswift force-pushed on Nov 12, 2017
  31. Add note about Valgrind suppressions file in developer-notes.md 4a426d8900
  32. practicalswift force-pushed on Nov 12, 2017
  33. practicalswift commented at 4:15 pm on November 12, 2017: contributor

    @laanwj Good point! I’ve now added a note about the suppressions file in developer-notes.md.

    The suppressions file is not meant to be Debian specific. It should work on all Linux Standard Base (LSB) systems.

    Looks good? :-)

  34. laanwj commented at 2:00 pm on November 13, 2017: member
    Yes, thank you, looks good to me now. utACK 4a426d8
  35. laanwj merged this on Nov 13, 2017
  36. laanwj closed this on Nov 13, 2017
  37. laanwj referenced this in commit 927e5280bd on Nov 13, 2017
  38. in doc/developer-notes.md:180 in 4a426d8900 
    175+
176+```shell
177+$ valgrind --suppressions=contrib/valgrind.supp src/test/test_bitcoin
178+$ valgrind --suppressions=contrib/valgrind.supp --leak-check=full \
179+      --show-leak-kinds=all src/test/test_bitcoin --log_level=test_suite
180+$ valgrind -v --leak-check=full src/bitcoind -printtoconsole
    MarcoFalke commented at 7:09 pm on April 29, 2018:
    I think this also needs the --suppressions=contrib/valgrind.supp, otherwise the active suppressions are not printed, which seems to be the purpose of this line.
  39. MarcoFalke commented at 7:11 pm on April 29, 2018: member

    Post merge ACK 4a426d89002034ed1c127624cae95b4a7f540dd1

    Note that I can reproduce the Memcheck:Cond module warnings by building from our ./depends on current master

  40. PastaPastaPasta referenced this in commit a6ed595d26 on Jan 17, 2020
  41. PastaPastaPasta referenced this in commit fa3d73321d on Jan 22, 2020
  42. PastaPastaPasta referenced this in commit d5da430292 on Jan 22, 2020
  43. PastaPastaPasta referenced this in commit 4cca1e04d4 on Jan 29, 2020
  44. PastaPastaPasta referenced this in commit d666b3af3e on Jan 29, 2020
  45. PastaPastaPasta referenced this in commit 46d2cc6159 on Jan 29, 2020
  46. ckti referenced this in commit 50a4edf9f1 on Mar 28, 2021
  47. MarcoFalke locked this on Sep 8, 2021


practicalswift TheBlueMatt laanwj sipa luke-jr MarcoFalke

Labels
Tests Scripts and tools

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-11-17 09:12 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me