upleb.uk

Public git repos — served from a NIP-34 GRASP relay at git.upleb.uk

summaryrefslogtreecommitdiff
path: root/src/util.h
diff options
context:
space:
mode:
authorBitflash <btf01@proton.me>2026-07-30 10:38:12 -0300
committerGitHub <noreply@github.com>2026-07-30 10:38:12 -0300
commit2ed9aed2ea9e302b3b36b37806ec03f2cccaaed1 (patch)
treeab30eade9435c074ee08c70303b8a15c08770b4f /src/util.h
parentf30f45849ecd45be6a073b6b1d308e1c12b8b52b (diff)
build: check format strings, which nothing ever did (#44)
The segfault in issue #5 was a format string: `%I64d` is MSVC-only, glibc parses it as the GNU `I` flag with a width of 64 and an `int` conversion, the vararg run desynchronises, and the following `%s` dereferences an integer. It killed the node mid-sync at the first block carrying a real transaction, deterministically. GCC can catch this. It never got the chance, for two reasons that had to be fixed together: - `-w` sat in both makefiles and suppressed every diagnostic. It cannot be narrowed; it wins over any -W or -Werror= that follows, so it had to go. The four -Wno- entries replacing it are the categories this 2009-era tree actually produces (measured: 248 deprecated-declarations, 157 write-strings, 18 narrowing, 1 unused-result), silenced by name instead of wholesale. - Even with -Wformat on, nothing was checked. GCC only inspects format strings for functions it knows are printf-like, and every format string here goes through strprintf, error, my_snprintf, or OutputDebugStringF -- `printf` itself is #defined to the last of those. None carried a format attribute, so the entire tree was invisible to the check. They are annotated now. Annotating surfaced 77 warnings across 17 distinct sites, all fixed here. Most were size_t or int64 passed to %d, harmless on x86-64 but wrong. Two were not cosmetic: - CBlockIndex::ToString printed pprev/pnext with %08x, truncating both pointers to 32 bits, so the two fields that line exists to show were unreliable on every 64-bit build. - CInv::GetCommand threw "type=% unknown type", where the % ate the space as a flag and the u as its conversion. The message read "type= 3nknown type". PRId64 is now one spelling for every target. Its MSVCRT branch was what produced "I64d" in the first place, it is pointless against UCRT which takes the C99 forms, the rest of the tree already prints int64 with %lld on Windows, and its only user (i64tostr) is dead code. Collapsing it lets both builds share the gnu_printf archetype -- ms_printf would reject %zu, which this tree uses and UCRT supports, while waving through the %I64d that crashes. Verified on both platforms: clean build, and reintroducing the issue #5 specifier now fails compilation rather than shipping. main.h:410:50: error: unknown conversion type character 'I' in format main.h:410:70: error: format '%s' expects argument of type 'char*', but argument 3 has type 'long long int' That second line is the crash itself, caught at build time. Runtime checked on both: node syncs past the block from issue #5, and the repaired log lines read correctly -- before: CTxOut(nValue= 50.%08I64d, scriptPubKey=(null)) after: CTxOut(nValue=50.00000000, scriptPubKey=0xE1C7EF3E706002) Four '_WIN32_WINNT redefined' warnings remain on Windows. Silencing them means letting a system header pick the Windows API level instead of this tree, which is a separate decision from format checking.
Diffstat (limited to 'src/util.h')
-rw-r--r--src/util.h44
1 files changed, 34 insertions, 10 deletions
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;
14#define for if (false) ; else for 14#define for if (false) ; else for
15#endif 15#endif
16 16
17#ifndef _MSC_VER 17// MinGW already provides __forceinline for MSVC compatibility, so defining it
18// unconditionally warned ten times per build once -w was removed.
19#if !defined(_MSC_VER) && !defined(__forceinline)
18#define __forceinline inline 20#define __forceinline inline
19#endif 21#endif
20 22
@@ -40,17 +42,17 @@ typedef unsigned long long uint64;
40#endif 42#endif
41#define snprintf my_snprintf 43#define snprintf my_snprintf
42 44
45// One definition for every target. The MSVCRT branch that used to sit here
46// produced "I64d", which is the specifier that segfaults on glibc (issue #5) --
47// and it was pointless besides: this build targets UCRT, which accepts the C99
48// forms, and the rest of the tree already prints int64 with %lld on Windows.
49// Keeping one spelling everywhere is also what lets the format attributes below
50// use a single archetype, so a bad specifier fails the build on both platforms.
43#ifndef PRId64 51#ifndef PRId64
44#if defined(_MSC_VER) || defined(__BORLANDC__) || defined(__MSVCRT__)
45#define PRId64 "I64d"
46#define PRIu64 "I64u"
47#define PRIx64 "I64x"
48#else
49#define PRId64 "lld" 52#define PRId64 "lld"
50#define PRIu64 "llu" 53#define PRIu64 "llu"
51#define PRIx64 "llx" 54#define PRIx64 "llx"
52#endif 55#endif
53#endif
54 56
55// This is needed because the foreach macro can't get over the comma in pair<t1, t2> 57// This is needed because the foreach macro can't get over the comma in pair<t1, t2>
56#define PAIRTYPE(t1, t2) pair<t1, t2> 58#define PAIRTYPE(t1, t2) pair<t1, t2>
@@ -102,10 +104,30 @@ void LogPrintDedup(const char* category, const char* file, int line, const std::
102#define LogPrint(category, ...) \ 104#define LogPrint(category, ...) \
103 do { if (LogAcceptsCategory(category)) LogPrintDedup((category), __FILE__, __LINE__, strprintf(__VA_ARGS__)); } while (0) 105 do { if (LogAcceptsCategory(category)) LogPrintDedup((category), __FILE__, __LINE__, strprintf(__VA_ARGS__)); } while (0)
104 106
107// Every format string in this project goes through one of the four functions
108// below -- `printf` itself is remapped to OutputDebugStringF just above, and
109// snprintf to my_snprintf. None of them carried a format attribute, and GCC
110// only checks format strings for functions it knows are printf-like, so
111// -Wformat had nothing to inspect and the whole tree was unchecked. That is
112// how `%I64d` (MSVC-only, and a segfault on glibc) survived in
113// CTxOut::ToString until it crashed a node mid-sync -- see issue #5.
114//
115// gnu_printf on both platforms, deliberately. MinGW's other option, ms_printf,
116// models the legacy msvcrt: it rejects %zu, which this tree uses and UCRT
117// supports, and it accepts %I64d, which is the specifier that crashes on glibc.
118// It would give false positives where the code is right and stay silent where it
119// is dangerous. gnu_printf describes UCRT's C99 behaviour accurately, so one
120// archetype covers both builds and a bad specifier fails either one.
121#if defined(__GNUC__)
122#define BF_FORMAT(fmt, args) __attribute__((format(gnu_printf, fmt, args)))
123#else
124#define BF_FORMAT(fmt, args)
125#endif
126
105void RandAddSeed(bool fPerfmon=false); 127void RandAddSeed(bool fPerfmon=false);
106int my_snprintf(char* buffer, size_t limit, const char* format, ...); 128int my_snprintf(char* buffer, size_t limit, const char* format, ...) BF_FORMAT(3, 4);
107string strprintf(const char* format, ...); 129string strprintf(const char* format, ...) BF_FORMAT(1, 2);
108bool error(const char* format, ...); 130bool error(const char* format, ...) BF_FORMAT(1, 2);
109void PrintException(std::exception* pex, const char* pszThread); 131void PrintException(std::exception* pex, const char* pszThread);
110void ParseString(const string& str, char c, vector<string>& v); 132void ParseString(const string& str, char c, vector<string>& v);
111string FormatMoney(int64 n, bool fPlus=false); 133string FormatMoney(int64 n, bool fPlus=false);
@@ -266,6 +288,8 @@ void PrintHex(const T pbegin, const T pend, const char* pszFormat="%s", bool fSp
266// Forward declaration -- defined in main.cpp 288// Forward declaration -- defined in main.cpp
267string GetAppDir(); 289string GetAppDir();
268 290
291inline int OutputDebugStringF(const char* pszFormat, ...) BF_FORMAT(1, 2);
292
269inline int OutputDebugStringF(const char* pszFormat, ...) 293inline int OutputDebugStringF(const char* pszFormat, ...)
270{ 294{
271#if 1 // debug.log always enabled 295#if 1 // debug.log always enabled