ereport bug

Started by Dmitry Voroninabout 11 years ago6 messages
#1Dmitry Voronin
carriingfate92@yandex.ru
1 attachment(s)

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Attachments:

test.ctext/x-c; name=test.cDownload
#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Dmitry Voronin (#1)
Re: ereport bug

Dmitry Voronin <carriingfate92@yandex.ru> writes:

<div><div><div><div data-lang="2"><div>Hello, postgresmen!</div><div>�</div><div>I found incorrect execution of ereport() macro. <br />If we pass into ereport() function 2 or more arguments, the macro errcontext does not correct execute. So, ereport() call stack is:</div><div>�</div><div>errstart<br />errcontext_msg<br />set_errcontext_domain<br />errmsg<br />errfinish<br />pg_unreachable</div><div>�</div><div><span lang="en"><span>This bug</span> <span>causes that error messages (for example, in PL/TCL) are </span></span><span lang="en"><span>not localized.<br /><br />Solutions:<br />- Wrap all errcontext() macro in </span></span><span lang="en"><span><span lang="en"><span>brackets, that is errcontext("error message %s", "end message") -&gt; (</span></span></span></span><span lang="en"><span><span lang="en"><span>errcontext("error message %s", "end message"))</span></span></span></span></div><div><span lang="en"><span><span lang="en"><span>- Rewrite this macro</span></spa!

n></span></span></div><div><span lang="en"><span><span lang="en"><span>- ???</span></span></span></span></div><div>�</div><div><span lang="en"><span>I am attaching</span> <span>to this letter</span> <span>a test case</span> <span>that shows</span> <span>the behavior errcontext() macro and the way to fix it.<br /></span></span></div><div><br />I am using postgresql 9.4 and test it on gcc 4.7 and gcc 4.8.1.<br /><br /></div><div>-- Best regards, Dmitry Voronin</div></div></div></div></div>

(Please don't post HTML-only mail to the PG mailing lists ...)

Hm ... the initial thought was that errcontext would never be used
directly in an ereport() macro, but you're right that we now have some
places that violate that rule. So the comma expression turns into a
couple of arguments to errfinish, meaning the order of evaluation becomes
compiler-dependent which is bad.

I think the easiest fix is to have errstart initialize context_domain
to the same value as domain. The order of evaluation is still
compiler-dependent, but it no longer matters because any errcontext
calls occurring textually within an ereport should be trying to select
the same domain as the ereport anyway.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#3Robert Haas
robertmhaas@gmail.com
In reply to: Dmitry Voronin (#1)
Re: ereport bug

On Mon, Jan 12, 2015 at 6:27 AM, Dmitry Voronin
<carriingfate92@yandex.ru> wrote:

I am attaching to this letter a test case that shows the behavior
errcontext() macro and the way to fix it.

So the upshot of this is that given errfinish(A, B, C), where A, B,
and C are expressions, my gcc is choosing to evaluate C, then B, then
A, then the errfinish call itself. But whoever wrote the errcontext()
macro evidently thought, in this kind of situation, the compiler would
be certain to evaluate A, then B, then C, then errfinish. But it
doesn't.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Robert Haas (#3)
Re: ereport bug

Robert Haas <robertmhaas@gmail.com> writes:

On Mon, Jan 12, 2015 at 6:27 AM, Dmitry Voronin
<carriingfate92@yandex.ru> wrote:

I am attaching to this letter a test case that shows the behavior
errcontext() macro and the way to fix it.

So the upshot of this is that given errfinish(A, B, C), where A, B,
and C are expressions, my gcc is choosing to evaluate C, then B, then
A, then the errfinish call itself. But whoever wrote the errcontext()
macro evidently thought, in this kind of situation, the compiler would
be certain to evaluate A, then B, then C, then errfinish. But it
doesn't.

http://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=1f9bf05e539646103c518bcbb49c04919b238f7a

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#5Robert Haas
robertmhaas@gmail.com
In reply to: Tom Lane (#4)
Re: ereport bug

On Wed, Jan 14, 2015 at 1:40 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Robert Haas <robertmhaas@gmail.com> writes:

On Mon, Jan 12, 2015 at 6:27 AM, Dmitry Voronin
<carriingfate92@yandex.ru> wrote:

I am attaching to this letter a test case that shows the behavior
errcontext() macro and the way to fix it.

So the upshot of this is that given errfinish(A, B, C), where A, B,
and C are expressions, my gcc is choosing to evaluate C, then B, then
A, then the errfinish call itself. But whoever wrote the errcontext()
macro evidently thought, in this kind of situation, the compiler would
be certain to evaluate A, then B, then C, then errfinish. But it
doesn't.

http://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=1f9bf05e539646103c518bcbb49c04919b238f7a

Oops, sorry. I missed that.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#6Воронин Дмитрий
carriingfate92@yandex.ru
In reply to: Tom Lane (#4)
Re: ereport bug

Hello all

I see your patch, Tom. It works on my postgres 9.4 (intel x86_64). I write a letter if I can test it on others platform.

Thank you!

14.01.2015, 21:40, "Tom Lane" <tgl@sss.pgh.pa.us>:

Robert Haas <robertmhaas@gmail.com> writes:

О©╫On Mon, Jan 12, 2015 at 6:27 AM, Dmitry Voronin
О©╫<carriingfate92@yandex.ru> wrote:

О©╫I am attaching to this letter a test case that shows the behavior
О©╫errcontext() macro and the way to fix it.

О©╫So the upshot of this is that given errfinish(A, B, C), where A, B,
О©╫and C are expressions, my gcc is choosing to evaluate C, then B, then
О©╫A, then the errfinish call itself. О©╫But whoever wrote the errcontext()
О©╫macro evidently thought, in this kind of situation, the compiler would
О©╫be certain to evaluate A, then B, then C, then errfinish. О©╫But it
О©╫doesn't.

http://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=1f9bf05e539646103c518bcbb49c04919b238f7a

О©╫О©╫О©╫О©╫О©╫О©╫О©╫О©╫О©╫О©╫О©╫О©╫О©╫О©╫О©╫О©╫О©╫О©╫О©╫О©╫О©╫О©╫О©╫О©╫regards, tom lane

--
О©╫ О©╫О©╫О©╫О©╫О©╫О©╫О©╫О©╫О©╫, О©╫О©╫О©╫О©╫О©╫О©╫О©╫ О©╫О©╫О©╫О©╫О©╫О©╫О©╫

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers