From fa7f04c8a7b7cbb4a1728bf2c9c6c7c8408b432a Mon Sep 17 00:00:00 2001
From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz>
Date: Wed, 14 May 2025 09:42:40 +0200
Subject: [PATCH] refactor: Remove UB in prevector reverse iterators
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

rend() creates a pointer with offset -1. This is UB, according to the
C++ standard: https://eel.is/c++draft/expr.add#4:

    When an expression J that has integral type is added to [...] an
    expression P of pointer type, the result has the type of P.

    ... if P points to a (possibly-hypothetical) array element i of an
    array object x with n elements [...] the expressions P + J and J + P
    (where J has the value j) point to the (possibly-hypothetical) array
    element i+j of x if 0≤i+j≤n [...]

    Otherwise, the behavior is undefined.

Also, it is unclear why the functions exist at all, when stdlib utils
such as std::reverse_iterator{it} or std::views::reverse can be used out
of the box.

So remove them, along with the ubsan suppressions, that are no longer
used.
---
 src/prevector.h                   | 48 +------------------------------
 test/sanitizer_suppressions/ubsan |  2 --
 2 files changed, 1 insertion(+), 49 deletions(-)

diff --git a/src/prevector.h b/src/prevector.h
index d14e5f64e9d..f25827bdcb2 100644
--- a/src/prevector.h
+++ b/src/prevector.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2015-2022 The Bitcoin Core developers
+// Copyright (c) 2015-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.
 
@@ -77,26 +77,6 @@ public:
         bool operator<(iterator x) const { return ptr < x.ptr; }
     };
 
-    class reverse_iterator {
-        T* ptr{};
-    public:
-        typedef Diff difference_type;
-        typedef T value_type;
-        typedef T* pointer;
-        typedef T& reference;
-        typedef std::bidirectional_iterator_tag iterator_category;
-        reverse_iterator() = default;
-        reverse_iterator(T* ptr_) : ptr(ptr_) {}
-        T& operator*() const { return *ptr; }
-        T* operator->() const { return ptr; }
-        reverse_iterator& operator--() { ptr++; return *this; }
-        reverse_iterator& operator++() { ptr--; return *this; }
-        reverse_iterator operator++(int) { reverse_iterator copy(*this); ++(*this); return copy; }
-        reverse_iterator operator--(int) { reverse_iterator copy(*this); --(*this); return copy; }
-        bool operator==(reverse_iterator x) const { return ptr == x.ptr; }
-        bool operator!=(reverse_iterator x) const { return ptr != x.ptr; }
-    };
-
     class const_iterator {
         const T* ptr{};
     public:
@@ -129,27 +109,6 @@ public:
         bool operator<(const_iterator x) const { return ptr < x.ptr; }
     };
 
-    class const_reverse_iterator {
-        const T* ptr{};
-    public:
-        typedef Diff difference_type;
-        typedef const T value_type;
-        typedef const T* pointer;
-        typedef const T& reference;
-        typedef std::bidirectional_iterator_tag iterator_category;
-        const_reverse_iterator() = default;
-        const_reverse_iterator(const T* ptr_) : ptr(ptr_) {}
-        const_reverse_iterator(reverse_iterator x) : ptr(&(*x)) {}
-        const T& operator*() const { return *ptr; }
-        const T* operator->() const { return ptr; }
-        const_reverse_iterator& operator--() { ptr++; return *this; }
-        const_reverse_iterator& operator++() { ptr--; return *this; }
-        const_reverse_iterator operator++(int) { const_reverse_iterator copy(*this); ++(*this); return copy; }
-        const_reverse_iterator operator--(int) { const_reverse_iterator copy(*this); --(*this); return copy; }
-        bool operator==(const_reverse_iterator x) const { return ptr == x.ptr; }
-        bool operator!=(const_reverse_iterator x) const { return ptr != x.ptr; }
-    };
-
 private:
 #pragma pack(push, 1)
     union direct_or_indirect {
@@ -304,11 +263,6 @@ public:
     iterator end() { return iterator(item_ptr(size())); }
     const_iterator end() const { return const_iterator(item_ptr(size())); }
 
-    reverse_iterator rbegin() { return reverse_iterator(item_ptr(size() - 1)); }
-    const_reverse_iterator rbegin() const { return const_reverse_iterator(item_ptr(size() - 1)); }
-    reverse_iterator rend() { return reverse_iterator(item_ptr(-1)); }
-    const_reverse_iterator rend() const { return const_reverse_iterator(item_ptr(-1)); }
-
     size_t capacity() const {
         if (is_direct()) {
             return N;
diff --git a/test/sanitizer_suppressions/ubsan b/test/sanitizer_suppressions/ubsan
index acdae7415d2..55dc2caad89 100644
--- a/test/sanitizer_suppressions/ubsan
+++ b/test/sanitizer_suppressions/ubsan
@@ -54,7 +54,6 @@ unsigned-integer-overflow:DecompressAmount
 unsigned-integer-overflow:crypto/
 unsigned-integer-overflow:MurmurHash3
 unsigned-integer-overflow:TxConfirmStats::EstimateMedianVal
-unsigned-integer-overflow:prevector.h
 unsigned-integer-overflow:InsecureRandomContext::rand64
 unsigned-integer-overflow:InsecureRandomContext::SplitMix64
 unsigned-integer-overflow:bitset_detail::PopCount
@@ -62,7 +61,6 @@ implicit-integer-sign-change:SetStdinEcho
 implicit-integer-sign-change:compressor.h
 implicit-integer-sign-change:crypto/
 implicit-integer-sign-change:TxConfirmStats::removeTx
-implicit-integer-sign-change:prevector.h
 implicit-integer-sign-change:verify_flags
 implicit-integer-sign-change:EvalScript
 implicit-integer-sign-change:serialize.h