Merge bitcoin/bitcoin#33785: util: Allow Assert (et al.) in contexts without __func__

fad6efd3bef1d123f806d492f019e29530b03a5e refactor: Use STR_INTERNAL_BUG macro where possible (MarcoFalke)
fada379589a17e86396aa7c2ce458ff2ff602b84 doc: Remove unused bugprone-lambda-function-name suppression (MarcoFalke)
fae1d99651e29341e486a10e6340335c71a2144e refactor: Use const reference to std::source_location (MarcoFalke)
fa5fbcd61563942122623fe2840a677853081990 util: Allow Assert() in contexts without __func__ (MarcoFalke)

Pull request description:

  Without this, compile warnings could be hit about `__func__` being only valid inside functions.

  ```
  warning: predefined identifier is only valid inside function [-Wpredefined-identifier-outside-function] note: expanded from macro Assert
    115 | #define Assert(val) inline_assertion_check<true>(val, __FILE__, __LINE__, __func__, #val)
        |                                                                           ^
  ```

  Ref https://github.com/bitcoin/bitcoin/pull/32740#discussion_r2486258473

  This also introduces a slight behaviour change, because `std::source_location::function_name` usually includes the entire function signature instead of just the name.

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

Tree-SHA512: e78a2d812d5ae22e45c93db1661dafbcd22ef209b3d8d8d5f2ac514e92fd19a17c3f0a5db2ef5e7748aa2083b10c0465326eb36812e6a80e238972facd2c7e98
This commit is contained in:
merge-script 2025-11-10 11:56:09 +00:00
commit 490cb056f6
No known key found for this signature in database
GPG Key ID: 2EEB9F5CC09526C1
6 changed files with 42 additions and 51 deletions

View File

@ -19,4 +19,10 @@ export GOAL="install"
export CI_LIMIT_STACK_SIZE=1
# -Wno-psabi is to disable ABI warnings: "note: parameter passing for argument of type ... changed in GCC 7.1"
# This could be removed once the ABI change warning does not show up by default
export BITCOIN_CONFIG="-DREDUCE_EXPORTS=ON -DCMAKE_CXX_FLAGS='-Wno-psabi -Wno-error=maybe-uninitialized'"
#
# -Wno-error=dangling-reference helps to work around a GCC 13.1 false-positive,
# fixed in later versions.
export BITCOIN_CONFIG=" \
-DREDUCE_EXPORTS=ON \
-DCMAKE_CXX_FLAGS='-Wno-psabi -Wno-error=dangling-reference -Wno-error=maybe-uninitialized' \
"

View File

@ -13,5 +13,8 @@ export PACKAGES="g++-mingw-w64-x86-64-posix nsis"
export RUN_UNIT_TESTS=false
export RUN_FUNCTIONAL_TESTS=false
export GOAL="deploy"
# -Wno-error=dangling-reference helps to work around a GCC 13.1 false-positive,
# fixed in later versions.
export BITCOIN_CONFIG="-DREDUCE_EXPORTS=ON -DBUILD_GUI_TESTS=OFF -DBUILD_KERNEL_LIB=ON -DBUILD_KERNEL_TEST=ON \
-DCMAKE_CXX_FLAGS='-Wno-error=maybe-uninitialized'"
-DCMAKE_CXX_FLAGS='-Wno-error=dangling-reference -Wno-error=maybe-uninitialized' \
"

View File

@ -1,10 +1,8 @@
// Copyright (c) 2009-2010 Satoshi Nakamoto
// Copyright (c) 2009-2022 The Bitcoin Core developers
// Copyright (c) 2009-present The Bitcoin Core developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
#include <bitcoin-build-config.h> // IWYU pragma: keep
#include <rest.h>
#include <blockfilter.h>
@ -87,11 +85,7 @@ static NodeContext* GetNodeContext(const std::any& context, HTTPRequest* req)
{
auto node_context = util::AnyPtr<NodeContext>(context);
if (!node_context) {
RESTERR(req, HTTP_INTERNAL_SERVER_ERROR,
strprintf("%s:%d (%s)\n"
"Internal bug detected: Node context not found!\n"
"You may report this issue here: %s\n",
__FILE__, __LINE__, __func__, CLIENT_BUGREPORT));
RESTERR(req, HTTP_INTERNAL_SERVER_ERROR, STR_INTERNAL_BUG("Node context not found!"));
return nullptr;
}
return node_context;
@ -125,11 +119,7 @@ static ChainstateManager* GetChainman(const std::any& context, HTTPRequest* req)
{
auto node_context = util::AnyPtr<NodeContext>(context);
if (!node_context || !node_context->chainman) {
RESTERR(req, HTTP_INTERNAL_SERVER_ERROR,
strprintf("%s:%d (%s)\n"
"Internal bug detected: Chainman disabled or instance not found!\n"
"You may report this issue here: %s\n",
__FILE__, __LINE__, __func__, CLIENT_BUGREPORT));
RESTERR(req, HTTP_INTERNAL_SERVER_ERROR, STR_INTERNAL_BUG("Chainman disabled or instance not found!"));
return nullptr;
}
return node_context->chainman.get();

View File

@ -2,10 +2,7 @@
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
#include <bitcoin-build-config.h> // IWYU pragma: keep
#include <chain.h>
#include <clientversion.h>
#include <common/args.h>
#include <common/messages.h>
#include <common/types.h>
@ -678,10 +675,8 @@ UniValue RPCHelpMan::HandleRequest(const JSONRPCRequest& request) const
mismatch.size() == 1 ? mismatch[0].write(4) :
mismatch.write(4)};
throw std::runtime_error{
strprintf("Internal bug detected: RPC call \"%s\" returned incorrect type:\n%s\n%s %s\nPlease report this issue here: %s\n",
m_name, explain,
CLIENT_NAME, FormatFullVersion(),
CLIENT_BUGREPORT)};
STR_INTERNAL_BUG(strprintf("RPC call \"%s\" returned incorrect type:\n%s", m_name, explain)),
};
}
}
return ret;

View File

@ -1,4 +1,4 @@
// Copyright (c) 2022 The Bitcoin Core developers
// Copyright (c) 2022-present The Bitcoin Core developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
@ -11,30 +11,32 @@
#include <cstdio>
#include <cstdlib>
#include <source_location>
#include <string>
#include <string_view>
std::string StrFormatInternalBug(std::string_view msg, std::string_view file, int line, std::string_view func)
std::string StrFormatInternalBug(std::string_view msg, const std::source_location& loc)
{
return strprintf("Internal bug detected: %s\n%s:%d (%s)\n"
"%s %s\n"
"Please report this issue here: %s\n",
msg, file, line, func, CLIENT_NAME, FormatFullVersion(), CLIENT_BUGREPORT);
msg, loc.file_name(), loc.line(), loc.function_name(),
CLIENT_NAME, FormatFullVersion(), CLIENT_BUGREPORT);
}
NonFatalCheckError::NonFatalCheckError(std::string_view msg, std::string_view file, int line, std::string_view func)
: std::runtime_error{StrFormatInternalBug(msg, file, line, func)}
NonFatalCheckError::NonFatalCheckError(std::string_view msg, const std::source_location& loc)
: std::runtime_error{StrFormatInternalBug(msg, loc)}
{
}
bool g_detail_test_only_CheckFailuresAreExceptionsNotAborts{false};
void assertion_fail(std::string_view file, int line, std::string_view func, std::string_view assertion)
void assertion_fail(const std::source_location& loc, std::string_view assertion)
{
if (g_detail_test_only_CheckFailuresAreExceptionsNotAborts) {
throw NonFatalCheckError{assertion, file, line, func};
throw NonFatalCheckError{assertion, loc};
}
auto str = strprintf("%s:%s %s: Assertion `%s' failed.\n", file, line, func, assertion);
auto str = strprintf("%s:%s %s: Assertion `%s' failed.\n", loc.file_name(), loc.line(), loc.function_name(), assertion);
fwrite(str.data(), 1, str.size(), stderr);
std::abort();
}

View File

