ci: Run fuzz testing test cases (bitcoin-core/qa-assets) under valgrind to catch memory errors #18166

pull practicalswift wants to merge 5 commits into bitcoin:master from practicalswift:fuzz-test-cases-under-valgrind changing 4 files +51 −7
  1. practicalswift commented at 7:18 pm on February 17, 2020: contributor

    Run fuzz testing test cases (bitcoin-core/qa-assets) under valgrind.

    This would have caught util: Avoid potential uninitialized read in FormatISO8601DateTime(int64_t) by checking gmtime_s/gmtime_r return value (#18162) and similar cases.

  2. practicalswift renamed this:
    ci: Run fuzz testing test cases under valgrind
    ci: Run fuzz testing test cases (bitcoin-core/qa-assets) under valgrind
    on Feb 17, 2020
  3. practicalswift renamed this:
    ci: Run fuzz testing test cases (bitcoin-core/qa-assets) under valgrind
    ci: Run fuzz testing test cases (bitcoin-core/qa-assets) under valgrind to catch memory errors
    on Feb 17, 2020
  4. practicalswift commented at 7:56 pm on February 17, 2020: contributor

    Failing as intended in Travis:

     0…
 1Run integer with args ['valgrind', '--quiet', '--error-exitcode=1', '/home/travis/build/bitcoin/bitcoin/build/bitcoin-x86_64-pc-linux-gnu/src/test/fuzz/integer', '-runs=1', '-detect_leaks=0', '/home/travis/build/bitcoin/bitcoin/ci/scratch//qa-assets/fuzz_seed_corpus/integer']
 2Output: INFO: Seed: 3664174428
 3INFO: Loaded 1 modules   (154709 inline 8-bit counters): 154709 [0xea0bc8, 0xec681d), 
 4INFO: Loaded 1 PC tables (154709 PCs): 154709 [0xec6820,0x1122d70), 
 5INFO:       79 files found in /home/travis/build/bitcoin/bitcoin/ci/scratch//qa-assets/fuzz_seed_corpus/integer
 6INFO: -max_len is not provided; libFuzzer will not generate inputs larger than 4096 bytes
 7INFO: seed corpus: files: 79 min: 1b max: 83b total: 5116b rss: 129Mb
 8==26395== Conditional jump or move depends on uninitialised value(s)
 9==26395==    at 0x4F43C0A: std::ostreambuf_iterator<char, std::char_traits<char> > std::num_put<char, std::ostreambuf_iterator<char, std::char_traits<char> > >::_M_insert_int<long>(std::ostreambuf_iterator<char, std::char_traits<char> >, std::ios_base&, char, long) const (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.25)
10==26395==    by 0x4F501A4: std::ostream& std::ostream::_M_insert<long>(long) (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.25)
11==26395==    by 0x543C1C: formatValue<int> (tinyformat.h:358)
12==26395==    by 0x543C1C: void tinyformat::detail::FormatArg::formatImpl<int>(std::ostream&, char const*, char const*, int, void const*) (tinyformat.h:543)
13==26395==    by 0x532D1F: format (tinyformat.h:528)
14==26395==    by 0x532D1F: tinyformat::detail::formatImpl(std::ostream&, char const*, tinyformat::detail::FormatArg const*, int) (tinyformat.h:907)
15==26395==    by 0x5EE190: vformat (tinyformat.h:1054)
16==26395==    by 0x5EE190: format<int, int, int> (tinyformat.h:1064)
17==26395==    by 0x5EE190: std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > tinyformat::format<int, int, int>(char const*, int const&, int const&, int const&) (tinyformat.h:1073)
18==26395==    by 0x5EE0C0: FormatISO8601Date[abi:cxx11](long) (time.cpp:112)
19…

    Will fail until #18162 is merged.

  5. DrahtBot commented at 8:24 pm on February 17, 2020: member

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #16367 (Multiprocess build support by ryanofsky)

    If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

  6. DrahtBot added the label Tests on Feb 17, 2020
  7. in ci/test/00_setup_env_native_fuzz_with_valgrind.sh:9 in 71cd3bea89 outdated 
    0@@ -0,0 +1,18 @@
1+#!/usr/bin/env bash
2+#
3+# Copyright (c) 2019 The Bitcoin Core developers
4+# Distributed under the MIT software license, see the accompanying
5+# file COPYING or http://www.opensource.org/licenses/mit-license.php.
6+
7+export LC_ALL=C.UTF-8
8+
9+export CONTAINER_NAME=ci_native_fuzz
    MarcoFalke commented at 0:54 am on February 18, 2020: 
    0export CONTAINER_NAME=ci_native_fuzz_valgrind
    practicalswift commented at 6:56 am on February 18, 2020:
    Fixed!
  8. MarcoFalke approved
  9. MarcoFalke commented at 0:55 am on February 18, 2020: member
    ACK
  10. ci: Run fuzz testing test cases under valgrind a3b539a924
  11. practicalswift force-pushed on Feb 18, 2020
  12. practicalswift commented at 6:58 am on February 18, 2020: contributor
    @MarcoFalke Thanks for reviewing! CONTAINER_NAME now changed. Please re-review :)
  13. MarcoFalke commented at 11:24 pm on February 18, 2020: member
    It could make sense to add an --exclude argument to the test_runner to skip a test target (in this case --exclude integer).
  14. in test/fuzz/test_runner.py:150 in a3b539a924 outdated 
    149@@ -150,7 +150,7 @@ def run_once(*, corpus, test_list, build_dir, export_coverage, use_valgrind):
