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

ci: Remove unused errtrace trap ERR #27667

Merged
merged 1 commit into from
May 16, 2023
Merged

Conversation

maflcko
Copy link
Member

@maflcko maflcko commented May 16, 2023

This was added in commit 069752b, presumably at a time when the functional tests wouldn't capture stderr.

Now that all tests capture and print stderr on failure, it can be removed. Reference:

  • Unit tests capture via 2>&1:

)" -- DEBUG_LOG_OUT > "$$TEST_LOGFILE" 2>&1 || (cat "$$TEST_LOGFILE" && false)

  • Functional tests capture as well:

raise AssertionError("Unexpected stderr {} != {}".format(stderr, expected_stderr))

@DrahtBot
Copy link
Contributor

DrahtBot commented May 16, 2023

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

Reviews

See the guideline for information on the review process.

Type Reviewers
ACK fanquake, hebasto

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

@DrahtBot DrahtBot added the Tests label May 16, 2023
Copy link
Member

@fanquake fanquake left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK fad09b7

@maflcko
Copy link
Member Author

maflcko commented May 16, 2023

Can be tested with a diff injecting a fault:

diff --git a/src/init.cpp b/src/init.cpp
index 52c5780..95839c0 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -891,6 +891,9 @@ bool AppInitParameterInteraction(const ArgsManager& args, bool use_syscall_sandb
 
     // Signal NODE_COMPACT_FILTERS if peerblockfilters and basic filters index are both enabled.
     if (args.GetBoolArg("-peerblockfilters", DEFAULT_PEERBLOCKFILTERS)) {
+           int a{-1};
+           unsigned b = a;
+           (void)b;
         if (g_enabled_filter_types.count(BlockFilterType::BASIC) != 1) {
             return InitError(_("Cannot set -peerblockfilters without -blockfilterindex."));
         }
diff --git a/src/test/util_threadnames_tests.cpp b/src/test/util_threadnames_tests.cpp
index ae91393..9b7ad55 100644
--- a/src/test/util_threadnames_tests.cpp
+++ b/src/test/util_threadnames_tests.cpp
@@ -68,6 +68,9 @@ BOOST_AUTO_TEST_CASE(util_threadnames_test_rename_threaded)
         BOOST_CHECK(names.find(TEST_THREAD_NAME_BASE + ToString(i)) != names.end());
     }
 
+    int a{-1};
+    unsigned b = a;
+    (void)b;
 }
 
 BOOST_AUTO_TEST_SUITE_END()

To test for the unit tests:

podman kill ci_native_asan || true && MAKEJOBS="-j$(nproc)" FILE_ENV="./ci/test/00_setup_env_native_asan.sh" ./ci/test_run_all.sh

With result:

