Merge bitcoin/bitcoin#30546: util: Use consteval checked format string in FatalErrorf, LogConnectFailure

fa5bc450d5 util: Use compile-time check for LogConnectFailure (MarcoFalke)
fa7087b896 util: Use compile-time check for FatalErrorf (MarcoFalke)
faa62c0112 util: Add ConstevalFormatString (MarcoFalke)
fae7b83eb5 lint: Remove forbidden functions from lint-format-strings.py (MarcoFalke)

Pull request description:

  The `test/lint/lint-format-strings.py` was designed to count the number of format specifiers and assert that they are equal to the number of parameters passed to the format function. The goal seems reasonable, but the implementation has many problems:

  * It is written in Python, meaning that C++ code can not be parsed correctly. Currently it relies on brittle regex and string parsing.
  * Apart from the parsing errors, there are also many logic errors. For example, `count_format_specifiers` allows a mix of positional specifiers and non-positional specifiers, which can lead to runtime format bugs. Also, `count_format_specifiers` silently skipped over "special" format specifiers, which are valid in tinyformat, which again can lead to runtime format bugs being undetected.
  * The brittle logic has a history of breaking in pull requests that are otherwise fine. This causes the CI to fail and the pull request being blocked from progress until the bug in the linter is fixed, or the code is rewritten to work around the bug.
  * It is only run in the CI, or when the developer invokes the script. It would be better if the developer got the error message at compile-time, directly when writing the code.

  Fix all issues by using a `consteval` checked format string in `FatalErrorf` and `LogConnectFailure`.

  This is the first step toward https://github.com/bitcoin/bitcoin/issues/30530 and a follow-up will apply the approach to the other places.

ACKs for top commit:
  stickies-v:
    re-ACK fa5bc450d5
  l0rinc:
    ACK fa5bc450d5
  hodlinator:
    ACK fa5bc450d5
  ryanofsky:
    Code review ACK fa5bc450d5

Tree-SHA512: d6189096b16083143687ed1b1559cf4f92f97dd87bc5d00673e44f4fb9fce7bb7b215cfdfc39b6e6a24f0b75a79a03ededce966639e554f7172e1fc22cf015ae
This commit is contained in:
Ryan Ofsky
2024-09-12 12:37:36 -04:00
8 changed files with 166 additions and 20 deletions

View File

@@ -16,10 +16,7 @@ import re
import sys
FUNCTION_NAMES_AND_NUMBER_OF_LEADING_ARGUMENTS = [
'FatalErrorf,0',
'fprintf,1',
'tfm::format,1', # Assuming tfm::::format(std::ostream&, ...
'LogConnectFailure,1',
'LogError,0',
'LogWarning,0',
'LogInfo,0',
@@ -27,14 +24,7 @@ FUNCTION_NAMES_AND_NUMBER_OF_LEADING_ARGUMENTS = [
'LogTrace,1',
'LogPrintf,0',
'LogPrintLevel,2',
'printf,0',
'snprintf,2',
'sprintf,1',
'strprintf,0',
'vfprintf,1',
'vprintf,1',
'vsnprintf,1',
'vsprintf,1',
'WalletLogPrintf,0',
]
RUN_LINT_FILE = 'test/lint/run-lint-format-strings.py'

View File

@@ -13,10 +13,6 @@ import re
import sys
FALSE_POSITIVES = [
("src/dbwrapper.cpp", "vsnprintf(p, limit - p, format, backup_ap)"),
("src/index/base.cpp", "FatalErrorf(const char* fmt, const Args&... args)"),
("src/index/base.h", "FatalErrorf(const char* fmt, const Args&... args)"),
("src/netbase.cpp", "LogConnectFailure(bool manual_connection, const char* fmt, const Args&... args)"),
("src/clientversion.cpp", "strprintf(_(COPYRIGHT_HOLDERS).translated, COPYRIGHT_HOLDERS_SUBSTITUTION)"),
("src/test/translation_tests.cpp", "strprintf(format, arg)"),
("src/validationinterface.cpp", "LogDebug(BCLog::VALIDATION, fmt \"\\n\", __VA_ARGS__)"),