mirror of
https://git.postgresql.org/git/postgresql.git
synced 2024-09-30 16:31:22 +02:00
Fix mislabeling of PROC_QUEUE->links as PGPROC, fixing UBSan on 32bit
ProcSleep() used a PGPROC* variable to point to PROC_QUEUE->links.next,
because that does "the right thing" with SHMQueueInsertBefore(). While that
largely works, it's certainly not correct and unnecessary - we can just use
SHM_QUEUE* to point to the insertion point.
Noticed when testing a 32bit of postgres with undefined behavior
sanitizer. UBSan noticed that sometimes the supposed PGPROC wasn't
sufficiently aligned (required since 46d6e5f567
, ensured indirectly, via
ShmemAllocRaw() guaranteeing cacheline alignment).
For now fix this by using a SHM_QUEUE* for the insertion point. Subsequently
we should replace all the use of PROC_QUEUE and SHM_QUEUE with ilist.h, but
that's a larger change that we don't want to backpatch.
Backpatch to all supported versions - it's useful to be able to run postgres
under UBSan.
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/20221117014230.op5kmgypdv2dtqsf@awork3.anarazel.de
Backpatch: 11-
This commit is contained in:
parent
d3d388831d
commit
c13667b518
@ -1067,11 +1067,11 @@ ProcSleep(LOCALLOCK *locallock, LockMethod lockMethodTable)
|
||||
uint32 hashcode = locallock->hashcode;
|
||||
LWLock *partitionLock = LockHashPartitionLock(hashcode);
|
||||
PROC_QUEUE *waitQueue = &(lock->waitProcs);
|
||||
SHM_QUEUE *waitQueuePos;
|
||||
LOCKMASK myHeldLocks = MyProc->heldLocks;
|
||||
bool early_deadlock = false;
|
||||
bool allow_autovacuum_cancel = true;
|
||||
int myWaitStatus;
|
||||
PGPROC *proc;
|
||||
PGPROC *leader = MyProc->lockGroupLeader;
|
||||
int i;
|
||||
|
||||
@ -1119,13 +1119,16 @@ ProcSleep(LOCALLOCK *locallock, LockMethod lockMethodTable)
|
||||
* we are only considering the part of the wait queue before my insertion
|
||||
* point.
|
||||
*/
|
||||
if (myHeldLocks != 0)
|
||||
if (myHeldLocks != 0 && waitQueue->size > 0)
|
||||
{
|
||||
LOCKMASK aheadRequests = 0;
|
||||
SHM_QUEUE *proc_node;
|
||||
|
||||
proc = (PGPROC *) waitQueue->links.next;
|
||||
proc_node = waitQueue->links.next;
|
||||
for (i = 0; i < waitQueue->size; i++)
|
||||
{
|
||||
PGPROC *proc = (PGPROC *) proc_node;
|
||||
|
||||
/*
|
||||
* If we're part of the same locking group as this waiter, its
|
||||
* locks neither conflict with ours nor contribute to
|
||||
@ -1133,7 +1136,7 @@ ProcSleep(LOCALLOCK *locallock, LockMethod lockMethodTable)
|
||||
*/
|
||||
if (leader != NULL && leader == proc->lockGroupLeader)
|
||||
{
|
||||
proc = (PGPROC *) proc->links.next;
|
||||
proc_node = proc->links.next;
|
||||
continue;
|
||||
}
|
||||
/* Must he wait for me? */
|
||||
@ -1168,24 +1171,25 @@ ProcSleep(LOCALLOCK *locallock, LockMethod lockMethodTable)
|
||||
}
|
||||
/* Nope, so advance to next waiter */
|
||||
aheadRequests |= LOCKBIT_ON(proc->waitLockMode);
|
||||
proc = (PGPROC *) proc->links.next;
|
||||
proc_node = proc->links.next;
|
||||
}
|
||||
|
||||
/*
|
||||
* If we fall out of loop normally, proc points to waitQueue head, so
|
||||
* we will insert at tail of queue as desired.
|
||||
* If we iterated through the whole queue, cur points to the waitQueue
|
||||
* head, so we will insert at tail of queue as desired.
|
||||
*/
|
||||
waitQueuePos = proc_node;
|
||||
}
|
||||
else
|
||||
{
|
||||
/* I hold no locks, so I can't push in front of anyone. */
|
||||
proc = (PGPROC *) &(waitQueue->links);
|
||||
waitQueuePos = &waitQueue->links;
|
||||
}
|
||||
|
||||
/*
|
||||
* Insert self into queue, ahead of the given proc (or at tail of queue).
|
||||
* Insert self into queue, at the position determined above.
|
||||
*/
|
||||
SHMQueueInsertBefore(&(proc->links), &(MyProc->links));
|
||||
SHMQueueInsertBefore(waitQueuePos, &MyProc->links);
|
||||
waitQueue->size++;
|
||||
|
||||
lock->waitMask |= LOCKBIT_ON(lockmode);
|
||||
|
Loading…
Reference in New Issue
Block a user