The functions in the "0003" patch haven't surfaced in my "make
installcheck-parallel" runs with Valgrind, or the "make check" with
MemorySanitizer. However, I could hit most of them with some fuzzing. The
only exception was `xl_hash_vacuum_one_page` but that's probably also
triggerable.

I noticed that we also use `sizeof` in some WAL functions, so probably the
tail padding can also be written to WAL? For example, consider this:
(gdb) ptype/o gistxlogPageSplit
type = struct gistxlogPageSplit {
/*      0      |       4 */    BlockNumber origrlink;
/* XXX  4-byte hole      */
/*      8      |       8 */    GistNSN orignsn;
/*     16      |       1 */    _Bool origleaf;
/* XXX  1-byte hole      */
/*     18      |       2 */    uint16 npage;
/*     20      |       1 */    _Bool markfollowright;
/* XXX  3-byte padding   */

                               /* total size (bytes):   24 */
                             }

And then we do  XLogRegisterData((char *) &xlrec,
sizeof(gistxlogPageSplit));


In general, I'm wondering what our approach to this should be. Several
potential improvements were mentioned, but I think for now we could focus
on removing the Valgrind suppression. This is a meaningful improvement that
uses the existing test tools. Do we want to defensively zero-initialize
every case that seems to be potentially affected, i.e. written to WAL and
has holes/tail padding? That sounds cheap and simple and probably even
backportable. In the "0001" patch, there are several cases where no padding
goes into WAL, I can remove these. For example, the use of
xl_brin_createidx in brinbuild() does not have this problem.

Reply via email to