diff options
| author | Tom Lane <tgl@sss.pgh.pa.us> | 2022-03-03 19:03:17 -0500 |
|---|---|---|
| committer | Tom Lane <tgl@sss.pgh.pa.us> | 2022-03-03 19:03:35 -0500 |
| commit | 5c9d17e94c5cc0d24c1f0dbfe030000238d65afb (patch) | |
| tree | af73da24a9f6e754d6e3eb03b3d72f07a2aebef5 | |
| parent | b0bc196e52e606fe0116fb63da20f57fb577745b (diff) | |
| download | postgresql-5c9d17e94c5cc0d24c1f0dbfe030000238d65afb.tar.gz postgresql-5c9d17e94c5cc0d24c1f0dbfe030000238d65afb.zip | |
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
| -rw-r--r-- | src/include/storage/block.h | 2 |
1 files changed, 1 insertions, 1 deletions
diff --git a/src/include/storage/block.h b/src/include/storage/block.h index 4a5c476d2d0..e7d86e5fe60 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 */ |