Re: longjmp clobber warnings are utterly broken in modern gcc

From: Andres Freund <andres(at)2ndquadrant(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: longjmp clobber warnings are utterly broken in modern gcc
Date: 2015-01-26 18:34:43
Message-ID: 20150126183443.GA5568@awork2.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> >> Aside from any reduction in the need
> >> for volatile, this might actually perform slightly better, because
> >> sigsetjmp() is a system call on some platforms. There are probably
> >> few cases where that actually matters, but the one in pq_getmessage(),
> >> for example, might not be entirely discountable.
> >
> > Hm. How would you implement PG_TRY_WITH_CLEANUP without a sigsetjmp()?
>
> Posit:
>
> struct cleanup_entry {
> void (*callback)(void *);
> void *callback_arg;
> struct cleanup_entry *next;
> };
> cleanup_entry *cleanup_stack = NULL;
>
> So PG_TRY_WITH_CLEANUP(_cb, _cb_arg) does (approximately) this:
>
> {
> cleanup_entry e;
> cleanup_entry *orige;
> e.callback = (_cb);
> e.callback_arg = (_cb_arg);
> e.next = cleanup_stack;
> orige = cleanup_stack;
> cleanup_stack = &e;
>
> And when you PG_END_TRY_WITH_CLEANUP, we just do this:
>
> cleanup_stack = orige;
> }

Hm. Not bad.

[ponder]

I guess we'd need to tie it into PG_exception_stack levels, so it
correctly handles nesting with sigsetjmp locations. In contrast to
sigsetjmp() style handlers we can't rely on PG_CATCH cleaning up that
state.

I wonder how hard it is to make that reliable for errors thrown in a
cleanup callback. Those certainly are possible in some of the PG_CATCHes
we have right now.

> And before doing sigsetjmp to the active handler, we run all the
> functions on the stack. There shouldn't be any need for volatile; the
> compiler has to know that once we make it possible to get at a pointer
> to cb_arg via a global variable (cleanup_stack), any function we call
> in another translation unit could decide to call that function and it
> would need to see up-to-date values of everything cb_arg points to.
> So before calling such a function it had better store that data to
> memory, not just leave it lying around in a register somewhere.

Given that we, at the moment at least, throw ERRORs from signal
handlers, I don't think that necessarily holds true. I think we're not
that far away from getting rid of all of those though.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Stephen Frost 2015-01-26 18:47:02 Re: Additional role attributes && superuser review
Previous Message Robert Haas 2015-01-26 18:34:34 Re: Proposal: knowing detail of config files via SQL