Improve memory-usage accounting in regular-expression compiler.

This code previously counted the number of NFA states it created, and
complained if a limit was exceeded, so as to prevent bizarre regex patterns
from consuming unreasonable time or memory.  That's fine as far as it went,
but the code paid no attention to how many arcs linked those states.  Since
regexes can be contrived that have O(N) states but will need O(N^2) arcs
after fixempties() processing, it was still possible to blow out memory,
and take a long time doing it too.  To fix, modify the bookkeeping to count
space used by both states and arcs.

I did not bother with including the "color map" in the accounting; it
can only grow to a few megabytes, which is not a lot in comparison to
what we're allowing for states+arcs (about 150MB on 64-bit machines
or half that on 32-bit machines).

Looking at some of the larger real-world regexes captured in the Tcl
regression test suite suggests that the most that is likely to be needed
for regexes found in the wild is under 10MB, so I believe that the current
limit has enough headroom to make it okay to keep it as a hard-wired limit.

In connection with this, redefine REG_ETOOBIG as meaning "regular
expression is too complex"; the previous wording of "nfa has too many
states" was already somewhat inapropos because of the error code's use
for stack depth overrun, and it was not very user-friendly either.

Back-patch to all supported branches.

diff --git a/src/backend/regex/regc_nfa.c b/src/backend/regex/regc_nfa.c
index ecc91a8ede9657c7e968b6a3d2df114d0d8f9196..fe8a4a12d1783a870e782667fb5cfb055bdd5b9b 100644 (file)
--- a/src/backend/regex/regc_nfa.c
+++ b/src/backend/regex/regc_nfa.c
@@ -63,7 +63,6 @@ newnfa(struct vars * v,
    nfa->nstates = 0;
    nfa->cm = cm;
    nfa->v = v;
-   nfa->size = 0;
    nfa->bos[0] = nfa->bos[1] = COLORLESS;
    nfa->eos[0] = nfa->eos[1] = COLORLESS;
    nfa->parent = parent;       /* Precedes newfstate so parent is valid. */
@@ -92,57 +91,6 @@ newnfa(struct vars * v,
    return nfa;
 }
 
-/*
- * TooManyStates - checks if the max states exceeds the compile-time value
- */
-static int
-TooManyStates(struct nfa * nfa)
-{
-   struct nfa *parent = nfa->parent;
-   size_t      sz = nfa->size;
-
-   while (parent != NULL)
-   {
-       sz = parent->size;
-       parent = parent->parent;
-   }
-   if (sz > REG_MAX_STATES)
-       return 1;
-   return 0;
-}
-
-/*
- * IncrementSize - increases the tracked size of the NFA and its parents.
- */
-static void
-IncrementSize(struct nfa * nfa)
-{
-   struct nfa *parent = nfa->parent;
-
-   nfa->size++;
-   while (parent != NULL)
-   {
-       parent->size++;
-       parent = parent->parent;
-   }
-}
-
-/*
- * DecrementSize - decreases the tracked size of the NFA and its parents.
- */
-static void
-DecrementSize(struct nfa * nfa)
-{
-   struct nfa *parent = nfa->parent;
-
-   nfa->size--;
-   while (parent != NULL)
-   {
-       parent->size--;
-       parent = parent->parent;
-   }
-}
-
 /*
  * freenfa - free an entire NFA
  */
@@ -188,12 +136,6 @@ newstate(struct nfa * nfa)
        return NULL;
    }
 
-   if (TooManyStates(nfa))
-   {
-       NERR(REG_ETOOBIG);
-       return NULL;
-   }
-
    if (nfa->free != NULL)
    {
        s = nfa->free;
@@ -201,12 +143,18 @@ newstate(struct nfa * nfa)
    }
    else
    {
+       if (nfa->v->spaceused >= REG_MAX_COMPILE_SPACE)
+       {
+           NERR(REG_ETOOBIG);
+           return NULL;
+       }
        s = (struct state *) MALLOC(sizeof(struct state));
        if (s == NULL)
        {
            NERR(REG_ESPACE);
            return NULL;
        }
+       nfa->v->spaceused += sizeof(struct state);
        s->oas.next = NULL;
        s->free = NULL;
        s->noas = 0;
@@ -230,8 +178,6 @@ newstate(struct nfa * nfa)
    }
    s->prev = nfa->slast;
    nfa->slast = s;
-   /* track the current size and the parent size */
-   IncrementSize(nfa);
    return s;
 }
 
