qa debug: Add –debugnode/-waitfordebugger [DRAFT] #31723

pull hodlinator wants to merge 1 commits into bitcoin:master from hodlinator:2024/06/wait_for_debugger changing 5 files +84 −2
  1. hodlinator commented at 1:18 pm on January 23, 2025: contributor

    Facilitates debugging bitcoind in the context of Python tests, inside of an editor/IDE.

    Makes bitcoind spin during startup, waiting for a debugger to be attached.

    Example usage

    1. Set breakpoints in editor/IDE.
    2. Start Python test:
    0₿ build/test/functional/feature_config_args.py --debugnode 0
    12025-01-23T13:51:09.143000Z TestFramework (INFO): PRNG seed is: 1495347861660308752
    22025-01-23T13:51:09.143000Z TestFramework (INFO): Initializing test directory /tmp/bitcoin_func_test_hx8vd5bv
    32025-01-23T13:51:09.144000Z TestFramework.node0 (INFO): bitcoind started, waiting for debugger, PID: 1382744
    
    1. Navigate in editor/IDE and attach to process 1382744, which will now continue executing under the context of the test.
    2. Experience how debugger hits breakpoints.

    Reasons for draft status

    • Untested on Mac & Windows.
    • Find a better way to enable WAIT_FOR_DEBUGGER in CMake that doesn’t leak into the interface since it’s not needed?
  2. DrahtBot commented at 1:18 pm on January 23, 2025: contributor

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

    Code Coverage & Benchmarks

    For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/31723.

    Reviews

    See the guideline for information on the review process. A summary of reviews will appear here.

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #31711 (build: set build type and per-build-type flags as early as possible by theuni)
    • #31375 (multiprocess: Add bitcoin wrapper executable by ryanofsky)
    • #31250 (wallet: Disable creating and loading legacy wallets by achow101)
    • #28710 (Remove the legacy wallet and BDB dependency by achow101)

    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.

  3. hodlinator marked this as a draft on Jan 23, 2025
  4. in src/bitcoind.cpp:267 in 7d181da293 outdated
    262@@ -254,6 +263,51 @@ static bool AppInit(NodeContext& node)
    263 
    264 MAIN_FUNCTION
    265 {
    266+    for (int i = 0; i < argc; ++i) {
    267+        if (strcmp(argv[i], "-waitfordebugger") == 0) {
    


    maflcko commented at 1:46 pm on January 23, 2025:
    Wouldn’t it be easier to just start with a debugger instead of first starting without one and then attaching one?

    hodlinator commented at 1:58 pm on January 23, 2025:

    Updated PR description to clarify intended usage.

    One could possibly launch bitcoind as a parameter to lldb/gdb, but I don’t know if there is a way of doing that if one also wants to have the debugging view with stepping etc inside of the editor/IDE.

  5. fanquake commented at 1:47 pm on January 23, 2025: member

    Makes bitcoind spin during startup, waiting for a debugger to be attached.

    Can you explain more? If you want to run under a debugger, then why not just start bitcoind under a debugger; why do we need to add this option?

  6. hodlinator commented at 1:55 pm on January 23, 2025: contributor

    Can you explain more? If you want to run under a debugger, then why not just start bitcoind under a debugger; why do we need to add this option?

    Clarified PR description text and added example usage. But I’m curious to hear other ways of doing the same thing if there are any.

  7. maflcko commented at 2:00 pm on January 23, 2025: member
    I think if you want to debug the start-up sequence specifically, it may be easier to do it outside the tests? Then anything else, if needed, can happen without patching Bitcoin Core source code itself.
  8. hodlinator commented at 2:19 pm on January 23, 2025: contributor

    I think if you want to debug the start-up sequence specifically, it may be easier to do it outside the tests? Then anything else, if needed, can happen without patching Bitcoin Core source code itself.

    This is not about debugging the start-up sequence specifically, but rather debugging from an early stage. Maybe one has set 5 breakpoints, some of which are hit during bitcoind init, and some which are hit by re-running a Python test which calls a set of RPCs.

  9. fanquake commented at 2:24 pm on January 23, 2025: member

    Ideally we wouldn’t be adding debug code, to production binaries, to facilitate using an IDE.

    Can you configure your IDE to do something like the equivalent of using gdb/lldb, and process attach --name bitcoind --waitfor? So it just grabs bitcoind once it spins up?

  10. hodlinator commented at 2:37 pm on January 23, 2025: contributor

    Ideally we wouldn’t be adding debug code, to production binaries, to facilitate using an IDE.

    Agree it should not be in production binaries, should be #ifdefed away. Added as another reason for draft status.

    Can you configure your IDE to do something like the equivalent of using gdb/lldb, and process attach --name bitcoind --waitfor? So it just grabs bitcoind once it spins up?

    I’ve never seen something like that in an IDE/editor, but I’ll have a look, might exist a debugger console which allows that kind of thing.

    This PR does however allow for attaching to a specific node instance of bitcoind if a Python tests runs several nodes at once. (I’m considering changing the index to refer to how many bitcoind-launches one has done, some tests just relaunch node 0 a bunch of times, and one might be interested in debugging it on the 3rd execution).

  11. i-am-yuvi commented at 1:46 pm on January 27, 2025: contributor
    Agree with @maflcko @fanquake, not sure if we want to add that to the production code.
  12. qa debug: Add --debugnode/-waitfordebugger
    Makes bitcoind spin during startup, waiting for a debugger to be attached.
    
    TODO: Confirm code works on Mac & Windows.
    7dd86091aa
  13. hodlinator force-pushed on Jan 27, 2025
  14. hodlinator commented at 7:15 pm on January 27, 2025: contributor
    Added tentative #ifdef solution in latest push. CMake part could probably be improved since now I think the #define leaks into the CMake project’s public interface, which should not be necessary.

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: 2025-02-07 18:12 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me