Re: Potential us of initialized memory in xlogrecovery.c

[Heikki Linnakangas]

On 06/06/2023 10:24, Tristan Partin wrote:

Hello,

Today, I compiled the master branch of Postgres with the following GCC
version:

gcc (GCC) 13.1.1 20230511 (Red Hat 13.1.1-2)

I got the following warning:

[701/2058] Compiling C object 
src/backend/postgres_lib.a.p/access_transam_xlogrecovery.c.o
In function ‘recoveryStopsAfter’,
 inlined from ‘PerformWalRecovery’ at 
../src/backend/access/transam/xlogrecovery.c:1749:8:
../src/backend/access/transam/xlogrecovery.c:2756:42: warning: ‘recordXtime’ 
may be used uninitialized [-Wmaybe-uninitialized]
  2756 | recoveryStopTime = recordXtime;
   | ~^
../src/backend/access/transam/xlogrecovery.c: In function ‘PerformWalRecovery’:
../src/backend/access/transam/xlogrecovery.c:2647:21: note: ‘recordXtime’ was 
declared here
  2647 | TimestampTz recordXtime;
   | ^~~

Investigating this issue I see a potential assignment in
xlogrecovery.c:2715. Best I can tell the warning looks real. Similar
functions in this file seem to initialize recordXtime to 0. Attached is
a patch which does just that.


Thank you! My refactoring in commit c945af80cf introduced this. Looking 
at getRecordTimestamp(), it will always return true and set recordXtime 
for the commit and abort records, and some compilers can deduce that.


Initializing to 0 makes sense, I'll commit that fix later tonight.

--
Heikki Linnakangas
Neon (https://neon.tech)

Potential us of initialized memory in xlogrecovery.c

[Tristan Partin]

Hello,

Today, I compiled the master branch of Postgres with the following GCC
version:

gcc (GCC) 13.1.1 20230511 (Red Hat 13.1.1-2)

I got the following warning:

[701/2058] Compiling C object 
src/backend/postgres_lib.a.p/access_transam_xlogrecovery.c.o
In function ‘recoveryStopsAfter’,
inlined from ‘PerformWalRecovery’ at 
../src/backend/access/transam/xlogrecovery.c:1749:8:
../src/backend/access/transam/xlogrecovery.c:2756:42: warning: ‘recordXtime’ 
may be used uninitialized [-Wmaybe-uninitialized]
 2756 | recoveryStopTime = recordXtime;
  | ~^
../src/backend/access/transam/xlogrecovery.c: In function ‘PerformWalRecovery’:
../src/backend/access/transam/xlogrecovery.c:2647:21: note: ‘recordXtime’ was 
declared here
 2647 | TimestampTz recordXtime;
  | ^~~

Investigating this issue I see a potential assignment in
xlogrecovery.c:2715. Best I can tell the warning looks real. Similar
functions in this file seem to initialize recordXtime to 0. Attached is
a patch which does just that.

-- 
Tristan Partin
Neon (https://neon.tech)
From 74cc0fcd0570383d273f49dbc7d669caf7474453 Mon Sep 17 00:00:00 2001
From: Tristan Partin 
Date: Tue, 6 Jun 2023 09:22:56 -0500
Subject: [PATCH v1] Initialize potentially uninitialized memory in
 xlogrecovery.c

Seems that in certain cases recordXtime might be used prior to
assignment.
---
 src/backend/access/transam/xlogrecovery.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/backend/access/transam/xlogrecovery.c b/src/backend/access/transam/xlogrecovery.c
index 4883fcb512..becc2bda62 100644
--- a/src/backend/access/transam/xlogrecovery.c
+++ b/src/backend/access/transam/xlogrecovery.c
@@ -2644,7 +2644,7 @@ recoveryStopsAfter(XLogReaderState *record)
 	uint8		info;
 	uint8		xact_info;
 	uint8		rmid;
-	TimestampTz recordXtime;
+	TimestampTz recordXtime = 0;
 
 	/*
 	 * Ignore recovery target settings when not in archive recovery (meaning
-- 
-- 
Tristan Partin
Neon (https://neon.tech)