Merge remote-tracking branch 'upstream/develop' into pr/base58-tests

* upstream/develop:
  test: Check for some unlikely null dereferences in tests (#5004)
  Add Antithesis intrumentation (#5042)

CMakeLists.txt

@@ -73,6 +73,7 @@ set(SECP256K1_INSTALL TRUE)
 add_subdirectory(external/secp256k1)
 add_library(secp256k1::secp256k1 ALIAS secp256k1)
 add_subdirectory(external/ed25519-donna)
+add_subdirectory(external/antithesis-sdk)
 find_package(gRPC REQUIRED)
 find_package(lz4 REQUIRED)
 # Target names with :: are not allowed in a generator expression.

CONTRIBUTING.md

@@ -343,6 +343,68 @@ pip3 install pre-commit
 pre-commit install
 ```
 
+## Contracts and instrumentation
+
+We are using [Antithesis](https://antithesis.com/) for continuous fuzzing,
+and keep a copy of [Antithesis C++ SDK](https://github.com/antithesishq/antithesis-sdk-cpp/)
+in `external/antithesis-sdk`. One of the aims of fuzzing is to identify bugs
+by finding external conditions which cause contracts violations inside `rippled`.
+The contracts are expressed as `ASSERT` or `UNREACHABLE` (defined in
+`include/xrpl/beast/utility/instrumentation.h`), which are effectively (outside
+of Antithesis) wrappers for `assert(...)` with added name. The purpose of name
+is to provide contracts with stable identity which does not rely on line numbers.
+
+When `rippled` is built with the Antithesis instrumentation enabled
+(using `voidstar` CMake option) and ran on the Antithesis platform, the
+contracts become
+[test properties](https://antithesis.com/docs/using_antithesis/properties.html);
+otherwise they are just like a regular `assert`.
+To learn more about Antithesis, see
+[How Antithesis Works](https://antithesis.com/docs/introduction/how_antithesis_works.html)
+and [C++ SDK](https://antithesis.com/docs/using_antithesis/sdk/cpp/overview.html#)
+
+We continue to use the old style `assert` or `assert(false)` in certain
+locations, where the reporting of contract violations on the Antithesis
+platform is either not possible or not useful.
+
+For this reason:
+* The locations where `assert` or `assert(false)` contracts should continue to be used:
+  * `constexpr` functions
+  * unit tests i.e. files under `src/test`
+  * unit tests-related modules (files under `beast/test` and `beast/unit_test`)
+* Outside of the listed locations, do not use `assert`; use `ASSERT` instead,
+  giving it unique name, with the short description of the contract.
+* Outside of the listed locations, do not use `assert(false)`; use
+  `UNREACHABLE` instead, giving it unique name, with the description of the
+  condition being violated
+* The contract name should start with a full name (including scope) of the
+  function, optionally a named lambda, followed by a colon ` : ` and a brief
+  (typically at most five words) description. `UNREACHABLE` contracts
+  can use slightly longer descriptions. If there are multiple overloads of the
+  function, use common sense to balance both brevity and unambiguity of the
+  function name. NOTE: the purpose of name is to provide stable means of
+  unique identification of every contract; for this reason try to avoid elements
+  which can change in some obvious refactors or when reinforcing the condition.
+* Contract description typically (except for `UNREACHABLE`) should describe the
+  _expected_ condition, as in "I assert that _expected_ is true".
+* Contract description for `UNREACHABLE` should describe the _unexpected_
+  situation which caused the line to have been reached.
+* Example good name for an
+  `UNREACHABLE` macro `"Json::operator==(Value, Value) : invalid type"`; example
+  good name for an `ASSERT` macro `"Json::Value::asCString : valid type"`.
+* Example **bad** name
+  `"RFC1751::insert(char* s, int x, int start, int length) : length is greater than or equal zero"`
+  (missing namespace, unnecessary full function signature, description too verbose).
+  Good name: `"ripple::RFC1751::insert : minimum length"`.
+* In **few** well-justified cases a non-standard name can be used, in which case a
+  comment should be placed to explain the rationale (example in `contract.cpp`)
+* Do **not** rename a contract without a good reason (e.g. the name no longer
+  reflects the location or the condition being checked)
+* Do not use `std::unreachable`
+* Do not put contracts where they can be violated by an external condition
+  (e.g. timing, data payload before mandatory validation etc.) as this creates
+  bogus bug reports (and causes crashes of Debug builds)
+
 ## Unit Tests
 To execute all unit tests:
 

cmake/RippledCompiler.cmake

@@ -120,7 +120,7 @@ else ()
   target_link_libraries (common
     INTERFACE
       -rdynamic
-      $<$<BOOL:${is_linux}>:-Wl,-z,relro,-z,now>
+      $<$<BOOL:${is_linux}>:-Wl,-z,relro,-z,now,--build-id>
       # link to static libc/c++ iff:
       #   * static option set and
       #   * NOT APPLE (AppleClang does not support static libc/c++) and
@@ -131,6 +131,17 @@ else ()
       >)
 endif ()
 
+# Antithesis instrumentation will only be built and deployed using machines running Linux.
+if (voidstar)
+  if (NOT CMAKE_BUILD_TYPE STREQUAL "Debug")
+    message(FATAL_ERROR "Antithesis instrumentation requires Debug build type, aborting...")
+  elseif (NOT is_linux)
+    message(FATAL_ERROR "Antithesis instrumentation requires Linux, aborting...")
+  elseif (NOT (is_clang AND CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL 16.0))
+    message(FATAL_ERROR "Antithesis instrumentation requires Clang version 16 or later, aborting...")
+  endif ()
+endif ()
+
 if (use_mold)
   # use mold linker if available
   execute_process (

cmake/RippledCore.cmake

@@ -74,6 +74,7 @@ target_compile_definitions(xrpl.libxrpl
 target_compile_options(xrpl.libxrpl
   PUBLIC
     $<$<BOOL:${is_gcc}>:-Wno-maybe-uninitialized>
+    $<$<BOOL:${voidstar}>:-DENABLE_VOIDSTAR>
 )
 
 target_link_libraries(xrpl.libxrpl
@@ -89,6 +90,7 @@ target_link_libraries(xrpl.libxrpl
     secp256k1::secp256k1
     xrpl.libpb
     xxHash::xxhash
+    $<$<BOOL:${voidstar}>:antithesis-sdk-cpp>
 )
 
 if(xrpld)
@@ -129,6 +131,19 @@ if(xrpld)
     target_compile_definitions(rippled PRIVATE RIPPLED_RUNNING_IN_CI)
   endif ()
 
+  if(voidstar)
+    target_compile_options(rippled
+      PRIVATE
+        -fsanitize-coverage=trace-pc-guard
+    )
+    # rippled requires access to antithesis-sdk-cpp implementation file
+    # antithesis_instrumentation.h, which is not exported as INTERFACE
+    target_include_directories(rippled
+      PRIVATE
+        ${CMAKE_SOURCE_DIR}/external/antithesis-sdk
+    )
+  endif()
+
   # any files that don't play well with unity should be added here
   if(tests)
     set_source_files_properties(

cmake/RippledInstall.cmake

@@ -10,6 +10,7 @@ install (
     ripple_boost
     xrpl.libpb
     xrpl.libxrpl
+    antithesis-sdk-cpp
   EXPORT RippleExports
   LIBRARY DESTINATION lib
   ARCHIVE DESTINATION lib

cmake/RippledSettings.cmake

@@ -18,6 +18,9 @@ if(unity)
     set(CMAKE_UNITY_BUILD_BATCH_SIZE 15 CACHE STRING "")
   endif()
 endif()
+if(is_clang AND is_linux)
+  option(voidstar "Enable Antithesis instrumentation." OFF)
+endif()
 if(is_gcc OR is_clang)
   option(coverage "Generates coverage info." OFF)
   option(profile "Add profiling flags" OFF)

external/README.md

@@ -6,6 +6,7 @@ The Conan recipes include patches we have not yet pushed upstream.
 
 | Folder          | Upstream                                     | Description |
 |:----------------|:---------------------------------------------|:------------|
+| `antithesis-sdk`| [Project](https://github.com/antithesishq/antithesis-sdk-cpp/) | [Antithesis](https://antithesis.com/docs/using_antithesis/sdk/cpp/overview.html) SDK for C++ |
 | `ed25519-donna` | [Project](https://github.com/floodyberry/ed25519-donna) | [Ed25519](http://ed25519.cr.yp.to/) digital signatures |
 | `rocksdb`       | [Recipe](https://github.com/conan-io/conan-center-index/tree/master/recipes/rocksdb) | Fast key/value database. (Supports rotational disks better than NuDB.) |
 | `secp256k1`     | [Project](https://github.com/bitcoin-core/secp256k1)    | ECDSA digital signatures using the **secp256k1** curve |

external/antithesis-sdk/.clang-format

@@ -0,0 +1,3 @@
+---
+DisableFormat: true
+SortIncludes: false

external/antithesis-sdk/CMakeLists.txt

@@ -0,0 +1,17 @@
+cmake_minimum_required(VERSION 3.25)
+
+# Note, version set explicitly by rippled project
+project(antithesis-sdk-cpp VERSION 0.4.2 LANGUAGES CXX)
+
+add_library(antithesis-sdk-cpp INTERFACE antithesis_sdk.h)
+
+# Note, both sections below created by rippled project
+target_include_directories(antithesis-sdk-cpp INTERFACE
+  $<INSTALL_INTERFACE:${CMAKE_INSTALL_INCLUDEDIR}>
+  $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}>
+)
+
+install(
+  FILES antithesis_sdk.h
+  DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}"
+)

external/antithesis-sdk/LICENSE

@@ -0,0 +1,21 @@
+MIT License
+
+Copyright (c) 2024 Antithesis
+
+Permission is hereby granted, free of charge, to any person obtaining a copy
+of this software and associated documentation files (the "Software"), to deal
+in the Software without restriction, including without limitation the rights
+to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+copies of the Software, and to permit persons to whom the Software is
+furnished to do so, subject to the following conditions:
+
+The above copyright notice and this permission notice shall be included in all
+copies or substantial portions of the Software.
+
+THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+SOFTWARE.

external/antithesis-sdk/README.md

@@ -0,0 +1,8 @@
+# Antithesis C++ SDK
+
+This library provides methods for C++ programs to configure the [Antithesis](https://antithesis.com) platform. It contains three kinds of functionality:
+* Assertion macros that allow you to define test properties about your software or workload.
+* Randomness functions for requesting both structured and unstructured randomness from the Antithesis platform.
+* Lifecycle functions that inform the Antithesis environment that particular test phases or milestones have been reached.
+
+For general usage guidance see the [Antithesis C++ SDK Documentation](https://antithesis.com/docs/using_antithesis/sdk/cpp/overview.html)

external/antithesis-sdk/antithesis_instrumentation.h

@@ -0,0 +1,113 @@
+#pragma once
+
+/*
+This header file enables code coverage instrumentation. It is distributed with the Antithesis C++ SDK.
+
+This header file can be used in both C and C++ programs. (The rest of the SDK works only for C++ programs.)
+
+You should include it in a single .cpp or .c file.
+
+The instructions (such as required compiler flags) and usage guidance are found at https://antithesis.com/docs/using_antithesis/sdk/cpp/overview.html.
+*/
+
+#include <unistd.h>
+#include <string.h>
+#include <dlfcn.h>
+#include <stdint.h>
+#include <stdio.h>
+#ifndef __cplusplus
+#include <stdbool.h>
+#include <stddef.h>
+#endif
+
+// If the libvoidstar(determ) library is present, 
+// pass thru trace_pc_guard related callbacks to it
+typedef void (*trace_pc_guard_init_fn)(uint32_t *start, uint32_t *stop);
+typedef void (*trace_pc_guard_fn)(uint32_t *guard, uint64_t edge);
+
+static trace_pc_guard_init_fn trace_pc_guard_init = NULL;
+static trace_pc_guard_fn trace_pc_guard = NULL;
+static bool did_check_libvoidstar = false;
+static bool has_libvoidstar = false;
+
+static __attribute__((no_sanitize("coverage"))) void debug_message_out(const char *msg) {
+  (void)printf("%s\n", msg);
+  return;
+}
+
+extern
+#ifdef __cplusplus
+    "C"
+#endif
+__attribute__((no_sanitize("coverage"))) void antithesis_load_libvoidstar() {
+#ifdef __cplusplus
+    constexpr
+#endif
+    const char* LIB_PATH = "/usr/lib/libvoidstar.so";
+
+    if (did_check_libvoidstar) {
+      return;
+    }
+    debug_message_out("TRYING TO LOAD libvoidstar");
+    did_check_libvoidstar = true;
+    void* shared_lib = dlopen(LIB_PATH, RTLD_NOW);
+    if (!shared_lib) {
+        debug_message_out("Can not load the Antithesis native library");
+        return;
+    }
+
+    void* trace_pc_guard_init_sym = dlsym(shared_lib, "__sanitizer_cov_trace_pc_guard_init");
+    if (!trace_pc_guard_init_sym) {
+        debug_message_out("Can not forward calls to libvoidstar for __sanitizer_cov_trace_pc_guard_init");
+        return;
+    }
+
+    void* trace_pc_guard_sym = dlsym(shared_lib, "__sanitizer_cov_trace_pc_guard_internal");
+    if (!trace_pc_guard_sym) {
+        debug_message_out("Can not forward calls to libvoidstar for __sanitizer_cov_trace_pc_guard");
+        return;
+    }
+
+    trace_pc_guard_init = (trace_pc_guard_init_fn)(trace_pc_guard_init_sym);
+    trace_pc_guard = (trace_pc_guard_fn)(trace_pc_guard_sym);
+    has_libvoidstar = true;
+    debug_message_out("LOADED libvoidstar");
+}
+
+// The following symbols are indeed reserved identifiers, since we're implementing functions defined
+// in the compiler runtime. Not clear how to get Clang on board with that besides narrowly suppressing
+// the warning in this case. The sample code on the CoverageSanitizer documentation page fails this 
+// warning!
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wreserved-identifier"
+extern 
+#ifdef __cplusplus
+    "C"
+#endif
+void __sanitizer_cov_trace_pc_guard_init(uint32_t *start, uint32_t *stop) {
+    debug_message_out("SDK forwarding to libvoidstar for __sanitizer_cov_trace_pc_guard_init()");
+    if (!did_check_libvoidstar) {
+        antithesis_load_libvoidstar();
+    }
+    if (has_libvoidstar) {
+        trace_pc_guard_init(start, stop);
+    }
+    return;
+}
+
+extern 
+#ifdef __cplusplus
+    "C"
+#endif
+void __sanitizer_cov_trace_pc_guard( uint32_t *guard ) {
+    if (has_libvoidstar) {
+        uint64_t edge = (uint64_t)(__builtin_return_address(0));
+        trace_pc_guard(guard, edge);
+    } else {
+        if (guard) {
+          *guard = 0;
+        }
+    }
+    return;
+}
+#pragma clang diagnostic pop