10 files changed, 92 insertions, 31 deletions

diff --git a/src/Makefile b/src/Makefile
index 9846d8e..f0c6706 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -9,7 +9,26 @@ RANDOMX_DIR ?= $(HOME)/RandomX
 
 IMGUI_DIR = imgui
 
-CXXFLAGS = -std=gnu++14 -O2 -w -fpermissive -pthread \
+# -w used to sit here and suppressed every diagnostic in the build, including
+# the one that mattered: `%I64d` is MSVC-only, glibc mis-parses it as the GNU
+# `I` flag with a width of 64 and an `int` conversion, and the resulting vararg
+# desynchronisation segfaulted the node while logging a received block (issue
+# #5, deterministic at height 1062 -- the first block carrying a real
+# transaction). GCC had been reporting it all along:
+#
+#   warning: format '%d' expects argument of type 'int',
+#            but argument 4 has type 'int64_t'
+#
+# -w cannot be narrowed: it wins over any -W or -Werror= that follows it, so it
+# has to go. The four -Wno- entries below are the categories this 2009-era tree
+# genuinely produces (measured: 248 deprecated-declarations, 157 write-strings,
+# 18 narrowing, 1 unused-result); silencing them by name instead of silencing
+# everything keeps the build quiet while leaving format errors fatal.
+WARNFLAGS = -Wformat -Werror=format \
+            -Wno-deprecated-declarations -Wno-write-strings \
+            -Wno-narrowing -Wno-unused-result
+
+CXXFLAGS = -std=gnu++14 -O2 $(WARNFLAGS) -fpermissive -pthread \
            -DSECP256K1_STATIC \
            -I$(SECP_INC) -I$(RANDOMX_DIR)/src \
            -I/usr/include/nlohmann \
diff --git a/src/btfaddr.cpp b/src/btfaddr.cpp
index 55bb6c5..6edcc91 100644
--- a/src/btfaddr.cpp
+++ b/src/btfaddr.cpp
@@ -7,7 +7,10 @@
 #include "btfaddr.h"
 
 #include <openssl/sha.h>
+// See the note in nostr.cpp: the makefiles pass -DSECP256K1_STATIC too.
+#ifndef SECP256K1_STATIC
 #define SECP256K1_STATIC
+#endif
 #include <secp256k1.h>
 #include <secp256k1_schnorrsig.h>
 #include <secp256k1_extrakeys.h>
diff --git a/src/main.cpp b/src/main.cpp
index bb317d2..c882b0b 100644
--- a/src/main.cpp
+++ b/src/main.cpp
@@ -898,7 +898,7 @@ bool CTransaction::ConnectInputs(CTxDB& txdb, map<uint256, CTxIndex>& mapTestPoo
             }
 
             if (prevout.n >= txPrev.vout.size() || prevout.n >= txindex.vSpent.size())
-                return error("ConnectInputs() : %s prevout.n out of range %d %d %d", GetHash().ToString().substr(0,6).c_str(), prevout.n, txPrev.vout.size(), txindex.vSpent.size());
+                return error("ConnectInputs() : %s prevout.n out of range %d %d %d", GetHash().ToString().substr(0,6).c_str(), prevout.n, (int)txPrev.vout.size(), (int)txindex.vSpent.size());
 
             // If prev is coinbase, check that it's matured
             if (txPrev.IsCoinBase())
@@ -1690,14 +1690,14 @@ void PrintBlockTree()
             pindex->nBlockPos,
             block.GetHash().ToString().substr(0,14).c_str(),
             DateTimeStr(block.nTime).c_str(),
-            block.vtx.size());
+            (int)block.vtx.size());
 
         CRITICAL_BLOCK(cs_mapWallet)
         {
             if (mapWallet.count(block.vtx[0].GetHash()))
             {
                 CWalletTx& wtx = mapWallet[block.vtx[0].GetHash()];
-                printf("    mine:  %d  %d  %d", wtx.GetDepthInMainChain(), wtx.GetBlocksToMaturity(), wtx.GetCredit());
+                printf("    mine:  %d  %d  %lld", wtx.GetDepthInMainChain(), wtx.GetBlocksToMaturity(), wtx.GetCredit());
             }
         }
         printf("\n");
@@ -1759,7 +1759,7 @@ bool ProcessMessages(CNode* pfrom)
     CDataStream& vRecv = pfrom->vRecv;
     if (vRecv.empty())
         return true;
-    LogPrint("net", "ProcessMessages(%d bytes)\n", vRecv.size());
+    LogPrint("net", "ProcessMessages(%d bytes)\n", (int)vRecv.size());
 
     //
     // Message format
