Re: BUG #15943: Valgrind-detected error in SlruPhysicalWritePage

Hello Andres,

08.08.2019 13:33, Andres Freund wrote:
> Hi,
>
> On August 8, 2019 12:44:43 AM EDT, PG Bug reporting form <noreply(at)postgresql(dot)org> wrote:
>> The following bug has been logged on the website:
>>
>> Bug reference: 15943
>> Logged by: Alexander Lakhin
>> Email address: exclusion(at)gmail(dot)com
>> PostgreSQL version: 12beta2
>> Operating system: Ubuntu 18.04
>> Description:
>>
>> When running under valgrind (sitting on the master branch):
>> printf "SELECT pg_notify('notify_async1',NULL); %.0s" `seq 2048` | psql
>>
> That seems likely to be padding. Probably easier to see if you enable read-var-info and track-origins.
>
With "--track-origins=yes --read-var-info=yes" I get:

==00:00:00:30.800 24110== Syscall param write(buf) points to
uninitialised byte(s)
==00:00:00:30.801 24110==    at 0x5054281: write (write.c:27)
==00:00:00:30.801 24110==    by 0x2687F0: SlruPhysicalWritePage (slru.c:851)
==00:00:00:30.801 24110==    by 0x268E95: SlruInternalWritePage (slru.c:543)
==00:00:00:30.801 24110==    by 0x2690F1: SlruSelectLRUPage (slru.c:1095)
==00:00:00:30.801 24110==    by 0x269777: SimpleLruZeroPage (slru.c:269)
==00:00:00:30.801 24110==    by 0x31DABA: asyncQueueAddEntries
(async.c:1381)
==00:00:00:30.801 24110==    by 0x31F130: PreCommit_Notify (async.c:845)
==00:00:00:30.801 24110==    by 0x273C19: CommitTransaction (xact.c:2128)
==00:00:00:30.801 24110==    by 0x274A10: CommitTransactionCommand
(xact.c:2921)
==00:00:00:30.801 24110==    by 0x53BD7D: finish_xact_command
(postgres.c:2597)
==00:00:00:30.801 24110==    by 0x53E2F3: exec_simple_query
(postgres.c:1256)
==00:00:00:30.801 24110==    by 0x540545: PostgresMain (postgres.c:4256)
==00:00:00:30.801 24110==  Address 0xf4f95bf is in a rw- anonymous segment
==00:00:00:30.801 24110==  Uninitialised value was created by a stack
allocation
==00:00:00:30.801 24110==    at 0x31D967: asyncQueueAddEntries
(async.c:1306)

and
==00:00:00:48.177 24110== Syscall param write(buf) points to
uninitialised byte(s)
==00:00:00:48.177 24110==    at 0x5054281: write (write.c:27)
==00:00:00:48.177 24110==    by 0x2687F0: SlruPhysicalWritePage (slru.c:851)
==00:00:00:48.177 24110==    by 0x268E95: SlruInternalWritePage (slru.c:543)
==00:00:00:48.177 24110==    by 0x26A1BD: SimpleLruTruncate (slru.c:1234)
==00:00:00:48.177 24110==    by 0x31D5A5: asyncQueueAdvanceTail
(async.c:2024)
==00:00:00:48.177 24110==    by 0x31F231: ProcessCompletedNotifies
(async.c:1132)
==00:00:00:48.177 24110==    by 0x5403D1: PostgresMain (postgres.c:4176)
==00:00:00:48.177 24110==    by 0x4B16D9: BackendRun (postmaster.c:4431)
==00:00:00:48.177 24110==    by 0x4B481B: BackendStartup (postmaster.c:4122)
==00:00:00:48.177 24110==    by 0x4B4B32: ServerLoop (postmaster.c:1704)
==00:00:00:48.178 24110==    by 0x4B5F70: PostmasterMain (postmaster.c:1377)
==00:00:00:48.178 24110==    by 0x406428: main (main.c:228)
==00:00:00:48.178 24110==  Address 0xf4fb5bf is in a rw- anonymous segment
==00:00:00:48.178 24110==  Uninitialised value was created by a stack
allocation
==00:00:00:48.178 24110==    at 0x31D967: asyncQueueAddEntries
(async.c:1306)

Indeed, "memset(&qe, 0, sizeof(qe));" added in the beginning of
asyncQueueAddEntries fixes the issue.
More interestingly, "memset(qe.data, 0, sizeof(qe.data));" in the same
place fixes it too (the patch is attached).
So it seems, the padding in AsyncQueueEntry has an alibi.

Best regards,
Alexander