diff options
| author | Thomas Munro <tmunro@postgresql.org> | 2025-02-15 13:13:19 +1300 |
|---|---|---|
| committer | Thomas Munro <tmunro@postgresql.org> | 2025-02-21 15:16:37 +1300 |
| commit | 57dca6faa9bdfd3c2ad2805fc088dc768d36fcf0 (patch) | |
| tree | 9932d516aa67a96de366ee6e14303ad52de9f550 /src | |
| parent | 29cce279bbc5919b8a85fb1ebc71d24395b2451b (diff) | |
Fix explicit valgrind interaction in read_stream.c.
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
Diffstat (limited to 'src')
| -rw-r--r-- | src/backend/storage/aio/read_stream.c | 38 |
1 files changed, 32 insertions, 6 deletions
diff --git a/src/backend/storage/aio/read_stream.c b/src/backend/storage/aio/read_stream.c index 9b962c301bf..b5be92d4611 100644 --- a/src/backend/storage/aio/read_stream.c +++ b/src/backend/storage/aio/read_stream.c @@ -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. */ |