Re: define pg_structiszero(addr, s, r)

On Tue, Nov 12, 2024 at 06:09:04AM +0000, Bertrand Drouvot wrote:
> I think that the 64b len check done in v11 is mandatory for safety reasons.
>
> The loop above reads 64 bytes at once, so would read beyond the memory area bounds
> if len < 64: That could cause crash or read invalid data.

Sorry, I was not following your argument. You're right that we need
something else here. However..

+ /*
+ * For len < 64, compare byte per byte to ensure we'll not read beyond the
+ * memory area.
+ */
+ if (len < sizeof(size_t) * 8)
+ {
+ while (p < end)
+ {
+ if (*p++ != 0)
+ return false;
+ }
+ return true;
+ }
+
+ /* Compare bytes until the pointer "p" is aligned */
+ while (((uintptr_t) p & (sizeof(size_t) - 1)) != 0)
+ {
+ if (p == end)
+ return true;
+
+ if (*p++ != 0)
+ return false;
+ }
+

Still, this is not optimal, based on what's been discussed upthread.
The byte-per-byte check is more expensive than the size_t check, so
shouldn't you make sure that you stack some size_t checks if dealing
with something smaller than 64 bytes? That would be a bit more
complex, sure, but if you leave that within the block doing "len <
sizeof(size_t) * 8", perhaps that's OK.. Or just do what you
mentioned upthread with a second macro for sizes <= 64. You'd need
three steps in this first block rather than one:
- byte-per-byte, up to aligned location.
- size_t loop.
- Again byte-per-byte, until the end,
--
Michael