From 4b0696b36e5ae4fd1224c03011eadfeb4aa508dd 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 04ab4c6d71..9c61103cd8 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 */