Date: Sun, 17 Nov 2024 08:12:10 -0800
From: Noah Misch <noah@leadboat.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
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"
Message-ID: <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>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <2988928.1731824798@sss.pgh.pa.us>
User-Agent: Mutt/2.2.12 (2023-09-09)
Archived-At: 
 <https://www.postgresql.org/message-id/20241117161210.54.nmisch%40google.com>
Precedence: bulk

On Sun, Nov 17, 2024 at 01:26:38AM -0500, Tom Lane wrote:
> 	arcarray = (struct arc **) MALLOC(totalinarcs * sizeof(struct arc *));
> 	if (arcarray == NULL)
> 	{
> 		NERR(REG_ESPACE);
> 		...
> 
> On a machine where malloc(0) returns NULL, this mistakenly
> thinks that's an error.
> 
> I verified that
> 
> -	if (arcarray == NULL)
> +	if (arcarray == NULL && totalinarcs != 0)
> 
> makes the failure go away, but I wonder if any other places in
> backend/regex/ are at the same hazard.  Maybe the smartest fix
> would be to put in a wrapper layer that does what pg_malloc
> does:
> 
> 	/* Avoid unportable behavior of malloc(0) */
> 	if (size == 0)
> 		size = 1;

Either of those sound reasonable.  The consequence of missing this hazard, 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.

> One other point is that this theory fails to explain why
> hornet didn't fail in the v16 branch ... oh, wait:
> v15 has 
> 
> #define MALLOC(n)		malloc(n)
> 
> where later branches have
> 
> #define MALLOC(n)		palloc_extended((n), MCXT_ALLOC_NO_OOM)
> 
> 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.