Fix asserts in fast-path locking code
authorTomas Vondra <tomas.vondra@postgresql.org>
Mon, 23 Sep 2024 09:37:12 +0000 (11:37 +0200)
committerTomas Vondra <tomas.vondra@postgresql.org>
Mon, 23 Sep 2024 09:37:12 +0000 (11:37 +0200)
Commit c4d5cb71d229 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

src/backend/storage/lmgr/lock.c
src/backend/storage/lmgr/proc.c

index 613b0d4994438d89eda5f0fefee9586a3b50c55a..bbd444745fd96958f9a1ddd1beb725b4c6f4286e 100644 (file)
@@ -218,8 +218,8 @@ 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)))
 
 /*
@@ -227,10 +227,10 @@ int           FastPathLockGroupsPerBackend = 0;
  * 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 */
index 9b72829725ac7477f72aba7d68cad64504c8c442..0d8162a2cca3b45b8f800121830075c484627236 100644 (file)
@@ -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