Repository: bitcoin/bitcoin (language: cpp)

Issue:
build: Windows debug cross-build fails with `-O0`
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

When cross-compiling for Windows with the `-O0` optimization level the build fails.

The error can be fixed by the `-Wa,-mbig-obj` assembler flag. However, @theuni [suggested](https://github.com/hebasto/bitcoin/pull/18#discussion_r1267219453):
> We should definitely look into breaking up that object.

### Expected behaviour

The build completes with no error.

### Steps to reproduce

```
$ make -C depends HOST=x86_64-w64-mingw32 DEBUG=1 NO_QT=1
$ ./configure CONFIG_SITE=$PWD/depends/x86_64-w64-mingw32/share/config.site --enable-debug CFLAGS=-O0 CXXFLAGS=-O0
$ make
```

### Relevant log output

```
...
/usr/bin/x86_64-w64-mingw32-as: wallet/libbitcoin_wallet_a-wallet.o: too many sections (46006)
{standard input}: Assembler messages:
{standard input}: Fatal error: can't write 47 bytes to section .text of wallet/libbitcoin_wallet_a-wallet.o: 'file too big'
/usr/bin/x86_64-w64-mingw32-as: wallet/libbitcoin_wallet_a-wallet.o: too many sections (46006)
{standard input}: Fatal error: wallet/libbitcoin_wallet_a-wallet.o: file too big
...
```

### How did you obtain Bitcoin Core

Compiled from source

### What version of Bitcoin Core are you using?

master@5608e1d3b4ba3a23c05937918046e428be890505

### Operating system and version

Ubuntu 23.04

### Machine specifications

_No response_


Discussion / hints:
Ping @achow101.

Being unable to build with `-O0` is not a big deal.

However, that means we're relying on compiler optimizations to reduce the code size first in order for tools to be able to process them with sane defaults. Breaking up `wallet.cpp` somewhat is prudent, imo.
How big is too big?

`wallet.cpp` is pretty much entirely `CWallet`'s members, so breaking it up will require breaking up `CWallet` which I'm not sure we want to do? I suppose not all of the members need to be in `CWallet`, but I think it'll still be necessary for that class to be kinda big.
> How big is too big?

Good question :)

> 
> `wallet.cpp` is pretty much entirely `CWallet`'s members, so breaking it up will require breaking up `CWallet` which I'm not sure we want to do?

Yeah, agreed that we shouldn't be breaking up classes for something like this. I suspect this actually has more to do with the _giant_ list of includes. Specifically, getting rid of boost signals ([here's a POC from a while back](https://github.com/theuni/bitcoin/commits/replace-boost-signals)) would probably be enough to fix the issue for good. Another reason to kill them off :)
Maybe every `class` other than `CWallet` can be moved to a separate module? 


Also, a bunch of includes can be killed, according to iwyu:

```
wallet/wallet.h should remove these lines:
- #include <interfaces/wallet.h>  // lines 12-12
- #include <psbt.h>  // lines 16-16
- #include <util/message.h>  // lines 20-20
- #include <util/strencodings.h>  // lines 22-22
- #include <validationinterface.h>  // lines 26-26
- #include <wallet/walletdb.h>  // lines 30-30
- #include <algorithm>  // lines 33-33
- #include <stdexcept>  // lines 39-39
- class CScript;  // lines 51-51
- enum class FeeEstimateMode;  // lines 52-52
- namespace wallet { class CWalletTx; }  // lines 121-121
- namespace wallet { class ReserveDestination; }  // lines 122-122
```

```
wallet/wallet.cpp should remove these lines:
- #include <policy/fees.h>  // lines 20-20
- #include <policy/policy.h>  // lines 21-21
- #include <util/bip32.h>  // lines 31-31
- #include <util/fees.h>  // lines 34-34
- #include <util/rbf.h>  // lines 38-38
- #include <wallet/fees.h>  // lines 44-44

I don't see any reason not to do the iwyu trimming. Though sadly it doesn't look like it'll help too much. Preprocessed (gcc -E -O0) numbers for `wallet/wallet.cpp` before and after @MarcoFalke's suggested removals:
```
Before: 191,084 lines
After: 190,899 lines
```
There is no requirement that an entire class is defined within a single compilation unit, I believe. If you don't have function definitions inside the `class` itself, then those definitions can be given in any .cpp file.

That said, for understandability I'd strongly prefer not splitting a class over multiple .cpp files...
Looking at `-ftime-trace=/tmp/`, I don't think there is anything that can be done, other than to nuke boost, or to define a smaller limited interface of our boost usage in a new header and hide the boost includes in the corresponding cpp file?

![boost_sad](https://github.com/bitcoin/bitcoin/assets/6399679/a92b0951-1d36-4b08-95c9-9986972866ec)


Fix patch:
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 4b21646ca18f5..2dba6f255d466 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -298,6 +298,15 @@ if(WIN32)
     try_append_linker_flag("-Wl,--major-subsystem-version,6" TARGET core_interface)
     try_append_linker_flag("-Wl,--minor-subsystem-version,2" TARGET core_interface)
   endif()
+
+  # Workaround producing large object files, which cannot be handled by the assembler.
+  # More likely to happen with no, or lower levels of optimisation.
+  # See discussion in https://github.com/bitcoin/bitcoin/issues/28109.
+  if(CMAKE_CXX_COMPILER_ID STREQUAL "MSVC")
+    try_append_cxx_flags("/bigobj" TARGET core_interface_debug SKIP_LINK)
+  else()
+    try_append_cxx_flags("-Wa,-mbig-obj" TARGET core_interface_debug SKIP_LINK)
+  endif()
 endif()
 
 # Use 64-bit off_t on 32-bit Linux.


Test patch: