From: Tom Lane <tgl@sss.pgh.pa.us>
To: Noah Misch <noah@leadboat.com>
cc: Alexander Lakhin <exclusion@gmail.com>, pgsql-bugs@lists.postgresql.org,
        Aleksander Alekseev <aleksander@timescale.com>, dhyan@nataraj.su
Subject: Re: BUG #18708: regex problem: (?:[^\d\D]){0} asserts with "lp->nouts
 == 0 && rp->nins == 0"
In-reply-to: <20241117161210.54.nmisch@google.com>
References: <2242483.1731627374@sss.pgh.pa.us>
 <2344149.1731636867@sss.pgh.pa.us>
 <CAJ7c6TMXLaoBB6GmgkkSRH_WoLxqSU_K6ORnw9rP+3JexuKSMQ@mail.gmail.com>
 <CAJ7c6TP+0MHu7t3QDdXHMf1DW9r56qnyr_7NsTQOB5+pKQEkcg@mail.gmail.com>
 <2631851.1731715737@sss.pgh.pa.us>
 <661fd81b-f069-8f30-1a41-e195c57449b4@gmail.com>
 <2745175.1731772978@sss.pgh.pa.us>
 <7dd96d86-bac0-4879-2555-276d2ef4a8db@gmail.com>
 <2958975.1731817729@sss.pgh.pa.us> <2988928.1731824798@sss.pgh.pa.us>
 <20241117161210.54.nmisch@google.com>
Comments: In-reply-to Noah Misch <noah@leadboat.com>
	message dated "Sun, 17 Nov 2024 08:12:10 -0800"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <3068211.1731864164.1@sss.pgh.pa.us>
Content-Transfer-Encoding: quoted-printable
Date: Sun, 17 Nov 2024 12:22:44 -0500
Message-ID: <3068212.1731864164@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/3068212.1731864164%40sss.pgh.pa.us>
Precedence: bulk

Noah Misch <noah@leadboat.com> writes:
> On Sun, Nov 17, 2024 at 01:26:38AM -0500, Tom Lane wrote:
>> arcarray =3D (struct arc **) MALLOC(totalinarcs * sizeof(struct arc *))=
;
>> if (arcarray =3D=3D NULL)

>> On a machine where malloc(0) returns NULL, this mistakenly
>> thinks that's an error.

> Either of those sound reasonable.  The consequence of missing this hazar=
d, a
> deterministic ERROR, is modest.  This affects just one platform, in the =
oldest
> branches.  There's a lack of complaints.  To me, all that would make the
> one-line diff tempting.

I dug through the MALLOC and REALLOC calls in backend/regex/ and
convinced myself that this is the only one that's at risk.  (Some
of those conclusions depend on the assumption that a regex NFA
never has nstates =3D=3D 0, but I think that's okay: we create start
and end states to begin with and never remove them.)  So the
one-liner fix is looking attractive.  I'd prefer a malloc wrapper
for future-proofing if this code were likely to receive a lot of
churn in the pre-v16 branches, but that seems pretty improbable
at this point.

>> So the right answer seems to be to figure out why we didn't
>> back-patch that change.

> I don't recall a specific reason or see one in the discussion of commit
> bea3d7e38.  It was done mainly to unblock commit db4f21e, which in turn
> unblocked commit 0da096d.  The last commit is heavy, so I can understand=
 it
> skipping the back branches.  If I were making a (weak) argument against
> back-patching bea3d7e38, I might cite the extra memory use from
> RegexpCacheMemoryContext and children.

I think just on minimum-risk grounds, I wouldn't consider
back-patching bea3d7e38.  I had more in mind a bespoke
three-line malloc wrapper function.  But the one-line fix
seems sufficient for the problem at hand.

			regards, tom lane