Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

kernel: Handle fatal errors through return values #29642

Draft
wants to merge 27 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
440f674
refactor: Drop util::Result operator=
ryanofsky Sep 15, 2022
882577b
refactor: Avoid copying util::Result values
ryanofsky Sep 6, 2023
93cee98
refactor: Add util::Result failure values
ryanofsky Jul 18, 2022
015b011
refactor: Use util::Result class in LoadChainstate and VerifyLoadedCh…
ryanofsky Jul 21, 2022
c7d4f43
refactor: Add util::Result multiple error and warning messages
ryanofsky Jul 18, 2022
5e38b42
test: add static test for util::Result memory usage
ryanofsky Jul 20, 2023
e5936fe
Add bitcoin-fatal-error clang-tidy check
TheCharlatan Feb 29, 2024
b16787d
kernel: Handle ImportBlocks fatal errors with Result<T, FatalError>
TheCharlatan Jun 15, 2023
e641470
refactor: Replace call to fatalError with AbortNode in init
TheCharlatan Feb 22, 2024
cf55203
kernel: Handle LoadExternalBlockFile fatal errors with Result<T, Fata…
TheCharlatan Jun 15, 2023
1850f3b
kernel: Handle ActivateSnapshot fatal errors with Result<T, FatalError>
TheCharlatan Jun 15, 2023
93b9c3b
kernel: Add [[nodiscard]] attribute for LoadChainstate
TheCharlatan Jun 15, 2023
4bfa9da
kernel: Handle ValidatedSnapshotCleanup fatal errors with Result<T, F…
TheCharlatan Jun 15, 2023
4e2de7b
kernel: Handle MaybeCompleteSnapshotValidation fatal errors with Resu…
TheCharlatan Jun 16, 2023
0758b72
kernel: Handle AcceptBlock fatal errors with Result<T, FatalError>
TheCharlatan Jun 16, 2023
6eceef8
kernel: Handle ActivateBestChainStep fatal errors with Result<T, Fata…
TheCharlatan Feb 23, 2024
11a0d3f
kernel: Handle ConnectTip fatal errors with Result<T, FatalError>
TheCharlatan Feb 23, 2024
42943e5
kernel: Handle FlushStateToDisk fatal errors with Result<T, FatalError>
TheCharlatan Jun 17, 2023
a555387
kernel: Handle ConnectBlock fatal errors with Result<T, FatalError>
TheCharlatan Feb 21, 2024
16dee3c
kernel: Handle SaveBlockToDisk fatal errors with Result<T, FatalError>
TheCharlatan Mar 11, 2024
f9b1773
kernel: Handle FindBlockPos fatal errors with Result<T, FatalError>
TheCharlatan Mar 11, 2024
57b7430
kernel: Handle FlushBlockFile fatal errors with Result<T, FatalError>
TheCharlatan Mar 11, 2024
738e850
kernel: Handle WriteUndoDataForBlock fatal errors with Result<T, Fata…
TheCharlatan Mar 12, 2024
0273acf
kernel: Handle FlushUndoFile fatal errors with Result<T, FatalError>
TheCharlatan Jun 18, 2023
76f1420
kernel: Handle FindUndoPos fatal errors with Result<T, FatalError>
TheCharlatan Jun 18, 2023
e2c336b
kernel: Handle LoadBlockIndex fatal errors with Result<T, FatalError>
TheCharlatan Feb 24, 2024
bd0a9bb
Remove flushError, fatalError from kernel notifications
TheCharlatan Feb 24, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 3 additions & 2 deletions contrib/devtools/bitcoin-tidy/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ find_program(CLANG_TIDY_EXE NAMES "clang-tidy-${LLVM_VERSION_MAJOR}" "clang-tidy
message(STATUS "Found LLVM ${LLVM_PACKAGE_VERSION}")
message(STATUS "Found clang-tidy: ${CLANG_TIDY_EXE}")

add_library(bitcoin-tidy MODULE bitcoin-tidy.cpp logprintf.cpp)
add_library(bitcoin-tidy MODULE bitcoin-tidy.cpp logprintf.cpp fatal_error.cpp)
target_include_directories(bitcoin-tidy SYSTEM PRIVATE ${LLVM_INCLUDE_DIRS})

# Disable RTTI and exceptions as necessary
Expand Down Expand Up @@ -58,8 +58,9 @@ else()
endif()

# Create a dummy library that runs clang-tidy tests as a side-effect of building
add_library(bitcoin-tidy-tests OBJECT EXCLUDE_FROM_ALL example_logprintf.cpp)
add_library(bitcoin-tidy-tests OBJECT EXCLUDE_FROM_ALL example_logprintf.cpp example_fatal_error.cpp)
add_dependencies(bitcoin-tidy-tests bitcoin-tidy)
target_include_directories(bitcoin-tidy-tests PRIVATE "${PROJECT_SOURCE_DIR}/../../../src")

set_target_properties(bitcoin-tidy-tests PROPERTIES CXX_CLANG_TIDY "${CLANG_TIDY_COMMAND}")

Expand Down
2 changes: 2 additions & 0 deletions contrib/devtools/bitcoin-tidy/bitcoin-tidy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.

#include "fatal_error.h"
#include "logprintf.h"

#include <clang-tidy/ClangTidyModule.h>
Expand All @@ -13,6 +14,7 @@ class BitcoinModule final : public clang::tidy::ClangTidyModule
void addCheckFactories(clang::tidy::ClangTidyCheckFactories& CheckFactories) override
{
CheckFactories.registerCheck<bitcoin::LogPrintfCheck>("bitcoin-unterminated-logprintf");
CheckFactories.registerCheck<bitcoin::FatalErrorCheck>("bitcoin-fatal-error");
}
};

