]> git.ipfire.org Git - thirdparty/postgresql.git/commitdiff
Fix explicit valgrind interaction in read_stream.c.
authorThomas Munro <tmunro@postgresql.org>
Sat, 15 Feb 2025 00:13:19 +0000 (13:13 +1300)
committerThomas Munro <tmunro@postgresql.org>
Fri, 21 Feb 2025 02:16:37 +0000 (15:16 +1300)
This is a back-patch of commits 2a8a0067 and 2509b857 into
REL_17_STABLE.  It's doesn't fix any known live bug in PostgreSQL v17
itself, but an extension could in theory have used the per-buffer data
feature and seen spurious errors under Valgrind.

Discussion: https://postgr.es/m/CA%2BhUKG%2Bg6aXpi2FEHqeLOzE%2BxYw%3DOV%2B-N5jhOEnnV%2BF0USM9xA%40mail.gmail.com

src/backend/storage/aio/read_stream.c

index 9b962c301bff620a297259c4fd580a9219271172..b5be92d4611283a76152ae9f99eab921e9a4974d 100644 (file)
@@ -176,9 +176,20 @@ read_stream_get_block(ReadStream *stream, void *per_buffer_data)
        if (blocknum != InvalidBlockNumber)
                stream->buffered_blocknum = InvalidBlockNumber;
        else
+       {
+               /*
+                * Tell Valgrind that the per-buffer data is undefined.  That replaces
+                * the "noaccess" state that was set when the consumer moved past this
+                * entry last time around the queue, and should also catch callbacks
+                * that fail to initialize data that the buffer consumer later
+                * accesses.  On the first go around, it is undefined already.
+                */
+               VALGRIND_MAKE_MEM_UNDEFINED(per_buffer_data,
+                                                                       stream->per_buffer_data_size);
                blocknum = stream->callback(stream,
                                                                        stream->callback_private_data,
                                                                        per_buffer_data);
+       }
 
        return blocknum;
 }
@@ -687,8 +698,11 @@ read_stream_next_buffer(ReadStream *stream, void **per_buffer_data)
        }
 
 #ifdef CLOBBER_FREED_MEMORY
-       /* Clobber old buffer and per-buffer data for debugging purposes. */
+       /* Clobber old buffer for debugging purposes. */
        stream->buffers[oldest_buffer_index] = InvalidBuffer;
+#endif
+
+#if defined(CLOBBER_FREED_MEMORY) || defined(USE_VALGRIND)
 
        /*
         * The caller will get access to the per-buffer data, until the next call.
@@ -697,11 +711,23 @@ read_stream_next_buffer(ReadStream *stream, void **per_buffer_data)
         * that is holding a dangling pointer to it.
         */
        if (stream->per_buffer_data)
-               wipe_mem(get_per_buffer_data(stream,
-                                                                        oldest_buffer_index == 0 ?
-                                                                        stream->queue_size - 1 :
-                                                                        oldest_buffer_index - 1),
-                                stream->per_buffer_data_size);
+       {
+               void       *per_buffer_data;
+
+               per_buffer_data = get_per_buffer_data(stream,
+                                                                                         oldest_buffer_index == 0 ?
+                                                                                         stream->queue_size - 1 :
+                                                                                         oldest_buffer_index - 1);
+
+#if defined(CLOBBER_FREED_MEMORY)
+               /* This also tells Valgrind the memory is "noaccess". */
+               wipe_mem(per_buffer_data, stream->per_buffer_data_size);
+#elif defined(USE_VALGRIND)
+               /* Tell it ourselves. */
+               VALGRIND_MAKE_MEM_NOACCESS(per_buffer_data,
+                                                                  stream->per_buffer_data_size);
+#endif
+       }
 #endif
 
        /* Pin transferred to caller. */