From 401950a76f26f9827715eaf0df7fcc4076aaf151 Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Fri, 29 Sep 2023 15:13:00 +0200 Subject: [PATCH] QAtomicScopedValueRollback: fix UB (passing rel/acq_rel to std::atomic::load()) It's explicitly undefined behavior to pass release/acq_rel memory_order to load(), so don't. This is private API, so no ChangeLog needed. Reported-by: Fabian Kosmale Task-number: QTBUG-115107 Pick-to: 6.5 Change-Id: Iee119303d790c31937238ef92d900a25020e9713 Reviewed-by: Fabian Kosmale (cherry picked from commit 5797f29e8c249e13da6285ae9b2d50fff4ffb505) Reviewed-by: Qt Cherry-pick Bot --- .../tools/qatomicscopedvaluerollback_p.h | 22 +++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/src/corelib/tools/qatomicscopedvaluerollback_p.h b/src/corelib/tools/qatomicscopedvaluerollback_p.h index 1e2b28125f5..c3b48f8855b 100644 --- a/src/corelib/tools/qatomicscopedvaluerollback_p.h +++ b/src/corelib/tools/qatomicscopedvaluerollback_p.h @@ -45,6 +45,24 @@ class QAtomicScopedValueRollback #if !defined(Q_CC_GNU_ONLY) || (Q_CC_GNU >= 900) // NOLINTNEXTLINE(qt-use-unreachable-return): Triggers on Clang, breaking GCC 8 Q_UNREACHABLE(); +#endif + return std::memory_order_seq_cst; + } + + static constexpr std::memory_order load_part(std::memory_order mo) noexcept + { + switch (mo) { + case std::memory_order_relaxed: + case std::memory_order_release: return std::memory_order_relaxed; + case std::memory_order_consume: return std::memory_order_consume; + case std::memory_order_acquire: + case std::memory_order_acq_rel: return std::memory_order_acquire; + case std::memory_order_seq_cst: return std::memory_order_seq_cst; + } + // GCC 8.x does not treat __builtin_unreachable() as constexpr +#if !defined(Q_CC_GNU_ONLY) || (Q_CC_GNU >= 900) + // NOLINTNEXTLINE(qt-use-unreachable-return): Triggers on Clang, breaking GCC 8 + Q_UNREACHABLE(); #endif return std::memory_order_seq_cst; } @@ -56,7 +74,7 @@ public: explicit constexpr QAtomicScopedValueRollback(std::atomic &var, std::memory_order mo = std::memory_order_seq_cst) - : m_atomic(var), m_value(var.load(mo)), m_mo(mo) {} + : m_atomic(var), m_value(var.load(load_part(mo))), m_mo(mo) {} Q_NODISCARD_CTOR explicit constexpr @@ -104,7 +122,7 @@ public: constexpr void commit() { - m_value = m_atomic.load(m_mo); + m_value = m_atomic.load(load_part(m_mo)); } };