From 6a8067ed5faffefc646fcd5824509ea268c203a5 Mon Sep 17 00:00:00 2001 From: James Clarke Date: Sat, 7 May 2016 22:09:09 +0100 Subject: [PATCH] Fix locking on big-endian architectures 4e4d8002e5fe9968b4d5a493fdb567ed773ccbab extended locks to have 64-bit keys in most cases, but some were left as 32-bit. However, code using these 32-bit locks assumed that the significant bytes of the key's long value started from lck_string[0], which is false on big-endian architectures. This commit adds Lock::getKeyString(), which gets a pointer to the first used byte of lck_string, and should be used in place of accessing lck_string directly. --- src/jrd/GlobalRWLock.cpp | 2 +- src/jrd/Relation.cpp | 2 +- src/jrd/btr.cpp | 4 ++-- src/jrd/cch.cpp | 2 +- src/jrd/lck.cpp | 10 +++++----- src/jrd/lck.h | 11 ++++++++++- src/jrd/met.epp | 4 ++-- 7 files changed, 22 insertions(+), 13 deletions(-) diff --git a/src/jrd/GlobalRWLock.cpp b/src/jrd/GlobalRWLock.cpp index 97c10cb4ea..8258b07951 100644 --- a/src/jrd/GlobalRWLock.cpp +++ b/src/jrd/GlobalRWLock.cpp @@ -78,7 +78,7 @@ GlobalRWLock::GlobalRWLock(thread_db* tdbb, MemoryPool& p, lck_t lckType, cachedLock = FB_NEW_RPT(getPool(), lockLen) Lock(tdbb, lockLen, lckType, this, lockCaching ? blocking_ast_cached_lock : NULL); - memcpy(&cachedLock->lck_key, lockStr, lockLen); + memcpy(cachedLock->getKeyString(), lockStr, lockLen); } GlobalRWLock::~GlobalRWLock() diff --git a/src/jrd/Relation.cpp b/src/jrd/Relation.cpp index 360faa195a..b32c8ba704 100644 --- a/src/jrd/Relation.cpp +++ b/src/jrd/Relation.cpp @@ -308,7 +308,7 @@ Lock* jrd_rel::createLock(thread_db* tdbb, MemoryPool* pool, jrd_rel* relation, const USHORT relLockLen = relation->getRelLockKeyLength(); Lock* lock = FB_NEW_RPT(*pool, relLockLen) Lock(tdbb, relLockLen, lckType, relation); - relation->getRelLockKey(tdbb, &lock->lck_key.lck_string[0]); + relation->getRelLockKey(tdbb, lock->getKeyString()); lock->lck_type = lckType; switch (lckType) diff --git a/src/jrd/btr.cpp b/src/jrd/btr.cpp index 99354a0dbd..b608f32940 100644 --- a/src/jrd/btr.cpp +++ b/src/jrd/btr.cpp @@ -223,7 +223,7 @@ BtrPageGCLock::~BtrPageGCLock() void BtrPageGCLock::disablePageGC(thread_db* tdbb, const PageNumber &page) { - page.getLockStr(lck_key.lck_string); + page.getLockStr(getKeyString()); LCK_lock(tdbb, this, LCK_read, LCK_WAIT); } @@ -235,7 +235,7 @@ void BtrPageGCLock::enablePageGC(thread_db* tdbb) bool BtrPageGCLock::isPageGCAllowed(thread_db* tdbb, const PageNumber& page) { BtrPageGCLock lock(tdbb); - page.getLockStr(lock.lck_key.lck_string); + page.getLockStr(lock.getKeyString()); ThreadStatusGuard temp_status(tdbb); diff --git a/src/jrd/cch.cpp b/src/jrd/cch.cpp index ea1651c830..e241fd8360 100644 --- a/src/jrd/cch.cpp +++ b/src/jrd/cch.cpp @@ -4055,7 +4055,7 @@ static LockState lock_buffer(thread_db* tdbb, BufferDesc* bdb, const SSHORT wait fb_assert(lock->lck_ast != NULL); } - bdb->bdb_page.getLockStr(lock->lck_key.lck_string); + bdb->bdb_page.getLockStr(lock->getKeyString()); if (LCK_lock_opt(tdbb, lock, lock_type, wait)) { if (!lock->lck_ast) diff --git a/src/jrd/lck.cpp b/src/jrd/lck.cpp index 8176af9fe3..be62b0c53c 100644 --- a/src/jrd/lck.cpp +++ b/src/jrd/lck.cpp @@ -739,7 +739,7 @@ SINT64 LCK_read_data(thread_db* tdbb, Lock* lock) const SINT64 data = dbb->dbb_lock_mgr->readData2(lock->lck_type, - lock->lck_key.lck_string, lock->lck_length, + lock->getKeyString(), lock->lck_length, lock->lck_owner_handle); fb_assert(LCK_CHECK_LOCK(lock)); return data; @@ -909,7 +909,7 @@ static void enqueue(thread_db* tdbb, CheckStatusWrapper* statusVector, Lock* loc fb_assert(LCK_CHECK_LOCK(lock)); lock->lck_id = dbb->dbb_lock_mgr->enqueue(tdbb, statusVector, lock->lck_id, - lock->lck_type, lock->lck_key.lck_string, lock->lck_length, + lock->lck_type, lock->getKeyString(), lock->lck_length, level, lock->lck_ast, lock->lck_object, lock->lck_data, wait, lock->lck_owner_handle); @@ -1036,7 +1036,7 @@ static Lock* hash_get_lock(Lock* lock, USHORT* hash_slot, Lock*** prior) if (!att->att_compatibility_table) hash_allocate(lock); - const USHORT hash_value = hash_func((UCHAR*) &lock->lck_key, lock->lck_length); + const USHORT hash_value = hash_func(lock->getKeyString(), lock->lck_length); if (hash_slot) *hash_slot = hash_value; @@ -1061,7 +1061,7 @@ static Lock* hash_get_lock(Lock* lock, USHORT* hash_slot, Lock*** prior) { // check that the keys are the same - if (!memcmp(lock->lck_key.lck_string, collision->lck_key.lck_string, lock->lck_length)) + if (!memcmp(lock->getKeyString(), collision->getKeyString(), lock->lck_length)) return collision; } @@ -1426,7 +1426,7 @@ static bool internal_enqueue(thread_db* tdbb, CheckStatusWrapper* statusVector, // with the local ast handler, passing it the lock block itself lock->lck_id = dbb->dbb_lock_mgr->enqueue(tdbb, statusVector, lock->lck_id, - lock->lck_type, (const UCHAR*) &lock->lck_key, lock->lck_length, + lock->lck_type, lock->getKeyString(), lock->lck_length, level, external_ast, lock, lock->lck_data, wait, lock->lck_owner_handle); // If the lock exchange failed, set the lock levels appropriately diff --git a/src/jrd/lck.h b/src/jrd/lck.h index 71596f9b10..6dcb1be67f 100644 --- a/src/jrd/lck.h +++ b/src/jrd/lck.h @@ -134,10 +134,19 @@ public: union { - UCHAR lck_string[1]; + UCHAR lck_string[8]; SINT64 lck_long; } lck_key; // Lock key string + UCHAR* getKeyString() + { +#ifdef WORDS_BIGENDIAN + if (lck_length <= 8) + return &lck_key.lck_string[8-lck_length]; +#endif + return &lck_key.lck_string[0]; + } + UCHAR lck_tail[1]; // Makes the allocator happy }; diff --git a/src/jrd/met.epp b/src/jrd/met.epp index 61210258d4..1bd4ba46dc 100644 --- a/src/jrd/met.epp +++ b/src/jrd/met.epp @@ -1390,7 +1390,7 @@ void MET_dsql_cache_release(thread_db* tdbb, int type, const MetaName& name, con const USHORT key_length = item->lock->lck_length; AutoPtr temp_lock(FB_NEW_RPT(*tdbb->getDefaultPool(), key_length) Lock(tdbb, key_length, LCK_dsql_cache)); - memcpy(temp_lock->lck_key.lck_string, item->lock->lck_key.lck_string, key_length); + memcpy(temp_lock->getKeyString(), item->lock->getKeyString(), key_length); if (LCK_lock(tdbb, temp_lock, LCK_EX, LCK_WAIT)) LCK_release(tdbb, temp_lock); @@ -4134,7 +4134,7 @@ static DSqlCacheItem* get_dsql_cache_item(thread_db* tdbb, int type, const Quali item->locked = false; item->lock = FB_NEW_RPT(*attachment->att_pool, key.length()) Lock(tdbb, key.length(), LCK_dsql_cache, item, blocking_ast_dsql_cache); - memcpy(item->lock->lck_key.lck_string, key.c_str(), key.length()); + memcpy(item->lock->getKeyString(), key.c_str(), key.length()); } else {