Public commit record
69/100 · Adequate
What the developer wrote
Authored by jeffro256
Misc clang 21 fixes
* Use -O3 and other flags instead of -Ofast
- https://discourse.llvm.org/t/rfc-deprecate-ofast/78687
- https://gcc.gnu.org/onlinedocs/gcc/Optimize-Options.html
* Find libunwind library based on C++ compiler type, not C compiler type
* In stack_trace.cpp, pass stream modifiers to `std::stringstream` first, then send string to log
* In stack_trace.cpp, remove dead code related to `stack_trace_log` path
* Remove `virtual` method attributues from `final` class `cryptonote::core`
* Remove unused `this` capture in cryptonote protocol handler
* Remove unused variable `bad` in net node
* Use `std::make_unsigned` instead of `boost::make_unsigned`
- https://github.com/boostorg/type_traits/issues/171
- https://github.com/boostorg/type_traits/issues/202
- https://github.com/boostorg/type_traits/pull/199
* Cleanup `include`s in pair serialization
* Test convergence b/t `std::make_unsigned` and `boost::make_unsigned`
Fixes compilation and silences warnings on:
clang version 21.1.6
Linux 6.18.8-3-cachyos
✓ Subject identifies a change✓ Provides detailed explanatory context✓ Mentions testing or verification✓ Links an issue, advisory, or supporting reference
What changed, and why it matters
This is a routine maintenance patch that makes Monero compile cleanly on the upcoming Clang 21 compiler. The most notable change is replacing the aggressive '-Ofast' optimization flag with '-O3 -ffast-math -fno-semantic-interposition', which removes a compiler option that could theoretically allow unsafe floating-point and memory optimizations. The rest of the patch removes dead code, unused variables, and compiler warnings. There is no direct evidence this fixes an active security vulnerability, but the compiler-flag change is a sensible hardening step.
Recommended action
Treat as a normal build-compatibility and hardening patch. Review the new optimization flags in release builds to ensure performance and correctness are acceptable. No urgent security response is warranted based on the commit content alone.
Security signals we found
01
Compiler optimization flag changed from -Ofast to -O3 -ffast-math -fno-semantic-interposition, reducing risk of unsafe compiler transformations
02
Dead code and unused variables removed, improving maintainability
03
std::make_unsigned used instead of boost::make_unsigned, with unit-test convergence checks
04
No explicit security bug, CVE, or vulnerability disclosure referenced in commit
Evidence from the diff
The commit updates build and source code to support Clang 21. Key changes: (1) CMakeLists.txt switches release optimization from ‘-Ofast’ to ‘-O3 -ffast-math -fno-semantic-interposition’ (notably omitting ‘-fallow-store-data-races’), and selects libunwind based on the C++ compiler ID rather than the C compiler ID. (2) stack_trace.cpp/h removes the unused set_stack_trace_log() API and dead stack_trace_log variable, and routes stream modifiers through a std::stringstream before logging. (3) cryptonote_core.h drops redundant ‘virtual’ on ‘final’ methods. (4) cryptonote_protocol_handler.inl removes an unused ‘this’ lambda capture. (5) net_node.inl removes an unused ‘bad’ counter. (6) binary_archive.h and pair.h replace boost::make_unsigned with std::make_unsigned and clean includes. (7) A unit test verifies std::make_unsigned and boost::make_unsigned behave the same for scoped/unscoped enums. No vulnerability is described or patched directly.
Changed components
CMakeLists.txt build configurationsrc/common/stack_trace.cpp and stack_trace.hsrc/cryptonote_core/cryptonote_core.hsrc/cryptonote_protocol/cryptonote_protocol_handler.inlsrc/daemon/main.cppsrc/p2p/net_node.inlsrc/serialization/binary_archive.hsrc/serialization/pair.htests/unit_tests/serialization.cppInspect captured patch +82 / −37
diff --git a/CMakeLists.txt b/CMakeLists.txt
index c1433a4..2e58c1c 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -351,7 +351,7 @@ endif()
if(WIN32 OR ARM OR PPC64LE OR PPC64 OR PPC)
set(OPT_FLAGS_RELEASE "-O2")
else()
- set(OPT_FLAGS_RELEASE "-Ofast")
+ set(OPT_FLAGS_RELEASE "-O3 -ffast-math -fno-semantic-interposition") # not present: -fallow-store-data-races
endif()
# BUILD_TAG is used to select the build type to check for a new version
@@ -508,7 +508,7 @@ if (APPLE OR NETBSD)
elseif (DEPENDS AND NOT LINUX)
set(DEFAULT_STACK_TRACE OFF)
set(LIBUNWIND_LIBRARIES "")
-elseif(CMAKE_C_COMPILER_ID STREQUAL "GNU" AND NOT MINGW)
+elseif(CMAKE_CXX_COMPILER_ID STREQUAL "GNU" AND NOT MINGW)
set(DEFAULT_STACK_TRACE ON)
set(STACK_TRACE_LIB "easylogging++") # for diag output only
set(LIBUNWIND_LIBRARIES "")
diff --git a/src/common/stack_trace.cpp b/src/common/stack_trace.cpp
index 5268daf..b45de06 100644
--- a/src/common/stack_trace.cpp
+++ b/src/common/stack_trace.cpp
@@ -35,6 +35,7 @@
#include <stdexcept>
#include <iomanip>
+#include <sstream>
#ifdef USE_UNWIND
#define UNW_LOCAL_ONLY
#include <libunwind.h>
@@ -54,7 +55,9 @@
do { \
auto elpp = ELPP; \
if (elpp) { \
- CINFO(el::base::Writer,el::base::DispatchAction::FileOnlyLog,MONERO_DEFAULT_LOG_CATEGORY) << x; \
+ std::stringstream ss; \
+ ss << x; \
+ CINFO(el::base::Writer,el::base::DispatchAction::FileOnlyLog,MONERO_DEFAULT_LOG_CATEGORY) << ss.str(); \
} \
else { \
std::cout << x << std::endl; \
@@ -105,19 +108,9 @@ void CXA_THROW(void *ex, CXA_THROW_INFO_T *info, void (*dest)(void*))
__real___cxa_throw(ex, info, dest);
}
-namespace
-{
- std::string stack_trace_log;
-}
-
namespace tools
{
-void set_stack_trace_log(const std::string &log)
-{
- stack_trace_log = log;
-}
-
void log_stack_trace(const char *msg)
{
#ifdef USE_UNWIND
@@ -127,7 +120,6 @@ void log_stack_trace(const char *msg)
unsigned level;
char sym[512], *dsym;
int status;
- const char *log = stack_trace_log.empty() ? NULL : stack_trace_log.c_str();
#endif
if (msg)
diff --git a/src/common/stack_trace.h b/src/common/stack_trace.h
index a7cd4af..b6e3cd4 100644
--- a/src/common/stack_trace.h
+++ b/src/common/stack_trace.h
@@ -29,12 +29,9 @@
#ifndef MONERO_EXCEPTION_H
#define MONERO_EXCEPTION_H
-#include <string>
-
namespace tools
{
-void set_stack_trace_log(const std::string &log);
void log_stack_trace(const char *msg);
} // namespace tools
diff --git a/src/cryptonote_core/cryptonote_core.h b/src/cryptonote_core/cryptonote_core.h
index 41b004a..6e69757 100644
--- a/src/cryptonote_core/cryptonote_core.h
+++ b/src/cryptonote_core/cryptonote_core.h
@@ -227,15 +227,15 @@ namespace cryptonote
*
* @return true if the block was added to the main chain, otherwise false
*/
- virtual bool handle_block_found(block& b, block_verification_context &bvc) override;
+ bool handle_block_found(block& b, block_verification_context &bvc) final;
/**
* @copydoc Blockchain::create_block_template
*
* @note see Blockchain::create_block_template
*/
- virtual bool get_block_template(block& b, const account_public_address& adr, difficulty_type& diffic, uint64_t& height, uint64_t& expected_reward, uint64_t &cumulative_weight, const blobdata& ex_nonce, uint64_t &seed_height, crypto::hash &seed_hash) override;
- virtual bool get_block_template(block& b, const crypto::hash *prev_block, const account_public_address& adr, difficulty_type& diffic, uint64_t& height, uint64_t& expected_reward, uint64_t &cumulative_weight, const blobdata& ex_nonce, uint64_t &seed_height, crypto::hash &seed_hash);
+ bool get_block_template(block& b, const account_public_address& adr, difficulty_type& diffic, uint64_t& height, uint64_t& expected_reward, uint64_t &cumulative_weight, const blobdata& ex_nonce, uint64_t &seed_height, crypto::hash &seed_hash) final;
+ bool get_block_template(block& b, const crypto::hash *prev_block, const account_public_address& adr, difficulty_type& diffic, uint64_t& height, uint64_t& expected_reward, uint64_t &cumulative_weight, const blobdata& ex_nonce, uint64_t &seed_height, crypto::hash &seed_hash);
/**
* @copydoc Blockchain::get_miner_data
@@ -248,7 +248,7 @@ namespace cryptonote
* @brief called when a transaction is relayed.
* @note Should only be invoked from `levin_notify`.
*/
- virtual void on_transactions_relayed(epee::span<const cryptonote::blobdata> tx_blobs, relay_method tx_relay) final;
+ void on_transactions_relayed(epee::span<const cryptonote::blobdata> tx_blobs, relay_method tx_relay) final;
/**
@@ -340,7 +340,7 @@ namespace cryptonote
*
* @note see Blockchain::get_current_blockchain_height()
*/
- virtual uint64_t get_current_blockchain_height() const final;
+ uint64_t get_current_blockchain_height() const final;
/**
* @brief get the hash and height of the most recent block
@@ -671,7 +671,7 @@ namespace cryptonote
*
* @return core synchronization status
*/
- virtual bool is_synchronized() const final;
+ bool is_synchronized() const final;
/**
* @copydoc miner::on_synchronized
diff --git a/src/cryptonote_protocol/cryptonote_protocol_handler.inl b/src/cryptonote_protocol/cryptonote_protocol_handler.inl
index aba2b33..229338a 100644
--- a/src/cryptonote_protocol/cryptonote_protocol_handler.inl
+++ b/src/cryptonote_protocol/cryptonote_protocol_handler.inl
@@ -2645,7 +2645,7 @@ skip:
{
// sort peers between fluffy ones and others
std::vector<std::pair<epee::net_utils::zone, boost::uuids::uuid>> fluffyConnections;
- m_p2p->for_each_connection([this, &exclude_context, &fluffyConnections](connection_context& context, nodetool::peerid_type peer_id, uint32_t support_flags)
+ m_p2p->for_each_connection([&exclude_context, &fluffyConnections](connection_context& context, nodetool::peerid_type peer_id, uint32_t support_flags)
{
// peer_id also filters out connections before handshake
if (peer_id && exclude_context.m_connection_id != context.m_connection_id && context.m_remote_address.get_zone() == epee::net_utils::zone::public_)
diff --git a/src/daemon/main.cpp b/src/daemon/main.cpp
index 3ae8212..78e907a 100644
--- a/src/daemon/main.cpp
+++ b/src/daemon/main.cpp
@@ -291,10 +291,6 @@ int main(int argc, char const * argv[])
// after logs initialized
tools::create_directories_if_necessary(data_dir.string());
-#ifdef STACK_TRACE
- tools::set_stack_trace_log(log_file_path.filename().string());
-#endif // STACK_TRACE
-
if (!command_line::is_arg_defaulted(vm, daemon_args::arg_max_concurrency))
tools::set_max_concurrency(command_line::get_arg(vm, daemon_args::arg_max_concurrency));
diff --git a/src/p2p/net_node.inl b/src/p2p/net_node.inl
index 1be3fc4..6f68645 100644
--- a/src/p2p/net_node.inl
+++ b/src/p2p/net_node.inl
@@ -2153,7 +2153,7 @@ namespace nodetool
if (!tools::dns_utils::load_txt_records_from_dns(records, dns_urls))
return true;
- unsigned good = 0, bad = 0;
+ unsigned good = 0;
for (const auto& record : records)
{
std::vector<std::string> ips;
@@ -2177,7 +2177,6 @@ namespace nodetool
continue;
}
MWARNING("Invalid IP address or subnet from DNS blocklist: " << ip << " - " << parsed_addr.error());
- ++bad;
}
}
if (good > 0)
diff --git a/src/serialization/binary_archive.h b/src/serialization/binary_archive.h
index 59b6363..216d9b2 100644
--- a/src/serialization/binary_archive.h
+++ b/src/serialization/binary_archive.h
@@ -36,8 +36,10 @@
#include <cassert>
#include <iostream>
#include <iterator>
+#include <type_traits>
+
#include <boost/endian/conversion.hpp>
-#include <boost/type_traits/make_unsigned.hpp>
+#include <boost/mpl/bool.hpp>
#include "common/varint.h"
#include "span.h"
@@ -106,7 +108,7 @@ struct binary_archive<false> : public binary_archive_base<false>
template <class T>
void serialize_int(T &v)
{
- serialize_uint(*(typename boost::make_unsigned<T>::type *)&v);
+ serialize_uint(*(typename std::make_unsigned<T>::type *)&v);
}
/*! \fn serialize_uint
@@ -137,7 +139,7 @@ struct binary_archive<false> : public binary_archive_base<false>
template <class T>
void serialize_varint(T &v)
{
- serialize_uvarint(*(typename boost::make_unsigned<T>::type *)(&v));
+ serialize_uvarint(*(typename std::make_unsigned<T>::type *)(&v));
}
template <class T>
@@ -190,7 +192,7 @@ struct binary_archive<true> : public binary_archive_base<true>
template <class T>
void serialize_int(T v)
{
- serialize_uint(static_cast<typename boost::make_unsigned<T>::type>(v));
+ serialize_uint(static_cast<typename std::make_unsigned<T>::type>(v));
}
template <class T>
void serialize_uint(T v)
@@ -209,7 +211,7 @@ struct binary_archive<true> : public binary_archive_base<true>
template <class T>
void serialize_varint(T &v)
{
- serialize_uvarint(*(typename boost::make_unsigned<T>::type *)(&v));
+ serialize_uvarint(*(typename std::make_unsigned<T>::type *)(&v));
}
template <class T>
diff --git a/src/serialization/pair.h b/src/serialization/pair.h
index a08583d..4c4fc22 100644
--- a/src/serialization/pair.h
+++ b/src/serialization/pair.h
@@ -29,8 +29,9 @@
// Parts of this file are originally copyright (c) 2012-2013 The Cryptonote developers
#pragma once
-#include <memory>
-#include <boost/type_traits/make_unsigned.hpp>
+
+#include <cstdint>
+
#include "serialization.h"
namespace serialization
diff --git a/tests/unit_tests/serialization.cpp b/tests/unit_tests/serialization.cpp
index d15e3f3..6cf9be8 100644
--- a/tests/unit_tests/serialization.cpp
+++ b/tests/unit_tests/serialization.cpp
@@ -35,6 +35,7 @@
#include <vector>
#include <boost/foreach.hpp>
#include <boost/archive/portable_binary_iarchive.hpp>
+#include <boost/type_traits/make_unsigned.hpp>
#include "cryptonote_basic/cryptonote_basic.h"
#include "cryptonote_basic/cryptonote_basic_impl.h"
#include "ringct/rctSigs.h"
@@ -56,6 +57,63 @@ static_assert(!std::is_trivially_copyable<std::vector<unsigned char>>(),
static_assert(!std::is_trivially_copyable<std::string>(),
"should fail to compile when applying blob serializer");
+/**
+ * Test convergence of std::make_unsigned against boost::make_unsigned for
+ * scoped enums with explicit underlying types
+ */
+#define DEFINE_SCOPED_UNDERLYING_ENUM(u) \
+ enum class E_ ## u : u { A_##u = static_cast<u>(-1), B_##u = 0, C_##u }; \
+ static_assert(std::is_same_v<std::underlying_type_t<E_##u>, u>); \
+ static_assert(sizeof(std::make_unsigned_t<E_##u>) == sizeof(std::make_unsigned_t<u>)); \
+ static_assert(std::is_same_v<std::make_unsigned_t<E_##u>, boost::make_unsigned_t<E_##u>>);
+
+DEFINE_SCOPED_UNDERLYING_ENUM(uint8_t)
+DEFINE_SCOPED_UNDERLYING_ENUM(uint16_t)
+DEFINE_SCOPED_UNDERLYING_ENUM(uint32_t)
+DEFINE_SCOPED_UNDERLYING_ENUM(uint64_t)
+//DEFINE_SCOPED_UNDERLYING_ENUM(__uint128_t)
+DEFINE_SCOPED_UNDERLYING_ENUM(int8_t)
+DEFINE_SCOPED_UNDERLYING_ENUM(int16_t)
+DEFINE_SCOPED_UNDERLYING_ENUM(int32_t)
+DEFINE_SCOPED_UNDERLYING_ENUM(int64_t)
+//DEFINE_SCOPED_UNDERLYING_ENUM(__int128_t)
+DEFINE_SCOPED_UNDERLYING_ENUM(size_t)
+
+/**
+ * Test convergence of std::make_unsigned against boost::make_unsigned for
+ * unscoped enums with explicit underlying types
+ */
+#define DEFINE_UNSCOPED_UNDERLYING_ENUM(u) \
+ enum class UE_ ## u : u { A_##u = static_cast<u>(-1), B_##u = 0, C_##u }; \
+ static_assert(std::is_same_v<std::underlying_type_t<UE_##u>, u>); \
+ static_assert(sizeof(std::make_unsigned_t<UE_##u>) == sizeof(std::make_unsigned_t<u>)); \
+ static_assert(std::is_same_v<std::make_unsigned_t<UE_##u>, boost::make_unsigned_t<UE_##u>>);
+
+DEFINE_UNSCOPED_UNDERLYING_ENUM(uint8_t)
+DEFINE_UNSCOPED_UNDERLYING_ENUM(uint16_t)
+DEFINE_UNSCOPED_UNDERLYING_ENUM(uint32_t)
+DEFINE_UNSCOPED_UNDERLYING_ENUM(uint64_t)
+//DEFINE_UNSCOPED_UNDERLYING_ENUM(__uint128_t)
+DEFINE_UNSCOPED_UNDERLYING_ENUM(int8_t)
+DEFINE_UNSCOPED_UNDERLYING_ENUM(int16_t)
+DEFINE_UNSCOPED_UNDERLYING_ENUM(int32_t)
+DEFINE_UNSCOPED_UNDERLYING_ENUM(int64_t)
+//DEFINE_UNSCOPED_UNDERLYING_ENUM(__int128_t)
+DEFINE_UNSCOPED_UNDERLYING_ENUM(size_t)
+
+/**
+ * Test convergence of std::make_unsigned against boost::make_unsigned for
+ * unscoped enums with implicit underlying types
+ */
+#define TEST_MAKE_UNSIGNED_ENUM(e) \
+ static_assert(std::is_same_v<std::make_unsigned_t<e>, boost::make_unsigned_t<e>>);
+enum IUE1 { A1, B1, C1 };
+#if defined(__GNUC__) && !defined(__clang__)
+ TEST_MAKE_UNSIGNED_ENUM(IUE1) // may break for GCC later too, or Boost might fix it first
+#endif
+enum IUE2 { A2 = -1, B2, C2 };
+TEST_MAKE_UNSIGNED_ENUM(IUE2)
+
struct Struct
{
int32_t a;
Risk score
Our methodology →Why this scored 27/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.