@ -1,4 +1,4 @@
// Copyright (c) 2019-2022 The Bitcoin Core developers
// Copyright (c) 2019-present The Bitcoin Core developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
@ -9,6 +9,7 @@
#include <atomic>
#include <cassert> // IWYU pragma: export
#include <source_location>
#include <stdexcept>
#include <string>
#include <string_view>
@ -52,26 +53,26 @@ struct test_only_CheckFailuresAreExceptionsNotAborts {
~test_only_CheckFailuresAreExceptionsNotAborts() { g_detail_test_only_CheckFailuresAreExceptionsNotAborts = false; };
};
std::string StrFormatInternalBug(std::string_view msg, std::string_view file, int line, std::string_view func);
std::string StrFormatInternalBug(std::string_view msg, const std::source_location& loc);
class NonFatalCheckError : public std::runtime_error
{
public:
NonFatalCheckError(std::string_view msg, std::string_view file, int line, std::string_view func);
NonFatalCheckError(std::string_view msg, const std::source_location& loc);
};
/** Internal helper */
void assertion_fail(std::string_view file, int line, std::string_view func, std::string_view assertion);
void assertion_fail(const std::source_location& loc, std::string_view assertion);
/** Helper for CHECK_NONFATAL() */
template <typename T>
T&& inline_check_non_fatal(LIFETIMEBOUND T&& val, const char* file, int line, const char* func, const char* assertion)
T&& inline_check_non_fatal(LIFETIMEBOUND T&& val, const std::source_location& loc, std::string_view assertion)
{
if (!val) {
if constexpr (G_ABORT_ON_FAILED_ASSUME) {
assertion_fail(file, line, func, assertion);
assertion_fail(loc, assertion);
}
throw NonFatalCheckError{assertion, file, line, func};
throw NonFatalCheckError{assertion, loc};
}
return std::forward<T>(val);
}
@ -82,20 +83,17 @@ T&& inline_check_non_fatal(LIFETIMEBOUND T&& val, const char* file, int line, co
/** Helper for Assert()/Assume() */
template <bool IS_ASSERT, typename T>
constexpr T&& inline_assertion_check(LIFETIMEBOUND T&& val, [[maybe_unused]] const char* file, [[maybe_unused]] int line, [[maybe_unused]] const char* func, [[maybe_unused]] const char* assertion)
constexpr T&& inline_assertion_check(LIFETIMEBOUND T&& val, [[maybe_unused]] const std::source_location& loc, [[maybe_unused]] std::string_view assertion)
{
if (IS_ASSERT || std::is_constant_evaluated() || G_ABORT_ON_FAILED_ASSUME) {
if (!val) {
assertion_fail(file, line, func, assertion);
assertion_fail(loc, assertion);
}
}
return std::forward<T>(val);
}
// All macros may use __func__ inside a lambda, so put them under nolint.
// NOLINTBEGIN(bugprone-lambda-function-name)
#define STR_INTERNAL_BUG(msg) StrFormatInternalBug((msg), __FILE__, __LINE__, __func__)
#define STR_INTERNAL_BUG(msg) StrFormatInternalBug((msg), std::source_location::current())
/**
* Identity function. Throw a NonFatalCheckError when the condition evaluates to false
@ -109,10 +107,10 @@ constexpr T&& inline_assertion_check(LIFETIMEBOUND T&& val, [[maybe_unused]] con
* caller, which can then report the issue to the developers.
*/
#define CHECK_NONFATAL(condition) \
inline_check_non_fatal(condition, __FILE__, __LINE__, __func__, #condition)
inline_check_non_fatal(condition, std::source_location::current(), #condition)
/** Identity function. Abort if the value compares equal to zero */
#define Assert(val) inline_assertion_check<true>(val, __FILE__, __LINE__, __func__, #val)
#define Assert(val) inline_assertion_check<true>(val, std::source_location::current(), #val)
/**
* Assume is the identity function.
@ -124,16 +122,13 @@ constexpr T&& inline_assertion_check(LIFETIMEBOUND T&& val, [[maybe_unused]] con
* - For non-fatal errors in interactive sessions (e.g. RPC or command line
* interfaces), CHECK_NONFATAL() might be more appropriate.
*/
#define Assume(val) inline_assertion_check<false>(val, __FILE__, __LINE__, __func__, #val)
#define Assume(val) inline_assertion_check<false>(val, std::source_location::current(), #val)
/**
* NONFATAL_UNREACHABLE() is a macro that is used to mark unreachable code. It throws a NonFatalCheckError.
*/
#define NONFATAL_UNREACHABLE() \
throw NonFatalCheckError( \
"Unreachable code reached (non-fatal)", __FILE__, __LINE__, __func__)
// NOLINTEND(bugprone-lambda-function-name)
#define NONFATAL_UNREACHABLE() \
throw NonFatalCheckError { "Unreachable code reached (non-fatal)", std::source_location::current() }
#if defined(__has_feature)
# if __has_feature(address_sanitizer)