Fix asserts in fast-path locking code

tvondra · tvondra · commit a7e5237f268e · 2024-09-23T11:37:12.000+02:00
Commit c4d5cb7 introduced a couple asserts in the fast-path locking code, upsetting Coverity. The assert in InitProcGlobal() is clearly wrong, as it assigns instead of checking the value. This is harmless, but doesn't check anything. The asserts in FAST_PATH_ macros are written as if for signed values, but the macros are only called for unsigned ones. That makes the check for (val >= 0) useless. Checks written as ((uint32) x < max) work for both signed and unsigned values. Negative values should wrap to values greater than INT32_MAX. Per Coverity, report by Tom Lane. Reported-by: Tom Lane Discussion: https://postgr.es/m/2891628.1727019959@sss.pgh.pa.us
diff --git a/src/backend/storage/lmgr/lock.c b/src/backend/storage/lmgr/lock.c
@@ -218,19 +218,19 @@ int			FastPathLockGroupsPerBackend = 0;
  * of fast-path lock slots.
  */
 #define FAST_PATH_SLOT(group, index) \
-	(AssertMacro(((group) >= 0) && ((group) < FastPathLockGroupsPerBackend)), \
-	 AssertMacro(((index) >= 0) && ((index) < FP_LOCK_SLOTS_PER_GROUP)), \
+	(AssertMacro((uint32) (group) < FastPathLockGroupsPerBackend), \
+	 AssertMacro((uint32) (index) < FP_LOCK_SLOTS_PER_GROUP), \
 	 ((group) * FP_LOCK_SLOTS_PER_GROUP + (index)))
 
 /*
  * Given a slot index (into the whole per-backend array), calculated using
  * the FAST_PATH_SLOT macro, split it into group and index (in the group).
  */
 #define FAST_PATH_GROUP(index)	\
-	(AssertMacro(((index) >= 0) && ((index) < FP_LOCK_SLOTS_PER_BACKEND)), \
+	(AssertMacro((uint32) (index) < FP_LOCK_SLOTS_PER_BACKEND), \
 	 ((index) / FP_LOCK_SLOTS_PER_GROUP))
 #define FAST_PATH_INDEX(index)	\
-	(AssertMacro(((index) >= 0) && ((index) < FP_LOCK_SLOTS_PER_BACKEND)), \
+	(AssertMacro((uint32) (index) < FP_LOCK_SLOTS_PER_BACKEND), \
 	 ((index) % FP_LOCK_SLOTS_PER_GROUP))
 
 /* Macros for manipulating proc->fpLockBits */
diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.c
@@ -322,7 +322,7 @@ InitProcGlobal(void)
 	}
 
 	/* Should have consumed exactly the expected amount of fast-path memory. */
-	Assert(fpPtr = fpEndPtr);
+	Assert(fpPtr == fpEndPtr);
 
 	/*
 	 * Save pointers to the blocks of PGPROC structures reserved for auxiliary

Original file line number	Diff line number	Diff line change
`@@ -322,7 +322,7 @@ InitProcGlobal(void)`
`322`	`322`	`}`
`323`	`323`
`324`	`324`	`/* Should have consumed exactly the expected amount of fast-path memory. */`
`325`		`- Assert(fpPtr = fpEndPtr);`
	`325`	`+ Assert(fpPtr == fpEndPtr);`
`326`	`326`
`327`	`327`	`/*`
`328`	`328`	`* Save pointers to the blocks of PGPROC structures reserved for auxiliary`