qa debug: Add --debug_runs/-waitfordebugger [DRAFT]

hodlinator commented at 1:18 pm on January 23, 2025: contributor

Facilitates debugging specific instances of 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

Generate build files with -DWAIT_FOR_DEBUGGER=ON & build.
Set breakpoints in editor/IDE.
Start Python test:

0₿ build/test/functional/feature_config_args.py --debug_runs 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 (run [#0](/bitcoin-bitcoin/0/), node [#0](/bitcoin-bitcoin/0/)), waiting for debugger, PID: 1382744

Navigate in editor/IDE and attach to process 1382744, which will now continue executing under the context of the test.
Experience how debugger hits breakpoints.

Reasons for draft status

Untested on Mac.

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:

#31866 (test, refactor: Add TestNode.binaries to hold binary paths by ryanofsky)
#31375 (multiprocess: Add bitcoin wrapper executable by ryanofsky)
#31260 (scripted-diff: Type-safe settings retrieval by ryanofsky)
#31250 (wallet: Disable creating and loading legacy wallets 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.

hodlinator marked this as a draft on Jan 23, 2025

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.

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?

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.

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.

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.

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?

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).

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.

hodlinator force-pushed on Jan 27, 2025

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.

DrahtBot added the label Needs rebase on Feb 12, 2025

qa debug: Add --debug_runs/-waitfordebugger

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

Useful for debugging one or several bitcoind nodes running in the context of a functional test.

TODO: Confirm code works on Mac.

a1da3bfc12

hodlinator force-pushed on Feb 12, 2025

DrahtBot removed the label Needs rebase on Feb 12, 2025

hodlinator commented at 8:05 am on February 13, 2025: contributor

Added CMake option WAIT_FOR_DEBUGGER.
Changed the Python logic from debugging specific nodes, to debugging specific node runs/executions, which is more specific.
Use prctl() on Linux to allow any process to attach to bitcoind. This removes the necessity to disable kernel.yama.ptrace_scope on some systems, which may be seen as decreasing security, but is compiled out unless WAIT_FOR_DEBUGGER=ON.
(Confirmed PR works on Windows).
(Rebased).

hodlinator renamed this:
~~qa debug: Add --debugnode/-waitfordebugger [DRAFT]~~
qa debug: Add --debug_runs/-waitfordebugger [DRAFT]
on Feb 13, 2025

maflcko commented at 11:45 am on February 13, 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.

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.

I’d still say it is easier to debug the init sequence separately than to start a new build with a new build flag for the edge case where someone wants to debug both in an IDE and also the init sequence , but no strong opinion.

hodlinator commented at 3:49 pm on February 13, 2025: contributor

fanquake:

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.

The only debugging console I get with the IDE states “No Active Debug Session” when I try to input commands. :(

maflcko:

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.

I’d still say it is easier to debug the init sequence separately than to start a new build with a new build flag for the edge case where someone wants to debug both in an IDE and also the init sequence , but no strong opinion.

Maybe part of this is down to me not getting used to using a Python debugger, breaking in Python at appropriate times and then attaching a C++ debugger to the new process. Being able to specify which bitcoind-executions one wants to debug still seems like a smoother workflow when testing the same scenario repeatedly.

I recently used a version of this PR when digging into #31734. Being able to step through the code (with a panel for inspecting local variables and watches), as the Python test framework is issuing RPCs, in the safety of one’s IDE, is nice. Knowing that bitcoind has not passed through one’s breakpoints (unless there’s some rare static initialization going on) helps build certainty about what unfamiliar code is involved in.

Granted, using --waitfor straight in gdb/lldb would give definite certainty what code runs. But the IDE GUI would not be there. I know gdb has some kind of TUI, and there are probably commands for saving and restoring state. It just feels like operating the computer with my feet. Happy to be proven wrong though. Please shill your setups. :)

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

Example usage

Reasons for draft status

Code Coverage & Benchmarks

Reviews

Conflicts