Horton

Thread

Commits

  1. Initialize 'recordXtime' to silence compiler warning.

  2. Refactor checking whether we've reached the recovery target.

  1. Potential us of initialized memory in xlogrecovery.c

    Tristan Partin <tristan@neon.tech> — 2023-06-06T14:24:56Z

    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)

  2. Re: Potential us of initialized memory in xlogrecovery.c

    Heikki Linnakangas <hlinnaka@iki.fi> — 2023-06-06T16:31:14Z

    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)