150             corpus_path,
    MarcoFalke commented at 11:26 pm on February 18, 2020:

    Unrelated to this pull, but it why is detect_leaks disabled?

    I’d suggest to include this commit:

     0commit ffff1245c11428968b71454e4ef85417d33c9893 (HEAD)
 1Author: MarcoFalke <falke.marco@gmail.com>
 2Date:   Fri Jan 3 10:27:04 2020 -0800
 3
 4    test: Enable leak detection for fuzzers
 5    
 6    This has been disabled in 16f0a186dcee563bb1000e1ffc51da87e7623bc6 for
 7    no given reason.
 8
 9diff --git a/test/fuzz/test_runner.py b/test/fuzz/test_runner.py
10index b638e6bac6..5174e21e2a 100755
11--- a/test/fuzz/test_runner.py
12+++ b/test/fuzz/test_runner.py
13@@ -146,7 +146,6 @@ def run_once(*, corpus, test_list, build_dir, export_coverage, use_valgrind):
14         args = [
15             os.path.join(build_dir, 'src', 'test', 'fuzz', t),
16             '-runs=1',
17-            '-detect_leaks=0',
18             corpus_path,
19         ]
20         if use_valgrind:
    practicalswift commented at 1:17 pm on February 19, 2020:
    -detect_leaks=0 was required at some points to make test/fuzz/test_runner.py pass. If that is not required any more that is great news! I’ll try if Travis is happy without it.
  15. tests: Remove -detect_leaks=0 from test/fuzz/test_runner.py - no longer needed 555236f769
  16. tests: Add support for excluding fuzz targets using -x/--exclude 5ea81449f3
  17. tests: Add --exclude integer,parse_iso8601 (temporarily) to make Travis pass until uninitialized read issue in FormatISO8601DateTime is fixed 733bbec34f
  18. tests: Improve test runner output in case of target errors f2472f6460
  19. MarcoFalke commented at 4:14 pm on February 19, 2020: member

    ACK f2472f64604a0c583f950c56e8753d0bee246388 👼

    Signature:

     0-----BEGIN PGP SIGNED MESSAGE-----
 1Hash: SHA512
 2
 3ACK f2472f64604a0c583f950c56e8753d0bee246388 👼
 4-----BEGIN PGP SIGNATURE-----
 5
 6iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
 7pUiMtgv/eLFsfqh9fC/1+Xam7Wy9imO1pDZigPBciBNC65SfSHx8iOWLFaN/AoNs
 8CcCvOSnjd6yPsja6uajgjiz2q83DjCGBLNKph8D03yJ+WMQt2DcLJUF52EZffSVq
 9XU7w+81kmxtQMmpFQOYFb/re1ScaGJ4U1bUeISsVeHlWhljw6XinSuLFFyJW4OA0
10ahD9Wrs1VF8zypuIPEKwNTzkCjyj6eeUsZLyE7JCvJ3LXhPn/HX7dt18ErekvTFV
117IHr3shTot5aMhmDnugjmLJ/NeMO1ynFBFjFioNNzU4Ztf/PaUtTt3jxe/umfPyK
124kp4ntEOGTzSBW2WDN+qXGNwrBmToIolOnRcVyYTgwLn3kMC3x/952Om2+D8m1WI
139up5/fEd5UEHLv5GVi3BXXFwVI/qZcCvpy3IqxnDMSwi3juhIJ5QonmsIEpqv2yo
14KWriT4nvULRhFBpfmlHaDoWLIQkl5nJTeJZLlEt2/t5/ioT/hkz8Ig7wNjAowRaQ
15K3ltYj3u
16=adP/
17-----END PGP SIGNATURE-----

    Timestamp of file with hash 70c5ad8b8dd4b65a0d243dba35a2ed1c733a95d5036423b9984096ba45d53f36 -

  20. MarcoFalke referenced this in commit eddcbfb109 on Feb 19, 2020
  21. MarcoFalke merged this on Feb 19, 2020
  22. MarcoFalke closed this on Feb 19, 2020
  23. sidhujag referenced this in commit 0d366a0195 on Feb 22, 2020
  24. MarcoFalke referenced this in commit ead6d686eb on Jun 25, 2020
  25. MarcoFalke referenced this in commit 7027c67cac on Jul 2, 2020
  26. jasonbcox referenced this in commit 5ab819115d on Nov 5, 2020
  27. sidhujag referenced this in commit f98402e2f7 on Nov 10, 2020
  28. practicalswift deleted the branch on Apr 10, 2021
  29. random-zebra referenced this in commit 44b5327e61 on May 28, 2021
  30. kittywhiskers referenced this in commit a8493e05a1 on Aug 15, 2021
  31. vijaydasmp referenced this in commit dfa262c93f on Oct 4, 2021
  32. DrahtBot locked this on Aug 18, 2022


practicalswift DrahtBot MarcoFalke

Labels
Tests

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 06:12 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me