Hi,

In QAtomic, we use compare_exchange_strong with a single memory_order 
argument. The failure mode is therefore calculated by libctdc++, the relevant 
code in atomic_base.h has not changed from 4.8.0 to 6.1.0, and appears to do 
the correct thing. It does use signed bitmasking operations with enums that 
have values with the high bit set (on 32-bit platforms), so theoretically 
there can be a miscompile there.

A workaround for Qt might be to pass an explicit failure mode, which would 
avoid aforementioned code in libstdc++.

If the failure is reproducible, then someone with access to the configuration 
could try the attached patch.

Thanks,
Marc

On Monday 23 May 2016 13:54:16 Sean Harmer wrote:
> Bump, is anybody able to help here please?
> 
> Thanks,
> 
> Sean
> 
> On Friday 20 May 2016 10:13:51 Sean Harmer wrote:
> > Hi,
> > 
> > Trying to submit a simple patch to Qt 3D we are hitting a weird
> > compilation error on Android CI configurations. The change is:
> > 
> > https://codereview.qt-project.org/#/c/157592/
> > 
> > and the compilation errors can be seen in full at:
> > 
> > http://testresults.qt.io/logs/qt/qt3d/489c6abe13e098eb87fa2c0a8639d43dfca
> > 8c0
> > 2f/LinuxRHEL_6_6x86_64AndroidAndroid_22armv7GCCRelease_DisableTests_Open
> > GLES
> > 2_NoUseGoldLinker/1e29b40895b69af3bcc7f2f8a4b041b69e05b286/buildlog.txt.
> > gz
> > 
> > but in brief they are:
> > 
> > In file included from /opt/android/ndk/sources/cxx-stl/gnu-
> > libstdc++/4.8/include/atomic:41:0,
> > 
> >                  from
> > 
> > /home/qt/work/install/include/QtCore/qatomic_cxx11.h:45, from
> > /home/qt/work/install/include/QtCore/qbasicatomic.h:53, from
> > /home/qt/work/install/include/QtCore/qatomic.h:46, from
> > /home/qt/work/install/include/QtCore/qglobal.h:1145, from
> > ../../include/Qt3DCore/../../src/core/qt3dcore_global.h:43,
> > 
> >                  from ../../include/Qt3DCore/qt3dcore_global.h:1,
> >                  from
> > 
> > ../../include/Qt3DRender/../../src/render/qt3drender_global.h:43,
> > 
> >                  from ../../include/Qt3DRender/qt3drender_global.h:1,
> >                  from
> > 
> > ../../include/Qt3DRender/5.7.0/Qt3DRender/private/../../../../../src/rend
> > er/ backend/backendnode_p.h:54, from
> > ../../include/Qt3DRender/5.7.0/Qt3DRender/private/backendnode_p.h:1,
> > 
> >                  from backend/entity_p.h:55,
> > 
> >                  from backend/entity.cpp:40:
> > /opt/android/ndk/sources/cxx-stl/gnu-libstdc++/4.8/include/bits/atomic_ba
> > se. h: In member function 'virtual void
> > Qt3DRender::Render::Entity::sceneChangeEvent(const QSceneChangePtr&)':
> > /opt/android/ndk/sources/cxx-stl/gnu-
> > libstdc++/4.8/include/bits/atomic_base.h:577:70: error: failure memory
> > model cannot be stronger than success memory model for
> > '__atomic_compare_exchange' return __atomic_compare_exchange_n(&_M_i,
> > &__i1, __i2, 0, __m1, __m2); ^ /opt/android/ndk/sources/cxx-stl/gnu-
> > libstdc++/4.8/include/bits/atomic_base.h:577:70: error: failure memory
> > model cannot be stronger than success memory model for
> > '__atomic_compare_exchange' return __atomic_compare_exchange_n(&_M_i,
> > &__i1, __i2, 0, __m1, __m2); ^ /opt/android/ndk/sources/cxx-stl/gnu-
> > libstdc++/4.8/include/bits/atomic_base.h:577:70: error: failure memory
> > model cannot be stronger than success memory model for
> > '__atomic_compare_exchange' return __atomic_compare_exchange_n(&_M_i,
> > &__i1, __i2, 0, __m1, __m2); ^ /opt/android/ndk/sources/cxx-stl/gnu-
> > libstdc++/4.8/include/bits/atomic_base.h:577:70: error: failure memory
> > model cannot be stronger than success memory model for
> > '__atomic_compare_exchange' return __atomic_compare_exchange_n(&_M_i,
> > &__i1, __i2, 0, __m1, __m2); ^
> > /opt/android/ndk/sources/cxx-stl/gnu-libstdc++/4.8/include/bits/atomic_ba
> > se .h: In member function 'virtual void
> > Qt3DRender::Render::Entity::initializeFromPeer(const
> > QNodeCreatedChangeBasePtr&)':
> > /opt/android/ndk/sources/cxx-stl/gnu-
> > libstdc++/4.8/include/bits/atomic_base.h:577:70: error: failure memory
> > model cannot be stronger than success memory model for
> > '__atomic_compare_exchange' return __atomic_compare_exchange_n(&_M_i,
> > &__i1, __i2, 0, __m1, __m2); ^
> > 
> > The patch makes no direct use of atomics, only via QSharedPointer. BogDan
> > reports that it builds fine with latest NDK.
> > 
> > Is this a genuine error on our part or some corner case issue with the
> > CI's installed NDK?
> > 
> > I'm at a total loss with this one and have no idea how to
> > investigate/fix.
> > 
> > Any help would be greatly appreciated. :)
> > 
> > Kind regards,
> > 
> > Sean
> > --
> > Dr Sean Harmer | sean.har...@kdab.com | Managing Director UK
> > Klarälvdalens Datakonsult AB, a KDAB Group company
> > Tel. UK +44 (0)1625 809908, Sweden (HQ) +46-563-540090
> > KDAB - Qt Experts - Platform-independent software solutions
> > _______________________________________________
> > Development mailing list
> > Development@qt-project.org
> > http://lists.qt-project.org/mailman/listinfo/development

