PG_TRY()/CATCH() in a loop reports uninitialized variables

Started by Adam Leeabout 7 years ago2 messageshackers
Beta feature

Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.

won't retrysuccessCI history

You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:

docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t41052
psql -h localhost -U postgres

Built from patchset v1 (message #1), July 28, 2026 at 03:54 AM.

Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:

git clone --branch t41052_1 https://github.com/hackorum-dev/postgres.git

In a checkout you already have, add the fork once:

git remote add hackorum https://github.com/hackorum-dev/postgres.git

then, for this patchset and every later one:

git fetch hackorum t41052_1 && git checkout t41052_1

Patchset v1 (message #1) is on t41052_1

Jump to latest
#1Adam Lee
ali@pivotal.io

Hi, hackers

"The local variables that do not have the volatile type and have been changed
between the setjmp() invocation and longjmp() call are indeterminate". This is
what the POSIX (and C standard for setjmp) says.

That's fine actually, but if we put the PG_TRY()/CATCH() in a loop, high
version gcc might complain.

Version:
$ gcc-9 --version
gcc-9 (Ubuntu 9.1.0-2ubuntu2~19.04) 9.1.0
(Actually from gcc 7)

Reproducer:
```
#include <setjmp.h>

extern int other(void);
extern void trigger(int *cond1);
extern sigjmp_buf *PG_exception_stack;

void
trigger(int *cond1)
{
while (1)
{
if (*cond1 == 0)
*cond1 = other();

while (*cond1)
{
sigjmp_buf *save_exception_stack = PG_exception_stack;
sigjmp_buf local_sigjmp_buf;

if (sigsetjmp(local_sigjmp_buf, 0) == 0)
PG_exception_stack = &local_sigjmp_buf;
else
PG_exception_stack = (sigjmp_buf *) save_exception_stack;

PG_exception_stack = (sigjmp_buf *) save_exception_stack;
}
}
}
```

```
$ gcc-9 -O1 -Werror=uninitialized -fexpensive-optimizations -ftree-pre -c -o /dev/null reproducer.c
reproducer.c: In function 'trigger':
reproducer.c:17:16: error: 'save_exception_stack' is used uninitialized in this function [-Werror=uninitialized]
17 | sigjmp_buf *save_exception_stack = PG_exception_stack;
| ^~~~~~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors
```

Codes re-ordering matters, when it warns:
```
sigjmp_buf *save_exception_stack = PG_exception_stack;
2f: 48 8b 1d 00 00 00 00 mov 0x0(%rip),%rbx # 36 <trigger+0x36>
36: 48 89 5c 24 18 mov %rbx,0x18(%rsp)
sigjmp_buf local_sigjmp_buf;

if (sigsetjmp(local_sigjmp_buf, 0) == 0)
```

When it doesn't complain:
```
sigjmp_buf *save_exception_stack = PG_exception_stack;
sigjmp_buf local_sigjmp_buf;

if (sigsetjmp(local_sigjmp_buf, 0) == 0)
29: 48 8d 44 24 20 lea 0x20(%rsp),%rax
2e: 48 89 44 24 08 mov %rax,0x8(%rsp)
...
sigjmp_buf *save_exception_stack = PG_exception_stack;
3c: 48 8b 1d 00 00 00 00 mov 0x0(%rip),%rbx # 43 <trigger+0x43>
43: 48 89 5c 24 18 mov %rbx,0x18(%rsp)
```

Greenplum had an issue reporting save_exception_stack and save_context_stack
not initialized.
https://github.com/greenplum-db/gpdb/issues/8262

I filed a bug report for gcc, they think it's expected.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91395

Since high version gcc thinks it's supposed to report warning, we need to make
these two variables volatile? Or have I missed something?

--
Adam Lee

Attachments:

t41052_1
make_save_stack_volatile.patchtext/plain; charset=us-asciiDownload+2-2
#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Adam Lee (#1)
Re: PG_TRY()/CATCH() in a loop reports uninitialized variables

Adam Lee <ali@pivotal.io> writes:

That's fine actually, but if we put the PG_TRY()/CATCH() in a loop, high
version gcc might complain.

I'd be inclined to say "so don't do that then". Given this interpretation
(which sure looks like a bug to me, gcc maintainers' opinion or no),
you're basically going to have to mark EVERYTHING in that function
volatile. Better to structure the code so you don't have to do that,
which would mean not putting the TRY and the loop in the same level
of function.

I've seen other weird-maybe-bug gcc behaviors in the vicinity of
setjmp calls, too, which is another factor that pushes me not to
want to assume too much about what you can do in the same function
as a TRY call.

regards, tom lane