@@ -294,7 +240,6 @@ freestate(struct nfa * nfa,
    s->prev = NULL;
    s->next = nfa->free;        /* don't delete it, put it on the free list */
    nfa->free = s;
-   DecrementSize(nfa);
 }
 
 /*
@@ -312,11 +257,13 @@ destroystate(struct nfa * nfa,
    {
        abnext = ab->next;
        FREE(ab);
+       nfa->v->spaceused -= sizeof(struct arcbatch);
    }
    s->ins = NULL;
    s->outs = NULL;
    s->next = NULL;
    FREE(s);
+   nfa->v->spaceused -= sizeof(struct state);
 }
 
 /*
@@ -437,12 +384,18 @@ allocarc(struct nfa * nfa,
        struct arcbatch *newAb;
        int         i;
 
+       if (nfa->v->spaceused >= REG_MAX_COMPILE_SPACE)
+       {
+           NERR(REG_ETOOBIG);
+           return NULL;
+       }
        newAb = (struct arcbatch *) MALLOC(sizeof(struct arcbatch));
        if (newAb == NULL)
        {
            NERR(REG_ESPACE);
            return NULL;
        }
+       nfa->v->spaceused += sizeof(struct arcbatch);
        newAb->next = s->oas.next;
        s->oas.next = newAb;
 

diff --git a/src/backend/regex/regcomp.c b/src/backend/regex/regcomp.c
index 6b031075e7551299feaaba062989b6c4a1524ab4..324fea5ffb0e52c1360c2c1c8e5f8467875fcf31 100644 (file)
--- a/src/backend/regex/regcomp.c
+++ b/src/backend/regex/regcomp.c
@@ -248,6 +248,7 @@ struct vars
    struct cvec *cv2;           /* utility cvec */
    struct subre *lacons;       /* lookahead-constraint vector */
    int         nlacons;        /* size of lacons */
+   size_t      spaceused;      /* approx. space used for compilation */
 };
 
 /* parsing macros; most know that `v' is the struct vars pointer */
@@ -363,6 +364,7 @@ pg_regcomp(regex_t *re,
    v->cv2 = NULL;
    v->lacons = NULL;
    v->nlacons = 0;
+   v->spaceused = 0;
    re->re_magic = REMAGIC;
    re->re_info = 0;            /* bits get set during parse */
    re->re_csize = sizeof(chr);

diff --git a/src/include/regex/regerrs.h b/src/include/regex/regerrs.h
index 809b511266047a32b403ac05f0f3b22d3437b5dc..41e25f7ff00e5cae1e2f07555653ce6db017dead 100644 (file)
--- a/src/include/regex/regerrs.h
+++ b/src/include/regex/regerrs.h
@@ -75,7 +75,7 @@
 },
 
 {
-   REG_ETOOBIG, "REG_ETOOBIG", "nfa has too many states"
+   REG_ETOOBIG, "REG_ETOOBIG", "regular expression is too complex"
 },
 
 {

diff --git a/src/include/regex/regex.h b/src/include/regex/regex.h
index 3020b0ff0f76d1a6fc520062803f28f43822cca0..5e1b692d26c130d09cbe4f7e3150ba6cbba6f66a 100644 (file)
--- a/src/include/regex/regex.h
+++ b/src/include/regex/regex.h
@@ -152,7 +152,7 @@ typedef struct
 #define REG_INVARG 16          /* invalid argument to regex function */
 #define REG_MIXED  17          /* character widths of regex and string differ */
 #define REG_BADOPT 18          /* invalid embedded option */
-#define REG_ETOOBIG 19         /* nfa has too many states */
+#define REG_ETOOBIG 19         /* regular expression is too complex */
 #define REG_ECOLORS 20         /* too many colors */
 #define REG_CANCEL 21          /* operation cancelled */
 /* two specials for debugging and testing */

diff --git a/src/include/regex/regguts.h b/src/include/regex/regguts.h
index 357891aa254847f9eba290b9e4f749ff59cd2bd9..19fe991c74faa5213261ce3b060071a8c7f2eec2 100644 (file)
--- a/src/include/regex/regguts.h
+++ b/src/include/regex/regguts.h
@@ -334,9 +334,6 @@ struct nfa
    struct colormap *cm;        /* the color map */
    color       bos[2];         /* colors, if any, assigned to BOS and BOL */
    color       eos[2];         /* colors, if any, assigned to EOS and EOL */
-   size_t      size;           /* Current NFA size; differs from nstates as
-                                * it also counts the number of states in
-                                * children of this NFA. */
    struct vars *v;             /* simplifies compile error reporting */
    struct nfa *parent;         /* parent NFA, if any */
 };
@@ -384,10 +381,16 @@ struct cnfa
 #define NULLCNFA(cnfa) ((cnfa).nstates == 0)
 
 /*
- * Used to limit the maximum NFA size to something sane. [Tcl Bug 1810264]
+ * This symbol limits the transient heap space used by the regex compiler,
+ * and thereby also the maximum complexity of NFAs that we'll deal with.
+ * Currently we only count NFA states and arcs against this; the other
+ * transient data is generally not large enough to notice compared to those.
+ * Note that we do not charge anything for the final output data structures
+ * (the compacted NFA and the colormap).
  */
-#ifndef REG_MAX_STATES
-#define REG_MAX_STATES 100000
+#ifndef REG_MAX_COMPILE_SPACE
+#define REG_MAX_COMPILE_SPACE  \
+   (100000 * sizeof(struct state) + 100000 * sizeof(struct arcbatch))
 #endif
 
 /*