Re: BUG #14174: Expanded-datum bug accessing freed memory

andrew@tao11.riddles.org.uk writes:
> This is a testcase distilled from a problem reported on IRC:

> create or replace function afn1(integer) returns int[] language plpgsql
>  as $f$ declare r int[]; begin select into r array[$1,$1]; return r; end;
> $f$;
> create or replace function afn2(int[]) returns integer language plpgsql
>  as $f$ begin return $1[1]; end; $f$;
> create or replace view v_test as select i, a from (select afn1(1) as a
> offset 0) s, lateral afn2(a) i;
> create function tstfn(integer) returns setof v_test language plpgsql
>  as $f$ begin return query select * from v_test where i=$1; end; $f$;
> select * from tstfn(1);

> ERROR:  cache lookup failed for type 2139062143

Hm, you don't need tstfn(), nor even the view:

regression=# select i, a from (select afn1(1) as a offset 0) s, lateral afn2(a) i;
ERROR:  cache lookup failed for type 2139062143

The core of the problem here is that afn1() is returning a read-write
pointer to the expanded object holding "r", and then when that's passed
to afn2(), it supposes that it can take ownership of it as a read-write
local variable; which means the value gets destroyed when afn2() exits.
That'd be all right except there's another reference to "a" still to be
evaluated :-(

We should have forestalled this by applying MakeExpandedObjectReadOnly
to the output of the first sub-select.  But we didn't because that's
currently only done by SubqueryNext, and the SubqueryScan node that
would have fixed this has been optimized away:

 Nested Loop  (cost=0.25..0.54 rows=1 width=36)
   Output: i.i, (afn1(1))
   ->  Result  (cost=0.00..0.26 rows=1 width=32)
         Output: afn1(1)
   ->  Function Scan on public.afn2 i  (cost=0.25..0.26 rows=1 width=4)
         Output: i.i
         Function Call: afn2((afn1(1)))

I am thinking maybe we need to have ExecProject do
MakeExpandedObjectReadOnly on each result, rather than assuming that
SubqueryScan is the place for that.  This would slightly increase the
general overhead attributable to the expanded-object feature, which is
unfortunate, but right now it's not clear that anything less is safe.
Making Result nodes do that would fix this particular instance but
there are plenty of other node types that might appear at the top of
a sub-select.

A possible future improvement is to teach the planner to detect which
variables are actually multiply referenced, and force read-only-ness
for only those values.  But that's clearly not something that would be
reasonable to back-patch into 9.5, or even 9.6 at this point.

            regards, tom lane