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

Поиск
Список
Период
Сортировка
От Adam Lee
Тема PG_TRY()/CATCH() in a loop reports uninitialized variables
Дата
Msg-id 20190808082008.GC23836@mars.local
обсуждение исходный текст
Ответы Re: PG_TRY()/CATCH() in a loop reports uninitialized variables  (Tom Lane <tgl@sss.pgh.pa.us>)
Список pgsql-hackers
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

Вложения

В списке pgsql-hackers по дате отправления:

Предыдущее
От: Amit Langote
Дата:
Сообщение: Re: Problem with default partition pruning
Следующее
От: amul sul
Дата:
Сообщение: Re: Some memory not freed at the exit of RelationBuildPartitionDesc()