From dff7fa4fa8d25a8d3f21f4445a6496fd263e4a52 Mon Sep 17 00:00:00 2001 From: Colin McEwan Date: Wed, 30 Jun 2021 09:16:28 +0100 Subject: [PATCH] Fix alignment of fields for atomic accesses (#761) * Assert alignment is suitable for atomic accesses * Move fields and pad to meet alignment constraints * Add padding to JackEngineControl to account for inherited data * Parenthesise padding length for clarity * Revert "Parenthesise padding length for clarity" This reverts commit 1f757b9ece5e3b032743c6c5ac49e83c3928e3de. * Revert "Add padding to JackEngineControl to account for inherited data" This reverts commit 3d8c7d83ad9483280f623171af7e40ccc76cef38. * Revert "Move fields and pad to meet alignment constraints" This reverts commit ff631bbbdc2279df05f3a18dd44e8fd68be2e04d. * Assure alignment by using 'alignas' on fields --- common/JackActivationCount.h | 7 +++++-- common/JackAtomicArrayState.h | 5 ++++- common/JackAtomicState.h | 5 ++++- common/JackConnectionManager.cpp | 3 +++ common/JackConnectionManager.h | 2 +- common/JackEngineControl.h | 10 +++++++--- common/JackMessageBuffer.cpp | 5 ++++- common/JackMessageBuffer.h | 2 +- common/JackTransportEngine.cpp | 2 ++ common/JackTransportEngine.h | 2 +- macosx/JackAtomic_os.h | 4 ++++ 11 files changed, 36 insertions(+), 11 deletions(-) diff --git a/common/JackActivationCount.h b/common/JackActivationCount.h index 7efc031c..f9ef8366 100644 --- a/common/JackActivationCount.h +++ b/common/JackActivationCount.h @@ -39,13 +39,16 @@ class JackActivationCount private: - SInt32 fValue; + alignas(SInt32) SInt32 fValue; SInt32 fCount; public: JackActivationCount(): fValue(0), fCount(0) - {} + { + static_assert(offsetof(JackActivationCount, fValue) % sizeof(fValue) == 0, + "fValue must be aligned within JackActivationCount"); + } bool Signal(JackSynchro* synchro, JackClientControl* control); diff --git a/common/JackAtomicArrayState.h b/common/JackAtomicArrayState.h index 51ec65d9..b664e815 100644 --- a/common/JackAtomicArrayState.h +++ b/common/JackAtomicArrayState.h @@ -23,6 +23,7 @@ #include "JackAtomic.h" #include "JackCompilerDeps.h" #include // for memcpy +#include namespace Jack { @@ -121,7 +122,7 @@ class JackAtomicArrayState // fState[2] ==> request T fState[3]; - volatile AtomicArrayCounter fCounter; + alignas(UInt32) volatile AtomicArrayCounter fCounter; UInt32 WriteNextStateStartAux(int state, bool* result) { @@ -159,6 +160,8 @@ class JackAtomicArrayState JackAtomicArrayState() { + static_assert(offsetof(JackAtomicArrayState, fCounter) % sizeof(fCounter) == 0, + "fCounter must be aligned within JackAtomicArrayState"); Counter1(fCounter) = 0; } diff --git a/common/JackAtomicState.h b/common/JackAtomicState.h index 6f90d66a..26d6a565 100644 --- a/common/JackAtomicState.h +++ b/common/JackAtomicState.h @@ -23,6 +23,7 @@ Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. #include "JackAtomic.h" #include "JackCompilerDeps.h" #include // for memcpy +#include namespace Jack { @@ -93,7 +94,7 @@ class JackAtomicState protected: T fState[2]; - volatile AtomicCounter fCounter; + alignas(UInt32) volatile AtomicCounter fCounter; SInt32 fCallWriteCounter; UInt32 WriteNextStateStartAux() @@ -131,6 +132,8 @@ class JackAtomicState JackAtomicState() { + static_assert(offsetof(JackAtomicState, fCounter) % sizeof(fCounter) == 0, + "fCounter must be aligned within JackAtomicState"); Counter(fCounter) = 0; fCallWriteCounter = 0; } diff --git a/common/JackConnectionManager.cpp b/common/JackConnectionManager.cpp index a0cc0439..db240d22 100644 --- a/common/JackConnectionManager.cpp +++ b/common/JackConnectionManager.cpp @@ -32,6 +32,9 @@ namespace Jack JackConnectionManager::JackConnectionManager() { int i; + static_assert(offsetof(JackConnectionManager, fInputCounter) % sizeof(UInt32) == 0, + "fInputCounter must be aligned within JackConnectionManager"); + jack_log("JackConnectionManager::InitConnections size = %ld ", sizeof(JackConnectionManager)); for (i = 0; i < PORT_NUM_MAX; i++) { diff --git a/common/JackConnectionManager.h b/common/JackConnectionManager.h index ead32092..bd071685 100644 --- a/common/JackConnectionManager.h +++ b/common/JackConnectionManager.h @@ -417,7 +417,7 @@ class SERVER_EXPORT JackConnectionManager JackFixedArray1 fInputPort[CLIENT_NUM]; /*! Table of input port per refnum : to find a refnum for a given port */ JackFixedArray fOutputPort[CLIENT_NUM]; /*! Table of output port per refnum : to find a refnum for a given port */ JackFixedMatrix fConnectionRef; /*! Table of port connections by (refnum , refnum) */ - JackActivationCount fInputCounter[CLIENT_NUM]; /*! Activation counter per refnum */ + alignas(UInt32) JackActivationCount fInputCounter[CLIENT_NUM]; /*! Activation counter per refnum */ JackLoopFeedback fLoopFeedback; /*! Loop feedback connections */ bool IsLoopPathAux(int ref1, int ref2) const; diff --git a/common/JackEngineControl.h b/common/JackEngineControl.h index 8626a161..a2bb7b11 100644 --- a/common/JackEngineControl.h +++ b/common/JackEngineControl.h @@ -64,7 +64,7 @@ struct SERVER_EXPORT JackEngineControl : public JackShmMem int fClientPriority; int fMaxClientPriority; char fServerName[JACK_SERVER_NAME_SIZE+1]; - JackTransportEngine fTransport; + alignas(UInt32) JackTransportEngine fTransport; jack_timer_type_t fClockSource; int fDriverNum; bool fVerbose; @@ -86,14 +86,18 @@ struct SERVER_EXPORT JackEngineControl : public JackShmMem UInt64 fConstraint; // Timer - JackFrameTimer fFrameTimer; + alignas(UInt32) JackFrameTimer fFrameTimer; #ifdef JACK_MONITOR JackEngineProfiling fProfiler; #endif JackEngineControl(bool sync, bool temporary, long timeout, bool rt, long priority, bool verbose, jack_timer_type_t clock, const char* server_name) - { + { + static_assert(offsetof(JackEngineControl, fTransport) % sizeof(UInt32) == 0, + "fTransport must be aligned within JackEngineControl"); + static_assert(offsetof(JackEngineControl, fFrameTimer) % sizeof(UInt32) == 0, + "fFrameTimer must be aligned within JackEngineControl"); fBufferSize = 512; fSampleRate = 48000; fPeriodUsecs = jack_time_t(1000000.f / fSampleRate * fBufferSize); diff --git a/common/JackMessageBuffer.cpp b/common/JackMessageBuffer.cpp index cd197993..f821cf4f 100644 --- a/common/JackMessageBuffer.cpp +++ b/common/JackMessageBuffer.cpp @@ -38,7 +38,10 @@ JackMessageBuffer::JackMessageBuffer() fOutBuffer(0), fOverruns(0), fRunning(false) -{} +{ + static_assert(offsetof(JackMessageBuffer, fOverruns) % sizeof(fOverruns) == 0, + "fOverruns must be aligned within JackMessageBuffer"); +} JackMessageBuffer::~JackMessageBuffer() {} diff --git a/common/JackMessageBuffer.h b/common/JackMessageBuffer.h index fc0d7177..3c796bde 100644 --- a/common/JackMessageBuffer.h +++ b/common/JackMessageBuffer.h @@ -64,7 +64,7 @@ class JackMessageBuffer : public JackRunnableInterface JackProcessSync fGuard; volatile unsigned int fInBuffer; volatile unsigned int fOutBuffer; - SInt32 fOverruns; + alignas(SInt32) SInt32 fOverruns; bool fRunning; void Flush(); diff --git a/common/JackTransportEngine.cpp b/common/JackTransportEngine.cpp index 5c0fa8d0..09bf44d7 100644 --- a/common/JackTransportEngine.cpp +++ b/common/JackTransportEngine.cpp @@ -36,6 +36,8 @@ namespace Jack JackTransportEngine::JackTransportEngine(): JackAtomicArrayState() { + static_assert(offsetof(JackTransportEngine, fWriteCounter) % sizeof(fWriteCounter) == 0, + "fWriteCounter must be first member of JackTransportEngine to ensure its alignment"); fTransportState = JackTransportStopped; fTransportCmd = fPreviousCmd = TransportCommandStop; fSyncTimeout = 10000000; /* 10 seconds default... diff --git a/common/JackTransportEngine.h b/common/JackTransportEngine.h index e00a3138..afdfcf17 100644 --- a/common/JackTransportEngine.h +++ b/common/JackTransportEngine.h @@ -104,7 +104,7 @@ class SERVER_EXPORT JackTransportEngine : public JackAtomicArrayState #if defined(__ppc__) || defined(__ppc64__) @@ -67,8 +68,11 @@ static inline char CAS(volatile UInt32 value, UInt32 newvalue, volatile void* ad #else +#include static inline char CAS(volatile UInt32 value, UInt32 newvalue, volatile void* addr) { + // Assert pointer is 32-bit aligned + assert(((long)addr & (sizeof(int)-1)) == 0); return __sync_bool_compare_and_swap ((UInt32*)addr, value, newvalue); }