-- 
Marc Mutz <marc.m...@kdab.com> | Senior Software Engineer
KDAB (Deutschland) GmbH & Co.KG, a KDAB Group Company
Tel: +49-30-521325470
KDAB - Qt, C++ and OpenGL Experts
From c5d7425f8ad391112758db161e3e08f18dc9d299 Mon Sep 17 00:00:00 2001
From: Marc Mutz <marc.m...@kdab.com>
Date: Thu, 26 May 2016 08:30:26 +0200
Subject: [PATCH] QAtomic: pass explicit failure mode to
 std::atomic::compare_exchange_strong

... in an attempt to avoid GCC 4.8 errors such as

  bits/atomic_base.h:577:70: error: failure memory model cannot be stronger than success memory model for '__atomic_compare_exchange'
  return __atomic_compare_exchange_n(&_M_i, &__i1, __i2, 0, __m1, __m2);
                                                                      ^

as seen on Android.

Change-Id: If046e735888cf331d2d6506d8d5ca9aa7402f9ad
---
 src/corelib/arch/qatomic_cxx11.h |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/corelib/arch/qatomic_cxx11.h b/src/corelib/arch/qatomic_cxx11.h
index bb49aae..d6731ec 100644
--- a/src/corelib/arch/qatomic_cxx11.h
+++ b/src/corelib/arch/qatomic_cxx11.h
@@ -153,7 +153,7 @@ template <typename X> struct QAtomicOps
     template <typename T>
     static bool testAndSetRelaxed(std::atomic<T> &_q_value, T expectedValue, T newValue, T *currentValue = Q_NULLPTR) Q_DECL_NOTHROW
     {
-        bool tmp = _q_value.compare_exchange_strong(expectedValue, newValue, std::memory_order_relaxed);
+        bool tmp = _q_value.compare_exchange_strong(expectedValue, newValue, std::memory_order_relaxed, std::memory_order_relaxed);
         if (currentValue)
             *currentValue = expectedValue;
         return tmp;
@@ -162,7 +162,7 @@ template <typename X> struct QAtomicOps
     template <typename T>
     static bool testAndSetAcquire(std::atomic<T> &_q_value, T expectedValue, T newValue, T *currentValue = Q_NULLPTR) Q_DECL_NOTHROW
     {
-        bool tmp = _q_value.compare_exchange_strong(expectedValue, newValue, std::memory_order_acquire);
+        bool tmp = _q_value.compare_exchange_strong(expectedValue, newValue, std::memory_order_acquire, std::memory_order_acquire);
         if (currentValue)
             *currentValue = expectedValue;
         return tmp;
@@ -171,7 +171,7 @@ template <typename X> struct QAtomicOps
     template <typename T>
     static bool testAndSetRelease(std::atomic<T> &_q_value, T expectedValue, T newValue, T *currentValue = Q_NULLPTR) Q_DECL_NOTHROW
     {
-        bool tmp = _q_value.compare_exchange_strong(expectedValue, newValue, std::memory_order_release);
+        bool tmp = _q_value.compare_exchange_strong(expectedValue, newValue, std::memory_order_release, std::memory_order_relaxed);
         if (currentValue)
             *currentValue = expectedValue;
         return tmp;
@@ -180,7 +180,7 @@ template <typename X> struct QAtomicOps
     template <typename T>
     static bool testAndSetOrdered(std::atomic<T> &_q_value, T expectedValue, T newValue, T *currentValue = Q_NULLPTR) Q_DECL_NOTHROW
     {
-        bool tmp = _q_value.compare_exchange_strong(expectedValue, newValue, std::memory_order_acq_rel);
+        bool tmp = _q_value.compare_exchange_strong(expectedValue, newValue, std::memory_order_acq_rel, std::memory_order_acquire);
         if (currentValue)
             *currentValue = expectedValue;
         return tmp;
-- 
1.7.10.4

_______________________________________________
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Reply via email to