@@ -1783,7 +1783,7 @@ bool ProcessMessages(CNode* pfrom)
             break;
         }
         if (pstart - vRecv.begin() > 0)
-            if (LogAcceptsCategory("net")) printf("\n\nPROCESSMESSAGE SKIPPED %d BYTES\n\n", pstart - vRecv.begin());
+            if (LogAcceptsCategory("net")) printf("\n\nPROCESSMESSAGE SKIPPED %d BYTES\n\n", (int)(pstart - vRecv.begin()));
         vRecv.erase(vRecv.begin(), pstart);
 
         // Read header
@@ -1836,7 +1836,7 @@ bool ProcessMessages(CNode* pfrom)
 bool ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv)
 {
     static map<unsigned int, vector<unsigned char> > mapReuseKey;
-    if (LogAcceptsCategory("net")) { printf("received: %-12s (%d bytes)  ", strCommand.c_str(), vRecv.size()); for (int i = 0; i < min(vRecv.size(), (unsigned int)25); i++) printf("%02x ", vRecv[i] & 0xff); printf("\n"); }
+    if (LogAcceptsCategory("net")) { printf("received: %-12s (%d bytes)  ", strCommand.c_str(), (int)vRecv.size()); for (int i = 0; i < min(vRecv.size(), (unsigned int)25); i++) printf("%02x ", vRecv[i] & 0xff); printf("\n"); }
     if (nDropMessagesTest > 0 && GetRand(nDropMessagesTest) == 0)
     {
         if (LogAcceptsCategory("net")) printf("dropmessages DROPPING RECV MESSAGE\n");
@@ -2171,7 +2171,7 @@ bool ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv)
 
 
     if (!vRecv.empty())
-        if (LogAcceptsCategory("net")) printf("ProcessMessage(%s) : %d extra bytes\n", strCommand.c_str(), vRecv.size());
+        if (LogAcceptsCategory("net")) printf("ProcessMessage(%s) : %d extra bytes\n", strCommand.c_str(), (int)vRecv.size());
 
     return true;
 }
@@ -2885,7 +2885,7 @@ bool BitcoinMiner()
         }
         pblock->nBits = nBits;
         pblock->vtx[0].vout[0].nValue = pblock->GetBlockValue(nBestHeight + 1, nFees);
-        if (LogAcceptsCategory("net")) printf("\n\nRunning BitcoinMiner with %d transactions in block\n", pblock->vtx.size());
+        if (LogAcceptsCategory("net")) printf("\n\nRunning BitcoinMiner with %d transactions in block\n", (int)pblock->vtx.size());
 
 
         //
