[RFC] Align debugging flags to -O0 #29796

pull fanquake wants to merge 1 commits into bitcoin:master from fanquake:depends_0g_debug_flags changing 6 files +6 −6
  1. fanquake commented at 11:12 am on April 3, 2024: member

    We currently have two issues in relation to debugging optimisation flags:

    • A depends build with DEBUG=1 and using cmake -B build -DCMAKE_BUILD_TYPE=Debug do not align, which is inconsistent/confusing. A depends build will -O1, while CMAKE_BUILD_TYPE=Debug will set -O0.
    • -O0 is unusable in various ways:
      • Compiling certain assembly under -O0 doesn’t currently work: #29407 (comment) (probably an upstream issue?).
      • Windows cross-compilation at -O0 is broken without additional flags: #28109.

    I’m not completely sure yet what the right choice is (previously it seems that -Og was unusable with Clang? (note that for Clang it looks like -Og and -O1 are basically equivalent), but we should at least align the two systems to be using the same thing for debugging.

    Configure history: -O0 added in #3833 Switched to -Og in #13005. Switched back to -O0 #16435

    Depends DEBUG=1 mode has used -O1 since inception.

  2. DrahtBot commented at 11:12 am on April 3, 2024: 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/29796.

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    Concept NACK achow101
    Concept ACK emc99, theuni
    Stale ACK ryanofsky, TheCharlatan, hebasto

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

    Conflicts

    No conflicts as of last run.

  3. hebasto commented at 11:24 am on April 3, 2024: member
    • A depends build with DEBUG=1 and using ./configure --enable-debug do not align, which is inconsistent/confusing.

    Concept ACK on fixing that.

  4. DrahtBot added the label CI failed on Apr 3, 2024
  5. emc99 commented at 3:34 pm on April 3, 2024: none
    Concept ACK
  6. theuni commented at 5:09 pm on April 3, 2024: member

    Concept ACK.

    I think msan is a good proxy for what we want enabled. From its docs:

    To get a reasonable performance add -O1 or higher. To get meaningful stack traces in error messages add -fno-omit-frame-pointer. To get perfect stack traces you may need to disable inlining (just use -O1) and tail call elimination (-fno-optimize-sibling-calls).

    From gcc’s docs on -Og

    -Og should be the optimization level of choice for the standard edit-compile-debug cycle, offering a reasonable level of optimization while maintaining fast compilation and a good debugging experience. It is a better choice than -O0 for producing debuggable code because some compiler passes that collect debug information are disabled at -O0. Like -O0, -Og completely disables a number of optimization passes so that individual options controlling them have no effect. Otherwise -Og enables all -O1 optimization flags except for those that may interfere with debugging

    From what I can tell, -O1 (and thus -Og) will disable the frame pointer. So we may also want to add -fno-omit-frame-pointer as recommended by llvm, but IMO we can wait and see if it turns out to be necessary for anyone first.

  7. achow101 commented at 6:33 pm on April 3, 2024: member

    While this does resolve the compile issue I was having, it does seem to change how gdb is able to debug things, possibly in a meaningful way? This is just an example of a difference that I noticed while stepping through CWallet::Create with and without this PR:

    On master:

     0Thread 4 "b-httpworker.0" hit Breakpoint 1, wallet::CWallet::Create (context=..., name="funds", database=std::unique_ptr<wallet::WalletDatabase> = {...}, 
 1    wallet_creation_flags=0, error=..., warnings=std::vector of length 0, capacity 0) at wallet/wallet.cpp:2948
 22948	{
 3(gdb) n
 42949	   interfaces::Chain* chain = context.chain;
 5(gdb) 
 62950	   ArgsManager& args = *Assert(context.args);
 7(gdb) 
 82024-04-03T18:24:24.238066Z [addcon] [logging/timer.h:58] [Log] [lock] Enter: lock contention m_nodes_mutex, net.cpp:2768 started
 92024-04-03T18:24:24.238169Z [addcon] [logging/timer.h:58] [Log] [lock] Enter: lock contention m_nodes_mutex, net.cpp:2768 completed (1μs)
102024-04-03T18:24:24.238246Z [net] [logging/timer.h:58] [Log] [lock] Enter: lock contention m_nodes_mutex, net.cpp:1860 started
112024-04-03T18:24:24.238343Z [net] [logging/timer.h:58] [Log] [lock] Enter: lock contention m_nodes_mutex, net.cpp:1860 completed (3μs)
122951	   const std::string& walletFile = database->Filename();
13(gdb) 
142953	   const auto start{SteadyClock::now()};
15(gdb) 
162024-04-03T18:24:25.288339Z [net] [logging/timer.h:58] [Log] [lock] Enter: lock contention m_nodes_mutex, net.cpp:1860 started
172024-04-03T18:24:25.288437Z [net] [logging/timer.h:58] [Log] [lock] Enter: lock contention m_nodes_mutex, net.cpp:1860 completed (3μs)
182956	   std::shared_ptr<CWallet> walletInstance(new CWallet(chain, name, std::move(database)), ReleaseWallet);
19(gdb) 
202024-04-03T18:24:25.789076Z [net] [logging/timer.h:58] [Log] [lock] Enter: lock contention m_nodes_mutex, net.cpp:1932 started
212024-04-03T18:24:25.789145Z [net] [logging/timer.h:58] [Log] [lock] Enter: lock contention m_nodes_mutex, net.cpp:1932 completed (2μs)
222957	   walletInstance->m_keypool_size = std::max(args.GetIntArg("-keypool", DEFAULT_KEYPOOL_SIZE), int64_t{1});
23(gdb) 
242958	   walletInstance->m_notify_tx_changed_script = args.GetArg("-walletnotify", "");
25(gdb) 
262961	   bool rescan_required = false;
27(gdb) 
282962	   DBErrors nLoadWalletRet = walletInstance->LoadWallet();
29(gdb) 
302024-04-03T18:24:27.971293Z [net] [logging/timer.h:58] [Log] [lock] Enter: lock contention connman.m_nodes_mutex, ./net.h:1622 started
312024-04-03T18:24:27.971368Z [net] [logging/timer.h:58] [Log] [lock] Enter: lock contention connman.m_nodes_mutex, ./net.h:1622 completed (2μs)
322024-04-03T18:24:27.972260Z [httpworker.0] [wallet/wallet.h:933] [WalletLogPrintf] [funds] Wallet file version = 10500, last client version = 279900
332024-04-03T18:24:28.845346Z [httpworker.0] [wallet/wallet.h:933] [WalletLogPrintf] [funds] Descriptors: 8, Descriptor Keys: 1 plaintext, 0 encrypted, 1 total.
342024-04-03T18:24:29.556576Z [httpworker.0] [wallet/wallet.h:933] [WalletLogPrintf] [funds] Setting spkMan to active: id = 7bb07d3df110f36af92db5568fb8d4cc1381dd0a6d24b17182ed87cad6ab1d33, type = legacy, internal = false
352024-04-03T18:24:29.556711Z [httpworker.0] [wallet/wallet.h:933] [WalletLogPrintf] [funds] Setting spkMan to active: id = 5cb8f1604559d7b3612008ed3307ca1e21328301d1117b3acdd8b22d5b277219, type = p2sh-segwit, internal = false
362024-04-03T18:24:29.556751Z [httpworker.0] [wallet/wallet.h:933] [WalletLogPrintf] [funds] Setting spkMan to active: id = 68a4834fa805e997434b5c4f2f31625be6f3c5aebae0757293c4215675614a1b, type = bech32, internal = false
372024-04-03T18:24:29.556788Z [httpworker.0] [wallet/wallet.h:933] [WalletLogPrintf] [funds] Setting spkMan to active: id = 19d3e9f0d167b24e3c5330462a0c6cbc3d6e4e06e469967a395835102c0aea4a, type = bech32m, internal = false
382024-04-03T18:24:29.556851Z [httpworker.0] [wallet/wallet.h:933] [WalletLogPrintf] [funds] Setting spkMan to active: id = 09212e585c214fb06d197863920ed50181b984dc6bace73a865ff06a84ec3e61, type = legacy, internal = true
392024-04-03T18:24:29.556892Z [httpworker.0] [wallet/wallet.h:933] [WalletLogPrintf] [funds] Setting spkMan to active: id = 05db3056830116eafc425f192ee58033d44772bbf3bdd62ab50e8d014ecab772, type = p2sh-segwit, internal = true
402024-04-03T18:24:29.556927Z [httpworker.0] [wallet/wallet.h:933] [WalletLogPrintf] [funds] Setting spkMan to active: id = 2fe605d1acd8e46f55fbb47c8162545449df0ab59839219a7541b770a948fa3c, type = bech32, internal = true
412024-04-03T18:24:29.556961Z [httpworker.0] [wallet/wallet.h:933] [WalletLogPrintf] [funds] Setting spkMan to active: id = 115308b94096b7f96245e3ed280290ac5c627c5d666abf5249e4a16bcd5ac79e, type = bech32m, internal = true
422963	   if (nLoadWalletRet != DBErrors::LOAD_OK) {
43(gdb) 
443006	   const bool fFirstRun = walletInstance->m_spk_managers.empty() &&
45(gdb) 
462024-04-03T18:24:31.018609Z [msghand] [logging/timer.h:58] [Log] [lock] Enter: lock contention connman.m_nodes_mutex, ./net.h:1622 started
472024-04-03T18:24:31.018740Z [msghand] [logging/timer.h:58] [Log] [lock] Enter: lock contention connman.m_nodes_mutex, ./net.h:1622 completed (5μs)
483007	                    !walletInstance->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS) &&

    This PR:

     0Thread 4 "b-httpworker.0" hit Breakpoint 1, wallet::CWallet::Create (context=..., name="funds", database=std::unique_ptr<wallet::WalletDatabase> = {...}, 
 1    wallet_creation_flags=0, error=..., warnings=std::vector of length 0, capacity 0) at wallet/wallet.cpp:2948
 22948	{
 3(gdb) n
 42949	   interfaces::Chain* chain = context.chain;
 5(gdb) 
 62024-04-03T18:21:31.566625Z [net] [logging/timer.h:58] [Log] [lock] Enter: lock contention m_nodes_mutex, net.cpp:1860 started
 72024-04-03T18:21:31.566715Z [net] [logging/timer.h:58] [Log] [lock] Enter: lock contention m_nodes_mutex, net.cpp:1860 completed (1μs)
 82950	   ArgsManager& args = *Assert(context.args);
 9(gdb) 
102024-04-03T18:21:33.071865Z [msghand] [logging/timer.h:58] [Log] [lock] Enter: lock contention connman.m_nodes_mutex, ./net.h:1622 started
112024-04-03T18:21:33.071947Z [msghand] [logging/timer.h:58] [Log] [lock] Enter: lock contention connman.m_nodes_mutex, ./net.h:1622 completed (1μs)
122024-04-03T18:21:33.071995Z [net] [logging/timer.h:58] [Log] [lock] Enter: lock contention m_nodes_mutex, net.cpp:1932 started
132024-04-03T18:21:33.072075Z [net] [logging/timer.h:58] [Log] [lock] Enter: lock contention m_nodes_mutex, net.cpp:1932 completed (1μs)
142951	   const std::string& walletFile = database->Filename();
15(gdb) n
162024-04-03T18:21:47.648919Z [net] [logging/timer.h:58] [Log] [lock] Enter: lock contention m_nodes_mutex, net.cpp:1932 started
172024-04-03T18:21:47.649023Z [net] [logging/timer.h:58] [Log] [lock] Enter: lock contention m_nodes_mutex, net.cpp:1932 completed (1μs)
182953	   const auto start{SteadyClock::now()};
19(gdb) 
202956	   std::shared_ptr<CWallet> walletInstance(new CWallet(chain, name, std::move(database)), ReleaseWallet);
21(gdb) 
222024-04-03T18:21:49.734108Z [msghand] [logging/timer.h:58] [Log] [lock] Enter: lock contention connman.m_nodes_mutex, ./net.h:1622 started
232024-04-03T18:21:49.734220Z [msghand] [logging/timer.h:58] [Log] [lock] Enter: lock contention connman.m_nodes_mutex, ./net.h:1622 completed (3μs)
241765		_M_enable_shared_from_this_with(_Yp*) noexcept
25(gdb) 
262957	   walletInstance->m_keypool_size = std::max(args.GetIntArg("-keypool", DEFAULT_KEYPOOL_SIZE), int64_t{1});
27(gdb) 
2888	     __new_allocator() _GLIBCXX_USE_NOEXCEPT { }
29(gdb) 
302024-04-03T18:21:55.765728Z [addcon] [logging/timer.h:58] [Log] [lock] Enter: lock contention m_reconnections_mutex, net.cpp:3814 started
312024-04-03T18:21:55.765826Z [addcon] [logging/timer.h:58] [Log] [lock] Enter: lock contention m_reconnections_mutex, net.cpp:3814 completed (1μs)
322957	   walletInstance->m_keypool_size = std::max(args.GetIntArg("-keypool", DEFAULT_KEYPOOL_SIZE), int64_t{1});
33(gdb) 
3488	     __new_allocator() _GLIBCXX_USE_NOEXCEPT { }
35(gdb) 
361665	     get() const noexcept
37(gdb) 
382024-04-03T18:21:59.177777Z [net] [logging/timer.h:58] [Log] [lock] Enter: lock contention m_reconnections_mutex, net.cpp:1923 started
392024-04-03T18:21:59.177864Z [net] [logging/timer.h:58] [Log] [lock] Enter: lock contention m_reconnections_mutex, net.cpp:1923 completed (1μs)
402958	   walletInstance->m_notify_tx_changed_script = args.GetArg("-walletnotify", "");
41(gdb) 
422024-04-03T18:22:01.358006Z [net] [logging/timer.h:58] [Log] [lock] Enter: lock contention m_nodes_mutex, net.cpp:1860 started
432024-04-03T18:22:01.358112Z [net] [logging/timer.h:58] [Log] [lock] Enter: lock contention m_nodes_mutex, net.cpp:1860 completed (1μs)
441665	     get() const noexcept
45(gdb) 
462024-04-03T18:22:02.173997Z [net] [logging/timer.h:58] [Log] [lock] Enter: lock contention m_reconnections_mutex, net.cpp:1923 started
472024-04-03T18:22:02.174068Z [net] [logging/timer.h:58] [Log] [lock] Enter: lock contention m_reconnections_mutex, net.cpp:1923 completed (3μs)
482024-04-03T18:22:02.174550Z [httpworker.0] [wallet/wallet.h:933] [WalletLogPrintf] [funds] Wallet file version = 10500, last client version = 279900
492024-04-03T18:22:02.743008Z [httpworker.0] [wallet/wallet.h:933] [WalletLogPrintf] [funds] Descriptors: 8, Descriptor Keys: 1 plaintext, 0 encrypted, 1 total.
502024-04-03T18:22:02.920795Z [httpworker.0] [wallet/wallet.h:933] [WalletLogPrintf] [funds] Setting spkMan to active: id = 7bb07d3df110f36af92db5568fb8d4cc1381dd0a6d24b17182ed87cad6ab1d33, type = legacy, internal = false
512024-04-03T18:22:02.920836Z [httpworker.0] [wallet/wallet.h:933] [WalletLogPrintf] [funds] Setting spkMan to active: id = 5cb8f1604559d7b3612008ed3307ca1e21328301d1117b3acdd8b22d5b277219, type = p2sh-segwit, internal = false
522024-04-03T18:22:02.920851Z [httpworker.0] [wallet/wallet.h:933] [WalletLogPrintf] [funds] Setting spkMan to active: id = 68a4834fa805e997434b5c4f2f31625be6f3c5aebae0757293c4215675614a1b, type = bech32, internal = false
532024-04-03T18:22:02.920864Z [httpworker.0] [wallet/wallet.h:933] [WalletLogPrintf] [funds] Setting spkMan to active: id = 19d3e9f0d167b24e3c5330462a0c6cbc3d6e4e06e469967a395835102c0aea4a, type = bech32m, internal = false
542024-04-03T18:22:02.920891Z [httpworker.0] [wallet/wallet.h:933] [WalletLogPrintf] [funds] Setting spkMan to active: id = 09212e585c214fb06d197863920ed50181b984dc6bace73a865ff06a84ec3e61, type = legacy, internal = true
552024-04-03T18:22:02.920907Z [httpworker.0] [wallet/wallet.h:933] [WalletLogPrintf] [funds] Setting spkMan to active: id = 05db3056830116eafc425f192ee58033d44772bbf3bdd62ab50e8d014ecab772, type = p2sh-segwit, internal = true
562024-04-03T18:22:02.920919Z [httpworker.0] [wallet/wallet.h:933] [WalletLogPrintf] [funds] Setting spkMan to active: id = 2fe605d1acd8e46f55fbb47c8162545449df0ab59839219a7541b770a948fa3c, type = bech32, internal = true
572024-04-03T18:22:02.920932Z [httpworker.0] [wallet/wallet.h:933] [WalletLogPrintf] [funds] Setting spkMan to active: id = 115308b94096b7f96245e3ed280290ac5c627c5d666abf5249e4a16bcd5ac79e, type = bech32m, internal = true
582963	   if (nLoadWalletRet != DBErrors::LOAD_OK) {

    One thing that jumps out to me as possibly being problematic, depending on what’s being debugged, is that the line bool rescan_required = false; is optimized away and no longer appears when going step by step. This is not necessarily a problem as it’s still fairly easy to follow what’s going on and the variable’s value can be checked even though the debugger doesn’t indicate that it was initialized.

    Another thing that is kind of annoying is the appearance of the __new_allocator() and get() calls. We don’t see these in the actual code and their appearance means stepping through some more lines that are probably irrelevant to debugging.

  8. theuni commented at 7:39 pm on April 3, 2024: member

    @achow101 Yeah, I see the same. Sadly that’s the trade-off. Basically -O0 is a poor approximation of what real optimized code will look like. But anything above is bound to optimize some things out.

    So we have to make a choice: pure debug-ability (including things like inlines which don’t actually exist), or more realistic binaries while sacrificing some source code in the debugger. Personally I prefer the latter, but you probably live in gdb more than me :)

  9. theuni commented at 7:46 pm on April 3, 2024: member

    Note that for local hacking (with clang), you can use:

     0diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
 1index 96c4397504..6bc408e843 100644
 2--- a/src/wallet/wallet.cpp
 3+++ b/src/wallet/wallet.cpp
 4@@ -2944,7 +2944,7 @@ std::unique_ptr<WalletDatabase> MakeWalletDatabase(const std::string& name, cons
 5     return MakeDatabase(wallet_path, options, status, error_string);
 6 }
 7
 8-std::shared_ptr<CWallet> CWallet::Create(WalletContext& context, const std::string& name, std::unique_ptr<WalletDatabase> database, uint64_t wallet_creation_flags, bilingual_str& error, std::vector<bilingual_str>& warnings)
 9+std::shared_ptr<CWallet> CWallet::Create(WalletContext& context, const std::string& name, std::unique_ptr<WalletDatabase> database, uint64_t wallet_creation_flags, bilingual_str& error, std::vector<bilingual_str>& warnings) __attribute__ ((optnone))
10 {
11     interfaces::Chain* chain = context.chain;
12     ArgsManager& args = *Assert(context.args);

    To get the best of both worlds:

     0Thread 1 "b-test" hit Breakpoint 1, wallet::CWallet::Create (context=..., name="/tmp/test_common_Bitcoin Core/231b391a2079bb9568d880b8e884ab0b1c6a21c39361735a83653cf8256a247e/test_wallet_2950041828", database=std::unique_ptr<wallet::WalletDatabase> = {...}, wallet_creation_flags=25769803776, error=..., warnings=std::vector of length 0, capacity 0) at ./src/wallet/wallet.cpp:2949
 12949	    interfaces::Chain* chain = context.chain;
 2(gdb) n
 32950	    ArgsManager& args = *Assert(context.args);
 4(gdb)
 52951	    const std::string& walletFile = database->Filename();
 6(gdb)
 72953	    const auto start{SteadyClock::now()};
 8(gdb)
 92956	    std::shared_ptr<CWallet> walletInstance(new CWallet(chain, name, std::move(database)), ReleaseWallet);
10(gdb)
112957	    walletInstance->m_keypool_size = std::max(args.GetIntArg("-keypool", DEFAULT_KEYPOOL_SIZE), int64_t{1});
12(gdb)
132958	    walletInstance->m_notify_tx_changed_script = args.GetArg("-walletnotify", "");
14(gdb)
152961	    bool rescan_required = false;
  10. fanquake commented at 3:40 pm on April 4, 2024: member
    One thing to investigate is if -ggdb(n) is useful here at all.
  11. fanquake force-pushed on Apr 7, 2024
  12. DrahtBot removed the label CI failed on Apr 7, 2024
  13. fanquake force-pushed on Jun 26, 2024
  14. fanquake force-pushed on Aug 9, 2024
  15. fanquake commented at 3:28 pm on August 9, 2024: member
    @theuni reminder that you might want to followup here at some point, re recent discussion/debugging.
  16. hebasto added the label Needs CMake port on Aug 16, 2024
  17. fanquake force-pushed on Aug 28, 2024
  18. fanquake removed the label Needs CMake port on Aug 28, 2024
  19. fanquake force-pushed on Sep 10, 2024
  20. ryanofsky approved
  21. ryanofsky commented at 2:47 pm on September 10, 2024: contributor

    Code review ACK a4350695d9b5551a7720766a8af99c4ad5667e23. I don’t know all of the tradeoffs between flags, but this change makes sense and seems to do what is described.

    I did notice this is changing darwin, linux, and windows flags from O1 to Og but not freebsd/netbsd/openbsd flags so was curious if that was intentional.

  22. DrahtBot requested review from hebasto on Sep 10, 2024
  23. DrahtBot requested review from theuni on Sep 10, 2024
  24. fanquake force-pushed on Sep 10, 2024
  25. fanquake commented at 2:50 pm on September 10, 2024: member

    I did notice this is changing darwin, linux, and windows flags from O1 to Og but not freebsd/netbsd/openbsd flags so was curious if that was intentional.

    It was not. Have fixed that now. Will take this out of draft.

  26. fanquake marked this as ready for review on Sep 10, 2024
  27. ryanofsky approved
  28. ryanofsky commented at 3:17 pm on September 10, 2024: contributor

    Code review ACK 296def1f6fd936d72d49ce98f9473b4a5d2f9c4b.

    However, reading more about this it seems like there are real examples of where -Og hurts debuggability #16435 (comment) (“Yes. Please! On macOS, without that fix, debugging with lldb/gdb is impossible.”) and earlier in this thread.

    Also if the goal is to make depends build and main build more consistent, I’m not sure why they couldn’t both use -O0 or if the other problems mentioned with -O0 prevent that.

    But overall it seems like an improvement to use -Og not -O1 in debug depends builds, and to experiment -Og it in the main build to be more consistent. However, If it turns out -Og does negatively impact debugging in the main build, I think we should consider breaking consistency and using -O0 again in the main build by default. We could also recommend manually switching to -O0 in developer notes when using GDB.

  29. DrahtBot added the label CI failed on Sep 11, 2024
  30. DrahtBot removed the label CI failed on Sep 15, 2024
  31. TheCharlatan approved
  32. TheCharlatan commented at 9:20 am on September 16, 2024: contributor
    ACK 296def1f6fd936d72d49ce98f9473b4a5d2f9c4b
  33. hebasto approved
  34. hebasto commented at 4:33 pm on September 16, 2024: member

    ACK 296def1f6fd936d72d49ce98f9473b4a5d2f9c4b, I have reviewed the code and it looks OK.

    This PR effectively aligns flags between the depends build subsystem and the main build system. This change makes the current undocumented behaviour–where flags from depends override flags form the main build system–more flexible, and it can be considered in the ongoing discussion.

    Users will still be able to adjust their debugging experience by manually providing additional flags.

  35. achow101 commented at 6:59 pm on September 16, 2024: member

    NACK using -Og for the main build.

    Since I spend a considerable amount of time in gdb, I find this to be detrimental to debugability. One big issue that I’m seeing is that when stepping through line by line, some lines get skipped entirely, including lines that call other functions so it is no longer possible to step into those functions from that caller. Instead a breakpoint needs to be set on that function directly, but this is not always desirable. This can actually be seen in my earlier comment where the LoadWallet() line entirely disappears when stepping through the -Og build.

  36. ryanofsky commented at 12:58 pm on September 30, 2024: contributor

    If there disadvantages to setting -Og everywhere, as achow is suggesting, maybe it makes sense for depends build to be able to pass flags to the bitcoin build, but let the bitcoin build be free to override them, and keep using -O0 for itself on linux but switch to -Og if needed to avoid problems on other platforms.

    I think this would be compatible with approach I suggested in #30813, where depends build provides flags that the main build can use to choose default flag values, and users can freely edit and override the saved values with cmake or ccmake.

  37. fanquake force-pushed on Dec 10, 2024
  38. fanquake renamed this:
    [RFC] Switch and/or align debugging flags (back) to `-Og`
    [RFC] Switch and/or align debugging flags to `-O0`
    on Dec 10, 2024
  39. fanquake marked this as a draft on Dec 10, 2024
  40. DrahtBot commented at 6:16 pm on December 10, 2024: contributor

    🚧 At least one of the CI tasks failed. Debug: https://github.com/bitcoin/bitcoin/runs/34203968633

    Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

    • Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

    • A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.

    • An intermittent issue.

    Leave a comment here, if you need help tracking down a confusing failure.

  41. DrahtBot added the label CI failed on Dec 10, 2024
  42. fanquake referenced this in commit ea9e64ff3c on Dec 12, 2024
  43. fanquake force-pushed on Dec 12, 2024
  44. fanquake renamed this:
    [RFC] Switch and/or align debugging flags to `-O0`
    [RFC] Align debugging flags to `-O0`
    on Dec 12, 2024
  45. fanquake referenced this in commit 1251a23642 on Dec 17, 2024
  46. depends: use -O0 in depends DEBUG=1 flags 2d827e041f
  47. fanquake force-pushed on Dec 17, 2024
  48. fanquake marked this as ready for review on Dec 17, 2024
  49. maflcko commented at 9:33 pm on December 19, 2024: member

    The CI failure:

     0[08:34:27.775] ********* Start testing of WalletTests *********
 1[08:34:27.775] Config: Using QtTest library 5.15.14, Qt 5.15.14 (i386-little_endian-ilp32 static debug build; by GCC 13.2.0), ubuntu 24.04
 2[08:34:27.775] PASS   : WalletTests::initTestCase()
 3[08:34:27.775] QDEBUG : WalletTests::walletTests() NotifyUnload
 4[08:34:27.775] QWARN  : WalletTests::walletTests() This plugin does not support propagateSizeHints()
 5[08:34:27.775] QDEBUG : WalletTests::walletTests() "NotifyTransactionChanged: 23d0e149f4c487be26bfd20c5a7735fa78757fa84c3368a720ee755f07b6e1d5 status= 0"
 6[08:34:27.775] QDEBUG : WalletTests::walletTests() "NotifyTransactionChanged: 20bb379fd28f090730ac2d207b92c6e95c69dfa48a8118577edb20e068c9a65d status= 1"
 7[08:34:27.775] QDEBUG : WalletTests::walletTests() "NotifyTransactionChanged: ec76e4ab5ae7066ba51a42c5e09a79adaf9708a4a3cdbd02a28d378179f3024e status= 1"
 8[08:34:27.775] QDEBUG : WalletTests::walletTests() "NotifyAddressBookChanged: mfWxJ45yp2SFn7UciZyNpvDKrzbhyfKrY8  isMine=0 purpose=1 status=0"
 9[08:34:27.775] QWARN  : WalletTests::walletTests() This plugin does not support propagateSizeHints()
10[08:34:27.775] QDEBUG : WalletTests::walletTests() "TransactionTablePriv::updateWallet: 23d0e149f4c487be26bfd20c5a7735fa78757fa84c3368a720ee755f07b6e1d5 0"
11[08:34:27.775] QDEBUG : WalletTests::walletTests() "    inModel=0 Index=96-96 showTransaction=1 derivedStatus=0"
12[08:34:27.775] QDEBUG : WalletTests::walletTests() "TransactionTablePriv::updateWallet: 20bb379fd28f090730ac2d207b92c6e95c69dfa48a8118577edb20e068c9a65d 1"
13[08:34:27.775] QDEBUG : WalletTests::walletTests() "    inModel=1 Index=33-34 showTransaction=1 derivedStatus=1"
14[08:34:27.775] QDEBUG : WalletTests::walletTests() "TransactionTablePriv::updateWallet: ec76e4ab5ae7066ba51a42c5e09a79adaf9708a4a3cdbd02a28d378179f3024e 1"
15[08:34:27.775] QDEBUG : WalletTests::walletTests() "    inModel=1 Index=26-27 showTransaction=1 derivedStatus=1"
16[08:34:27.775] QDEBUG : WalletTests::walletTests() "NotifyTransactionChanged: f51a5fd85b12b19bf82e2ed01583b06de4d91ca145afe516e1371cf8a0845f9b status= 0"
17[08:34:27.775] QDEBUG : WalletTests::walletTests() "NotifyTransactionChanged: 1d5b20ad1872c813e6fd16b0715d68fa59f782c5912a94b5d5f64e39b50e1865 status= 1"
18[08:34:27.775] QDEBUG : WalletTests::walletTests() "TransactionTablePriv::updateWallet: f51a5fd85b12b19bf82e2ed01583b06de4d91ca145afe516e1371cf8a0845f9b 0"
19[08:34:27.775] QDEBUG : WalletTests::walletTests() "    inModel=0 Index=67-67 showTransaction=1 derivedStatus=0"
20[08:34:27.775] QDEBUG : WalletTests::walletTests() "TransactionTablePriv::updateWallet: 1d5b20ad1872c813e6fd16b0715d68fa59f782c5912a94b5d5f64e39b50e1865 1"
21[08:34:27.775] QDEBUG : WalletTests::walletTests() "    inModel=1 Index=36-37 showTransaction=1 derivedStatus=1"
22[08:34:27.775] QWARN  : WalletTests::walletTests() This plugin does not support propagateSizeHints()
23[08:34:27.775] QWARN  : WalletTests::walletTests() This plugin does not support propagateSizeHints()
24[08:34:27.775] QWARN  : WalletTests::walletTests() This plugin does not support propagateSizeHints()
25[08:34:27.775] QDEBUG : WalletTests::walletTests() "NotifyTransactionChanged: fe93875e30d86470a0861a46df809993cb589a49a9fd9941ad601fc35a140e50 status= 0"
26[08:34:27.775] QDEBUG : WalletTests::walletTests() "NotifyTransactionChanged: 1d5b20ad1872c813e6fd16b0715d68fa59f782c5912a94b5d5f64e39b50e1865 status= 1"
27[08:34:27.775] QDEBUG : WalletTests::walletTests() "NotifyTransactionChanged: f51a5fd85b12b19bf82e2ed01583b06de4d91ca145afe516e1371cf8a0845f9b status= 1"
28[08:34:27.775] QDEBUG : WalletTests::walletTests() "TransactionTablePriv::updateWallet: f51a5fd85b12b19bf82e2ed01583b06de4d91ca145afe516e1371cf8a0845f9b 1"
29[08:34:27.775] QDEBUG : WalletTests::walletTests() "    inModel=1 Index=67-68 showTransaction=1 derivedStatus=1"
30[08:34:27.775] QDEBUG : WalletTests::walletTests() "TransactionTablePriv::updateWallet: fe93875e30d86470a0861a46df809993cb589a49a9fd9941ad601fc35a140e50 0"
31[08:34:27.775] QDEBUG : WalletTests::walletTests() "    inModel=0 Index=29-29 showTransaction=1 derivedStatus=0"
32[08:34:27.775] QDEBUG : WalletTests::walletTests() "TransactionTablePriv::updateWallet: 1d5b20ad1872c813e6fd16b0715d68fa59f782c5912a94b5d5f64e39b50e1865 1"
33[08:34:27.775] QDEBUG : WalletTests::walletTests() "    inModel=1 Index=37-38 showTransaction=1 derivedStatus=1"
34[08:34:27.775] QDEBUG : WalletTests::walletTests() "TransactionTablePriv::updateWallet: f51a5fd85b12b19bf82e2ed01583b06de4d91ca145afe516e1371cf8a0845f9b 1"
35[08:34:27.775] QDEBUG : WalletTests::walletTests() "    inModel=1 Index=68-69 showTransaction=1 derivedStatus=1"
36[08:34:27.775] 
37[08:34:27.775] === Received signal at function time: 8293ms, total time: 8294ms, dumping stack ===
38[08:34:27.775] === End of stack trace ===
39[08:34:27.775] QFATAL : WalletTests::walletTests() Received signal 4
40[08:34:27.775]          Function time: 8293ms Total time: 8293ms
41[08:34:27.775] FAIL!  : WalletTests::walletTests() Received a fatal error.
42[08:34:27.775]    Loc: [Unknown file(0)]
43[08:34:27.775] Totals: 1 passed, 1 failed, 0 skipped, 0 blacklisted, 8306ms
44[08:34:27.775] ********* Finished testing of WalletTests *********
45[08:34:27.775] 
46[08:34:27.948]  16/139 Test  [#10](/bitcoin-bitcoin/10/): addrman_tests ........................   Passed   10.89 sec
47[08:34:29.354]  17/139 Test  [#24](/bitcoin-bitcoin/24/): blockfilter_index_tests ..............   Passed    2.37 sec
48[08:34:32.408]  18/139 Test  [#21](/bitcoin-bitcoin/21/): bip324_tests .........................   Passed   11.24 sec
49[08:34:40.010]  19/139 Test  [#13](/bitcoin-bitcoin/13/): argsman_tests ........................   Passed   22.84 sec
50[08:35:19.511]  20/139 Test   [#7](/bitcoin-bitcoin/7/): secp256k1_exhaustive_tests ...........   Passed   62.48 sec
51[08:35:27.660]  21/139 Test  [#17](/bitcoin-bitcoin/17/): base58_tests .........................   Passed   69.47 sec
52[08:37:38.670]  22/139 Test   [#5](/bitcoin-bitcoin/5/): secp256k1_noverify_tests .............   Passed  201.65 sec
53[08:39:43.315]  23/139 Test   [#9](/bitcoin-bitcoin/9/): bench_sanity_check_high_priority .....   Passed  326.26 sec
54[08:40:22.068]  24/139 Test   [#6](/bitcoin-bitcoin/6/): secp256k1_tests ......................   Passed  365.03 sec
55[08:40:22.069] 
56[08:40:22.069] 96% tests passed, 1 tests failed out of 24
57[08:40:22.069] 
58[08:40:22.069] Total Test time (real) = 365.08 sec
59[08:40:22.070] 
60[08:40:22.070] The following tests FAILED:
61[08:40:22.070] 	  8 - test_bitcoin-qt (Subprocess aborted)
62[08:40:22.070] Errors while running CTest
63[08:40:22.805] 
64[08:40:22.805] Exit status: 8


fanquake DrahtBot hebasto emc99 theuni achow101 ryanofsky TheCharlatan maflcko


theuni

Labels
CI failed

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