Expand Down
237 changes: 237 additions & 0 deletions contrib/devtools/bitcoin-tidy/example_fatal_error.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,237 @@
// Copyright (c) 2024-present Bitcoin Developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.

#include "util/result.h"

#include <functional>
#include <string>
#include <thread>

// Test for bitcoin-fatal-error

using util::Result;

enum class FatalError
{
Fatal
};

enum class ChainstateLoadError
{
Error
};

Result<void, FatalError> void_fatal_func()
{
return {};
}

Result<bool, FatalError> bool_fatal_func()
{
return true;
}

Result<int, FatalError> int_fatal_func()
{
return 1;
}

class Test {
public:
Result<void, FatalError> fatal_func() {
return void_fatal_func();
}
};

template<typename T>
void CheckFatalBad(Result<T, FatalError> fatal, int i, int j) {
return;
}

template<typename T>
void CheckFatal(Result<T, FatalError> fatal, int i, int j) {
return;
}

void TraceThread(std::string name, std::function<void()> thread_func) {
thread_func();
}

Result<void, FatalError> good_fatal_func_1()
{
auto lambda_1 = ([&]() {
return void_fatal_func();
});

auto lambda_2 = ([&]() {
Result<void, FatalError> result{};
result.Set(void_fatal_func());
});

auto lambda_3 = ([&]() {
Result<void, FatalError> result{};
result.MoveMessages(void_fatal_func());
});

auto lambda_4 = ([&]() {
Result<void, FatalError> result{};
auto res{void_fatal_func()};
result.MoveMessages(res);
});
return {};
}

Result<void, FatalError> good_fatal_func_2()
{
Result<void, FatalError> result{};
auto res{void_fatal_func()};
result.MoveMessages(res);
return result;
}

Result<void, FatalError> good_fatal_func_3()
{
auto res{void_fatal_func()};
return res;
}

Result<void, FatalError> good_fatal_func_4()
{
Result<void, FatalError> result{};
result.Set(void_fatal_func());
result.MoveMessages(void_fatal_func());
if (result)
{
return void_fatal_func();
}
return result;
}

Result<void, FatalError> good_fatal_func_5()
{
Result<void, FatalError> result{};
if (auto res{void_fatal_func()}; !res)
{
result.MoveMessages(res);
return result;
}
return {};
}

Result<int, FatalError> good_fatal_func_6()
{
Test testy{};
auto res{testy.fatal_func()};
Result<void, FatalError> result{};
result.MoveMessages(res);
return 0;
}

Result<int, FatalError> good_fatal_func_7()
{
Result<int, FatalError> result{1};
result.Set([&]()
{
return int_fatal_func();
}());
return int_fatal_func();
}

Result<void, FatalError> good_fatal_func_8()
{
Result<void, FatalError> result{};
result.Set(void_fatal_func());
auto res{void_fatal_func()};
if (!res) {
result.Set(std::move(res));
} else {
result.MoveMessages(res);
}
return result;
}

void good_fatal_func_9()
{
CheckFatal(bool_fatal_func(), 0, 0);
}

void good_fatal_func_10()
{
[&]()
{
CheckFatal(void_fatal_func(), 0, 0);
}();
}

int good_fatal_func_11()
{
int i = 0;
if (1) {
i += 1;
}
auto handle = std::thread(&TraceThread, "", [] {
CheckFatal(void_fatal_func(), 0, 0);
});
return i;
}

Result<int, ChainstateLoadError> good_fatal_func_12()
{
bool_fatal_func();
return {0};
}

void bad_fatal_func_1()
{
void_fatal_func();
auto testy{void_fatal_func()};
}

void bad_fatal_func_2()
{
Test testy{};
testy.fatal_func();
}

void bad_fatal_func_3()
{
[&]()
{
void_fatal_func();
}();
}

void bad_fatal_func_4()
{
CheckFatalBad(void_fatal_func(), 0, 0);
}

void bad_fatal_func_5()
{
CheckFatal(void_fatal_func(), 0, 0);
void_fatal_func();
}

void bad_fatal_func_6()
{
int zero{0};
void_fatal_func();
CheckFatal(void_fatal_func(), 0, 0);
}

Result<int, FatalError> bad_fatal_func_7()
{
[&]()
{
return void_fatal_func();
}();
return {0};
}

Result<int, FatalError> bad_fatal_func_8()
{
auto res{void_fatal_func()};
if (!res) return {util::Error{}, util::MoveMessages(res), res.GetFailure()};
return 0;
}