diff --git a/src/main.h b/src/main.h
index fde3a4e..7e5bdf2 100644
--- a/src/main.h
+++ b/src/main.h
@@ -650,11 +650,11 @@ public:
     string ToString() const
     {
         string str;
-        str += strprintf("CTransaction(hash=%s, ver=%d, vin.size=%d, vout.size=%d, nLockTime=%d)\n",
+        str += strprintf("CTransaction(hash=%s, ver=%d, vin.size=%d, vout.size=%d, nLockTime=%u)\n",
             GetHash().ToString().substr(0,6).c_str(),
             nVersion,
-            vin.size(),
-            vout.size(),
+            (int)vin.size(),
+            (int)vout.size(),
             nLockTime);
         for (int i = 0; i < vin.size(); i++)
             str += "    " + vin[i].ToString() + "\n";
@@ -1081,7 +1081,7 @@ public:
             hashPrevBlock.ToString().substr(0,14).c_str(),
             hashMerkleRoot.ToString().substr(0,6).c_str(),
             nTime, nBits, nNonce,
-            vtx.size());
+            (int)vtx.size());
         for (int i = 0; i < vtx.size(); i++)
         {
             printf("  ");
@@ -1223,8 +1223,10 @@ public:
 
     string ToString() const
     {
-        return strprintf("CBlockIndex(nprev=%08x, pnext=%08x, nFile=%d, nBlockPos=%-6d nHeight=%d, merkle=%s, hashBlock=%s)",
-            pprev, pnext, nFile, nBlockPos, nHeight,
+        // pprev/pnext are pointers: %08x truncated them to 32 bits on a 64-bit
+        // build, so the two fields this line exists to show were unreliable.
+        return strprintf("CBlockIndex(nprev=%p, pnext=%p, nFile=%d, nBlockPos=%-6d nHeight=%d, merkle=%s, hashBlock=%s)",
+            (void*)pprev, (void*)pnext, nFile, nBlockPos, nHeight,
             hashMerkleRoot.ToString().substr(0,6).c_str(),
             GetBlockHash().ToString().substr(0,14).c_str());
     }
diff --git a/src/makefile.mingw b/src/makefile.mingw
index 7d35d92..01f7702 100644
--- a/src/makefile.mingw
+++ b/src/makefile.mingw
@@ -7,7 +7,14 @@ DEPS      = ../deps
 RANDOMX   = ../RandomX
 IMGUI_DIR = imgui
 
-CXXFLAGS = -std=gnu++14 -O2 -w -fpermissive -Wno-deprecated-declarations -pthread \
+# See the long note in Makefile: -w hid the format bug that segfaulted the node
+# on glibc (issue #5), and -w cannot be narrowed, so format diagnostics are now
+# fatal and only the categories this tree actually produces are silenced.
+WARNFLAGS = -Wformat -Werror=format \
+            -Wno-deprecated-declarations -Wno-write-strings \
+            -Wno-narrowing -Wno-unused-result
+
+CXXFLAGS = -std=gnu++14 -O2 $(WARNFLAGS) -fpermissive -pthread \
            -DWIN32 -DNOPCH \
            -DSECP256K1_STATIC \
            -I$(DEPS)/include -I$(RANDOMX)/src \
diff --git a/src/net.cpp b/src/net.cpp
index 08a3348..395092d 100644
--- a/src/net.cpp
+++ b/src/net.cpp
@@ -920,7 +920,7 @@ void ThreadSocketHandler2(void* parg)
 
                     if (pnodeExtra->GetRefCount() <= (pnodeExtra->fNetworkNode ? 1 : 0))
                     {
-                        LogPrint("net", "(%d nodes) disconnecting duplicate: %s\n", vNodes.size(), pnodeExtra->addr.ToString().c_str());
+                        LogPrint("net", "(%d nodes) disconnecting duplicate: %s\n", (int)vNodes.size(), pnodeExtra->addr.ToString().c_str());
                         if (pnodeExtra->fNetworkNode && !pnode->fNetworkNode)
                         {
                             pnode->AddRef();
diff --git a/src/net.h b/src/net.h
index d13de61..364fbfb 100644
--- a/src/net.h
+++ b/src/net.h
@@ -392,7 +392,9 @@ public:
     const char* GetCommand() const
     {
         if (!IsKnownType())
-            throw std::out_of_range(strprintf("CInv::GetCommand() : type=% unknown type", type));
+            // Was "type=% unknown type": the % swallowed the space as a flag and
+            // the u as the conversion, so this read "type= 3nknown type".
+            throw std::out_of_range(strprintf("CInv::GetCommand() : type=%d unknown type", type));
         return ppszTypeName[type];
     }
 
diff --git a/src/nostr.cpp b/src/nostr.cpp
index 482a835..088174a 100644
--- a/src/nostr.cpp
+++ b/src/nostr.cpp
@@ -6,8 +6,12 @@
 #include "headers.h"
 #include <openssl/ssl.h>
 #include <openssl/err.h>
-// We link libsecp256k1 statically (without this the header assumes DLL import)
+// We link libsecp256k1 statically (without this the header assumes DLL import).
+// Both makefiles also pass -DSECP256K1_STATIC; the guard keeps that from
+// warning while leaving this file correct if built without the flag.
+#ifndef SECP256K1_STATIC
 #define SECP256K1_STATIC
+#endif
 #include <secp256k1.h>
 #include <secp256k1_schnorrsig.h>
 #include <secp256k1_extrakeys.h>
diff --git a/src/util.cpp b/src/util.cpp
index e4e314f..d52ca88 100644
--- a/src/util.cpp
+++ b/src/util.cpp
@@ -180,7 +180,7 @@ void RandAddSeed(bool fPerfmon)
             RAND_add(&hash, sizeof(hash), min(nSize/500.0, (double)sizeof(hash)));
             hash = 0;
             memset(pdata, 0, nSize);
-            printf("RandAddSeed() got %d bytes of performance data\n", nSize);
+            printf("RandAddSeed() got %lu bytes of performance data\n", nSize);
         }
     }
 #else
@@ -472,7 +472,7 @@ void AddTimeData(unsigned int ip, int64 nTime)
     if (vTimeOffsets.empty())
         vTimeOffsets.push_back(0);
     vTimeOffsets.push_back(nOffsetSample);
-    printf("Added time data, samples %d, ip %08x, offset %+lld (%+lld minutes)\n", vTimeOffsets.size(), ip, vTimeOffsets.back(), vTimeOffsets.back()/60);
+    printf("Added time data, samples %d, ip %08x, offset %+lld (%+lld minutes)\n", (int)vTimeOffsets.size(), ip, vTimeOffsets.back(), vTimeOffsets.back()/60);
     if (vTimeOffsets.size() >= 5 && vTimeOffsets.size() % 2 == 1)
     {
         sort(vTimeOffsets.begin(), vTimeOffsets.end());
diff --git a/src/util.h b/src/util.h
index b583248..797ed27 100644
--- a/src/util.h
+++ b/src/util.h
@@ -14,7 +14,9 @@ typedef unsigned long long  uint64;
 #define for  if (false) ; else for
 #endif
 
-#ifndef _MSC_VER
+// MinGW already provides __forceinline for MSVC compatibility, so defining it
+// unconditionally warned ten times per build once -w was removed.
+#if !defined(_MSC_VER) && !defined(__forceinline)
 #define __forceinline  inline
 #endif
 
@@ -40,17 +42,17 @@ typedef unsigned long long  uint64;
 #endif
 #define snprintf my_snprintf
 
+// One definition for every target. The MSVCRT branch that used to sit here
+// produced "I64d", which is the specifier that segfaults on glibc (issue #5) --
+// and it was pointless besides: this build targets UCRT, which accepts the C99
+// forms, and the rest of the tree already prints int64 with %lld on Windows.
+// Keeping one spelling everywhere is also what lets the format attributes below
+// use a single archetype, so a bad specifier fails the build on both platforms.
 #ifndef PRId64
-#if defined(_MSC_VER) || defined(__BORLANDC__) || defined(__MSVCRT__)
-#define PRId64  "I64d"
-#define PRIu64  "I64u"
-#define PRIx64  "I64x"
-#else
 #define PRId64  "lld"
 #define PRIu64  "llu"
 #define PRIx64  "llx"
 #endif
-#endif
 
 // This is needed because the foreach macro can't get over the comma in pair<t1, t2>
 #define PAIRTYPE(t1, t2)    pair<t1, t2>
@@ -102,10 +104,30 @@ void LogPrintDedup(const char* category, const char* file, int line, const std::
 #define LogPrint(category, ...) \
     do { if (LogAcceptsCategory(category)) LogPrintDedup((category), __FILE__, __LINE__, strprintf(__VA_ARGS__)); } while (0)
 
+// Every format string in this project goes through one of the four functions
+// below -- `printf` itself is remapped to OutputDebugStringF just above, and
+// snprintf to my_snprintf. None of them carried a format attribute, and GCC
+// only checks format strings for functions it knows are printf-like, so
+// -Wformat had nothing to inspect and the whole tree was unchecked. That is
+// how `%I64d` (MSVC-only, and a segfault on glibc) survived in
+// CTxOut::ToString until it crashed a node mid-sync -- see issue #5.
+//
+// gnu_printf on both platforms, deliberately. MinGW's other option, ms_printf,
+// models the legacy msvcrt: it rejects %zu, which this tree uses and UCRT
+// supports, and it accepts %I64d, which is the specifier that crashes on glibc.
+// It would give false positives where the code is right and stay silent where it
+// is dangerous. gnu_printf describes UCRT's C99 behaviour accurately, so one
+// archetype covers both builds and a bad specifier fails either one.
+#if defined(__GNUC__)
+#define BF_FORMAT(fmt, args) __attribute__((format(gnu_printf, fmt, args)))
+#else
+#define BF_FORMAT(fmt, args)
+#endif
+
 void RandAddSeed(bool fPerfmon=false);
-int my_snprintf(char* buffer, size_t limit, const char* format, ...);
-string strprintf(const char* format, ...);
-bool error(const char* format, ...);
+int my_snprintf(char* buffer, size_t limit, const char* format, ...) BF_FORMAT(3, 4);
+string strprintf(const char* format, ...) BF_FORMAT(1, 2);
+bool error(const char* format, ...) BF_FORMAT(1, 2);
 void PrintException(std::exception* pex, const char* pszThread);
 void ParseString(const string& str, char c, vector<string>& v);
 string FormatMoney(int64 n, bool fPlus=false);
@@ -266,6 +288,8 @@ void PrintHex(const T pbegin, const T pend, const char* pszFormat="%s", bool fSp
 // Forward declaration -- defined in main.cpp
 string GetAppDir();
 
+inline int OutputDebugStringF(const char* pszFormat, ...) BF_FORMAT(1, 2);
+
 inline int OutputDebugStringF(const char* pszFormat, ...)
 {
 #if 1 // debug.log always enabled