From 97031f440485a01627cb1cc442c0fb10d27b97bd Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Thu, 3 Mar 2022 19:03:17 -0500 Subject: [PATCH] Fix bogus casting in BlockIdGetBlockNumber(). This macro cast the result to BlockNumber after shifting, not before, which is the wrong thing. Per the C spec, the uint16 fields would promote to int not unsigned int, so that (for 32-bit int) the shift potentially shifts a nonzero bit into the sign position. I doubt there are any production systems where this would actually end with the wrong answer, but it is undefined behavior per the C spec, and clang's -fsanitize=undefined option reputedly warns about it on some platforms. (I can't reproduce that right now, but the code is undeniably wrong per spec.) It's easy to fix by casting to BlockNumber (uint32) in the proper places. It's been wrong for ages, so back-patch to all supported branches. Report and patch by Zhihong Yu (cosmetic tweaking by me) Discussion: https://postgr.es/m/CALNJ-vT9r0DSsAOw9OXVJFxLENoVS_68kJ5x0p44atoYH+H4dg@mail.gmail.com --- src/include/storage/block.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/include/storage/block.h b/src/include/storage/block.h index d73e393051..a87039bf61 100644 --- a/src/include/storage/block.h +++ b/src/include/storage/block.h @@ -115,7 +115,7 @@ typedef BlockIdData *BlockId; /* block identifier */ #define BlockIdGetBlockNumber(blockId) \ ( \ AssertMacro(BlockIdIsValid(blockId)), \ - (BlockNumber) (((blockId)->bi_hi << 16) | ((uint16) (blockId)->bi_lo)) \ + ((((BlockNumber) (blockId)->bi_hi) << 16) | ((BlockNumber) (blockId)->bi_lo)) \ ) #endif /* BLOCK_H */