Re: empty array case in plperl_ref_from_pg_array not handled correctly

From: Alex Hunsaker <badalex(at)gmail(dot)com>
To: Andres Freund <andres(at)anarazel(dot)de>
Cc: Alexey Klyukin <alexk(at)hintbits(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: empty array case in plperl_ref_from_pg_array not handled correctly
Date: 2016-03-08 09:11:03
Message-ID: CAFaPBrTnLeunw+pJ_BRahYwkRpOprNsTUP1O6zjCNejjy5cGjg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Mar 7, 2016 at 11:32 PM, Andres Freund <andres(at)anarazel(dot)de> wrote:

> Hi,
>
> Per the new valgrind animal we get:
>
>
> http://pgbuildfarm.org/cgi-bin/show_log.pl?nm=skink&dt=2016-03-08%2004%3A22%3A00
> 2016-03-08 05:56:05.566 UTC [56de6971.723:5] LOG: statement: select
> plperl_sum_array('{}');
> ==1827== Invalid write of size 4
> ==1827== at 0x14E35DD1: plperl_ref_from_pg_array (plperl.c:1459)
>
>
[ I think you may have meant to CC me not Alexey K. I'm probably the person
responsible :D. ]

Indeed, I think the simplest fix is to make plperl_ref_from_pg_array()
return an "empty" array in that case. The attached fixes the valgrind
warning for me. (cassert enabled build on master).

>
> ISTM the assumption that an array always has a dimension is a bit more
> widespread... E.g. split_array() looks like it'd not work nicely with a
> zero dimensional array...
>

Hrm, plperl_ref_from_pg_array() is the only caller of split_array(), so I
added an Assert() to make its contract more clear.

Recursive calls to split_array() should be fine as nested 0 dim arrays get
collapsed down to single 0 dim array (I hand tested it nonetheless):
# select ARRAY[ARRAY[ARRAY[]]]::int[];
array
───────
{}
(1 row)

Looking around, everything that makes an array seems to pass through
plperl_ref_from_pg_array(), so I think once that is fixed all is good. I
also looked for dimensions and ARR_NDIM() just to make sure (didn't find
anything fishy).

Thanks,

Attachment Content-Type Size
plperl_array_valgrind.patch application/octet-stream 1.5 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Kyotaro HORIGUCHI 2016-03-08 09:19:35 Re: [PROPOSAL] VACUUM Progress Checker.
Previous Message Fabien COELHO 2016-03-08 08:28:15 Re: checkpointer continuous flushing - V18