From b65eb8549e54da3a6836b3daf7048e298083fb81 Mon Sep 17 00:00:00 2001 From: Bronek Kozicki Date: Tue, 18 Mar 2025 16:45:25 +0000 Subject: [PATCH] fix: Remove null pointer deref, just do abort (#5338) This change removes the existing undefined behavior from `LogicError`, so we can be certain that there will be always a stacktrace. De-referencing a null pointer is an old trick to generate `SIGSEGV`, which would typically also create a stacktrace. However it is also an undefined behaviour and compilers can do something else. A more robust way to create a stacktrace while crashing the program is to use `std::abort`, which we have also used in this location for a long time. If we combine the two, we might not get the expected behaviour - namely, the nullpointer deref followed by `std::abort`, as handled in certain compiler versions may not immediately cause a crash. We have observed stacktrace being wiped instead, and thread put in indeterminate state, then stacktrace created without any useful information. --- src/libxrpl/basics/contract.cpp | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) diff --git a/src/libxrpl/basics/contract.cpp b/src/libxrpl/basics/contract.cpp index b0cd88259..b5a7b3f36 100644 --- a/src/libxrpl/basics/contract.cpp +++ b/src/libxrpl/basics/contract.cpp @@ -27,19 +27,6 @@ namespace ripple { -namespace detail { - -[[noreturn]] void -accessViolation() noexcept -{ - // dereference memory location zero - int volatile* j = 0; - (void)*j; - std::abort(); -} - -} // namespace detail - void LogThrow(std::string const& title) { @@ -57,7 +44,7 @@ LogicError(std::string const& s) noexcept // here. // For the above reasons, we want this contract to stand out. UNREACHABLE("LogicError", {{"message", s}}); - detail::accessViolation(); + std::abort(); } } // namespace ripple