test/prevector_tests.cpp(17): Test suite "prevector_tests" is skipped because disabled
test/util_threadnames_tests.cpp(20): Entering test suite "util_threadnames_tests"
test/util_threadnames_tests.cpp(55): Entering test case "util_threadnames_test_rename_threaded"
test/util_threadnames_tests.cpp:72:18: runtime error: implicit conversion from type 'int' of value -1 (32-bit, signed) to type 'unsigned int' changed the value to 4294967295 (32-bit, unsigned)
    #0 0xaaaadddc5720 in util_threadnames_tests::util_threadnames_test_rename_threaded::test_method() src/test/util_threadnames_tests.cpp:72:18
    #1 0xaaaadddc38b8 in util_threadnames_tests::util_threadnames_test_rename_threaded_invoker() src/test/util_threadnames_tests.cpp:55:1
    #2 0xaaaadcbe5304 in boost::function0<void>::operator()() const /usr/include/boost/function/function_template.hpp:763:14
    #3 0xaaaadcc62fd0 in boost::detail::forward::operator()() /usr/include/boost/test/impl/execution_monitor.ipp:1388:32
    #4 0xaaaadcc62fd0 in boost::detail::function::function_obj_invoker0<boost::detail::forward, int>::invoke(boost::detail::function::function_buffer&) /usr/include/boost/function/function_template.hpp:137:18
    #5 0xaaaadcc5c438 in boost::function0<int>::operator()() const /usr/include/boost/function/function_template.hpp:763:14
    #6 0xaaaadcb32318 in int boost::detail::do_invoke<boost::shared_ptr<boost::detail::translator_holder_base>, boost::function<int ()>>(boost::shared_ptr<boost::detail::translator_holder_base> const&, boost::function<int ()> const&) /usr/include/boost/test/impl/execution_monitor.ipp:301:30
    #7 0xaaaadcb32318 in boost::execution_monitor::catch_signals(boost::function<int ()> const&) /usr/include/boost/test/impl/execution_monitor.ipp:903:16
    #8 0xaaaadcb32748 in boost::execution_monitor::execute(boost::function<int ()> const&) /usr/include/boost/test/impl/execution_monitor.ipp:1301:16
    #9 0xaaaadcb24758 in boost::execution_monitor::vexecute(boost::function<void ()> const&) /usr/include/boost/test/impl/execution_monitor.ipp:1397:5
    #10 0xaaaadcb2b444 in boost::unit_test::unit_test_monitor_t::execute_and_translate(boost::function<void ()> const&, unsigned long) /usr/include/boost/test/impl/unit_test_monitor.ipp:49:9
    #11 0xaaaadcba4eac in boost::unit_test::framework::state::execute_test_tree(unsigned long, unsigned long, boost::unit_test::framework::state::random_generator_helper const*) /usr/include/boost/test/impl/framework.ipp:815:44
    #12 0xaaaadcba3e90 in boost::unit_test::framework::state::execute_test_tree(unsigned long, unsigned long, boost::unit_test::framework::state::random_generator_helper const*) /usr/include/boost/test/impl/framework.ipp:784:58
    #13 0xaaaadcba3e90 in boost::unit_test::framework::state::execute_test_tree(unsigned long, unsigned long, boost::unit_test::framework::state::random_generator_helper const*) /usr/include/boost/test/impl/framework.ipp:784:58
    #14 0xaaaadcb297fc in boost::unit_test::framework::run(unsigned long, bool) /usr/include/boost/test/impl/framework.ipp:1721:29
    #15 0xaaaadcb5a9a8 in boost::unit_test::unit_test_main(boost::unit_test::test_suite* (*)(int, char**), int, char**) /usr/include/boost/test/impl/unit_test_main.ipp:250:9
    #16 0xffff9fb66dbc  (/lib/aarch64-linux-gnu/libc.so.6+0x26dbc) (BuildId: 9efe43e32e614cad22a180ef036eea6154eafdc2)
    #17 0xffff9fb66e94 in __libc_start_main (/lib/aarch64-linux-gnu/libc.so.6+0x26e94) (BuildId: 9efe43e32e614cad22a180ef036eea6154eafdc2)
    #18 0xaaaadca2bf6c in _start (/root/b-c-ci/ci/scratch/build/bitcoin-aarch64-unknown-linux-gnu/src/test/test_bitcoin+0x107bf6c) (BuildId: e6232accf8d12124a11d947626dab631bc8c9c37)

SUMMARY: UndefinedBehaviorSanitizer: implicit-integer-sign-change test/util_threadnames_tests.cpp:72:18 in 
make[3]: *** [Makefile:22120: test/util_threadnames_tests.cpp.test] Error 1
make[3]: *** Waiting for unfinished jobs....
make[3]: Leaving directory '/root/b-c-ci/ci/scratch/build/bitcoin-aarch64-unknown-linux-gnu/src'
make[2]: *** [Makefile:20096: check-am] Error 2
make[2]: Leaving directory '/root/b-c-ci/ci/scratch/build/bitcoin-aarch64-unknown-linux-gnu/src'
make[1]: *** [Makefile:19761: check-recursive] Error 1
make[1]: Leaving directory '/root/b-c-ci/ci/scratch/build/bitcoin-aarch64-unknown-linux-gnu/src'
make: *** [Makefile:816: check-recursive] Error 1

To test for the functional tests (skip unit tests):

podman kill ci_native_asan || true && RUN_UNIT_TESTS=false TEST_RUNNER_EXTRA="p2p_blockfilters" MAKEJOBS="-j$(nproc)" FILE_ENV="./ci/test/00_setup_env_native_asan.sh" ./ci/test_run_all.sh

With result:

Ran 11 tests in 2.543s

