Skip to content

Commit

Permalink
util: Add Assume() identity function
Browse files Browse the repository at this point in the history
Summary:
```
This is needed for #20138. Please refer to the added documentation for
motivation.
```

Backport of [[bitcoin/bitcoin#20255 | core#20255]].

Depends on D9681.

Comes with a linter fix for an edge case where a macro defining
`((void)(val))` would be linted as `(()(val))`.

Ref T1611.

Test Plan:
With and without debug:
  ninja all check

Reviewers: #bitcoin_abc, PiRK

Reviewed By: #bitcoin_abc, PiRK

Maniphest Tasks: T1611

Differential Revision: https://reviews.bitcoinabc.org/D9680
  • Loading branch information
practicalswift authored and Fabcien committed Jun 15, 2021
1 parent bf30e25 commit 12031e5
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 8 deletions.
2 changes: 1 addition & 1 deletion arcanist/linter/CppVoidParameterLinter.php
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ public function lintPath($path) {
$absPath = Filesystem::resolvePath($path, $this->getProjectRoot());
$fileContent = Filesystem::readFile($absPath);

if (preg_match_all('/[^\s{]+\s?\(void\)/', $fileContent, $voidParameters,
if (preg_match_all('/[^\s{(]+\s?\(void\)/', $fileContent, $voidParameters,
PREG_OFFSET_CAPTURE)) {
foreach ($voidParameters[0] as $voidParameter) {
list($function, $offset) = $voidParameter;
Expand Down
27 changes: 27 additions & 0 deletions doc/developer-notes.md
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,33 @@ The `-DCMAKE_BUILD_TYPE=Debug` cmake option adds `-DDEBUG_LOCKORDER` to the
compiler flags. This inserts run-time checks to keep track of which locks are
held, and adds warnings to the debug.log file if inconsistencies are detected.

### Assertions and Checks

The util file `src/util/check.h` offers helpers to protect against coding and
internal logic bugs. They must never be used to validate user, network or any
other input.

* `assert` or `Assert` should be used to document assumptions when any
violation would mean that it is not safe to continue program execution. The
code is always compiled with assertions enabled.
- For example, a nullptr dereference or any other logic bug in validation
code means the program code is faulty and must terminate immediately.
* `CHECK_NONFATAL` should be used for recoverable internal logic bugs. On
failure, it will throw an exception, which can be caught to recover from the
error.
- For example, a nullptr dereference or any other logic bug in RPC code
means that the RPC code is faulty and can not be executed. However, the
logic bug can be shown to the user and the program can continue to run.
* `Assume` should be used to document assumptions when program execution can
safely continue even if the assumption is violated. In debug builds it
behaves like `Assert`/`assert` to notify developers and testers about
nonfatal errors. In production it doesn't warn or log anything, though the
expression is always evaluated.
- For example it can be assumed that a variable is only initialized once,
but a failed assumption does not result in a fatal bug. A failed
assumption may or may not result in a slightly degraded user experience,
but it is safe to continue program execution.

### Valgrind suppressions file

Valgrind is a programming tool for memory debugging, memory leak detection, and
Expand Down
5 changes: 2 additions & 3 deletions src/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -112,9 +112,8 @@ foreach(LANGUAGE C CXX)
add_compile_options_to_configuration_for_language(Debug ${LANGUAGE} ${COMPILER_DEBUG_LEVEL})
endforeach()

# Define the debugging symbols DEBUG and DEBUG_LOCKORDER when the Debug build
# type is selected.
add_compile_definitions_to_configuration(Debug DEBUG DEBUG_LOCKORDER)
# Define some debugging symbols when the Debug build type is selected.
add_compile_definitions_to_configuration(Debug DEBUG DEBUG_LOCKORDER ABORT_ON_FAILED_ASSUME)

# Add -ftrapv when building in Debug
add_compile_options_to_configuration(Debug -ftrapv)
Expand Down
23 changes: 19 additions & 4 deletions src/util/check.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,18 +47,33 @@ class NonFatalCheckError : public std::runtime_error {
#error "Cannot compile without assertions!"
#endif

/** Helper for Assert(). TODO remove in C++14 and replace
* `decltype(get_pure_r_value(val))` with `T` (templated lambda) */
/** Helper for Assert() */
template <typename T> T get_pure_r_value(T &&val) {
return std::forward<T>(val);
}

/** Identity function. Abort if the value compares equal to zero */
#define Assert(val) \
[&]() -> decltype(get_pure_r_value(val)) { \
([&]() -> decltype(get_pure_r_value(val)) { \
auto &&check = (val); \
assert(#val &&check); \
return std::forward<decltype(get_pure_r_value(val))>(check); \
}()
}())

/**
* Assume is the identity function.
*
* - Should be used to run non-fatal checks. In debug builds it behaves like
* Assert()/assert() to notify developers and testers about non-fatal errors.
* In production it doesn't warn or log anything.
* - For fatal errors, use Assert().
* - For non-fatal errors in interactive sessions (e.g. RPC or command line
* interfaces), CHECK_NONFATAL() might be more appropriate.
*/
#ifdef ABORT_ON_FAILED_ASSUME
#define Assume(val) Assert(val)
#else
#define Assume(val) ((void)(val))
#endif

#endif // BITCOIN_UTIL_CHECK_H

0 comments on commit 12031e5

Please sign in to comment.