Further tweak memory management for regex DFAs.

tglsfdc · tglsfdc · commit 6c20bdb2a279 · 2021-03-08T16:32:29.000-05:00
Coverity is still unhappy after commit 190c798, and after looking closer I think it might be onto something. The callers of newdfa() typically drop out if v->err has been set nonzero, which newdfa() is faithfully doing if it fails. However, what if v->err was already nonzero before we entered newdfa()? Then newdfa() could succeed and the caller would promptly leak its result. I don't think this scenario can actually happen, but the predicate "v->err is always zero when newdfa() is called" seems difficult to be entirely sure of; there's a good deal of code that potentially could get that wrong. It seems better to adjust the callers to directly check for a null result instead of relying on ISERR() tests. This is slightly cheaper than the previous coding anyway. Lacking evidence that there's any real bug, no back-patch.
diff --git a/src/backend/regex/rege_dfa.c b/src/backend/regex/rege_dfa.c
@@ -604,6 +604,8 @@ lastcold(struct vars *v,
 
 /*
  * newdfa - set up a fresh DFA
+ *
+ * Returns NULL (and sets v->err) on failure.
  */
 static struct dfa *
 newdfa(struct vars *v,
diff --git a/src/backend/regex/regexec.c b/src/backend/regex/regexec.c
@@ -351,7 +351,7 @@ getsubdfa(struct vars *v,
 	if (d == NULL)
 	{
 		d = newdfa(v, &t->cnfa, &v->g->cmap, DOMALLOC);
-		if (ISERR())
+		if (d == NULL)
 			return NULL;
 		/* set up additional info if this is a backref node */
 		if (t->op == 'b')
@@ -381,8 +381,6 @@ getladfa(struct vars *v,
 		struct subre *sub = &v->g->lacons[n];
 
 		v->ladfas[n] = newdfa(v, &sub->cnfa, &v->g->cmap, DOMALLOC);
-		if (ISERR())
-			return NULL;
 		/* a LACON can't contain a backref, so nothing else to do */
 	}
 	return v->ladfas[n];
@@ -408,8 +406,8 @@ find(struct vars *v,
 
 	/* first, a shot with the search RE */
 	s = newdfa(v, &v->g->search, cm, &v->dfa1);
-	assert(!(ISERR() && s != NULL));
-	NOERR();
+	if (s == NULL)
+		return v->err;
 	MDEBUG(("\nsearch at %ld\n", LOFF(v->start)));
 	cold = NULL;
 	close = shortest(v, s, v->search_start, v->search_start, v->stop,
@@ -436,8 +434,8 @@ find(struct vars *v,
 	cold = NULL;
 	MDEBUG(("between %ld and %ld\n", LOFF(open), LOFF(close)));
 	d = newdfa(v, cnfa, cm, &v->dfa1);
-	assert(!(ISERR() && d != NULL));
-	NOERR();
+	if (d == NULL)
+		return v->err;
 	for (begin = open; begin <= close; begin++)
 	{
 		MDEBUG(("\nfind trying at %ld\n", LOFF(begin)));
@@ -493,11 +491,11 @@ cfind(struct vars *v,
 	int			ret;
 
 	s = newdfa(v, &v->g->search, cm, &v->dfa1);
-	NOERR();
+	if (s == NULL)
+		return v->err;
 	d = newdfa(v, cnfa, cm, &v->dfa2);
-	if (ISERR())
+	if (d == NULL)
 	{
-		assert(d == NULL);
 		freedfa(s);
 		return v->err;
 	}
