From 538b3b8b359fa77cb7e1507113efb788b4e159c9 Mon Sep 17 00:00:00 2001 From: Tom Lane <tgl@sss.pgh.pa.us> Date: Fri, 16 Oct 2015 15:36:16 -0400 Subject: [PATCH] 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. --- src/backend/regex/regc_nfa.c | 75 +++++++----------------------------- src/backend/regex/regcomp.c | 2 + src/include/regex/regerrs.h | 2 +- src/include/regex/regex.h | 2 +- src/include/regex/regguts.h | 15 +++++--- 5 files changed, 27 insertions(+), 69 deletions(-) diff --git a/src/backend/regex/regc_nfa.c b/src/backend/regex/regc_nfa.c index ecc91a8ede9..fe8a4a12d17 100644 --- 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 6b031075e75..324fea5ffb0 100644 --- 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 809b5112660..41e25f7ff00 100644 --- 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 3020b0ff0f7..5e1b692d26c 100644 --- 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 357891aa254..19fe991c74f 100644 --- 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 /* -- GitLab