OK
Running Unit Tests for Test Framework Modules
Remaining jobs: [p2p_blockfilters.py]
1/1 - p2p_blockfilters.py failed, Duration: 2 s

stdout:
2023-05-16T09:23:21.794000Z TestFramework (INFO): PRNG seed is: 5743461567751360021
2023-05-16T09:23:21.795000Z TestFramework (INFO): Initializing test directory /root/b-c-ci/ci/scratch/test_runner/test_runner_₿_🏃_20230516_092319/p2p_blockfilters_0
2023-05-16T09:23:23.801000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
  File "/root/b-c-ci/ci/scratch/build/bitcoin-aarch64-unknown-linux-gnu/test/functional/test_framework/test_framework.py", line 560, in start_nodes
    node.wait_for_rpc_connection()
  File "/root/b-c-ci/ci/scratch/build/bitcoin-aarch64-unknown-linux-gnu/test/functional/test_framework/test_node.py", line 231, in wait_for_rpc_connection
    raise FailedToStartError(self._node_msg(
test_framework.test_node.FailedToStartError: [node 0] bitcoind exited with status 1 during initialization

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/root/b-c-ci/ci/scratch/build/bitcoin-aarch64-unknown-linux-gnu/test/functional/test_framework/test_framework.py", line 131, in main
    self.setup()
  File "/root/b-c-ci/ci/scratch/build/bitcoin-aarch64-unknown-linux-gnu/test/functional/test_framework/test_framework.py", line 298, in setup
    self.setup_network()
  File "/root/b-c-ci/ci/scratch/build/bitcoin-aarch64-unknown-linux-gnu/test/functional/test_framework/test_framework.py", line 392, in setup_network
    self.setup_nodes()
  File "/root/b-c-ci/ci/scratch/build/bitcoin-aarch64-unknown-linux-gnu/test/functional/test_framework/test_framework.py", line 414, in setup_nodes
    self.start_nodes()
  File "/root/b-c-ci/ci/scratch/build/bitcoin-aarch64-unknown-linux-gnu/test/functional/test_framework/test_framework.py", line 563, in start_nodes
    self.stop_nodes()
  File "/root/b-c-ci/ci/scratch/build/bitcoin-aarch64-unknown-linux-gnu/test/functional/test_framework/test_framework.py", line 578, in stop_nodes
    node.stop_node(wait=wait, wait_until_stopped=False)
  File "/root/b-c-ci/ci/scratch/build/bitcoin-aarch64-unknown-linux-gnu/test/functional/test_framework/test_node.py", line 342, in stop_node
    self.stop(wait=wait)
    ^^^^^^^^^
  File "/root/b-c-ci/ci/scratch/build/bitcoin-aarch64-unknown-linux-gnu/test/functional/test_framework/test_node.py", line 190, in __getattr__
    assert self.rpc_connected and self.rpc is not None, self._node_msg("Error: no RPC connection")
                                  ^^^^^^^^^^^^^^^^^^^^
AssertionError: [node 0] Error: no RPC connection
2023-05-16T09:23:23.854000Z TestFramework (INFO): Stopping nodes
[node 1] Cleaning up leftover process
[node 0] Cleaning up leftover process


stderr:
Traceback (most recent call last):
  File "/root/b-c-ci/ci/scratch/build/bitcoin-aarch64-unknown-linux-gnu/test/functional/p2p_blockfilters.py", line 274, in <module>
    CompactFiltersTest().main()
  File "/root/b-c-ci/ci/scratch/build/bitcoin-aarch64-unknown-linux-gnu/test/functional/test_framework/test_framework.py", line 155, in main
    exit_code = self.shutdown()
                ^^^^^^^^^^^^^^^
  File "/root/b-c-ci/ci/scratch/build/bitcoin-aarch64-unknown-linux-gnu/test/functional/test_framework/test_framework.py", line 314, in shutdown
    self.stop_nodes()
  File "/root/b-c-ci/ci/scratch/build/bitcoin-aarch64-unknown-linux-gnu/test/functional/test_framework/test_framework.py", line 578, in stop_nodes
    node.stop_node(wait=wait, wait_until_stopped=False)
  File "/root/b-c-ci/ci/scratch/build/bitcoin-aarch64-unknown-linux-gnu/test/functional/test_framework/test_node.py", line 342, in stop_node
    self.stop(wait=wait)
    ^^^^^^^^^
  File "/root/b-c-ci/ci/scratch/build/bitcoin-aarch64-unknown-linux-gnu/test/functional/test_framework/test_node.py", line 190, in __getattr__
    assert self.rpc_connected and self.rpc is not None, self._node_msg("Error: no RPC connection")
                                  ^^^^^^^^^^^^^^^^^^^^
AssertionError: [node 0] Error: no RPC connection


Combine the logs and print the last 99999999 lines ...

============
Combined log for /root/b-c-ci/ci/scratch/test_runner//test_runner_₿_🏃_20230516_092319/p2p_blockfilters_0:
============

 test  2023-05-16T09:23:21.794000Z TestFramework (INFO): PRNG seed is: 5743461567751360021
 test  2023-05-16T09:23:21.794000Z TestFramework (DEBUG): Setting up network thread
 test  2023-05-16T09:23:21.795000Z TestFramework (INFO): Initializing test directory /root/b-c-ci/ci/scratch/test_runner/test_runner_₿_🏃_20230516_092319/p2p_blockfilters_0
 test  2023-05-16T09:23:21.797000Z TestFramework.node0 (DEBUG): bitcoind started, waiting for RPC to come up
 test  2023-05-16T09:23:21.797000Z TestFramework.node1 (DEBUG): bitcoind started, waiting for RPC to come up
 test  2023-05-16T09:23:23.801000Z TestFramework.node0 (DEBUG): Stopping node
 test  2023-05-16T09:23:23.801000Z TestFramework (ERROR): Assertion failed
                                   Traceback (most recent call last):
                                     File "/root/b-c-ci/ci/scratch/build/bitcoin-aarch64-unknown-linux-gnu/test/functional/test_framework/test_framework.py", line 560, in start_nodes
                                       node.wait_for_rpc_connection()
                                     File "/root/b-c-ci/ci/scratch/build/bitcoin-aarch64-unknown-linux-gnu/test/functional/test_framework/test_node.py", line 231, in wait_for_rpc_connection
                                       raise FailedToStartError(self._node_msg(
                                   test_framework.test_node.FailedToStartError: [node 0] bitcoind exited with status 1 during initialization
                                   During handling of the above exception, another exception occurred:
                                   Traceback (most recent call last):
                                     File "/root/b-c-ci/ci/scratch/build/bitcoin-aarch64-unknown-linux-gnu/test/functional/test_framework/test_framework.py", line 131, in main
                                       self.setup()
                                     File "/root/b-c-ci/ci/scratch/build/bitcoin-aarch64-unknown-linux-gnu/test/functional/test_framework/test_framework.py", line 298, in setup
                                       self.setup_network()
                                     File "/root/b-c-ci/ci/scratch/build/bitcoin-aarch64-unknown-linux-gnu/test/functional/test_framework/test_framework.py", line 392, in setup_network
                                       self.setup_nodes()
                                     File "/root/b-c-ci/ci/scratch/build/bitcoin-aarch64-unknown-linux-gnu/test/functional/test_framework/test_framework.py", line 414, in setup_nodes
                                       self.start_nodes()
                                     File "/root/b-c-ci/ci/scratch/build/bitcoin-aarch64-unknown-linux-gnu/test/functional/test_framework/test_framework.py", line 563, in start_nodes
                                       self.stop_nodes()
                                     File "/root/b-c-ci/ci/scratch/build/bitcoin-aarch64-unknown-linux-gnu/test/functional/test_framework/test_framework.py", line 578, in stop_nodes
                                       node.stop_node(wait=wait, wait_until_stopped=False)
                                     File "/root/b-c-ci/ci/scratch/build/bitcoin-aarch64-unknown-linux-gnu/test/functional/test_framework/test_node.py", line 342, in stop_node
                                       self.stop(wait=wait)
                                       ^^^^^^^^^
                                     File "/root/b-c-ci/ci/scratch/build/bitcoin-aarch64-unknown-linux-gnu/test/functional/test_framework/test_node.py", line 190, in __getattr__
                                       assert self.rpc_connected and self.rpc is not None, self._node_msg("Error: no RPC connection")
                                                                     ^^^^^^^^^^^^^^^^^^^^
                                   AssertionError: [node 0] Error: no RPC connection
 test  2023-05-16T09:23:23.804000Z TestFramework (DEBUG): Closing down network thread
 test  2023-05-16T09:23:23.854000Z TestFramework (INFO): Stopping nodes
 test  2023-05-16T09:23:23.855000Z TestFramework.node0 (DEBUG): Stopping node

 node0 stderr init.cpp:895:25: runtime error: implicit conversion from type 'int' of value -1 (32-bit, signed) to type 'unsigned int' changed the value to 4294967295 (32-bit, unsigned)
    #0 0xaaaac678ca58 in AppInitParameterInteraction(ArgsManager const&, bool) src/init.cpp:895:25
    #1 0xaaaac6758c44 in AppInit(node::NodeContext&, int, char**) src/bitcoind.cpp:176:14
    #2 0xaaaac6758c44 in main src/bitcoind.cpp:267:13
    #3 0xffff96676dbc  (/lib/aarch64-linux-gnu/libc.so.6+0x26dbc) (BuildId: 9efe43e32e614cad22a180ef036eea6154eafdc2)
    #4 0xffff96676e94 in __libc_start_main (/lib/aarch64-linux-gnu/libc.so.6+0x26e94) (BuildId: 9efe43e32e614cad22a180ef036eea6154eafdc2)
    #5 0xaaaac667f06c in _start (/root/b-c-ci/ci/scratch/build/bitcoin-aarch64-unknown-linux-gnu/src/bitcoind+0x98f06c) (BuildId: 5e23ce6e2e3afe05858f3446a0371be08b6d268a)

SUMMARY: UndefinedBehaviorSanitizer: implicit-integer-sign-change init.cpp:895:25 in

TEST                | STATUS    | DURATION

p2p_blockfilters.py | ✖ Failed  | 2 s

ALL                 | ✖ Failed  | 2 s (accumulated) 
Runtime: 2 s

Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK fad09b7, tested on Ubuntu 22.04: I can still see warnings from the sanitizers in both unit and functional tests.

@fanquake fanquake merged commit 904631e into bitcoin:master May 16, 2023
@maflcko maflcko deleted the 2305-ci-trap-err- branch May 16, 2023 15:07
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request May 17, 2023
fad09b7 ci: Remove unused errtrace trap ERR (MarcoFalke)

Pull request description:

  This was added in commit 069752b, presumably at a time when the functional tests wouldn't capture stderr.

  Now that all tests capture and print stderr on failure, it can be removed. Reference:

  * Unit tests capture via `2>&1`:

  https://github.com/bitcoin/bitcoin/blob/d7700d3a26478d9b1648463c188648c7047b1c60/src/Makefile.test.include#L421

  * Functional tests capture as well:

  https://github.com/bitcoin/bitcoin/blob/d7700d3a26478d9b1648463c188648c7047b1c60/test/functional/test_framework/test_node.py#L356

ACKs for top commit:
  fanquake:
    ACK fad09b7
  hebasto:
    ACK fad09b7, tested on Ubuntu 22.04: I can still see warnings from the sanitizers in both unit and functional tests.

Tree-SHA512: 1e786eee432a7a50eb9f78b06b2b157321cc16f91b613e3b476e9e51572592fe4bcf4dc15df176e5f019f24497ac68cf332d2037b55b57498c93f4e19613163c
fanquake added a commit that referenced this pull request Mar 27, 2024
fa22a43 ci: Print tsan errors to stderr (MarcoFalke)

Pull request description:

  This fixes a bug introduced in #27667

  All sanitizers print their errors to stderr, except for tsan, which prints to a file and expects the file to be read.

  Fix this by not using a log file in any sanitizer.

ACKs for top commit:
  dergoegge:
    utACK fa22a43

Tree-SHA512: 15dca57932a21bda145335fab6367bbf2ae67b25e0b7b61044d2c06ab7a8db3a452f057f6656b81a031726375b7bb238f5ced18ab8894f005e7ab254c7d1ef06
@bitcoin bitcoin locked and limited conversation to collaborators May 15, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants