Skip site navigation (1) Skip section navigation (2)

Re: [PATCH] unified frontend support for pg_malloc et al and palloc/pfree mulation (was xlogreader-v4)

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Andres Freund <andres(at)2ndquadrant(dot)com>
Cc: Heikki Linnakangas <hlinnakangas(at)vmware(dot)com>, Peter Eisentraut <peter_e(at)gmx(dot)net>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: [PATCH] unified frontend support for pg_malloc et al and palloc/pfree mulation (was xlogreader-v4)
Date: 2013-01-13 18:41:36
Message-ID: 13628.1358102496@sss.pgh.pa.us (view raw or flat)
Thread:
Lists: pgsql-hackers
Andres Freund <andres(at)2ndquadrant(dot)com> writes:
> On 2013-01-12 15:39:16 -0500, Tom Lane wrote:
>> This is actually a disadvantage of the proposal to replace the abort()
>> calls with __builtin_unreachable(), too.  The gcc boys interpret the
>> semantics of that as "if control reaches here, the behavior is
>> undefined"

> We could make add an Assert() additionally?

I see little value in that.  In a non-assert build it will do nothing at
all, and in an assert build it'd just add code bloat.

I think there might be a good argument for defining pg_unreachable() as
abort() in assert-enabled builds, even if the compiler has
__builtin_unreachable(): that way, if the impossible happens, we'll find
out about it in a predictable and debuggable way.  And by definition,
we're not so concerned about either performance or code bloat in assert
builds.

> Where my variant is:
> #define ereport_domain(elevel, domain, rest)                                \
>     do {                                                                    \
>         const int elevel_ = elevel;                                         \
>         if (errstart(elevel_,__FILE__, __LINE__, PG_FUNCNAME_MACRO, domain))\
>         {                                                                   \
>             errfinish rest;                                                 \
>         }                                                                   \
>         if (elevel_>= ERROR)                                                \
>             pg_unreachable();                                               \
>     } while(0)

This is the same implementation I'd arrived at after looking at
Heikki's.  I think we should probably use this for non-gcc builds.
However, for recent gcc (those with __builtin_constant_p) I think that
my original suggestion is better, ie don't use a local variable but
instead use __builtin_constant_p(elevel) && (elevel) >= ERROR in the
second test.  That way we don't have the problem with unreachability
tests failing at -O0.  We may still have some issues with bogus warnings
in other compilers, but I think most PG developers are using recent gcc.

			regards, tom lane


In response to

pgsql-hackers by date

Next:From: Andres FreundDate: 2013-01-13 19:06:37
Subject: Re: Re: [PATCH] unified frontend support for pg_malloc et al and palloc/pfree mulation (was xlogreader-v4)
Previous:From: Andrew DunstanDate: 2013-01-13 18:18:58
Subject: psql binary output

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group