pg.ddx.io  pgsql-committers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
pgsql: Avoid premature free of pass-by-reference CALL arguments.
3+ messages / 2 participants
[nested] [flat]

Commits from this thread:
  2018-03-25 038a2ed139  Stabilize regression test result.
* pgsql: Avoid premature free of pass-by-reference CALL arguments.
@ 2018-02-10 18:37 Tom Lane <tgl@sss.pgh.pa.us>
  2018-03-24 21:25 ` Re: pgsql: Avoid premature free of pass-by-reference CALL arguments. Andres Freund <andres@anarazel.de>
  0 siblings, 1 reply; 3+ messages in thread

From: Tom Lane @ 2018-02-10 18:37 UTC (permalink / raw)
  To: pgsql-committers

Avoid premature free of pass-by-reference CALL arguments.

Prematurely freeing the EState used to evaluate CALL arguments led, in some
cases, to passing dangling pointers to the procedure.  This was masked in
trivial cases because the argument pointers would point to Const nodes in
the original expression tree, and in some other cases because the result
value would end up in the standalone ExprContext rather than in memory
belonging to the EState --- but that wasn't exactly high quality
programming either, because the standalone ExprContext was never
explicitly freed, breaking assorted API contracts.

In addition, using a separate EState for each argument was just silly.

So let's use just one EState, and one ExprContext, and make the latter
belong to the former rather than be standalone, and clean up the EState
(and hence the ExprContext) post-call.

While at it, improve the function's commentary a bit.

Discussion: https://postgr.es/m/29173.1518282748@sss.pgh.pa.us

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/d02d4a6d4f27c223f48b03a5e651a22c8460b3c4

Modified Files
--------------
src/backend/commands/functioncmds.c            | 28 ++++++++++++++++++++------
src/test/regress/expected/create_procedure.out | 12 +++++++----
src/test/regress/sql/create_procedure.sql      |  4 +++-
3 files changed, 33 insertions(+), 11 deletions(-)



^ permalink  raw  reply  [nested|flat] 3+ messages in thread

* Re: pgsql: Avoid premature free of pass-by-reference CALL arguments.
  2018-02-10 18:37 pgsql: Avoid premature free of pass-by-reference CALL arguments. Tom Lane <tgl@sss.pgh.pa.us>
@ 2018-03-24 21:25 ` Andres Freund <andres@anarazel.de>
  2018-03-25 02:55   ` Re: pgsql: Avoid premature free of pass-by-reference CALL arguments. Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 1 reply; 3+ messages in thread

From: Andres Freund @ 2018-03-24 21:25 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; Peter Eisentraut <peter_e@gmx.net>; +Cc: pgsql-committers; pgsql-hackers@postgresql.org


On 2018-02-10 18:37:17 +0000, Tom Lane wrote:
> Avoid premature free of pass-by-reference CALL arguments.

> src/test/regress/expected/create_procedure.out | 12 +++++++----
> src/test/regress/sql/create_procedure.sql      |  4 +++-
> 3 files changed, 33 insertions(+), 11 deletions(-)

There's a recent llvm buildfarm animal failure related to this:
https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=pogona&dt=2018-03-24%2020%3A10%3A01

inline patch 1: /home/bf/build/buildfarm-pogona/HEAD/pgsql.build/src/test/regress/results/create_procedure.out (+1/-1 lines) download inline-1.patch*** /home/bf/build/buildfarm-pogona/HEAD/pgsql.build/../pgsql/src/test/regress/expected/create_procedure.out 2018-03-23 08:10:44.326010286 +0100 --- /home/bf/build/buildfarm-pogona/HEAD/pgsql.build/src/test/regress/results/create_procedure.out 2018-03-24 21:15:28.749352165 +0100 *************** *** 44,50 **** SELECT * FROM cp_test ORDER BY b COLLATE "C"; a | b ---+------- ! 1 | 0 1 | a 1 | xyzzy (3 rows) --- 44,50 ---- SELECT * FROM cp_test ORDER BY b COLLATE "C"; a | b ---+------- ! 1 | 9 1 | a 1 | xyzzy (3 rows)
With the differening output created by: CREATE PROCEDURE ptest1(x text) LANGUAGE SQL AS $$ INSERT INTO cp_test VALUES (1, x); $$; CALL ptest1(substring(random()::text, 1, 1)); -- ok, volatile arg At first I was gosh darned confused, this really didn't seem likely to be an LLVM induced failure. And it turns out it isn't. If the value returned by random() is very small, the text representation switches to scientific notation like 8.26204195618629e-05. So, perhaps we should choose a different volatile function here? Greetings, Andres Freund ^ permalink raw reply [nested|flat] 3+ messages in thread

* Re: pgsql: Avoid premature free of pass-by-reference CALL arguments.
  2018-02-10 18:37 pgsql: Avoid premature free of pass-by-reference CALL arguments. Tom Lane <tgl@sss.pgh.pa.us>
  2018-03-24 21:25 ` Re: pgsql: Avoid premature free of pass-by-reference CALL arguments. Andres Freund <andres@anarazel.de>
@ 2018-03-25 02:55   ` Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 0 replies; 3+ messages in thread

From: Tom Lane @ 2018-03-25 02:55 UTC (permalink / raw)
  To: Andres Freund <andres@anarazel.de>; +Cc: Peter Eisentraut <peter_e@gmx.net>; pgsql-committers; pgsql-hackers@postgresql.org

Andres Freund <andres@anarazel.de> writes:
> On 2018-02-10 18:37:17 +0000, Tom Lane wrote:
> CALL ptest1(substring(random()::text, 1, 1));  -- ok, volatile arg

> At first I was gosh darned confused, this really didn't seem likely to
> be an LLVM induced failure. And it turns out it isn't.  If the value
> returned by random() is very small, the text representation switches to
> scientific notation like 8.26204195618629e-05.

Ooops.  I'll do something about that tomorrow.

			regards, tom lane




^ permalink  raw  reply  [nested|flat] 3+ messages in thread


end of thread, other threads:[~2018-03-25 02:55 UTC | newest]

Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2018-02-10 18:37 pgsql: Avoid premature free of pass-by-reference CALL arguments. Tom Lane <tgl@sss.pgh.pa.us>
2018-03-24 21:25 ` Andres Freund <andres@anarazel.de>
2018-03-25 02:55   ` Tom Lane <tgl@sss.pgh.pa.us>

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox